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

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

[Ulf Hansson]

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.

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@google.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Reported-by: Sebin Francis <sebin.francis@ti.com>
Reported-by: Diederik de Haas <didi.debian@cknow.org>
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Ulf Hansson <ulf.hansson@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);
-- 
2.43.0

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

[Tomi Valkeinen]

On 25/09/2025 14:59, 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.
> 
> 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@google.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Reported-by: Sebin Francis <sebin.francis@ti.com>
> Reported-by: Diederik de Haas <didi.debian@cknow.org>
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@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");

I have no issue with this, but I also think that while the pending print
above could well be dev_dbg, this one is perhaps a bit more warning-ish.
It may be harmless to get the time-out, but it would be better not to
time-out (i.e. everything was already sync_stated, or startup scripts
handled forcing the sync state).

>  	sup->state_synced = true;
>  	get_device(sup);
>  	list_add_tail(&sup->links.defer_sync, data);

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

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

[Ulf Hansson]

On Thu, 25 Sept 2025 at 14:08, Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
>
>
>
> On 25/09/2025 14:59, 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.
> >
> > 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@google.com>
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Reported-by: Sebin Francis <sebin.francis@ti.com>
> > Reported-by: Diederik de Haas <didi.debian@cknow.org>
> > Reported-by: Jon Hunter <jonathanh@nvidia.com>
> > Signed-off-by: Ulf Hansson <ulf.hansson@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");
>
> I have no issue with this, but I also think that while the pending print
> above could well be dev_dbg, this one is perhaps a bit more warning-ish.
> It may be harmless to get the time-out, but it would be better not to
> time-out (i.e. everything was already sync_stated, or startup scripts
> handled forcing the sync state).

I agree. Perhaps we should consider moving the above "sync_state()
pending.." to dev_dbg at some later point, then we should probably
keep the "Timed out..." print at dev_info().

Anyway, I don't think using dev_warn makes sense, at least for
configurations with CONFIG_FW_DEVLINK_SYNC_STATE_TIMEOUT being set.

>
> >       sup->state_synced = true;
> >       get_device(sup);
> >       list_add_tail(&sup->links.defer_sync, data);
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>
>  Tomi
>

Thanks for reviewing!

Kind regards
Uffe

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

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 3346 bytes --]

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@google.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Reported-by: Sebin Francis <sebin.francis@ti.com>
> Reported-by: Diederik de Haas <didi.debian@cknow.org>
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@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);


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Ulf Hansson]

On Thu, 25 Sept 2025 at 15:59, Diederik de Haas <didi.debian@cknow.org> wrote:
>
> 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.

Sorry if I was too vague. See more below.

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

Well, why is it wrong to have drivers being built as modules? They
just happen to be probed at some point later, then why should we have
warnings printed in the log due to this?

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

I personally don't have a strong opinion about the log level here, but
the reports I have received so far, clearly indicated to me that we
should not be using the warn level for these messages.

In the end, it seems like these prints are not as straight-forward
when it comes to deciding what level to log things on.

Let's see what other people think about it.

[...]

Kind regards
Uffe

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

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]

On Thu Sep 25, 2025 at 4:26 PM CEST, Ulf Hansson wrote:
> On Thu, 25 Sept 2025 at 15:59, Diederik de Haas <didi.debian@cknow.org> wrote:
>> 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.
>
> Sorry if I was too vague. See more below.
>
>> 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.
>
> Well, why is it wrong to have drivers being built as modules? They

Nothing wrong with it at all. It just means I notice issues (like [1])
that others may not who have modules built-in.

[1] a52dffaa46c2 ("drm/rockchip: vop2: make vp registers nonvolatile")

> just happen to be probed at some point later, then why should we have
> warnings printed in the log due to this?

I thought the failure of the check was more important then it apparently
is. Then warning about it does seem excessive.

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Saravana Kannan]

