Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Naresh Kamboju]

The kernel warning noticed on arm64 juno-r2 device running linux
next-20200326 and next-20200327

[   36.077086] ------------[ cut here ]------------
[   36.081752] amba 20010000.etf: deferred probe timeout, ignoring dependency
[   36.081859] WARNING: CPU: 1 PID: 42 at drivers/base/dd.c:270
driver_deferred_probe_check_state+0x54/0x80
[   36.098242] Modules linked in: fuse
[   36.101753] CPU: 1 PID: 42 Comm: kworker/1:1 Not tainted
5.6.0-rc7-next-20200327 #1
[   36.109427] Hardware name: ARM Juno development board (r2) (DT)
[   36.115372] Workqueue: events amba_deferred_retry_func
[   36.120526] pstate: 60000005 (nZCv daif -PAN -UAO)
[   36.125334] pc : driver_deferred_probe_check_state+0x54/0x80
[   36.131010] lr : driver_deferred_probe_check_state+0x54/0x80
[   36.136680] sp : ffff000934e0fae0
[   36.140001] x29: ffff000934e0fae0 x28: ffff000934db5608
[   36.145337] x27: ffffa00013c63240 x26: ffff000934f2a800
[   36.150668] x25: 0000000000000001 x24: fffffffffffffffe
[   36.155996] x23: ffff000934c6ab80 x22: ffffa00011b39ea0
[   36.161322] x21: ffff000934f2a800 x20: ffffa00011905fe0
[   36.166649] x19: ffff000934f2a800 x18: 0000000000000000
[   36.171974] x17: 0000000000000000 x16: 0000000000000000
[   36.177299] x15: 0000000000000000 x14: 003d090000000000
[   36.182625] x13: 00003d0900000000 x12: ffff9400027ef445
[   36.187952] x11: 1ffff400027ef444 x10: ffff9400027ef444
[   36.193278] x9 : dfffa00000000000 x8 : 0000000000000000
[   36.198603] x7 : 0000000000000001 x6 : ffffa00013f7a220
[   36.203929] x5 : 0000000000000004 x4 : dfffa00000000000
[   36.209255] x3 : ffffa000101a74ec x2 : ffff8001269c1f26
[   36.214581] x1 : da1107b7b6a8fb00 x0 : 0000000000000000
[   36.219906] Call trace:
[   36.222369]  driver_deferred_probe_check_state+0x54/0x80
[   36.227698]  __genpd_dev_pm_attach+0x264/0x2a0
[   36.232154]  genpd_dev_pm_attach+0x68/0x78
[   36.236265]  dev_pm_domain_attach+0x6c/0x70
[   36.240463]  amba_device_try_add+0xec/0x3f8
[   36.244659]  amba_deferred_retry_func+0x84/0x158
[   36.249301]  process_one_work+0x3f0/0x660
[   36.253326]  worker_thread+0x74/0x698
[   36.256997]  kthread+0x218/0x220
[   36.260236]  ret_from_fork+0x10/0x1c
[   36.263819] ---[ end trace c637c10e549bd716 ]---#

Full test log,
https://lkft.validation.linaro.org/scheduler/job/1317079#L981

On Tue, 24 Mar 2020 at 18:24, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Tue, Mar 24, 2020 at 1:20 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > There is a place in the code where open-coded version of list entry accessors
> > list_last_entry() is used.
> >
> > Replace that with the standard macro.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> > ---
> > v2: no change
> >  drivers/base/dd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index efd0e4c16ba5..27a4d51b5bba 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -1226,7 +1226,7 @@ void driver_detach(struct device_driver *drv)
> >                         spin_unlock(&drv->p->klist_devices.k_lock);
> >                         break;
> >                 }
> > -               dev_prv = list_entry(drv->p->klist_devices.k_list.prev,
> > +               dev_prv = list_last_entry(&drv->p->klist_devices.k_list,
> >                                      struct device_private,
> >                                      knode_driver.n_node);
> >                 dev = dev_prv->device;

metadata:
  git branch: master
  git repo: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
  git describe: next-20200327
  kernel-config:
https://builds.tuxbuild.com/nqmmxorUbC1qTWp42iEKjQ/kernel.config


-- 
Linaro LKFT
https://lkft.linaro.org

Re: Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Robin Murphy]

