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

From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Saravana Kannan <saravanak@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	devicetree@vger.kernel.org, kernel-team@android.com,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1] driver core: Set fw_devlink to "permissive" behavior by default
Date: Fri, 27 Mar 2020 11:25:48 +0100	[thread overview]
Message-ID: <bd8b42d3-a35a-cc8e-0d06-2899416c2996@samsung.com> (raw)
In-Reply-To: <20200321210305.28937-1-saravanak@google.com>

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