On Thu, Sep 25, 2025 at 10:52 AM Diederik de Haas <didi.debian@cknow.org> wrote:
>
> On Thu Sep 25, 2025 at 4:26 PM CEST, Ulf Hansson wrote:
> > On Thu, 25 Sept 2025 at 15:59, Diederik de Haas <didi.debian@cknow.org> wrote:
> >> 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.
> >
> > Sorry if I was too vague. See more below.
> >
> >> 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.
> >
> > Well, why is it wrong to have drivers being built as modules? They

IIUC/Remember Kconfig correctly, FW_DEVLINK_SYNC_STATE_TIMEOUT should
be off by default? (if I don't give any default, what ends up
happening?)

If we can assume, timeout won't happen by default, then the only
people seeing this warning are people setting the flag or setting the
command line param. So I'm okay with making it an info.

But the "sync_state() pending due to" is for the default behavior. I'm
assuming without the sync_state() supported added to power domains,
the ones getting this warning would have been powered off? But after
the sync_state() support, it's not powered off anymore. That can cause
increased power usage. Seems like something worth warning about.

Also, another thing I want to understand is, for all the reports you
are getting for "sync_state() pending due to", is it actually correct
to have them pending? Or can fw_devlink do better in those cases and
turn them off? It could be a driver fix/fw_devlink fix depending on
the issue.

TL;DR:
1. For timeout messages, since timeout isn't default behavior, I'm
okay making it an info. But
2. For pending sync state messages, I think it should remain as a warning.

Thanks,
Saravana

>
> Nothing wrong with it at all. It just means I notice issues (like [1])
> that others may not who have modules built-in.
>
> [1] a52dffaa46c2 ("drm/rockchip: vop2: make vp registers nonvolatile")
>
> > just happen to be probed at some point later, then why should we have
> > warnings printed in the log due to this?
>
> I thought the failure of the check was more important then it apparently
> is. Then warning about it does seem excessive.
>
> Cheers,
>   Diederik

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

[Ulf Hansson]

On Thu, 25 Sept 2025 at 23:49, Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Sep 25, 2025 at 10:52 AM Diederik de Haas <didi.debian@cknow.org> wrote:
> >
> > On Thu Sep 25, 2025 at 4:26 PM CEST, Ulf Hansson wrote:
> > > On Thu, 25 Sept 2025 at 15:59, Diederik de Haas <didi.debian@cknow.org> wrote:
> > >> 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.
> > >
> > > Sorry if I was too vague. See more below.
> > >
> > >> 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.
> > >
> > > Well, why is it wrong to have drivers being built as modules? They
>
> IIUC/Remember Kconfig correctly, FW_DEVLINK_SYNC_STATE_TIMEOUT should
> be off by default? (if I don't give any default, what ends up
> happening?)

As "FW_DEVLINK_SYNC_STATE_STRICT" is default, the "sync_state()
pending due..." print gets printed for a lot of consumer devices that
have not been probed yet.

>
> If we can assume, timeout won't happen by default, then the only
> people seeing this warning are people setting the flag or setting the
> command line param. So I'm okay with making it an info.

I think you got this wrong. It's the default behaviour that triggers
lots of prints.

>
> But the "sync_state() pending due to" is for the default behavior. I'm
> assuming without the sync_state() supported added to power domains,
> the ones getting this warning would have been powered off? But after
> the sync_state() support, it's not powered off anymore. That can cause
> increased power usage. Seems like something worth warning about.

Yes, I certainly agree, but...

To me, it kind of sounds like we should change to use
FW_DEVLINK_SYNC_STATE_TIMEOUT as the default behaviour. In that case,
I don't think there is a need to change the log-level for
"sync_state() pending due to..".

>
> Also, another thing I want to understand is, for all the reports you
> are getting for "sync_state() pending due to", is it actually correct
> to have them pending? Or can fw_devlink do better in those cases and
> turn them off? It could be a driver fix/fw_devlink fix depending on
> the issue.

