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

On Thu, Sep 25, 2025 at 10:52 AM Diederik de Haas [off-list ref] wrote:

On Thu Sep 25, 2025 at 4:26 PM CEST, Ulf Hansson wrote:

quoted

On Thu, 25 Sept 2025 at 15:59, Diederik de Haas [off-list ref] wrote:

quoted

On Thu Sep 25, 2025 at 1:59 PM CEST, Ulf Hansson wrote:

quoted

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.

quoted

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

quoted

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