On 2020-03-27 5:56 pm, Naresh Kamboju wrote:
> The kernel warning noticed on arm64 juno-r2 device running linux
> next-20200326 and next-20200327

I suspect this is the correct expected behaviour manifesting. If you're 
using the upstream juno-r2.dts, the power domain being waited for here 
is provided by SCPI, however unless you're using an SCP firmware from at 
least 3 years ago you won't actually have SCPI since they switched it to 
the newer SCMI protocol, which is not yet supported upstream for Juno. 
See what happened earlier in the log:

[    2.741206] scpi_protocol scpi: incorrect or no SCP firmware found
[    2.747586] scpi_protocol: probe of scpi failed with error -110

Thus this is the "waiting for a dependency which will never appear" 
case, for which I assume the warning is intentional, since the system is 
essentially broken (i.e. the hardware/firmware doesn't actually match 
what the DT describes).

Robin.

> [   36.077086] ------------[ cut here ]------------
> [   36.081752] amba 20010000.etf: deferred probe timeout, ignoring dependency
> [   36.081859] WARNING: CPU: 1 PID: 42 at drivers/base/dd.c:270
> driver_deferred_probe_check_state+0x54/0x80
> [   36.098242] Modules linked in: fuse
> [   36.101753] CPU: 1 PID: 42 Comm: kworker/1:1 Not tainted
> 5.6.0-rc7-next-20200327 #1
> [   36.109427] Hardware name: ARM Juno development board (r2) (DT)
> [   36.115372] Workqueue: events amba_deferred_retry_func
> [   36.120526] pstate: 60000005 (nZCv daif -PAN -UAO)
> [   36.125334] pc : driver_deferred_probe_check_state+0x54/0x80
> [   36.131010] lr : driver_deferred_probe_check_state+0x54/0x80
> [   36.136680] sp : ffff000934e0fae0
> [   36.140001] x29: ffff000934e0fae0 x28: ffff000934db5608
> [   36.145337] x27: ffffa00013c63240 x26: ffff000934f2a800
> [   36.150668] x25: 0000000000000001 x24: fffffffffffffffe
> [   36.155996] x23: ffff000934c6ab80 x22: ffffa00011b39ea0
> [   36.161322] x21: ffff000934f2a800 x20: ffffa00011905fe0
> [   36.166649] x19: ffff000934f2a800 x18: 0000000000000000
> [   36.171974] x17: 0000000000000000 x16: 0000000000000000
> [   36.177299] x15: 0000000000000000 x14: 003d090000000000
> [   36.182625] x13: 00003d0900000000 x12: ffff9400027ef445
> [   36.187952] x11: 1ffff400027ef444 x10: ffff9400027ef444
> [   36.193278] x9 : dfffa00000000000 x8 : 0000000000000000
> [   36.198603] x7 : 0000000000000001 x6 : ffffa00013f7a220
> [   36.203929] x5 : 0000000000000004 x4 : dfffa00000000000
> [   36.209255] x3 : ffffa000101a74ec x2 : ffff8001269c1f26
> [   36.214581] x1 : da1107b7b6a8fb00 x0 : 0000000000000000
> [   36.219906] Call trace:
> [   36.222369]  driver_deferred_probe_check_state+0x54/0x80
> [   36.227698]  __genpd_dev_pm_attach+0x264/0x2a0
> [   36.232154]  genpd_dev_pm_attach+0x68/0x78
> [   36.236265]  dev_pm_domain_attach+0x6c/0x70
> [   36.240463]  amba_device_try_add+0xec/0x3f8
> [   36.244659]  amba_deferred_retry_func+0x84/0x158
> [   36.249301]  process_one_work+0x3f0/0x660
> [   36.253326]  worker_thread+0x74/0x698
> [   36.256997]  kthread+0x218/0x220
> [   36.260236]  ret_from_fork+0x10/0x1c
> [   36.263819] ---[ end trace c637c10e549bd716 ]---#
> 
> Full test log,
> https://lkft.validation.linaro.org/scheduler/job/1317079#L981
> 
> On Tue, 24 Mar 2020 at 18:24, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Tue, Mar 24, 2020 at 1:20 PM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>>
>>> There is a place in the code where open-coded version of list entry accessors
>>> list_last_entry() is used.
>>>
>>> Replace that with the standard macro.
>>>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>
>> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>>> ---
>>> v2: no change
>>>   drivers/base/dd.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>>> index efd0e4c16ba5..27a4d51b5bba 100644
>>> --- a/drivers/base/dd.c
>>> +++ b/drivers/base/dd.c
>>> @@ -1226,7 +1226,7 @@ void driver_detach(struct device_driver *drv)
>>>                          spin_unlock(&drv->p->klist_devices.k_lock);
>>>                          break;
>>>                  }
>>> -               dev_prv = list_entry(drv->p->klist_devices.k_list.prev,
>>> +               dev_prv = list_last_entry(&drv->p->klist_devices.k_list,
>>>                                       struct device_private,
>>>                                       knode_driver.n_node);
>>>                  dev = dev_prv->device;
> 
> metadata:
>    git branch: master
>    git repo: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>    git describe: next-20200327
>    kernel-config:
> https://builds.tuxbuild.com/nqmmxorUbC1qTWp42iEKjQ/kernel.config
> 
> 

Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Greg Kroah-Hartman]

