Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-4755-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
To: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>, "marcel@holtmann.org"
	<marcel@holtmann.org>, "luiz.dentz@gmail.com" <luiz.dentz@gmail.com>
CC: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Amitkumar
 Karwar <amitkumar.karwar@nxp.com>, Rohit Fule <rohit.fule@nxp.com>, Sherry
 Sun <sherry.sun@nxp.com>, Luke Wang <ziniu.wang_1@nxp.com>, Bough Chen
	<haibo.chen@nxp.com>, LnxRevLi <LnxRevLi@nxp.com>
Subject: RE: [PATCH v1 1/1] Bluetooth: btnxpuart: Add handling for
 boot-signature timeout errors
Thread-Topic: [PATCH v1 1/1] Bluetooth: btnxpuart: Add handling for
 boot-signature timeout errors
Thread-Index: AQHapd5qEJe1KSoTXkWbNdMsKRB8DrGbCEug
Date: Fri, 17 May 2024 07:16:57 +0000
Message-ID:
 <AS4PR04MB9692D509D7AAC5F5CDFA5032E7EE2@AS4PR04MB9692.eurprd04.prod.outlook.com>
References: <20240514090913.1507990-1-neeraj.sanjaykale@nxp.com>
In-Reply-To: <20240514090913.1507990-1-neeraj.sanjaykale@nxp.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?cEZ7rRKSzOrrBeChpWwNfPgPbLkRnEWucj2C5dHNHxWrX7imVNwn3Ed2N6i0?=
 =?us-ascii?Q?c8DBRBFELsljEflSSCV1VzODzfDJX9DvolYR/j3ZqkjzUBx6POxNmw/gmeI6?=
 =?us-ascii?Q?R4UzMV0LICxEVmxTjqAAqYOHhU24C+Dtbn6Hv/z7DzJYwXnQuo5g1Mn6JZCr?=
 =?us-ascii?Q?BPOv6mRtSSg1Ptk5DNOthm1xVxpqOpmo13Kkj2dV9JVlpI9WH5nxwH4qnkvF?=
 =?us-ascii?Q?gddScZRNhicLag5J2lD5qzaLbsmokf9SBKg1uUS5wp4hV6nZ4vTyQe7odQpS?=
 =?us-ascii?Q?d/HpYDQjSn64nxWuhszswcE130tBpDH8+or1eiJtPgkY2uLxn5jlR+LTmyYm?=
 =?us-ascii?Q?i7iLQRZpFSv5fwy0YJFh7cbUh6/I4KQ4feo6PPCPan3kleIuqDcWwckWWn8a?=
 =?us-ascii?Q?EdN1wiryzcmkQw/hGx9+KgDG8yrGyCeB0mzsJSiwqxqdmXgEvT/5bQX0vb1k?=
 =?us-ascii?Q?QAiVJQXStjX+WtikQkELszhkSrW7bUqqsSeVw4qBsukVbuUkTRmeNyW5xJnJ?=
 =?us-ascii?Q?n298YNIVQPFai25iMoh2ORuiYfjZOQkKaR2C6J0vvjGyWg5NvhLmmSWHv1ss?=
 =?us-ascii?Q?2XWUMiUxe7w6eBdSo/9rKrc/bTCwebsn0/p9e83qUaT7V8AGcnHfLZbfNwsl?=
 =?us-ascii?Q?ZihdoVN2GFi5DMevXWi67DKbHPoR7fIOhbf5D5P2cVHgmH84KW5bjINy3ZFP?=
 =?us-ascii?Q?/kVCdA7wvjBRiTokoEtbccN7JZmtgvHS/t+DjqyF4FSD1Zlzo8jfIH649t75?=
 =?us-ascii?Q?IG4yC0jsskK+ogYBta/t8CxWKgHVoohLp4rUKSXrTxbQpqLXEnaSqRo5LbD1?=
 =?us-ascii?Q?0wRdNJOniT73HXKuxzV6NkvZUTOL0N8ij82l3/wuyJ1UwdeyzxHtUWpBeIhy?=
 =?us-ascii?Q?wR9sYeOCmYU1hOCDIs6t+iizTZLPkg6RejOPck8DJpjrNMzWz8DK5jdhbH+T?=
 =?us-ascii?Q?7OCtyvUbTQzOI2ZYSxUNPLRwjl4PVsOiI6ExaPDYFxXwRCfxKv7iYFMdL4u+?=
 =?us-ascii?Q?QcH93vxe0QCJEaubT5kI2hcXXzTqScjz8MiGM0398GNyZcr/4giQgbD10/FI?=
 =?us-ascii?Q?N6V2b1IiQ7ENCsQIzgT5ww7bKRa2hdfvjS5mgW98SuXf4dvY0r8gntKLNOwR?=
 =?us-ascii?Q?9KapTpVsi7omtbPK0rXAD/cAcOBOJjhtkkqJhGWd5gpbzPtV/ObRU6AMgo4X?=
 =?us-ascii?Q?u6DvhWiGohp9WrQ2k51CtxUUqJI0mnthzSYxDK6mSexiWxM8YjM7T92gxxtJ?=
 =?us-ascii?Q?douxNGacKx1Y31V+z5udbpP62zHqdmkOH+KIZuwpeWMG8KX7nZn/hJuhj/jS?=
 =?us-ascii?Q?+51VC/iQz5AcK7xedkUjl4NAgBA7xGN/RsA+Cvc8ao8Su30miPsMf62LUaMe?=
 =?us-ascii?Q?GbhQ9MKGB2z7dMHc6q7NxujG5bpiEAlDan/CUTSsEzmm9rkc6xQ1I/MCuKRO?=
 =?us-ascii?Q?qoNMMBj9WxWv2akbB4X/j9/8dJ1ToT9AVbu0vFo0ZQBMc87XDLEvDwKvqhUY?=
 =?us-ascii?Q?CyMKvt1qkUh9yudifPTmYYF5K7nGBBx9IxMmrKjAl8vOLNkfHsDB1mvTLRr3?=
 =?us-ascii?Q?4mL7GR2WFvNnhkb6chmf3FgLRJJamGpVBsoMjagz?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-bluetooth@vger.kernel.org
