Re: [PATCH v7 1/4] firmware_loader: expand firmware error codes with up-to-date error

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The selftest script for firmware uploads does not inject and test the new 'firmware-duplicate' error.
- [Low] Missing ABI documentation for the newly added sysfs error string 'firmware-duplicate'.
--

commit b451d666b20500e69051bac63ead44572b79d708
Author: Marco Felsch <m.felsch@pengutronix.de>

firmware_loader: expand firmware error codes with up-to-date error

This commit adds FW_UPLOAD_ERR_DUPLICATE to allow drivers to inform the
firmware_loader that an update is not required. It also updates the
test_firmware module to support injecting this new error state.

> diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
> index efc33294212fd..82c4b1fedf4cd 100644
> --- a/drivers/base/firmware_loader/sysfs_upload.c
> +++ b/drivers/base/firmware_loader/sysfs_upload.c
> @@ -28,6 +28,7 @@ static const char * const fw_upload_err_str[] = {
>  	[FW_UPLOAD_ERR_RW_ERROR]     = "read-write-error",
>  	[FW_UPLOAD_ERR_WEAROUT]	     = "flash-wearout",
>  	[FW_UPLOAD_ERR_FW_INVALID]   = "firmware-invalid",
> +	[FW_UPLOAD_ERR_DUPLICATE]    = "firmware-duplicate",

[Severity: Low]
Since a new error string "firmware-duplicate" is exposed to userspace,
should this also be documented in
Documentation/ABI/testing/sysfs-class-firmware?

It looks like the new string is missing from the allowed error values
documented for the sysfs interface.

>  };
>  
>  static const char *fw_upload_progress(struct device *dev,

[ ... ]

> diff --git a/lib/test_firmware.c b/lib/test_firmware.c
> index 7459bba65444d..a822d9641eb4c 100644
> --- a/lib/test_firmware.c
> +++ b/lib/test_firmware.c
> @@ -1134,6 +1134,7 @@ static const char * const fw_upload_err_str[] = {
>  	[FW_UPLOAD_ERR_RW_ERROR]     = "read-write-error",
>  	[FW_UPLOAD_ERR_WEAROUT]	     = "flash-wearout",
>  	[FW_UPLOAD_ERR_FW_INVALID]   = "firmware-invalid",
> +	[FW_UPLOAD_ERR_DUPLICATE]    = "firmware-duplicate",

[Severity: Medium]
Does the test script also need to be updated to exercise this new code path?

While this properly adds injection support to the test module, the hardcoded
errors list in tools/testing/selftests/firmware/fw_upload.sh does not appear
to include "firmware-duplicate", meaning the kselftest won't actually
inject or test this new error type.

>  };
>  
>  static void upload_err_inject_error(struct test_firmware_upload *tst,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-v6-10-topic-touchscreen-axiom-v7-0-0d2a550a7ee8@pengutronix.de?part=1