On Fri, Mar 27, 2020 at 11:26:13PM +0530, Naresh Kamboju wrote:
> The kernel warning noticed on arm64 juno-r2 device running linux
> next-20200326 and next-20200327
> 
> [   36.077086] ------------[ cut here ]------------
> [   36.081752] amba 20010000.etf: deferred probe timeout, ignoring dependency
> [   36.081859] WARNING: CPU: 1 PID: 42 at drivers/base/dd.c:270
> driver_deferred_probe_check_state+0x54/0x80
> [   36.098242] Modules linked in: fuse
> [   36.101753] CPU: 1 PID: 42 Comm: kworker/1:1 Not tainted
> 5.6.0-rc7-next-20200327 #1
> [   36.109427] Hardware name: ARM Juno development board (r2) (DT)
> [   36.115372] Workqueue: events amba_deferred_retry_func
> [   36.120526] pstate: 60000005 (nZCv daif -PAN -UAO)
> [   36.125334] pc : driver_deferred_probe_check_state+0x54/0x80
> [   36.131010] lr : driver_deferred_probe_check_state+0x54/0x80
> [   36.136680] sp : ffff000934e0fae0
> [   36.140001] x29: ffff000934e0fae0 x28: ffff000934db5608
> [   36.145337] x27: ffffa00013c63240 x26: ffff000934f2a800
> [   36.150668] x25: 0000000000000001 x24: fffffffffffffffe
> [   36.155996] x23: ffff000934c6ab80 x22: ffffa00011b39ea0
> [   36.161322] x21: ffff000934f2a800 x20: ffffa00011905fe0
> [   36.166649] x19: ffff000934f2a800 x18: 0000000000000000
> [   36.171974] x17: 0000000000000000 x16: 0000000000000000
> [   36.177299] x15: 0000000000000000 x14: 003d090000000000
> [   36.182625] x13: 00003d0900000000 x12: ffff9400027ef445
> [   36.187952] x11: 1ffff400027ef444 x10: ffff9400027ef444
> [   36.193278] x9 : dfffa00000000000 x8 : 0000000000000000
> [   36.198603] x7 : 0000000000000001 x6 : ffffa00013f7a220
> [   36.203929] x5 : 0000000000000004 x4 : dfffa00000000000
> [   36.209255] x3 : ffffa000101a74ec x2 : ffff8001269c1f26
> [   36.214581] x1 : da1107b7b6a8fb00 x0 : 0000000000000000
> [   36.219906] Call trace:
> [   36.222369]  driver_deferred_probe_check_state+0x54/0x80
> [   36.227698]  __genpd_dev_pm_attach+0x264/0x2a0
> [   36.232154]  genpd_dev_pm_attach+0x68/0x78
> [   36.236265]  dev_pm_domain_attach+0x6c/0x70
> [   36.240463]  amba_device_try_add+0xec/0x3f8
> [   36.244659]  amba_deferred_retry_func+0x84/0x158
> [   36.249301]  process_one_work+0x3f0/0x660
> [   36.253326]  worker_thread+0x74/0x698
> [   36.256997]  kthread+0x218/0x220
> [   36.260236]  ret_from_fork+0x10/0x1c
> [   36.263819] ---[ end trace c637c10e549bd716 ]---#
> 
> Full test log,
> https://lkft.validation.linaro.org/scheduler/job/1317079#L981
> 
> On Tue, 24 Mar 2020 at 18:24, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Tue, Mar 24, 2020 at 1:20 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > There is a place in the code where open-coded version of list entry accessors
> > > list_last_entry() is used.
> > >
> > > Replace that with the standard macro.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > > ---
> > > v2: no change
> > >  drivers/base/dd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > > index efd0e4c16ba5..27a4d51b5bba 100644
> > > --- a/drivers/base/dd.c
> > > +++ b/drivers/base/dd.c
> > > @@ -1226,7 +1226,7 @@ void driver_detach(struct device_driver *drv)
> > >                         spin_unlock(&drv->p->klist_devices.k_lock);
> > >                         break;
> > >                 }
> > > -               dev_prv = list_entry(drv->p->klist_devices.k_list.prev,
> > > +               dev_prv = list_last_entry(&drv->p->klist_devices.k_list,
> > >                                      struct device_private,
> > >                                      knode_driver.n_node);
> > >                 dev = dev_prv->device;
> 
> metadata:
>   git branch: master
>   git repo: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>   git describe: next-20200327
>   kernel-config:
> https://builds.tuxbuild.com/nqmmxorUbC1qTWp42iEKjQ/kernel.config
> 

And you bisected the warning to this patch?  If you revert it, does it
go away?

confused,

greg k-h

Re: Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Sudeep Holla]

