[PATCH V2] accel/amdxdna: Use a different name for latest firmware

[PATCH V2] accel/amdxdna: Use a different name for latest firmware

[Lizhi Hou]

Using legacy driver with latest firmware causes a power off issue.

Fix this by assigning a different filename (npu_7.sbin) to the latest
firmware. The driver attempts to load the latest firmware first and falls
back to the previous firmware version if loading fails.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5009
Fixes: f1eac46fe5f7 ("accel/amdxdna: Update firmware version check for latest firmware")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/aie2_pci.c        | 20 +++++++++++++++++++-
 drivers/accel/amdxdna/amdxdna_pci_drv.c |  3 +++
 drivers/accel/amdxdna/npu1_regs.c       |  2 +-
 drivers/accel/amdxdna/npu4_regs.c       |  2 +-
 drivers/accel/amdxdna/npu5_regs.c       |  2 +-
 drivers/accel/amdxdna/npu6_regs.c       |  2 +-
 6 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
index 4b3e6bb97bd2..85079b6fc5d9 100644
--- a/drivers/accel/amdxdna/aie2_pci.c
+++ b/drivers/accel/amdxdna/aie2_pci.c
@@ -32,6 +32,11 @@ static int aie2_max_col = XRS_MAX_COL;
 module_param(aie2_max_col, uint, 0600);
 MODULE_PARM_DESC(aie2_max_col, "Maximum column could be used");
 
