2020-03-21 21:06:36

by Saravana Kannan

[permalink] [raw]
Subject: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default

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 <[email protected]>
Cc: Frank Rowand <[email protected]>
Cc: [email protected]
Signed-off-by: Saravana Kannan <[email protected]>
---

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


2020-03-27 10:26:46

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default

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 <[email protected]>
> Cc: Frank Rowand <[email protected]>
> Cc: [email protected]
> Signed-off-by: Saravana Kannan <[email protected]>

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

2020-03-27 11:31:25

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default

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 <[email protected]>
> > Cc: Frank Rowand <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Saravana Kannan <[email protected]>
>
> 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

2020-03-27 15:22:30

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default

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 <[email protected]>
> > Cc: Frank Rowand <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Saravana Kannan <[email protected]>
>
> 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

2020-03-27 18:32:12

by Saravana Kannan

[permalink] [raw]
Subject: Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default

On Fri, Mar 27, 2020 at 8:21 AM Greg Kroah-Hartman
<[email protected]> 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 <[email protected]>
> > > Cc: Frank Rowand <[email protected]>
> > > Cc: [email protected]
> > > Signed-off-by: Saravana Kannan <[email protected]>
> >
> > 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

2020-03-30 06:21:33

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default

Hi

On 2020-03-27 19:30, Saravana Kannan wrote:
> On Fri, Mar 27, 2020 at 8:21 AM Greg Kroah-Hartman
> <[email protected]> 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 <[email protected]>
>>>> Cc: Frank Rowand <[email protected]>
>>>> Cc: [email protected]
>>>> Signed-off-by: Saravana Kannan <[email protected]>
>>> 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

2020-03-30 18:43:42

by Saravana Kannan

[permalink] [raw]
Subject: Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default

On Sun, Mar 29, 2020 at 11:20 PM Marek Szyprowski
<[email protected]> wrote:
>
> Hi
>
> On 2020-03-27 19:30, Saravana Kannan wrote:
> > On Fri, Mar 27, 2020 at 8:21 AM Greg Kroah-Hartman
> > <[email protected]> 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 <[email protected]>
> >>>> Cc: Frank Rowand <[email protected]>
> >>>> Cc: [email protected]
> >>>> Signed-off-by: Saravana Kannan <[email protected]>
> >>> 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

2020-03-31 02:30:35

by Saravana Kannan

[permalink] [raw]
Subject: [PATCH v1] driver core: Fix handling of fw_devlink=permissive

When commit 8375e74f2bca ("driver core: Add fw_devlink kernel
commandline option") added fw_devlink, it didn't implement "permissive"
mode correctly.

That commit got the device links flags correct to make sure unprobed
suppliers don't block the probing of a consumer. However, if a consumer
is waiting for mandatory suppliers to register, that could still block a
consumer from probing.

This commit fixes that by making sure in permissive mode, all suppliers
to a consumer are treated as a optional suppliers. So, even if a
consumer is waiting for suppliers to register and link itself (using the
DL_FLAG_SYNC_STATE_ONLY flag) to the supplier, the consumer is never
blocked from probing.

Fixes: 8375e74f2bca ("driver core: Add fw_devlink kernel commandline option")
Reported-by: Marek Szyprowski <[email protected]>
Signed-off-by: Saravana Kannan <[email protected]>
---
Hi Marek,

If you pull in this patch and then add back in my patch that created the
boot problem for you, can you see if that fixes the boot issue for you?

Thanks,
Saravana

drivers/base/core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5e3cc1651c78..1be26a7f0866 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2370,6 +2370,11 @@ u32 fw_devlink_get_flags(void)
return fw_devlink_flags;
}

