Re: [PATCH V1 03/20] accel/amdxdna: Add CERT firmware version support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] amdxdna: redundant entry in npu3_cert_feature_table
--

commit 3d4c843c0c43dded98465c96cc3cb5ac8f46a7fb
Author: David Zhang <yidong.zhang@amd.com>

accel/amdxdna: Add CERT firmware version support

This commit adds support for querying and validating CERT firmware versions
on AIE4 platforms. It introduces mailbox opcodes, unifies firmware version
storage, and provides host queue protocol validation against driver
capabilities.

> diff --git a/drivers/accel/amdxdna/npu3_regs.c b/drivers/accel/amdxdna/npu3_regs.c
> index 8d287ef32fff7..31208c42ad5ff 100644
> --- a/drivers/accel/amdxdna/npu3_regs.c
> +++ b/drivers/accel/amdxdna/npu3_regs.c
> @@ -42,6 +42,12 @@ static const struct amdxdna_fw_feature_tbl npu3_fw_feature_table[] = {
>  	{ 0 }
>  };
>  
> +static const struct amdxdna_fw_feature_tbl npu3_cert_feature_table[] = {
> +	{ .major = 1, .min_minor = 0 },
> +	{ .features = BIT_U64(AIE4_HSA_COMMAND), .major = 1, .min_minor = 0 },

[Severity: Low]
Is this first entry without any features intentional?

When aie_check_protocol_impl() iterates through this table, both entries 
match major = 1 and min_minor >= 0, so it will process both. The first 
entry will perform a no-op bitwise OR on the feature mask 
(aie->feature_mask |= 0) before the second entry applies the 
AIE4_HSA_COMMAND feature.

This isn't a functional bug, but seems like a redundant configuration that 
could be simplified by removing the first entry.

> +	{ 0 }
> +};
> +
>  static const struct amdxdna_dev_priv npu3_dev_priv = {
>  	.npufw_path             = "npu.sbin",
>  	.certfw_path            = "cert.sbin",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930033233.1727265-1-yidong.zhang@amd.com?part=3