On Fri, Mar 27, 2020 at 07:40:25PM +0000, Robin Murphy wrote:
> On 2020-03-27 5:56 pm, Naresh Kamboju wrote:
> > The kernel warning noticed on arm64 juno-r2 device running linux
> > next-20200326 and next-20200327
>
> I suspect this is the correct expected behaviour manifesting. If you're
> using the upstream juno-r2.dts, the power domain being waited for here is
> provided by SCPI, however unless you're using an SCP firmware from at least
> 3 years ago you won't actually have SCPI since they switched it to the newer
> SCMI protocol, which is not yet supported upstream for Juno. See what
> happened earlier in the log:
>
> [    2.741206] scpi_protocol scpi: incorrect or no SCP firmware found
> [    2.747586] scpi_protocol: probe of scpi failed with error -110
>
> Thus this is the "waiting for a dependency which will never appear" case,
> for which I assume the warning is intentional,

Is that the case ?

Previously we used to get the warning:
"amba xx: ignoring dependency for device, assuming no driver"

Now we have the kernel warning in addition to the above.

> since the system is essentially broken (i.e. the hardware/firmware doesn't
> actually match what the DT describes).
>

Not sure if we can term it as "essentially broken". Definitely not 100%
functional but not broken if the situation like on Juno where SCP firmware
is fundamental for all OSPM but not essential for boot and other minimum
set of functionality.

