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

[Claude Code Review Bot <claude-review@example.com>]

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