[PATCH] driver core: fw_devlink: Don't warn in fw_devlink_dev_sync_state()

[Diederik de Haas]

Hi,

On Thu Sep 25, 2025 at 1:59 PM CEST, Ulf Hansson wrote:
> Due to the wider deployment of the ->sync_state() support, for PM domains
> for example, we are receiving reports about the messages that are being
> logged in fw_devlink_dev_sync_state(). In particular as they are at the
> warning level, which doesn't seem correct.
>
> Even if it certainly is useful to know that the ->sync_state() condition
> could not be met, there may be nothing wrong with it. For example, a driver
> may be built as module and are still waiting to be initialized/probed.

"there may be nothing wrong with it" doesn't sound very convincing.
So there *can* be something wrong with it, so warning sounds
appropriate? If there is (certainly) something wrong with it, I expect
an error.
FWIW: most of my drivers/modules are built as modules.
I do seem to run into 'problems' more then average because of that, but
to me it just signals there is something wrong ... which should be
fixed. Not silenced.

You're the expert, but I'm not so sure this is an improvement.
I do regularly check dmesg level 0, 1, 2, 3 and 4, hence it landed on my
radar. I do not regularly check all the dmesg msgs, so this change would
result it dropping off my (immediate) radar.

On Fri Sep 12, 2025 at 8:32 PM CEST, Saravana Kannan wrote:
> Please don't just disable fw_devlink using fw_devlink=off. We want to
> fix any issues you are hitting with it. I might even delete this "off"
> option sometime. It was meant as an early debug option.

That response made a lot of sense to me. I haven't gotten around to it
yet, but I want to test all my (Rockchip) devices to see if they 'need'
that parameter to 'silence' the warning. I have a (vague) recollection
that some don't need it (for that).

My 0.02

Cheers,
  Diederik

> Ideally these messages should be at the debug level, but since the
> ->sync_state() feature is under an ongoing deployment and the prints
> provides valuable information, let's move to the info level for now.
>
> Cc: Saravana Kannan <saravanak at google.com>
> Reported-by: Geert Uytterhoeven <geert at linux-m68k.org>
> Reported-by: Sebin Francis <sebin.francis at ti.com>
> Reported-by: Diederik de Haas <didi.debian at cknow.org>
> Reported-by: Jon Hunter <jonathanh at nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson at linaro.org>
> ---
>  drivers/base/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index d22d6b23e758..97eab79c2f3b 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1784,7 +1784,7 @@ static int fw_devlink_dev_sync_state(struct device *dev, void *data)
>  		return 0;
>  
>  	if (fw_devlink_sync_state == FW_DEVLINK_SYNC_STATE_STRICT) {
> -		dev_warn(sup, "sync_state() pending due to %s\n",
> +		dev_info(sup, "sync_state() pending due to %s\n",
>  			 dev_name(link->consumer));
>  		return 0;
>  	}
> @@ -1792,7 +1792,7 @@ static int fw_devlink_dev_sync_state(struct device *dev, void *data)
>  	if (!list_empty(&sup->links.defer_sync))
>  		return 0;
>  
> -	dev_warn(sup, "Timed out. Forcing sync_state()\n");
> +	dev_info(sup, "Timed out. Forcing sync_state()\n");
>  	sup->state_synced = true;
>  	get_device(sup);
>  	list_add_tail(&sup->links.defer_sync, data);

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20250925/1fe73786/attachment.sig>