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 <[email protected]> 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/[email protected]?part=3