Either way I am not against the warning, just wanted to get certain things
clarified myself.

--
Regards,
Sudeep

Re: Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Andy Shevchenko]

On Mon, Mar 30, 2020 at 11:13:21AM +0100, Sudeep Holla wrote:
> On Fri, Mar 27, 2020 at 07:40:25PM +0000, Robin Murphy wrote:
> > On 2020-03-27 5:56 pm, Naresh Kamboju wrote:
> > > The kernel warning noticed on arm64 juno-r2 device running linux
> > > next-20200326 and next-20200327
> >
> > I suspect this is the correct expected behaviour manifesting. If you're
> > using the upstream juno-r2.dts, the power domain being waited for here is
> > provided by SCPI, however unless you're using an SCP firmware from at least
> > 3 years ago you won't actually have SCPI since they switched it to the newer
> > SCMI protocol, which is not yet supported upstream for Juno. See what
> > happened earlier in the log:
> >
> > [    2.741206] scpi_protocol scpi: incorrect or no SCP firmware found
> > [    2.747586] scpi_protocol: probe of scpi failed with error -110
> >
> > Thus this is the "waiting for a dependency which will never appear" case,
> > for which I assume the warning is intentional,
> 
> Is that the case ?
> 
> Previously we used to get the warning:
> "amba xx: ignoring dependency for device, assuming no driver"
> 
> Now we have the kernel warning in addition to the above.
> 
> > since the system is essentially broken (i.e. the hardware/firmware doesn't
> > actually match what the DT describes).
> >
> 
> Not sure if we can term it as "essentially broken". Definitely not 100%
> functional but not broken if the situation like on Juno where SCP firmware
> is fundamental for all OSPM but not essential for boot and other minimum
> set of functionality.
> 
> Either way I am not against the warning, just wanted to get certain things
> clarified myself.

How this warning related to the patch in the subject? Does revert of the patch
gives you no warning? (I will be very surprised).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Robin Murphy]

On 2020-03-30 11:13 am, Sudeep Holla wrote:
> On Fri, Mar 27, 2020 at 07:40:25PM +0000, Robin Murphy wrote:
>> On 2020-03-27 5:56 pm, Naresh Kamboju wrote:
>>> The kernel warning noticed on arm64 juno-r2 device running linux
>>> next-20200326 and next-20200327
>>
>> I suspect this is the correct expected behaviour manifesting. If you're
>> using the upstream juno-r2.dts, the power domain being waited for here is
>> provided by SCPI, however unless you're using an SCP firmware from at least
>> 3 years ago you won't actually have SCPI since they switched it to the newer
>> SCMI protocol, which is not yet supported upstream for Juno. See what
>> happened earlier in the log:
>>
>> [    2.741206] scpi_protocol scpi: incorrect or no SCP firmware found
>> [    2.747586] scpi_protocol: probe of scpi failed with error -110
>>
>> Thus this is the "waiting for a dependency which will never appear" case,
>> for which I assume the warning is intentional,
> 
> Is that the case ?
> 
> Previously we used to get the warning:
> "amba xx: ignoring dependency for device, assuming no driver"
> 
> Now we have the kernel warning in addition to the above.

AFAICS the difference is down to whether deferred_probe_timeout has 
expired or not - I'm not familiar enough with this code to know 
*exactly* what the difference is supposed to represent, nor which change 
has actually pushed the Juno case from one state to the other (other 
than it almost certainly can't be $SUBJECT - if this series is to blame 
at all I'd assume it would be down to patch #1/3, but there's a bunch of 
other rework previously queued in -next that is probably also interacting)

>> since the system is essentially broken (i.e. the hardware/firmware doesn't
>> actually match what the DT describes).
>>
> 
> Not sure if we can term it as "essentially broken". Definitely not 100%
> functional but not broken if the situation like on Juno where SCP firmware
> is fundamental for all OSPM but not essential for boot and other minimum
> set of functionality.

