Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-5023-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <52ccf0c1-e5dd-412b-9e47-7829ca0f6ffc@molgen.mpg.de>
Date: Wed, 29 May 2024 12:05:42 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 2/2] Bluetooth: btnxpuart: Update firmware names
To: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
Cc: marcel@holtmann.org, luiz.dentz@gmail.com, davem@davemloft.net,
 edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, robh@kernel.org,
 krzk+dt@kernel.org, conor+dt@kernel.org, linux-bluetooth@vger.kernel.org,
 netdev@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org, amitkumar.karwar@nxp.com, rohit.fule@nxp.com,
 sherry.sun@nxp.com, ziniu.wang_1@nxp.com, haibo.chen@nxp.com,
 LnxRevLi@nxp.com, regressions@lists.linux.dev
References: <20240529095347.22186-1-neeraj.sanjaykale@nxp.com>
 <20240529095347.22186-3-neeraj.sanjaykale@nxp.com>
Content-Language: en-US
From: Paul Menzel <pmenzel@molgen.mpg.de>
In-Reply-To: <20240529095347.22186-3-neeraj.sanjaykale@nxp.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

[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@nxp.com>
> ---
> 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