| Message ID | 20200324122023.9649-3-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v2,1/3] driver core: Break infinite loop when deferred probe can't be satisfied | expand |
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; > -- > 2.25.1 >
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
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 > >
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
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
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).
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.
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.
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
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
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.
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
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;
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> --- v2: no change drivers/base/dd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)