* [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default @ 2020-03-21 21:03 ` Saravana Kannan 2020-03-27 10:25 ` Marek Szyprowski 0 siblings, 1 reply; 7+ messages in thread From: Saravana Kannan @ 2020-03-21 21:03 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki Cc: Saravana Kannan, Rob Herring, Frank Rowand, devicetree, kernel-team, linux-kernel Set fw_devlink to "permissive" behavior by default so that device links are automatically created (with DL_FLAG_SYNC_STATE_ONLY) by scanning the firmware. This ensures suppliers get their sync_state() calls only after all their consumers have probed successfully. Without this, suppliers will get their sync_state() calls at late_initcall_sync() even if their consuer Ideally, we'd want to set fw_devlink to "on" or "rpm" by default. But that needs more testing as it's known to break some corner case drivers/platforms. Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: devicetree@vger.kernel.org Signed-off-by: Saravana Kannan <saravanak@google.com> --- I think it's time to soak test this and see if anything fails or if anyone complains. Definitely not ready for 5.6. But pulling it in for 5.7 and having it go through all the rc testing would be helpful. I'm sure there'll be reports where some DT properties are ambiguously names and is breaking downstream or even some upstream platform. For example, a DT property like "nr-gpios" would have a dmesg log about parsing error because it looks like a valid "-gpios" DT binding. It'll be good to catch those case and fix them. Also, is there no way to look up current value of early_params? It'd be nice if there was a way to do that. -Saravana drivers/base/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 5e3cc1651c78..9fabf9749a06 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2345,7 +2345,7 @@ static int device_private_init(struct device *dev) return 0; } -static u32 fw_devlink_flags; +static u32 fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY; static int __init fw_devlink_setup(char *arg) { if (!arg) -- 2.25.1.696.g5e7596f4ac-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default 2020-03-21 21:03 ` [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default Saravana Kannan @ 2020-03-27 10:25 ` Marek Szyprowski 2020-03-27 11:30 ` Greg Kroah-Hartman 2020-03-27 15:21 ` Greg Kroah-Hartman 0 siblings, 2 replies; 7+ messages in thread From: Marek Szyprowski @ 2020-03-27 10:25 UTC (permalink / raw) To: Saravana Kannan, Greg Kroah-Hartman, Rafael J. Wysocki Cc: Rob Herring, Frank Rowand, devicetree, kernel-team, linux-kernel Hi, On 2020-03-21 22:03, Saravana Kannan wrote: > Set fw_devlink to "permissive" behavior by default so that device links > are automatically created (with DL_FLAG_SYNC_STATE_ONLY) by scanning the > firmware. > > This ensures suppliers get their sync_state() calls only after all their > consumers have probed successfully. Without this, suppliers will get > their sync_state() calls at late_initcall_sync() even if their consuer > > Ideally, we'd want to set fw_devlink to "on" or "rpm" by default. But > that needs more testing as it's known to break some corner case > drivers/platforms. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: devicetree@vger.kernel.org > Signed-off-by: Saravana Kannan <saravanak@google.com> This patch has just landed in linux-next 20200326. Sadly it breaks booting of the Raspberry Pi3b and Pi4 boards, either in 32bit or 64bit mode. There is no warning nor panic message, just a silent freeze. The last message shown on the earlycon is: [ 0.893217] Serial: 8250/16550 driver, 1 ports, IRQ sharing enabled > --- > > I think it's time to soak test this and see if anything fails or if > anyone complains. Definitely not ready for 5.6. But pulling it in for > 5.7 and having it go through all the rc testing would be helpful. > > I'm sure there'll be reports where some DT properties are ambiguously > names and is breaking downstream or even some upstream platform. For > example, a DT property like "nr-gpios" would have a dmesg log about > parsing error because it looks like a valid "-gpios" DT binding. It'll > be good to catch those case and fix them. > > Also, is there no way to look up current value of early_params? It'd be > nice if there was a way to do that. > > -Saravana > > drivers/base/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 5e3cc1651c78..9fabf9749a06 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -2345,7 +2345,7 @@ static int device_private_init(struct device *dev) > return 0; > } > > -static u32 fw_devlink_flags; > +static u32 fw_devlink_flags = DL_FLAG_SYNC_STATE_ONLY; > static int __init fw_devlink_setup(char *arg) > { > if (!arg) Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default 2020-03-27 10:25 ` Marek Szyprowski @ 2020-03-27 11:30 ` Greg Kroah-Hartman 2020-03-27 15:21 ` Greg Kroah-Hartman 1 sibling, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2020-03-27 11:30 UTC (permalink / raw) To: Marek Szyprowski Cc: Saravana Kannan, Rafael J. Wysocki, Rob Herring, Frank Rowand, devicetree, kernel-team, linux-kernel On Fri, Mar 27, 2020 at 11:25:48AM +0100, Marek Szyprowski wrote: > Hi, > > On 2020-03-21 22:03, Saravana Kannan wrote: > > Set fw_devlink to "permissive" behavior by default so that device links > > are automatically created (with DL_FLAG_SYNC_STATE_ONLY) by scanning the > > firmware. > > > > This ensures suppliers get their sync_state() calls only after all their > > consumers have probed successfully. Without this, suppliers will get > > their sync_state() calls at late_initcall_sync() even if their consuer > > > > Ideally, we'd want to set fw_devlink to "on" or "rpm" by default. But > > that needs more testing as it's known to break some corner case > > drivers/platforms. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Frank Rowand <frowand.list@gmail.com> > > Cc: devicetree@vger.kernel.org > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > This patch has just landed in linux-next 20200326. Sadly it breaks > booting of the Raspberry Pi3b and Pi4 boards, either in 32bit or 64bit > mode. There is no warning nor panic message, just a silent freeze. The > last message shown on the earlycon is: > > [ 0.893217] Serial: 8250/16550 driver, 1 ports, IRQ sharing enabled Ugh, not good. Saravana, mind if I revert this? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default 2020-03-27 10:25 ` Marek Szyprowski 2020-03-27 11:30 ` Greg Kroah-Hartman @ 2020-03-27 15:21 ` Greg Kroah-Hartman 2020-03-27 18:30 ` Saravana Kannan 1 sibling, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2020-03-27 15:21 UTC (permalink / raw) To: Marek Szyprowski Cc: Saravana Kannan, Rafael J. Wysocki, Rob Herring, Frank Rowand, devicetree, kernel-team, linux-kernel On Fri, Mar 27, 2020 at 11:25:48AM +0100, Marek Szyprowski wrote: > Hi, > > On 2020-03-21 22:03, Saravana Kannan wrote: > > Set fw_devlink to "permissive" behavior by default so that device links > > are automatically created (with DL_FLAG_SYNC_STATE_ONLY) by scanning the > > firmware. > > > > This ensures suppliers get their sync_state() calls only after all their > > consumers have probed successfully. Without this, suppliers will get > > their sync_state() calls at late_initcall_sync() even if their consuer > > > > Ideally, we'd want to set fw_devlink to "on" or "rpm" by default. But > > that needs more testing as it's known to break some corner case > > drivers/platforms. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Frank Rowand <frowand.list@gmail.com> > > Cc: devicetree@vger.kernel.org > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > This patch has just landed in linux-next 20200326. Sadly it breaks > booting of the Raspberry Pi3b and Pi4 boards, either in 32bit or 64bit > mode. There is no warning nor panic message, just a silent freeze. The > last message shown on the earlycon is: > > [ 0.893217] Serial: 8250/16550 driver, 1 ports, IRQ sharing enabled I've just reverted this for now. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default 2020-03-27 15:21 ` Greg Kroah-Hartman @ 2020-03-27 18:30 ` Saravana Kannan 2020-03-30 6:20 ` Marek Szyprowski 0 siblings, 1 reply; 7+ messages in thread From: Saravana Kannan @ 2020-03-27 18:30 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Marek Szyprowski, Rafael J. Wysocki, Rob Herring, Frank Rowand, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Android Kernel Team, LKML On Fri, Mar 27, 2020 at 8:21 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Fri, Mar 27, 2020 at 11:25:48AM +0100, Marek Szyprowski wrote: > > Hi, > > > > On 2020-03-21 22:03, Saravana Kannan wrote: > > > Set fw_devlink to "permissive" behavior by default so that device links > > > are automatically created (with DL_FLAG_SYNC_STATE_ONLY) by scanning the > > > firmware. > > > > > > This ensures suppliers get their sync_state() calls only after all their > > > consumers have probed successfully. Without this, suppliers will get > > > their sync_state() calls at late_initcall_sync() even if their consuer > > > > > > Ideally, we'd want to set fw_devlink to "on" or "rpm" by default. But > > > that needs more testing as it's known to break some corner case > > > drivers/platforms. > > > > > > Cc: Rob Herring <robh+dt@kernel.org> > > > Cc: Frank Rowand <frowand.list@gmail.com> > > > Cc: devicetree@vger.kernel.org > > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > > > This patch has just landed in linux-next 20200326. Sadly it breaks > > booting of the Raspberry Pi3b and Pi4 boards, either in 32bit or 64bit > > mode. There is no warning nor panic message, just a silent freeze. The > > last message shown on the earlycon is: > > > > [ 0.893217] Serial: 8250/16550 driver, 1 ports, IRQ sharing enabled Marek, Any chance you could get me a stack trace for when it's stuck? That'd be super helpful and I'd really appreciate it. Is it working fine on other variants of Raspberry? > > I've just reverted this for now. > Greg, I have no problem with reverting this. If there's any other tree/branch you can put this on where it could get more testing and reporting of issues, that'd be great. -Saravana ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default 2020-03-27 18:30 ` Saravana Kannan @ 2020-03-30 6:20 ` Marek Szyprowski 2020-03-30 18:41 ` Saravana Kannan 0 siblings, 1 reply; 7+ messages in thread From: Marek Szyprowski @ 2020-03-30 6:20 UTC (permalink / raw) To: Saravana Kannan, Greg Kroah-Hartman Cc: Rafael J. Wysocki, Rob Herring, Frank Rowand, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Android Kernel Team, LKML Hi On 2020-03-27 19:30, Saravana Kannan wrote: > On Fri, Mar 27, 2020 at 8:21 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: >> On Fri, Mar 27, 2020 at 11:25:48AM +0100, Marek Szyprowski wrote: >>> On 2020-03-21 22:03, Saravana Kannan wrote: >>>> Set fw_devlink to "permissive" behavior by default so that device links >>>> are automatically created (with DL_FLAG_SYNC_STATE_ONLY) by scanning the >>>> firmware. >>>> >>>> This ensures suppliers get their sync_state() calls only after all their >>>> consumers have probed successfully. Without this, suppliers will get >>>> their sync_state() calls at late_initcall_sync() even if their consuer >>>> >>>> Ideally, we'd want to set fw_devlink to "on" or "rpm" by default. But >>>> that needs more testing as it's known to break some corner case >>>> drivers/platforms. >>>> >>>> Cc: Rob Herring <robh+dt@kernel.org> >>>> Cc: Frank Rowand <frowand.list@gmail.com> >>>> Cc: devicetree@vger.kernel.org >>>> Signed-off-by: Saravana Kannan <saravanak@google.com> >>> This patch has just landed in linux-next 20200326. Sadly it breaks >>> booting of the Raspberry Pi3b and Pi4 boards, either in 32bit or 64bit >>> mode. There is no warning nor panic message, just a silent freeze. The >>> last message shown on the earlycon is: >>> >>> [ 0.893217] Serial: 8250/16550 driver, 1 ports, IRQ sharing enabled > Marek, > > Any chance you could get me a stack trace for when it's stuck? That'd > be super helpful and I'd really appreciate it. Is it working fine on > other variants of Raspberry? I have no access to other variants of Raspberry board. The issue seems to be related to bcm2835aux_serial_driver. I've added "initcall_debug" and "ignore_loglevel" to kernel cmdline and I got the following log: [ 4.595353] calling exar_pci_driver_init+0x0/0x30 @ 1 [ 4.600597] initcall exar_pci_driver_init+0x0/0x30 returned 0 after 44 usecs [ 4.607747] calling bcm2835aux_serial_driver_init+0x0/0x28 @ 1 The with some debug printk calls I've found that the clock lookup fails with -517 (-EPROBE_DEFER) in bcm2835aux_serial_driver: https://elixir.bootlin.com/linux/v5.6/source/drivers/tty/serial/8250/8250_bcm2835aux.c#L52 Without this patch, the lookup works fine. Please let me know if you need more information. The kernel cmdline I've use is: "8250.nr_uarts=1 console=ttyS0,115200n8 earlycon=uart8250,mmio32,0x3f215040 root=/dev/mmcblk0p2 rootwait rw", kernel is compiled with bcm2835_defconfig, booted on Raspberry Pi3b+ with arch/arm/boot/dts/bcm2837-rpi-3-b.dtb Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default 2020-03-30 6:20 ` Marek Szyprowski @ 2020-03-30 18:41 ` Saravana Kannan 0 siblings, 0 replies; 7+ messages in thread From: Saravana Kannan @ 2020-03-30 18:41 UTC (permalink / raw) To: Marek Szyprowski Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Rob Herring, Frank Rowand, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Android Kernel Team, LKML On Sun, Mar 29, 2020 at 11:20 PM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi > > On 2020-03-27 19:30, Saravana Kannan wrote: > > On Fri, Mar 27, 2020 at 8:21 AM Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > >> On Fri, Mar 27, 2020 at 11:25:48AM +0100, Marek Szyprowski wrote: > >>> On 2020-03-21 22:03, Saravana Kannan wrote: > >>>> Set fw_devlink to "permissive" behavior by default so that device links > >>>> are automatically created (with DL_FLAG_SYNC_STATE_ONLY) by scanning the > >>>> firmware. > >>>> > >>>> This ensures suppliers get their sync_state() calls only after all their > >>>> consumers have probed successfully. Without this, suppliers will get > >>>> their sync_state() calls at late_initcall_sync() even if their consuer > >>>> > >>>> Ideally, we'd want to set fw_devlink to "on" or "rpm" by default. But > >>>> that needs more testing as it's known to break some corner case > >>>> drivers/platforms. > >>>> > >>>> Cc: Rob Herring <robh+dt@kernel.org> > >>>> Cc: Frank Rowand <frowand.list@gmail.com> > >>>> Cc: devicetree@vger.kernel.org > >>>> Signed-off-by: Saravana Kannan <saravanak@google.com> > >>> This patch has just landed in linux-next 20200326. Sadly it breaks > >>> booting of the Raspberry Pi3b and Pi4 boards, either in 32bit or 64bit > >>> mode. There is no warning nor panic message, just a silent freeze. The > >>> last message shown on the earlycon is: > >>> > >>> [ 0.893217] Serial: 8250/16550 driver, 1 ports, IRQ sharing enabled > > Marek, > > > > Any chance you could get me a stack trace for when it's stuck? That'd > > be super helpful and I'd really appreciate it. Is it working fine on > > other variants of Raspberry? > > I have no access to other variants of Raspberry board. > > The issue seems to be related to bcm2835aux_serial_driver. I've added > "initcall_debug" and "ignore_loglevel" to kernel cmdline and I got the > following log: > > [ 4.595353] calling exar_pci_driver_init+0x0/0x30 @ 1 > [ 4.600597] initcall exar_pci_driver_init+0x0/0x30 returned 0 after > 44 usecs > [ 4.607747] calling bcm2835aux_serial_driver_init+0x0/0x28 @ 1 > > The with some debug printk calls I've found that the clock lookup fails > with -517 (-EPROBE_DEFER) in bcm2835aux_serial_driver: > https://elixir.bootlin.com/linux/v5.6/source/drivers/tty/serial/8250/8250_bcm2835aux.c#L52 > > Without this patch, the lookup works fine. > > Please let me know if you need more information. The kernel cmdline I've > use is: "8250.nr_uarts=1 console=ttyS0,115200n8 > earlycon=uart8250,mmio32,0x3f215040 root=/dev/mmcblk0p2 rootwait rw", > kernel is compiled with bcm2835_defconfig, booted on Raspberry Pi3b+ > with arch/arm/boot/dts/bcm2837-rpi-3-b.dtb Thanks for the details! I think it gave me an idea of what might be going wrong here. -Saravana ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-03-30 18:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20200327102554eucas1p1f848633a39f8e158472506b84877f98c@eucas1p1.samsung.com>
2020-03-21 21:03 ` [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default Saravana Kannan
2020-03-27 10:25 ` Marek Szyprowski
2020-03-27 11:30 ` Greg Kroah-Hartman
2020-03-27 15:21 ` Greg Kroah-Hartman
2020-03-27 18:30 ` Saravana Kannan
2020-03-30 6:20 ` Marek Szyprowski
2020-03-30 18:41 ` Saravana Kannan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).