+static char *npu_fw[] = {
+	"npu_7.sbin",
+	"npu.sbin"
+};
+
 /*
  * The management mailbox channel is allocated by firmware.
  * The related register and ring buffer information is on SRAM BAR.
@@ -489,6 +494,7 @@ static int aie2_init(struct amdxdna_dev *xdna)
 	struct psp_config psp_conf;
 	const struct firmware *fw;
 	unsigned long bars = 0;
+	char *fw_full_path;
 	int i, nvec, ret;
 
 	if (!hypervisor_is_type(X86_HYPER_NATIVE)) {
@@ -503,7 +509,19 @@ static int aie2_init(struct amdxdna_dev *xdna)
 	ndev->priv = xdna->dev_info->dev_priv;
 	ndev->xdna = xdna;
 
-	ret = request_firmware(&fw, ndev->priv->fw_path, &pdev->dev);
+	for (i = 0; i < ARRAY_SIZE(npu_fw); i++) {
+		fw_full_path = kasprintf(GFP_KERNEL, "%s%s", ndev->priv->fw_path, npu_fw[i]);
+		if (!fw_full_path)
+			return -ENOMEM;
+
+		ret = firmware_request_nowarn(&fw, fw_full_path, &pdev->dev);
+		kfree(fw_full_path);
+		if (!ret) {
+			XDNA_INFO(xdna, "Load firmware %s%s", ndev->priv->fw_path, npu_fw[i]);
+			break;
+		}
+	}
+
 	if (ret) {
 		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d",
 			 ndev->priv->fw_path, ret);
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index 4ada45d06fcf..a4384593bdcc 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -23,6 +23,9 @@ MODULE_FIRMWARE("amdnpu/1502_00/npu.sbin");
 MODULE_FIRMWARE("amdnpu/17f0_10/npu.sbin");
 MODULE_FIRMWARE("amdnpu/17f0_11/npu.sbin");
 MODULE_FIRMWARE("amdnpu/17f0_20/npu.sbin");
+MODULE_FIRMWARE("amdnpu/1502_00/npu_7.sbin");
+MODULE_FIRMWARE("amdnpu/17f0_10/npu_7.sbin");
+MODULE_FIRMWARE("amdnpu/17f0_11/npu_7.sbin");
 
 /*
  * 0.0: Initial version
diff --git a/drivers/accel/amdxdna/npu1_regs.c b/drivers/accel/amdxdna/npu1_regs.c
index 6f36a27b5a02..6e3d3ca69c04 100644
--- a/drivers/accel/amdxdna/npu1_regs.c
+++ b/drivers/accel/amdxdna/npu1_regs.c
@@ -72,7 +72,7 @@ static const struct aie2_fw_feature_tbl npu1_fw_feature_table[] = {
 };
 
 static const struct amdxdna_dev_priv npu1_dev_priv = {
-	.fw_path        = "amdnpu/1502_00/npu.sbin",
+	.fw_path        = "amdnpu/1502_00/",
 	.rt_config	= npu1_default_rt_cfg,
 	.dpm_clk_tbl	= npu1_dpm_clk_table,
 	.fw_feature_tbl = npu1_fw_feature_table,
diff --git a/drivers/accel/amdxdna/npu4_regs.c b/drivers/accel/amdxdna/npu4_regs.c
index a8d6f76dde5f..ce25eef5fc34 100644
--- a/drivers/accel/amdxdna/npu4_regs.c
+++ b/drivers/accel/amdxdna/npu4_regs.c
@@ -98,7 +98,7 @@ const struct aie2_fw_feature_tbl npu4_fw_feature_table[] = {
 };
 
 static const struct amdxdna_dev_priv npu4_dev_priv = {
-	.fw_path        = "amdnpu/17f0_10/npu.sbin",
+	.fw_path        = "amdnpu/17f0_10/",
 	.rt_config	= npu4_default_rt_cfg,
 	.dpm_clk_tbl	= npu4_dpm_clk_table,
 	.fw_feature_tbl = npu4_fw_feature_table,
diff --git a/drivers/accel/amdxdna/npu5_regs.c b/drivers/accel/amdxdna/npu5_regs.c
index c0a35cfd886c..c0ac5daf32ee 100644
--- a/drivers/accel/amdxdna/npu5_regs.c
+++ b/drivers/accel/amdxdna/npu5_regs.c
@@ -63,7 +63,7 @@
 #define NPU5_SRAM_BAR_BASE	MMNPU_APERTURE1_BASE
 
 static const struct amdxdna_dev_priv npu5_dev_priv = {
-	.fw_path        = "amdnpu/17f0_11/npu.sbin",
+	.fw_path        = "amdnpu/17f0_11/",
 	.rt_config	= npu4_default_rt_cfg,
 	.dpm_clk_tbl	= npu4_dpm_clk_table,
 	.fw_feature_tbl = npu4_fw_feature_table,
diff --git a/drivers/accel/amdxdna/npu6_regs.c b/drivers/accel/amdxdna/npu6_regs.c
index 1fb07df99186..ce591ed0d483 100644
--- a/drivers/accel/amdxdna/npu6_regs.c
+++ b/drivers/accel/amdxdna/npu6_regs.c
@@ -63,7 +63,7 @@
 #define NPU6_SRAM_BAR_BASE	MMNPU_APERTURE1_BASE
 
 static const struct amdxdna_dev_priv npu6_dev_priv = {
-	.fw_path        = "amdnpu/17f0_10/npu.sbin",
+	.fw_path        = "amdnpu/17f0_10/",
 	.rt_config	= npu4_default_rt_cfg,
 	.dpm_clk_tbl	= npu4_dpm_clk_table,
 	.fw_feature_tbl = npu4_fw_feature_table,
-- 
2.34.1

Re: [PATCH V2] accel/amdxdna: Use a different name for latest firmware

[Mario Limonciello (AMD) (kernel.org)]

On 2/25/2026 2:47 PM, Lizhi Hou wrote:
> Using legacy driver with latest firmware causes a power off issue.
> 
> Fix this by assigning a different filename (npu_7.sbin) to the latest
> firmware. The driver attempts to load the latest firmware first and falls
> back to the previous firmware version if loading fails.
> 
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5009
> Fixes: f1eac46fe5f7 ("accel/amdxdna: Update firmware version check for latest firmware")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>   drivers/accel/amdxdna/aie2_pci.c        | 20 +++++++++++++++++++-
>   drivers/accel/amdxdna/amdxdna_pci_drv.c |  3 +++
>   drivers/accel/amdxdna/npu1_regs.c       |  2 +-
>   drivers/accel/amdxdna/npu4_regs.c       |  2 +-
>   drivers/accel/amdxdna/npu5_regs.c       |  2 +-
>   drivers/accel/amdxdna/npu6_regs.c       |  2 +-
>   6 files changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
> index 4b3e6bb97bd2..85079b6fc5d9 100644
> --- a/drivers/accel/amdxdna/aie2_pci.c
> +++ b/drivers/accel/amdxdna/aie2_pci.c
> @@ -32,6 +32,11 @@ static int aie2_max_col = XRS_MAX_COL;
>   module_param(aie2_max_col, uint, 0600);
>   MODULE_PARM_DESC(aie2_max_col, "Maximum column could be used");
>   
> +static char *npu_fw[] = {
> +	"npu_7.sbin",
> +	"npu.sbin"
> +};
> +
>   /*
>    * The management mailbox channel is allocated by firmware.
>    * The related register and ring buffer information is on SRAM BAR.
> @@ -489,6 +494,7 @@ static int aie2_init(struct amdxdna_dev *xdna)
>   	struct psp_config psp_conf;
>   	const struct firmware *fw;
>   	unsigned long bars = 0;
> +	char *fw_full_path;
>   	int i, nvec, ret;
>   
>   	if (!hypervisor_is_type(X86_HYPER_NATIVE)) {
> @@ -503,7 +509,19 @@ static int aie2_init(struct amdxdna_dev *xdna)
>   	ndev->priv = xdna->dev_info->dev_priv;
>   	ndev->xdna = xdna;
>   
> -	ret = request_firmware(&fw, ndev->priv->fw_path, &pdev->dev);
> +	for (i = 0; i < ARRAY_SIZE(npu_fw); i++) {
> +		fw_full_path = kasprintf(GFP_KERNEL, "%s%s", ndev->priv->fw_path, npu_fw[i]);
> +		if (!fw_full_path)
> +			return -ENOMEM;
> +
> +		ret = firmware_request_nowarn(&fw, fw_full_path, &pdev->dev);
> +		kfree(fw_full_path);
> +		if (!ret) {
> +			XDNA_INFO(xdna, "Load firmware %s%s", ndev->priv->fw_path, npu_fw[i]);
> +			break;
> +		}
> +	}
> +
>   	if (ret) {
>   		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d",
>   			 ndev->priv->fw_path, ret);
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index 4ada45d06fcf..a4384593bdcc 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -23,6 +23,9 @@ MODULE_FIRMWARE("amdnpu/1502_00/npu.sbin");
>   MODULE_FIRMWARE("amdnpu/17f0_10/npu.sbin");
>   MODULE_FIRMWARE("amdnpu/17f0_11/npu.sbin");
>   MODULE_FIRMWARE("amdnpu/17f0_20/npu.sbin");
> +MODULE_FIRMWARE("amdnpu/1502_00/npu_7.sbin");
> +MODULE_FIRMWARE("amdnpu/17f0_10/npu_7.sbin");
> +MODULE_FIRMWARE("amdnpu/17f0_11/npu_7.sbin");
>   
>   /*
>    * 0.0: Initial version
> diff --git a/drivers/accel/amdxdna/npu1_regs.c b/drivers/accel/amdxdna/npu1_regs.c
> index 6f36a27b5a02..6e3d3ca69c04 100644
> --- a/drivers/accel/amdxdna/npu1_regs.c
> +++ b/drivers/accel/amdxdna/npu1_regs.c
> @@ -72,7 +72,7 @@ static const struct aie2_fw_feature_tbl npu1_fw_feature_table[] = {
>   };
>   
>   static const struct amdxdna_dev_priv npu1_dev_priv = {
> -	.fw_path        = "amdnpu/1502_00/npu.sbin",
> +	.fw_path        = "amdnpu/1502_00/",
>   	.rt_config	= npu1_default_rt_cfg,
>   	.dpm_clk_tbl	= npu1_dpm_clk_table,
>   	.fw_feature_tbl = npu1_fw_feature_table,
> diff --git a/drivers/accel/amdxdna/npu4_regs.c b/drivers/accel/amdxdna/npu4_regs.c
> index a8d6f76dde5f..ce25eef5fc34 100644
> --- a/drivers/accel/amdxdna/npu4_regs.c
> +++ b/drivers/accel/amdxdna/npu4_regs.c
> @@ -98,7 +98,7 @@ const struct aie2_fw_feature_tbl npu4_fw_feature_table[] = {
>   };
>   
>   static const struct amdxdna_dev_priv npu4_dev_priv = {
> -	.fw_path        = "amdnpu/17f0_10/npu.sbin",
> +	.fw_path        = "amdnpu/17f0_10/",
>   	.rt_config	= npu4_default_rt_cfg,
>   	.dpm_clk_tbl	= npu4_dpm_clk_table,
>   	.fw_feature_tbl = npu4_fw_feature_table,
> diff --git a/drivers/accel/amdxdna/npu5_regs.c b/drivers/accel/amdxdna/npu5_regs.c
> index c0a35cfd886c..c0ac5daf32ee 100644
> --- a/drivers/accel/amdxdna/npu5_regs.c
> +++ b/drivers/accel/amdxdna/npu5_regs.c
> @@ -63,7 +63,7 @@
>   #define NPU5_SRAM_BAR_BASE	MMNPU_APERTURE1_BASE
>   
>   static const struct amdxdna_dev_priv npu5_dev_priv = {
> -	.fw_path        = "amdnpu/17f0_11/npu.sbin",
> +	.fw_path        = "amdnpu/17f0_11/",
>   	.rt_config	= npu4_default_rt_cfg,
>   	.dpm_clk_tbl	= npu4_dpm_clk_table,
>   	.fw_feature_tbl = npu4_fw_feature_table,
> diff --git a/drivers/accel/amdxdna/npu6_regs.c b/drivers/accel/amdxdna/npu6_regs.c
> index 1fb07df99186..ce591ed0d483 100644
> --- a/drivers/accel/amdxdna/npu6_regs.c
> +++ b/drivers/accel/amdxdna/npu6_regs.c
> @@ -63,7 +63,7 @@
>   #define NPU6_SRAM_BAR_BASE	MMNPU_APERTURE1_BASE
>   
>   static const struct amdxdna_dev_priv npu6_dev_priv = {
> -	.fw_path        = "amdnpu/17f0_10/npu.sbin",
> +	.fw_path        = "amdnpu/17f0_10/",
>   	.rt_config	= npu4_default_rt_cfg,
>   	.dpm_clk_tbl	= npu4_dpm_clk_table,
>   	.fw_feature_tbl = npu4_fw_feature_table,

Re: [PATCH V2] accel/amdxdna: Use a different name for latest firmware

[Lizhi Hou]

Applied to drm-misc-fixes

On 2/25/26 12:49, Mario Limonciello (AMD) (kernel.org) wrote:
>
>
> On 2/25/2026 2:47 PM, Lizhi Hou wrote:
>> Using legacy driver with latest firmware causes a power off issue.
>>
>> Fix this by assigning a different filename (npu_7.sbin) to the latest
>> firmware. The driver attempts to load the latest firmware first and 
>> falls
>> back to the previous firmware version if loading fails.
>>
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5009
>> Fixes: f1eac46fe5f7 ("accel/amdxdna: Update firmware version check 
>> for latest firmware")
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>>   drivers/accel/amdxdna/aie2_pci.c        | 20 +++++++++++++++++++-
>>   drivers/accel/amdxdna/amdxdna_pci_drv.c |  3 +++
>>   drivers/accel/amdxdna/npu1_regs.c       |  2 +-
>>   drivers/accel/amdxdna/npu4_regs.c       |  2 +-
>>   drivers/accel/amdxdna/npu5_regs.c       |  2 +-
>>   drivers/accel/amdxdna/npu6_regs.c       |  2 +-
>>   6 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_pci.c 
>> b/drivers/accel/amdxdna/aie2_pci.c
>> index 4b3e6bb97bd2..85079b6fc5d9 100644
>> --- a/drivers/accel/amdxdna/aie2_pci.c
>> +++ b/drivers/accel/amdxdna/aie2_pci.c
>> @@ -32,6 +32,11 @@ static int aie2_max_col = XRS_MAX_COL;
>>   module_param(aie2_max_col, uint, 0600);
>>   MODULE_PARM_DESC(aie2_max_col, "Maximum column could be used");
>>   +static char *npu_fw[] = {
>> +    "npu_7.sbin",
>> +    "npu.sbin"
>> +};
>> +
>>   /*
>>    * The management mailbox channel is allocated by firmware.
>>    * The related register and ring buffer information is on SRAM BAR.
>> @@ -489,6 +494,7 @@ static int aie2_init(struct amdxdna_dev *xdna)
>>       struct psp_config psp_conf;
>>       const struct firmware *fw;
>>       unsigned long bars = 0;
>> +    char *fw_full_path;
>>       int i, nvec, ret;
>>         if (!hypervisor_is_type(X86_HYPER_NATIVE)) {
>> @@ -503,7 +509,19 @@ static int aie2_init(struct amdxdna_dev *xdna)
>>       ndev->priv = xdna->dev_info->dev_priv;
>>       ndev->xdna = xdna;
>>   -    ret = request_firmware(&fw, ndev->priv->fw_path, &pdev->dev);
>> +    for (i = 0; i < ARRAY_SIZE(npu_fw); i++) {
>> +        fw_full_path = kasprintf(GFP_KERNEL, "%s%s", 
>> ndev->priv->fw_path, npu_fw[i]);
>> +        if (!fw_full_path)
>> +            return -ENOMEM;
>> +
>> +        ret = firmware_request_nowarn(&fw, fw_full_path, &pdev->dev);
>> +        kfree(fw_full_path);
>> +        if (!ret) {
>> +            XDNA_INFO(xdna, "Load firmware %s%s", 
>> ndev->priv->fw_path, npu_fw[i]);
>> +            break;
>> +        }
>> +    }
>> +
>>       if (ret) {
>>           XDNA_ERR(xdna, "failed to request_firmware %s, ret %d",
>>                ndev->priv->fw_path, ret);
>> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c 
>> b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> index 4ada45d06fcf..a4384593bdcc 100644
>> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> @@ -23,6 +23,9 @@ MODULE_FIRMWARE("amdnpu/1502_00/npu.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f0_10/npu.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f0_11/npu.sbin");
>>   MODULE_FIRMWARE("amdnpu/17f0_20/npu.sbin");
>> +MODULE_FIRMWARE("amdnpu/1502_00/npu_7.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f0_10/npu_7.sbin");
>> +MODULE_FIRMWARE("amdnpu/17f0_11/npu_7.sbin");
>>     /*
>>    * 0.0: Initial version
>> diff --git a/drivers/accel/amdxdna/npu1_regs.c 
>> b/drivers/accel/amdxdna/npu1_regs.c
>> index 6f36a27b5a02..6e3d3ca69c04 100644
>> --- a/drivers/accel/amdxdna/npu1_regs.c
>> +++ b/drivers/accel/amdxdna/npu1_regs.c
>> @@ -72,7 +72,7 @@ static const struct aie2_fw_feature_tbl 
>> npu1_fw_feature_table[] = {
>>   };
>>     static const struct amdxdna_dev_priv npu1_dev_priv = {
>> -    .fw_path        = "amdnpu/1502_00/npu.sbin",
>> +    .fw_path        = "amdnpu/1502_00/",
>>       .rt_config    = npu1_default_rt_cfg,
>>       .dpm_clk_tbl    = npu1_dpm_clk_table,
>>       .fw_feature_tbl = npu1_fw_feature_table,
>> diff --git a/drivers/accel/amdxdna/npu4_regs.c 
>> b/drivers/accel/amdxdna/npu4_regs.c
>> index a8d6f76dde5f..ce25eef5fc34 100644
>> --- a/drivers/accel/amdxdna/npu4_regs.c
>> +++ b/drivers/accel/amdxdna/npu4_regs.c
>> @@ -98,7 +98,7 @@ const struct aie2_fw_feature_tbl 
>> npu4_fw_feature_table[] = {
>>   };
>>     static const struct amdxdna_dev_priv npu4_dev_priv = {
>> -    .fw_path        = "amdnpu/17f0_10/npu.sbin",
>> +    .fw_path        = "amdnpu/17f0_10/",
>>       .rt_config    = npu4_default_rt_cfg,
>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>       .fw_feature_tbl = npu4_fw_feature_table,
>> diff --git a/drivers/accel/amdxdna/npu5_regs.c 
>> b/drivers/accel/amdxdna/npu5_regs.c
>> index c0a35cfd886c..c0ac5daf32ee 100644
>> --- a/drivers/accel/amdxdna/npu5_regs.c
>> +++ b/drivers/accel/amdxdna/npu5_regs.c
>> @@ -63,7 +63,7 @@
>>   #define NPU5_SRAM_BAR_BASE    MMNPU_APERTURE1_BASE
>>     static const struct amdxdna_dev_priv npu5_dev_priv = {
>> -    .fw_path        = "amdnpu/17f0_11/npu.sbin",
>> +    .fw_path        = "amdnpu/17f0_11/",
>>       .rt_config    = npu4_default_rt_cfg,
>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>       .fw_feature_tbl = npu4_fw_feature_table,
>> diff --git a/drivers/accel/amdxdna/npu6_regs.c 
>> b/drivers/accel/amdxdna/npu6_regs.c
>> index 1fb07df99186..ce591ed0d483 100644
>> --- a/drivers/accel/amdxdna/npu6_regs.c
>> +++ b/drivers/accel/amdxdna/npu6_regs.c
>> @@ -63,7 +63,7 @@
>>   #define NPU6_SRAM_BAR_BASE    MMNPU_APERTURE1_BASE
>>     static const struct amdxdna_dev_priv npu6_dev_priv = {
>> -    .fw_path        = "amdnpu/17f0_10/npu.sbin",
>> +    .fw_path        = "amdnpu/17f0_10/",
>>       .rt_config    = npu4_default_rt_cfg,
>>       .dpm_clk_tbl    = npu4_dpm_clk_table,
>>       .fw_feature_tbl = npu4_fw_feature_table,
>

Claude review: accel/amdxdna: Use a different name for latest firmware

[Claude Code Review Bot]

Overall Series Review

Subject: accel/amdxdna: Use a different name for latest firmware
Author: Lizhi Hou <lizhi.hou@amd.com>
Patches: 3
Reviewed: 2026-02-27T12:53:21.622485

---

This is a single-patch fix for a firmware compatibility issue where a legacy driver loading the latest firmware causes a power-off problem. The approach — renaming the latest firmware to `npu_7.sbin` and using a fallback chain — is reasonable and follows a common kernel pattern. However, the patch has several issues that should be addressed: a missing `const` qualifier, a misleading error message after the semantic change to `fw_path`, a missing `MODULE_FIRMWARE` entry for the `17f0_20` variant, and the `XDNA_INFO` message being printed on every successful probe.

---

---
Generated by Claude Code Patch Reviewer

Claude review: accel/amdxdna: Use a different name for latest firmware

[Claude Code Review Bot]

Patch Review

**1. Missing `const` qualifiers on `npu_fw` array**

```c
static char *npu_fw[] = {
	"npu_7.sbin",
	"npu.sbin"
};
```

This should be `static const char * const npu_fw[]`. Both the pointers and the pointed-to strings are string literals and should never be modified. The kernel's `-Wwrite-strings` warning and general const-correctness conventions expect this.

**2. Misleading error message after `fw_path` semantic change**

The `fw_path` field has been changed from a full firmware path (e.g., `"amdnpu/1502_00/npu.sbin"`) to a directory prefix (e.g., `"amdnpu/1502_00/"`). But the error message after the loop is unchanged:

```c
if (ret) {
    XDNA_ERR(xdna, "failed to request_firmware %s, ret %d",
             ndev->priv->fw_path, ret);
```

This will now print something like `"failed to request_firmware amdnpu/1502_00/, ret -2"` which is incomplete — it doesn't tell the user which firmware files were actually attempted. Consider logging all attempted names, e.g.:

```c
XDNA_ERR(xdna, "failed to load firmware from %s (tried %s, %s), ret %d",
         ndev->priv->fw_path, npu_fw[0], npu_fw[1], ret);
```

Or alternatively, log each individual failure at `dev_dbg` level inside the loop.

**3. Missing `MODULE_FIRMWARE` for `17f0_20/npu_7.sbin`**

The existing code declares `MODULE_FIRMWARE("amdnpu/17f0_20/npu.sbin")` but the patch does not add a corresponding `MODULE_FIRMWARE("amdnpu/17f0_20/npu_7.sbin")`. Since the firmware loading loop is global (applies to all NPU variants), any device using the `17f0_20` directory will also attempt to load `npu_7.sbin`. The `MODULE_FIRMWARE` declarations should match what the driver may try to load so that packaging tools (like `dracut`, `mkinitcpio`) can include the right files. If `17f0_20` genuinely does not have an `npu_7.sbin` firmware, a comment explaining why would be helpful.

**4. `XDNA_INFO` on every successful probe is noisy**

```c
if (!ret) {
    XDNA_INFO(xdna, "Load firmware %s%s", ndev->priv->fw_path, npu_fw[i]);
    break;
}
```

This prints an info-level message on every successful device probe. Typically firmware load success is logged at debug level (or not at all, since `request_firmware` already logs). Consider using `XDNA_DBG` instead, or only logging when the fallback firmware is used (i.e., `i > 0`), which is the actually interesting case.

**5. `fw_path` field name no longer reflects its meaning**

The struct field `fw_path` in `struct amdxdna_dev_priv` (defined in `aie2_pci.h:249`) previously held a full firmware path and now holds a directory prefix. The name `fw_path` is now misleading. Consider renaming it to `fw_dir` or adding a comment at the struct definition to clarify the new semantics:

```c
const char  *fw_path;  /* firmware directory prefix, e.g. "amdnpu/1502_00/" */
```

**6. Minor: `fw_full_path` string is constructed twice**

The firmware path string is constructed with `kasprintf` and freed, then the same concatenation is repeated in the `XDNA_INFO` format string. This is fine functionally, but you could move the `XDNA_INFO` before the `kfree` and just print `fw_full_path` for cleaner code:

```c
ret = firmware_request_nowarn(&fw, fw_full_path, &pdev->dev);
if (!ret) {
    XDNA_INFO(xdna, "Loaded firmware %s", fw_full_path);
    kfree(fw_full_path);
    break;
}
kfree(fw_full_path);
```

**Overall**: The fix approach is sound — using a separate firmware filename with fallback is a clean way to handle the compatibility issue. The issues above are all addressable in a V3. The `const` fix and the error message fix are the most important; the rest are improvements.

---
Generated by Claude Code Patch Reviewer

Claude review: accel/amdxdna: Use a different name for latest firmware

[Claude Code Review Bot]

Overall Series Review

Subject: accel/amdxdna: Use a different name for latest firmware
Author: Lizhi Hou <lizhi.hou@amd.com>
Patches: 3
Reviewed: 2026-02-27T12:57:27.075175

---

This is a single-patch fix for a power-off issue caused by loading the latest firmware with a legacy driver. The approach is to introduce a new firmware filename (`npu_7.sbin`) for the latest firmware and implement a fallback mechanism: try the new firmware first, then fall back to the old `npu.sbin`. The concept is sound, but the implementation has several issues ranging from missing `const` qualifiers, to using `request_firmware()` (which logs errors) for an expected-to-fail probe, to a silently dropped `MODULE_FIRMWARE` entry, and an error message that now prints a directory instead of a filename.

---
Generated by Claude Code Patch Reviewer

Claude review: accel/amdxdna: Use a different name for latest firmware

[Claude Code Review Bot]

Patch Review

**1. Use `request_firmware_nowarn()` for the fallback probe**

The first iteration of the loop tries `npu_7.sbin`, which will not exist on systems with older firmware packages. Using `request_firmware()` will log a kernel error/warning to dmesg on every such system, alarming users with a spurious failure message. The non-final attempts should use `firmware_request_nowarn()` (or `request_firmware_direct()`) to suppress the expected failure, and only use `request_firmware()` for the final fallback.

```c
+	for (i = 0; i < ARRAY_SIZE(npu_fw); i++) {
+		fw_full_path = kasprintf(GFP_KERNEL, "%s%s", ndev->priv->fw_path,
+					 npu_fw[i]);
+		if (!fw_full_path)
+			return -ENOMEM;
+
+		ret = request_firmware(&fw, fw_full_path, &pdev->dev);
```

**2. Missing `const` qualifiers on `npu_fw[]`**

The array holds pointers to string literals. It should be `static const char * const npu_fw[]` to prevent both the pointers and the strings from being modified.

```c
+static char *npu_fw[] = {
+	"npu_7.sbin",
+	"npu.sbin"
+};
```

Should be:
```c
static const char * const npu_fw[] = {
	"npu_7.sbin",
	"npu.sbin",
};
```

Also missing a trailing comma after `"npu.sbin"` per kernel coding style.

**3. Silently dropped `MODULE_FIRMWARE("amdnpu/17f0_20/npu.sbin")`**

The patch removes the `17f0_20` MODULE_FIRMWARE entry without any explanation in the commit message:

```c
-MODULE_FIRMWARE("amdnpu/17f0_20/npu.sbin");
+MODULE_FIRMWARE("amdnpu/1502_00/npu_7.sbin");
+MODULE_FIRMWARE("amdnpu/17f0_10/npu_7.sbin");
+MODULE_FIRMWARE("amdnpu/17f0_11/npu_7.sbin");
```

If this path was previously required by some device variant, its removal is a regression. If it was stale/incorrect, that should be a separate fix or at least noted in the commit message. Additionally, no `npu_7.sbin` entry is added for `17f0_20`, which seems inconsistent.

**4. Degraded error message after firmware load failure**

When all firmware loading fails, the error message now prints just the directory prefix instead of an actual firmware filename:

```c
 	if (ret) {
 		XDNA_ERR(xdna, "failed to request_firmware %s, ret %d",
-			 ndev->priv->fw_path, ret);
+				 ndev->priv->fw_path, ret);
```

`ndev->priv->fw_path` is now `"amdnpu/1502_00/"` instead of `"amdnpu/1502_00/npu.sbin"`. The message won't tell the user which specific firmware files were tried. Consider printing the filenames that were attempted, or at minimum the last one.

**5. Unnecessary whitespace change in the error message**

The alignment of `ndev->priv->fw_path` in the error message changes from one tab to two tabs. This is a cosmetic change unrelated to the fix and adds noise to the diff.

**6. Semantic change to `fw_path` field meaning**

The `fw_path` field in `struct amdxdna_dev_priv` is changed from holding a full firmware file path to holding a directory prefix. This is a silent semantic change that could confuse future developers. The field name now no longer describes its contents. Consider renaming it to `fw_dir` or similar to make the new semantics clear.

**7. `npu_fw[]` scope is wider than necessary**

The `npu_fw[]` array is declared at module scope but only used inside `aie2_init()`. It could be declared locally within the function, or if kept at file scope, should at minimum have the `const` qualifiers mentioned above.

**8. Naming choice `npu_7.sbin`**

The commit message doesn't explain the significance of "7" in the firmware filename. A brief note on why this name was chosen (firmware version 7? protocol version 7?) would help future readers understand the naming convention.

---
Generated by Claude Code Patch Reviewer