There are some cases that we potentially could fix, in particular for
when a genpd-provider, provides multiple power-domains per device-node
(#power-domain-cells = <1>;). But let's discuss that separately.

>
> TL;DR:
> 1. For timeout messages, since timeout isn't default behavior, I'm
> okay making it an info. But
> 2. For pending sync state messages, I think it should remain as a warning.

See above.

This means drivers being built as modules will trigger a lot of
warnings, in the default behavior. Is this really what we want?

[...]

Kind regards
Uffe

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

[Rafael J. Wysocki]

On Fri, Sep 26, 2025 at 2:05 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 25 Sept 2025 at 23:49, Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Thu, Sep 25, 2025 at 10:52 AM Diederik de Haas <didi.debian@cknow.org> wrote:
> > >
> > > On Thu Sep 25, 2025 at 4:26 PM CEST, Ulf Hansson wrote:
> > > > On Thu, 25 Sept 2025 at 15:59, Diederik de Haas <didi.debian@cknow.org> wrote:
> > > >> 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.
> > > >
> > > > Sorry if I was too vague. See more below.
> > > >
> > > >> 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.
> > > >
> > > > Well, why is it wrong to have drivers being built as modules? They
> >
> > IIUC/Remember Kconfig correctly, FW_DEVLINK_SYNC_STATE_TIMEOUT should
> > be off by default? (if I don't give any default, what ends up
> > happening?)
>
> As "FW_DEVLINK_SYNC_STATE_STRICT" is default, the "sync_state()
> pending due..." print gets printed for a lot of consumer devices that
> have not been probed yet.
>
> >
> > If we can assume, timeout won't happen by default, then the only
> > people seeing this warning are people setting the flag or setting the
> > command line param. So I'm okay with making it an info.
>
> I think you got this wrong. It's the default behaviour that triggers
> lots of prints.
>
> >
> > But the "sync_state() pending due to" is for the default behavior. I'm
> > assuming without the sync_state() supported added to power domains,
> > the ones getting this warning would have been powered off? But after
> > the sync_state() support, it's not powered off anymore. That can cause
> > increased power usage. Seems like something worth warning about.
>
> Yes, I certainly agree, but...
>
> To me, it kind of sounds like we should change to use
> FW_DEVLINK_SYNC_STATE_TIMEOUT as the default behaviour. In that case,
> I don't think there is a need to change the log-level for
> "sync_state() pending due to..".
>
> >
> > Also, another thing I want to understand is, for all the reports you
> > are getting for "sync_state() pending due to", is it actually correct
> > to have them pending? Or can fw_devlink do better in those cases and
> > turn them off? It could be a driver fix/fw_devlink fix depending on
> > the issue.
>
> There are some cases that we potentially could fix, in particular for
> when a genpd-provider, provides multiple power-domains per device-node
> (#power-domain-cells = <1>;). But let's discuss that separately.
>
> >
> > TL;DR:
> > 1. For timeout messages, since timeout isn't default behavior, I'm
> > okay making it an info. But
> > 2. For pending sync state messages, I think it should remain as a warning.
>
> See above.
>
> This means drivers being built as modules will trigger a lot of
> warnings, in the default behavior. Is this really what we want?

No, at least I don't.

Printing a warn level message about anything that is not a clear
functional issue is seriously problematic IMV.

As for whether or not the messages should be info level or debug
level, it also depends on the amount of noise generated by them.  IMV,
generating a lot of noise at the info level is not really useful and
may cause some actually useful information to be missed.

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

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Thu Sep 25, 2025 at 3:59 PM CEST, Diederik de Haas wrote:
> 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.
>
> 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).

Not trying to revisit the discussion, only to report that data.
All tests were done with the 6.17 kernel and no 'fw_devlink=off'.

Device				sync_state warning
Rock64 (rk3328)			yes
RockPro64 (rk3399)		no
Quartz64 Model A (rk3566)	no
Quartz64 Model B (rk3566)	yes
PineTab2 (rk3566)		yes
NanoPi R5S (rk3568)		yes
Rock 5B (rk3588)		yes

Cheers,
  Diederik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]