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

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

[Saravana Kannan]

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

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

[Marek Szyprowski]

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

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

[Greg Kroah-Hartman]

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

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

[Greg Kroah-Hartman]

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

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

[Saravana Kannan]

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

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

[Marek Szyprowski]

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

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

[Saravana Kannan]

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

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

[Saravana Kannan]

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 <m.szyprowski@samsung.com>
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
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

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

[Marek Szyprowski]

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 <m.szyprowski@samsung.com>
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
> 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 <m.szyprowski@samsung.com>

>
> 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

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

[Saravana Kannan]

On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
<m.szyprowski@samsung.com> 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 <m.szyprowski@samsung.com>
> > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > ---
> > 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 <m.szyprowski@samsung.com>

Thanks!

Greg,

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

Thanks,
Saravana

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

[Marek Szyprowski]

Hi,

On 2020-03-31 08:18, Saravana Kannan wrote:
> On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
> <m.szyprowski@samsung.com> 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 <m.szyprowski@samsung.com>
>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
>>> ---
>>> 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

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

[Greg Kroah-Hartman]

On Mon, Mar 30, 2020 at 11:18:01PM -0700, Saravana Kannan wrote:
> On Mon, Mar 30, 2020 at 10:43 PM Marek Szyprowski
> <m.szyprowski@samsung.com> 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 <m.szyprowski@samsung.com>
> > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > ---
> > > 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 <m.szyprowski@samsung.com>
> 
> 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

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

[Saravana Kannan]

On Tue, Mar 31, 2020 at 12:29 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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
> > <m.szyprowski@samsung.com> 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 <m.szyprowski@samsung.com>
> > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > > ---
> > > > 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 <m.szyprowski@samsung.com>
> >
> > 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/20200321210305.28937-1-saravanak@google.com/

Thanks,
Saravana

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

[Greg Kroah-Hartman]

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
> <gregkh@linuxfoundation.org> 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
> > > <m.szyprowski@samsung.com> 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 <m.szyprowski@samsung.com>
> > > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > > > ---
> > > > > 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 <m.szyprowski@samsung.com>
> > >
> > > 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/20200321210305.28937-1-saravanak@google.com/

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

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

[Saravana Kannan]

On Tue, Apr 28, 2020 at 8:52 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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
> > <gregkh@linuxfoundation.org> 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
> > > > <m.szyprowski@samsung.com> 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 <m.szyprowski@samsung.com>
> > > > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > > > > ---
> > > > > > 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 <m.szyprowski@samsung.com>
> > > >
> > > > 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/20200321210305.28937-1-saravanak@google.com/
>
> 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