Re: [PATCH net v2 2/2] octeontx2-af: suppress kpu profile loading warning

[Simon Horman <horms@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
octeontx2-af: suppress kpu profile loading warning

This commit replaces request_firmware_direct() with
firmware_request_nowarn() in npc_load_kpu_profile_from_fs() to suppress
a warning observed when the KPU profile is loaded via the firmware
database method rather than being integrated into the kernel image.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index c7bc0b3a29b9..007d3f22b0c9 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -2246,7 +2246,7 @@ static int npc_load_kpu_profile_from_fs(struct rvu *rvu)
>  
>  	strcat(path, kpu_profile);
>  
> -	if (request_firmware_direct(&fw, path, rvu->dev))
> +	if (firmware_request_nowarn(&fw, path, rvu->dev))

[Medium]
Does this actually change the warning behavior the commit message
describes?

Looking at the two helpers in drivers/base/firmware_loader/main.c, both
pass FW_OPT_NO_WARN into _request_firmware():

  int firmware_request_nowarn(...) {
        ...
        ret = _request_firmware(firmware, name, device, NULL, 0, 0,
                                FW_OPT_UEVENT | FW_OPT_NO_WARN);
        ...
  }

  int request_firmware_direct(...) {
        ...
        ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
                                FW_OPT_UEVENT | FW_OPT_NO_WARN |
                                FW_OPT_NOFALLBACK_SYSFS);
        ...
  }

The dev_warn() calls in the firmware loader hot path (for example the
"Direct firmware load for %s failed" message, the "loading %s failed"
message, and the sysfs fallback notice) are all gated by
!(opt_flags & FW_OPT_NO_WARN), and have been since commit 1fe6e4f0b0c4
("firmware_loader: Suppress warning on FW_OPT_NO_WARN flag").

Given that request_firmware_direct() already sets FW_OPT_NO_WARN, where
does the spurious warning mentioned in the commit message
("causes a spurious warning from the Linux firmware subsystem ... due
to the absence of firmware in the kernel image") originate?

[Medium]
This change also silently drops FW_OPT_NOFALLBACK_SYSFS, which is not
mentioned in the commit message.

The kerneldoc for request_firmware_direct() says it "doesn't fall back
to usermode helper even if the firmware couldn't be loaded directly
from fs.  Hence it's useful for loading optional firmwares, which
aren't always present, without extra long timeouts of udev."

firmware_request_nowarn()'s kerneldoc states the opposite: "The sysfs
fallback mechanism is enabled if direct filesystem lookup fails."

On kernels built with CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y, or where
the force_sysfs_fallback sysctl is enabled, fw_run_sysfs_fallback() no
longer short-circuits on FW_OPT_NOFALLBACK_SYSFS, so the absent-firmware
case will now invoke firmware_fallback_sysfs() and wait on udev for the
loading timeout (default 60s) before returning -ENOENT.

The commit message itself states that "in most cases the profile is
loaded using the 2nd method", so this is precisely the optional-firmware
scenario that request_firmware_direct() was designed for. Was the
fallback-sysfs behavior change intentional, and is there a reason not
to keep the no-fallback semantics on configurations that enable the
usermode helper?

>  		return -ENOENT;
>  
>  	dev_info(rvu->dev, "Loading KPU profile from filesystem: %s\n",