It's "broken" in the sense that the underlying system is *not* the 
system described in the DT. Yes, all the parts that still happen to line 
up will mostly still function OK, but those that don't will 
fundamentally not work as the kernel has been told to expect. I'm not 
sure what you prefer to call "not working as the kernel expects", but I 
call it "broken" ;)

Robin.

Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Andy Shevchenko]

On Mon, Mar 30, 2020 at 3:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
> On 2020-03-30 11:13 am, Sudeep Holla wrote:
> > On Fri, Mar 27, 2020 at 07:40:25PM +0000, Robin Murphy wrote:

...

> AFAICS the difference is down to whether deferred_probe_timeout has
> expired or not - I'm not familiar enough with this code to know
> *exactly* what the difference is supposed to represent, nor which change
> has actually pushed the Juno case from one state to the other (other
> than it almost certainly can't be $SUBJECT - if this series is to blame
> at all I'd assume it would be down to patch #1/3, but there's a bunch of
> other rework previously queued in -next that is probably also interacting)

JFYI: patch #1/3 wasn't applied.


-- 
With Best Regards,
Andy Shevchenko

Re: Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Sudeep Holla]

On Mon, Mar 30, 2020 at 01:43:40PM +0300, Andy Shevchenko wrote:

[...]

>
> How this warning related to the patch in the subject? Does revert of the patch
> gives you no warning? (I will be very surprised).
>

I did a quick check reverting the $subject patch and it doesn't remove
extra warning. Sorry for the noise, I assumed so hastily, I was wrong.

--
Regards,
Sudeep

Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Sudeep Holla]

On Mon, Mar 30, 2020 at 01:45:32PM +0100, Robin Murphy wrote:
> On 2020-03-30 11:13 am, Sudeep Holla wrote:
> > On Fri, Mar 27, 2020 at 07:40:25PM +0000, Robin Murphy wrote:
> > > On 2020-03-27 5:56 pm, Naresh Kamboju wrote:
> > > > The kernel warning noticed on arm64 juno-r2 device running linux
> > > > next-20200326 and next-20200327
> > >
> > > I suspect this is the correct expected behaviour manifesting. If you're
> > > using the upstream juno-r2.dts, the power domain being waited for here is
> > > provided by SCPI, however unless you're using an SCP firmware from at least
> > > 3 years ago you won't actually have SCPI since they switched it to the newer
> > > SCMI protocol, which is not yet supported upstream for Juno. See what
> > > happened earlier in the log:
> > >
> > > [    2.741206] scpi_protocol scpi: incorrect or no SCP firmware found
> > > [    2.747586] scpi_protocol: probe of scpi failed with error -110
> > >
> > > Thus this is the "waiting for a dependency which will never appear" case,
> > > for which I assume the warning is intentional,
> >
> > Is that the case ?
> >
> > Previously we used to get the warning:
> > "amba xx: ignoring dependency for device, assuming no driver"
> >
> > Now we have the kernel warning in addition to the above.
>
> AFAICS the difference is down to whether deferred_probe_timeout has expired
> or not - I'm not familiar enough with this code to know *exactly* what the
> difference is supposed to represent, nor which change has actually pushed
> the Juno case from one state to the other

Me either

> (other than it almost certainly
> can't be $SUBJECT - if this series is to blame at all I'd assume it would be
> down to patch #1/3, but there's a bunch of other rework previously queued in
> -next that is probably also interacting)
>

I agree, I was assuming one of the patch in series but again I may be wrong.

> > > since the system is essentially broken (i.e. the hardware/firmware doesn't
> > > actually match what the DT describes).
> > >
> >
> > Not sure if we can term it as "essentially broken". Definitely not 100%
> > functional but not broken if the situation like on Juno where SCP firmware
> > is fundamental for all OSPM but not essential for boot and other minimum
> > set of functionality.
>
> It's "broken" in the sense that the underlying system is *not* the system
> described in the DT. Yes, all the parts that still happen to line up will
> mostly still function OK, but those that don't will fundamentally not work
> as the kernel has been told to expect. I'm not sure what you prefer to call
> "not working as the kernel expects", but I call it "broken" ;)
>