+static bool fw_devlink_is_permissive(void)
+{
+ return fw_devlink_flags == DL_FLAG_SYNC_STATE_ONLY;
+}
+
/**
* device_add - add device to device hierarchy.
* @dev: device.
@@ -2524,7 +2529,7 @@ int device_add(struct device *dev)
if (fw_devlink_flags && is_fwnode_dev &&
fwnode_has_op(dev->fwnode, add_links)) {
fw_ret = fwnode_call_int_op(dev->fwnode, add_links, dev);
- if (fw_ret == -ENODEV)
+ if (fw_ret == -ENODEV && !fw_devlink_is_permissive())
device_link_wait_for_mandatory_supplier(dev);
else if (fw_ret)
device_link_wait_for_optional_supplier(dev);
--
2.26.0.rc2.310.g2932bb562d-goog

2020-03-31 05:44:15

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH v1] driver core: Fix handling of fw_devlink=permissive

Hi,

On 2020-03-31 04:28, Saravana Kannan wrote:
> When commit 8375e74f2bca ("driver core: Add fw_devlink kernel
> commandline option") added fw_devlink, it didn't implement "permissive"
> mode correctly.
>
> That commit got the device links flags correct to make sure unprobed
> suppliers don't block the probing of a consumer. However, if a consumer
> is waiting for mandatory suppliers to register, that could still block a
> consumer from probing.
>
> This commit fixes that by making sure in permissive mode, all suppliers
> to a consumer are treated as a optional suppliers. So, even if a
> consumer is waiting for suppliers to register and link itself (using the
> DL_FLAG_SYNC_STATE_ONLY flag) to the supplier, the consumer is never
> blocked from probing.
>
> Fixes: 8375e74f2bca ("driver core: Add fw_devlink kernel commandline option")
> Reported-by: Marek Szyprowski <[email protected]>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> Hi Marek,
>
> If you pull in this patch and then add back in my patch that created the
> boot problem for you, can you see if that fixes the boot issue for you?

Indeed, this fixes booting on my Raspberry Pi3/4 boards with linux
next-20200327. Thanks! :)

Tested-by: Marek Szyprowski <[email protected]>

>
> Thanks,
> Saravana
>
> drivers/base/core.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 5e3cc1651c78..1be26a7f0866 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2370,6 +2370,11 @@ u32 fw_devlink_get_flags(void)
> return fw_devlink_flags;
> }
>
> +static bool fw_devlink_is_permissive(void)
> +{
> + return fw_devlink_flags == DL_FLAG_SYNC_STATE_ONLY;
> +}
> +
> /**
> * device_add - add device to device hierarchy.
> * @dev: device.
> @@ -2524,7 +2529,7 @@ int device_add(struct device *dev)
> if (fw_devlink_flags && is_fwnode_dev &&
> fwnode_has_op(dev->fwnode, add_links)) {
> fw_ret = fwnode_call_int_op(dev->fwnode, add_links, dev);
> - if (fw_ret == -ENODEV)
> + if (fw_ret == -ENODEV && !fw_devlink_is_permissive())
> device_link_wait_for_mandatory_supplier(dev);
> else if (fw_ret)
> device_link_wait_for_optional_supplier(dev);

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2020-03-31 06:20:43

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v1] driver core: Fix handling of fw_devlink=permissive

On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
<[email protected]> wrote:
>
> Hi,
>
> On 2020-03-31 04:28, Saravana Kannan wrote:
> > When commit 8375e74f2bca ("driver core: Add fw_devlink kernel
> > commandline option") added fw_devlink, it didn't implement "permissive"
> > mode correctly.
> >
> > That commit got the device links flags correct to make sure unprobed
> > suppliers don't block the probing of a consumer. However, if a consumer
> > is waiting for mandatory suppliers to register, that could still block a
> > consumer from probing.
> >
> > This commit fixes that by making sure in permissive mode, all suppliers
> > to a consumer are treated as a optional suppliers. So, even if a
> > consumer is waiting for suppliers to register and link itself (using the
> > DL_FLAG_SYNC_STATE_ONLY flag) to the supplier, the consumer is never
> > blocked from probing.
> >
> > Fixes: 8375e74f2bca ("driver core: Add fw_devlink kernel commandline option")
> > Reported-by: Marek Szyprowski <[email protected]>
> > Signed-off-by: Saravana Kannan <[email protected]>
> > ---
> > Hi Marek,
> >
> > If you pull in this patch and then add back in my patch that created the
> > boot problem for you, can you see if that fixes the boot issue for you?
>
> Indeed, this fixes booting on my Raspberry Pi3/4 boards with linux
> next-20200327. Thanks! :)

Hi Marek,

Thanks for testing, but I'm not able to find the tag next-20200327. I
can only find next-20200326 and next-20200330. I was just trying to
make sure that next-20200327 doesn't have the revert Greg did. I'm
guessing you meant next-20200326?

> Tested-by: Marek Szyprowski <[email protected]>

Thanks!

Greg,

Can you pull in my fix and then revert the revert?

Thanks,
Saravana

2020-03-31 07:09:20

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH v1] driver core: Fix handling of fw_devlink=permissive

Hi,

On 2020-03-31 08:18, Saravana Kannan wrote:
> On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
> <[email protected]> wrote:
>> Hi,
>>
>> On 2020-03-31 04:28, Saravana Kannan wrote:
>>> When commit 8375e74f2bca ("driver core: Add fw_devlink kernel
>>> commandline option") added fw_devlink, it didn't implement "permissive"
>>> mode correctly.
>>>
>>> That commit got the device links flags correct to make sure unprobed
>>> suppliers don't block the probing of a consumer. However, if a consumer
>>> is waiting for mandatory suppliers to register, that could still block a
>>> consumer from probing.
>>>
>>> This commit fixes that by making sure in permissive mode, all suppliers
>>> to a consumer are treated as a optional suppliers. So, even if a
>>> consumer is waiting for suppliers to register and link itself (using the
>>> DL_FLAG_SYNC_STATE_ONLY flag) to the supplier, the consumer is never
>>> blocked from probing.
>>>
>>> Fixes: 8375e74f2bca ("driver core: Add fw_devlink kernel commandline option")
>>> Reported-by: Marek Szyprowski <[email protected]>
>>> Signed-off-by: Saravana Kannan <[email protected]>
>>> ---
>>> Hi Marek,
>>>
>>> If you pull in this patch and then add back in my patch that created the
>>> boot problem for you, can you see if that fixes the boot issue for you?
>> Indeed, this fixes booting on my Raspberry Pi3/4 boards with linux
>> next-20200327. Thanks! :)
> Hi Marek,
>
> Thanks for testing, but I'm not able to find the tag next-20200327.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?h=next-20200327

> I
> can only find next-20200326 and next-20200330. I was just trying to
> make sure that next-20200327 doesn't have the revert Greg did. I'm
> guessing you meant next-20200326?

I did my test on next-20200327.

$ git log --oneline next-20200327 -- drivers/base/core.c
d3ef0815f924 Merge remote-tracking branch 'driver-core/driver-core-next'
c442a0d18744 driver core: Set fw_devlink to "permissive" behavior by default
4dbe191c046e driver core: Add device links from fwnode only for the
primary device
1d3435793123 Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
9a2dd570591e Merge 5.6-rc5 into driver-core-next
9211f0a6a91a driver core: fw_devlink_flags can be static
68464d79015a driver core: Add missing annotation for
device_links_write_lock()
ab7789c5174c driver core: Add missing annotation for
device_links_read_lock()
8375e74f2bca driver core: Add fw_devlink kernel commandline option
^C

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2020-03-31 07:30:42

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v1] driver core: Fix handling of fw_devlink=permissive

On Mon, Mar 30, 2020 at 11:18:01PM -0700, Saravana Kannan wrote:
> On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
> <[email protected]> wrote:
> >
> > Hi,
> >
> > On 2020-03-31 04:28, Saravana Kannan wrote:
> > > When commit 8375e74f2bca ("driver core: Add fw_devlink kernel
> > > commandline option") added fw_devlink, it didn't implement "permissive"
> > > mode correctly.
> > >
> > > That commit got the device links flags correct to make sure unprobed
> > > suppliers don't block the probing of a consumer. However, if a consumer
> > > is waiting for mandatory suppliers to register, that could still block a
> > > consumer from probing.
> > >
> > > This commit fixes that by making sure in permissive mode, all suppliers
> > > to a consumer are treated as a optional suppliers. So, even if a
> > > consumer is waiting for suppliers to register and link itself (using the
> > > DL_FLAG_SYNC_STATE_ONLY flag) to the supplier, the consumer is never
> > > blocked from probing.
> > >
> > > Fixes: 8375e74f2bca ("driver core: Add fw_devlink kernel commandline option")
> > > Reported-by: Marek Szyprowski <[email protected]>
> > > Signed-off-by: Saravana Kannan <[email protected]>
> > > ---
> > > Hi Marek,
> > >
> > > If you pull in this patch and then add back in my patch that created the
> > > boot problem for you, can you see if that fixes the boot issue for you?
> >
> > Indeed, this fixes booting on my Raspberry Pi3/4 boards with linux
> > next-20200327. Thanks! :)
>
> Hi Marek,
>
> Thanks for testing, but I'm not able to find the tag next-20200327. I
> can only find next-20200326 and next-20200330. I was just trying to
> make sure that next-20200327 doesn't have the revert Greg did. I'm
> guessing you meant next-20200326?
>
> > Tested-by: Marek Szyprowski <[email protected]>
>
> Thanks!
>
> Greg,
>
> Can you pull in my fix and then revert the revert?

After 5.7-rc1 is out I will, thanks.

greg k-h

2020-04-16 20:44:02

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v1] driver core: Fix handling of fw_devlink=permissive

On Tue, Mar 31, 2020 at 12:29 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Mon, Mar 30, 2020 at 11:18:01PM -0700, Saravana Kannan wrote:
> > On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
> > <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On 2020-03-31 04:28, Saravana Kannan wrote:
> > > > When commit 8375e74f2bca ("driver core: Add fw_devlink kernel
> > > > commandline option") added fw_devlink, it didn't implement "permissive"
> > > > mode correctly.
> > > >
> > > > That commit got the device links flags correct to make sure unprobed
> > > > suppliers don't block the probing of a consumer. However, if a consumer
> > > > is waiting for mandatory suppliers to register, that could still block a
> > > > consumer from probing.
> > > >
> > > > This commit fixes that by making sure in permissive mode, all suppliers
> > > > to a consumer are treated as a optional suppliers. So, even if a
> > > > consumer is waiting for suppliers to register and link itself (using the
> > > > DL_FLAG_SYNC_STATE_ONLY flag) to the supplier, the consumer is never
> > > > blocked from probing.
> > > >
> > > > Fixes: 8375e74f2bca ("driver core: Add fw_devlink kernel commandline option")
> > > > Reported-by: Marek Szyprowski <[email protected]>
> > > > Signed-off-by: Saravana Kannan <[email protected]>
> > > > ---
> > > > Hi Marek,
> > > >
> > > > If you pull in this patch and then add back in my patch that created the
> > > > boot problem for you, can you see if that fixes the boot issue for you?
> > >
> > > Indeed, this fixes booting on my Raspberry Pi3/4 boards with linux
> > > next-20200327. Thanks! :)
> >
> > Hi Marek,
> >
> > Thanks for testing, but I'm not able to find the tag next-20200327. I
> > can only find next-20200326 and next-20200330. I was just trying to
> > make sure that next-20200327 doesn't have the revert Greg did. I'm
> > guessing you meant next-20200326?
> >
> > > Tested-by: Marek Szyprowski <[email protected]>
> >
> > Thanks!
> >
> > Greg,
> >
> > Can you pull in my fix and then revert the revert?
>
> After 5.7-rc1 is out I will, thanks.

Hi Greg,

Just to clarify, this patch is a bug fix and I think this patch should
go into all the stable branches that support fw_devlink.

The only risky change that you needed to wait on for 5.7-rc1 is the
patch [1] that sets fw_devlink=permissive by default. Well, a revert
of the revert of [1].

[1] - https://lore.kernel.org/lkml/[email protected]/

Thanks,
Saravana

2020-04-28 15:54:25

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v1] driver core: Fix handling of fw_devlink=permissive

On Thu, Apr 16, 2020 at 11:25:47AM -0700, Saravana Kannan wrote:
> On Tue, Mar 31, 2020 at 12:29 AM Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > On Mon, Mar 30, 2020 at 11:18:01PM -0700, Saravana Kannan wrote:
> > > On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
> > > <[email protected]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On 2020-03-31 04:28, Saravana Kannan wrote:
> > > > > When commit 8375e74f2bca ("driver core: Add fw_devlink kernel
> > > > > commandline option") added fw_devlink, it didn't implement "permissive"
> > > > > mode correctly.
> > > > >
> > > > > That commit got the device links flags correct to make sure unprobed
> > > > > suppliers don't block the probing of a consumer. However, if a consumer
> > > > > is waiting for mandatory suppliers to register, that could still block a
> > > > > consumer from probing.
> > > > >
> > > > > This commit fixes that by making sure in permissive mode, all suppliers
> > > > > to a consumer are treated as a optional suppliers. So, even if a
> > > > > consumer is waiting for suppliers to register and link itself (using the
> > > > > DL_FLAG_SYNC_STATE_ONLY flag) to the supplier, the consumer is never
> > > > > blocked from probing.
> > > > >
> > > > > Fixes: 8375e74f2bca ("driver core: Add fw_devlink kernel commandline option")
> > > > > Reported-by: Marek Szyprowski <[email protected]>
> > > > > Signed-off-by: Saravana Kannan <[email protected]>
> > > > > ---
> > > > > Hi Marek,
> > > > >
> > > > > If you pull in this patch and then add back in my patch that created the
> > > > > boot problem for you, can you see if that fixes the boot issue for you?
> > > >
> > > > Indeed, this fixes booting on my Raspberry Pi3/4 boards with linux
> > > > next-20200327. Thanks! :)
> > >
> > > Hi Marek,
> > >
> > > Thanks for testing, but I'm not able to find the tag next-20200327. I
> > > can only find next-20200326 and next-20200330. I was just trying to
> > > make sure that next-20200327 doesn't have the revert Greg did. I'm
> > > guessing you meant next-20200326?
> > >
> > > > Tested-by: Marek Szyprowski <[email protected]>
> > >
> > > Thanks!
> > >
> > > Greg,
> > >
> > > Can you pull in my fix and then revert the revert?
> >
> > After 5.7-rc1 is out I will, thanks.
>
> Hi Greg,
>
> Just to clarify, this patch is a bug fix and I think this patch should
> go into all the stable branches that support fw_devlink.
>
> The only risky change that you needed to wait on for 5.7-rc1 is the
> patch [1] that sets fw_devlink=permissive by default. Well, a revert
> of the revert of [1].
>
> [1] - https://lore.kernel.org/lkml/[email protected]/

I don't understand, what kernels should this go back to? Your "Fixes:"
line just shows for a 5.7-rc1 patch, nothing older.

thanks,

greg k-h

2020-04-28 18:30:42

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH v1] driver core: Fix handling of fw_devlink=permissive

On Tue, Apr 28, 2020 at 8:52 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Thu, Apr 16, 2020 at 11:25:47AM -0700, Saravana Kannan wrote:
> > On Tue, Mar 31, 2020 at 12:29 AM Greg Kroah-Hartman
> > <[email protected]> wrote:
> > >
> > > On Mon, Mar 30, 2020 at 11:18:01PM -0700, Saravana Kannan wrote:
> > > > On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
> > > > <[email protected]> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On 2020-03-31 04:28, Saravana Kannan wrote:
> > > > > > When commit 8375e74f2bca ("driver core: Add fw_devlink kernel
> > > > > > commandline option") added fw_devlink, it didn't implement "permissive"
> > > > > > mode correctly.
> > > > > >
> > > > > > That commit got the device links flags correct to make sure unprobed
> > > > > > suppliers don't block the probing of a consumer. However, if a consumer
> > > > > > is waiting for mandatory suppliers to register, that could still block a
> > > > > > consumer from probing.
> > > > > >
> > > > > > This commit fixes that by making sure in permissive mode, all suppliers
> > > > > > to a consumer are treated as a optional suppliers. So, even if a
> > > > > > consumer is waiting for suppliers to register and link itself (using the
> > > > > > DL_FLAG_SYNC_STATE_ONLY flag) to the supplier, the consumer is never
> > > > > > blocked from probing.
> > > > > >
> > > > > > Fixes: 8375e74f2bca ("driver core: Add fw_devlink kernel commandline option")
> > > > > > Reported-by: Marek Szyprowski <[email protected]>
> > > > > > Signed-off-by: Saravana Kannan <[email protected]>
> > > > > > ---
> > > > > > Hi Marek,
> > > > > >
> > > > > > If you pull in this patch and then add back in my patch that created the
> > > > > > boot problem for you, can you see if that fixes the boot issue for you?
> > > > >
> > > > > Indeed, this fixes booting on my Raspberry Pi3/4 boards with linux
> > > > > next-20200327. Thanks! :)
> > > >
> > > > Hi Marek,
> > > >
> > > > Thanks for testing, but I'm not able to find the tag next-20200327. I
> > > > can only find next-20200326 and next-20200330. I was just trying to
> > > > make sure that next-20200327 doesn't have the revert Greg did. I'm
> > > > guessing you meant next-20200326?
> > > >
> > > > > Tested-by: Marek Szyprowski <[email protected]>
> > > >
> > > > Thanks!
> > > >
> > > > Greg,
> > > >
> > > > Can you pull in my fix and then revert the revert?
> > >
> > > After 5.7-rc1 is out I will, thanks.
> >
> > Hi Greg,
> >
> > Just to clarify, this patch is a bug fix and I think this patch should
> > go into all the stable branches that support fw_devlink.
> >
> > The only risky change that you needed to wait on for 5.7-rc1 is the
> > patch [1] that sets fw_devlink=permissive by default. Well, a revert
> > of the revert of [1].
> >
> > [1] - https://lore.kernel.org/lkml/[email protected]/
>
> I don't understand, what kernels should this go back to? Your "Fixes:"
> line just shows for a 5.7-rc1 patch, nothing older.

That's all. I was just stating the obvious I guess -- to pull this
into all the releases that have the "Fixes" commit. Btw you had
reverted the "Fixes" commit. So you'll have to revert the revert and
then apply this patch. Hope that makes sense.

-Saravana