Re: [PATCH v3 2/2] Bluetooth: btnxpuart: Update firmware names

[Paul Menzel]

[Cc: regressions@]

Dear Neeraj,


Am 29.05.24 um 11:53 schrieb Neeraj Sanjay Kale:
> This updates the firmware names of 3 chipsets: w8987, w8997, w9098.
> These changes are been done to standardize chip specific firmware
> file names.

Can you please describe the new naming schema in the commit message?

> To allow user to use older firmware file names, a new device tree
> property has been introduced called firmware-name, which will override
> the hardcoded firmware names in the driver.

So users updating the Linux kernel but not updating the devicetree with 
the new property are going to see a regression, right? I think this 
violates Linux’ no regression policy. If so, please implement a way to 
support old and new names.

> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@xxxxxxx>
> ---
> v2: Remove "nxp/" from all firmware name definitions to be inline with
> firware file name read from device tree file. (Krzysztof)

fir*m*ware

> ---
>   drivers/bluetooth/btnxpuart.c | 28 +++++++++++++++++-----------
>   1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index 0b93c2ff29e4..4442d911eba8 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -33,16 +33,16 @@
>   /* NXP HW err codes */
>   #define BTNXPUART_IR_HW_ERR		0xb0
>   
> -#define FIRMWARE_W8987		"nxp/uartuart8987_bt.bin"
> -#define FIRMWARE_W8997		"nxp/uartuart8997_bt_v4.bin"
> -#define FIRMWARE_W9098		"nxp/uartuart9098_bt_v1.bin"
> -#define FIRMWARE_IW416		"nxp/uartiw416_bt_v0.bin"
> -#define FIRMWARE_IW612		"nxp/uartspi_n61x_v1.bin.se"
> -#define FIRMWARE_IW624		"nxp/uartiw624_bt.bin"
> -#define FIRMWARE_SECURE_IW624	"nxp/uartiw624_bt.bin.se"
> -#define FIRMWARE_AW693		"nxp/uartaw693_bt.bin"
> -#define FIRMWARE_SECURE_AW693	"nxp/uartaw693_bt.bin.se"
> -#define FIRMWARE_HELPER		"nxp/helper_uart_3000000.bin"
> +#define FIRMWARE_W8987		"uart8987_bt_v0.bin"
> +#define FIRMWARE_W8997		"uart8997_bt_v4.bin"
> +#define FIRMWARE_W9098		"uart9098_bt_v1.bin"
> +#define FIRMWARE_IW416		"uartiw416_bt_v0.bin"
> +#define FIRMWARE_IW612		"uartspi_n61x_v1.bin.se"
> +#define FIRMWARE_IW624		"uartiw624_bt.bin"
> +#define FIRMWARE_SECURE_IW624	"uartiw624_bt.bin.se"
> +#define FIRMWARE_AW693		"uartaw693_bt.bin"
> +#define FIRMWARE_SECURE_AW693	"uartaw693_bt.bin.se"
> +#define FIRMWARE_HELPER		"helper_uart_3000000.bin"
>   
>   #define CHIP_ID_W9098		0x5c03
>   #define CHIP_ID_IW416		0x7201
> @@ -685,13 +685,19 @@ static bool process_boot_signature(struct btnxpuart_dev *nxpdev)
>   static int nxp_request_firmware(struct hci_dev *hdev, const char *fw_name)
>   {
>   	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
> +	const char *fw_name_dt;
>   	int err = 0;
>   
>   	if (!fw_name)
>   		return -ENOENT;
>   
>   	if (!strlen(nxpdev->fw_name)) {
> -		snprintf(nxpdev->fw_name, MAX_FW_FILE_NAME_LEN, "%s", fw_name);
> +		if (strcmp(fw_name, FIRMWARE_HELPER) &&
> +		    !device_property_read_string(&nxpdev->serdev->dev,
> +						 "firmware-name",
> +						 &fw_name_dt))
> +			fw_name = fw_name_dt;
> +		snprintf(nxpdev->fw_name, MAX_FW_FILE_NAME_LEN, "nxp/%s", fw_name);
>   
>   		bt_dev_dbg(hdev, "Request Firmware: %s", nxpdev->fw_name);
>   		err = request_firmware(&nxpdev->fw, nxpdev->fw_name, &hdev->dev);


Kind regards,

Paul