I agree with you in context of Juno and it's firmware story.

But I also have another development use-case. Unless the DT becomes the
integral part of firmware from start, we can end up having DT with full
DT components(e.g. all SCMI users) while the firmware can add the
features incremental way. I agree this is not common for most of the
kernel developer but practical for few.

--
Regards,
Sudeep

Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[Robin Murphy]

On 2020-03-30 2:11 pm, Andy Shevchenko wrote:
> On Mon, Mar 30, 2020 at 3:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
>> On 2020-03-30 11:13 am, Sudeep Holla wrote:
>>> On Fri, Mar 27, 2020 at 07:40:25PM +0000, Robin Murphy wrote:
> 
> ...
> 
>> AFAICS the difference is down to whether deferred_probe_timeout has
>> expired or not - I'm not familiar enough with this code to know
>> *exactly* what the difference is supposed to represent, nor which change
>> has actually pushed the Juno case from one state to the other (other
>> than it almost certainly can't be $SUBJECT - if this series is to blame
>> at all I'd assume it would be down to patch #1/3, but there's a bunch of
>> other rework previously queued in -next that is probably also interacting)
> 
> JFYI: patch #1/3 wasn't applied.

OK, so if anyone's invested enough to want to investigate, it must be 
something in John's earlier changes here:

https://lore.kernel.org/lkml/20200225050828.56458-1-john.stultz@linaro.org/

Thanks,
Robin.

Re: [PATCH v2 3/3] driver core: Replace open-coded list_last_entry()

[John Stultz]

On Mon, Mar 30, 2020 at 6:30 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-03-30 2:11 pm, Andy Shevchenko wrote:
> > On Mon, Mar 30, 2020 at 3:49 PM Robin Murphy <robin.murphy@arm.com> wrote:
> >> On 2020-03-30 11:13 am, Sudeep Holla wrote:
> >>> On Fri, Mar 27, 2020 at 07:40:25PM +0000, Robin Murphy wrote:
> >
> > ...
> >
> >> AFAICS the difference is down to whether deferred_probe_timeout has
> >> expired or not - I'm not familiar enough with this code to know
> >> *exactly* what the difference is supposed to represent, nor which change
> >> has actually pushed the Juno case from one state to the other (other
> >> than it almost certainly can't be $SUBJECT - if this series is to blame
> >> at all I'd assume it would be down to patch #1/3, but there's a bunch of
> >> other rework previously queued in -next that is probably also interacting)
> >
> > JFYI: patch #1/3 wasn't applied.
>
> OK, so if anyone's invested enough to want to investigate, it must be
> something in John's earlier changes here:
>
> https://lore.kernel.org/lkml/20200225050828.56458-1-john.stultz@linaro.org/

Hey all,
  Sorry, I just got a heads up about this thread.

So yea, it looks like the change is due likely to the first patch in
my series. Previously, after initcall_done, (since
deferred_probe_timeout was -1 unless manually specified) if the driver
wasn't already loaded we'd print "ignoring dependency for device,
assuming no driver" and return ENODEV.

Now, if modules are enabled (as without modules enabled, I believe
you'd see the same behavior as previous), we wait 30 seconds  (for
userspace to load any posssible modules that meet that dependency) and
then the driver_deferred_probe_timeout fires and we print "deferred
probe timeout, ignoring dependency".

It seems the issue here is the first message was printed with
dev_warn() and the second with dev_WARN() which provides the scary
backtrace.

I think functionally as mentioned above, there's no real behavioral
change here. But please correct me if that's wrong.

Since we are more likely to see the second message now, maybe we
should make both print via dev_warn()?

I'll spin up a patch.

thanks
-john