List-Id: <linux-bluetooth.vger.kernel.org>
List-Subscribe: <mailto:linux-bluetooth+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-bluetooth+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: AS4PR04MB9692.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 06816073-79bf-4fab-54da-08dc7641568d
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 May 2024 07:16:57.6270
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: a9Jkb3kyJaWxROQlw/+KNCtDM0vaBQPAocteiFpsTq77oYiXfhNYFJzEM9sn9lGw9AAVlu/+o/NEMbiEyjsuEo+n+kT3rDTczoKCoZVXi40=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8454

Hi Luiz,

I have re-sent this patch. Can you please review this?
Thank you for your time.

Thanks,
Neeraj

=20
> This handles the timeout errors seen in the bootloader signatures during =
FW
> download.
>=20
> When the bootloader does not receive a response packet from the host
> within a specific time, it adds an error code to the bootloader signature=
 while
> requesting for the FW chunk from the same offset.
>=20
> The host is expected to clear this error code with a NAK, and reply to on=
ly
> those bootloader signatures which have error code 0.
>=20
> This error handling is valid for data_req bootloader signatures for V3 an=
d
> future bootloader versions.
>=20
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
>  drivers/bluetooth/btnxpuart.c | 46 ++++++++++++++++++++++++++++++++---
>  1 file changed, 42 insertions(+), 4 deletions(-)
>=20
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.=
c
> index 0b93c2ff29e4..2018513fb961 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -187,6 +187,10 @@ struct btnxpuart_dev {
>  #define NXP_NAK_V3		0x7b
>  #define NXP_CRC_ERROR_V3	0x7c
>=20
> +#define NXP_ACK_RX_TIMEOUT	0x0002
> +#define NXP_HDR_RX_TIMEOUT	0x0003
> +#define NXP_DATA_RX_TIMEOUT	0x0004
> +
>  #define HDR_LEN			16
>=20
>  #define NXP_RECV_CHIP_VER_V1 \
> @@ -277,6 +281,12 @@ struct nxp_bootloader_cmd {
>  	__be32 crc;
>  } __packed;
>=20
> +struct nxp_v3_rx_timeout_nak {
> +	u8 nak;
> +	__le32 offset;
> +	u8 crc;
> +} __packed;
> +
>  static u8 crc8_table[CRC8_TABLE_SIZE];
>=20
>  /* Default configurations */
> @@ -899,6 +909,32 @@ static int nxp_recv_chip_ver_v3(struct hci_dev *hdev=
,
> struct sk_buff *skb)
>  	return 0;
>  }
>=20
> +static void nxp_handle_fw_dnld_error(struct hci_dev *hdev, struct
> +v3_data_req *req) {
> +	struct btnxpuart_dev *nxpdev =3D hci_get_drvdata(hdev);
> +	__u32 offset =3D __le32_to_cpu(req->offset);
> +	__u16 err =3D __le16_to_cpu(req->error);
> +	struct nxp_v3_rx_timeout_nak nak_tx_buf;
> +
> +	switch (err) {
> +	case NXP_ACK_RX_TIMEOUT:
> +	case NXP_HDR_RX_TIMEOUT:
> +	case NXP_DATA_RX_TIMEOUT:
> +		nak_tx_buf.nak =3D NXP_NAK_V3;
> +		nak_tx_buf.offset =3D __cpu_to_le32(offset);
> +		nak_tx_buf.crc =3D crc8(crc8_table, (u8 *)&nak_tx_buf,
> +				      sizeof(nak_tx_buf) - 1, 0xff);
> +		serdev_device_write_buf(nxpdev->serdev, (u8
> *)&nak_tx_buf,
> +					sizeof(nak_tx_buf));
> +		break;
> +	default:
> +		bt_dev_dbg(hdev, "Unknown bootloader error: %d", err);
> +		break;
> +
> +	}
> +
> +}
> +
>  static int nxp_recv_fw_req_v3(struct hci_dev *hdev, struct sk_buff *skb)=
  {
>  	struct btnxpuart_dev *nxpdev =3D hci_get_drvdata(hdev); @@ -913,7
> +949,12 @@ static int nxp_recv_fw_req_v3(struct hci_dev *hdev, struct
> sk_buff *skb)
>  	if (!req || !nxpdev->fw)
>  		goto free_skb;
>=20
> -	nxp_send_ack(NXP_ACK_V3, hdev);
> +	if (!req->error) {
> +		nxp_send_ack(NXP_ACK_V3, hdev);
> +	} else {
> +		nxp_handle_fw_dnld_error(hdev, req);
> +		goto free_skb;
> +	}
>=20
>  	len =3D __le16_to_cpu(req->len);
>=20
> @@ -940,9 +981,6 @@ static int nxp_recv_fw_req_v3(struct hci_dev *hdev,
> struct sk_buff *skb)
>  		wake_up_interruptible(&nxpdev->fw_dnld_done_wait_q);
>  		goto free_skb;
>  	}
> -	if (req->error)
> -		bt_dev_dbg(hdev, "FW Download received err 0x%02x from
> chip",
> -			   req->error);
>=20
>  	offset =3D __le32_to_cpu(req->offset);
>  	if (offset < nxpdev->fw_v3_offset_correction) {
> --
> 2.34.1