diff mbox series

[next,2/2] Bluetooth: btnxpuart: support multiple init baudrates

Message ID 20240823124239.2263107-2-catalin.popescu@leica-geosystems.com
State New
Headers show
Series [next,1/2] dt-bindings: net: bluetooth: nxp: support multiple init baudrates | expand

Commit Message

Catalin Popescu Aug. 23, 2024, 12:42 p.m. UTC 
We need to support simultaneously different versions of the same chip
that are using different baudrates (example: engineering and production
samples). This happens typically when OTP has been modified and new
primary baudrate is being used. As there's no other way to detect the
correct baudrate, we need the driver to go through the list of configured
baudrates and try each one out until it finds the matching one.

Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
---
 drivers/bluetooth/btnxpuart.c | 45 ++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 14 deletions(-)

Comments

Neeraj Sanjay Kale Aug. 26, 2024, 8:50 a.m. UTC | #1 
Hello Catalin,

Thank you for your patch. I have some comments below.

> 
> We need to support simultaneously different versions of the same chip that
> are using different baudrates (example: engineering and production samples).
> This happens typically when OTP has been modified and new primary
> baudrate is being used. As there's no other way to detect the correct
> baudrate, we need the driver to go through the list of configured baudrates
> and try each one out until it finds the matching one.
By design, it is expected for customer to know the OTP setting in the chip from respective module vendor documentation and mention the value in device tree property fw-init-baudrate.
Please check with your module vendor to synchronize the OTP settings in engineering and production chips.
Feel free to create a ticket in our customer support portal (https://www.nxp.com/support/support:SUPPORTHOME) for any queries.

> @@ static int nxp_setup(struct hci_dev *hdev)
>                 clear_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
>         }
> 
> -       serdev_device_set_baudrate(nxpdev->serdev, nxpdev-
> >fw_init_baudrate);
> -       nxpdev->current_baudrate = nxpdev->fw_init_baudrate;
> +       for (i = 0; i < ARRAY_SIZE(nxpdev->fw_init_baudrate); i++) {
> +               serdev_device_set_baudrate(nxpdev->serdev, nxpdev-
> >fw_init_baudrate[i]);
> +               nxpdev->current_baudrate = nxpdev->fw_init_baudrate[i];
> +
> +               if (nxpdev->current_baudrate != HCI_NXP_SEC_BAUDRATE) {
> +                       nxpdev->new_baudrate = HCI_NXP_SEC_BAUDRATE;
> +                       err = nxp_set_baudrate_cmd(hdev, NULL);
> +                       if (err)
> +                               continue;
> +               }
> 
> -       if (nxpdev->current_baudrate != HCI_NXP_SEC_BAUDRATE) {
> -               nxpdev->new_baudrate = HCI_NXP_SEC_BAUDRATE;
> -               hci_cmd_sync_queue(hdev, nxp_set_baudrate_cmd, NULL, NULL);
> +               nxpdev->fw_primary_baudrate = nxpdev->fw_init_baudrate[i];
> +               bt_dev_dbg(hdev, "Using primary baudrate %d", nxpdev-
> >fw_primary_baudrate);
> +               break;
>         }
Looping through a fw-init-baudrate array and finding correct baudrate through trial-and-error method would increase initialization time by 2 seconds for each failed iteration. (command timeout)
This will affect other customers who are critical about the time it takes for hci interface to be up after device bootup.

Thanks,
Neeraj
diff mbox series

Patch

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 7c2030cec10e..2bfb0ab114f4 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -180,7 +180,8 @@  struct btnxpuart_dev {
 
 	u32 new_baudrate;
 	u32 current_baudrate;
-	u32 fw_init_baudrate;
+	u32 fw_primary_baudrate;
+	u32 fw_init_baudrate[8];
 	bool timeout_changed;
 	bool baudrate_changed;
 	bool helper_downloaded;
@@ -1159,7 +1160,7 @@  static int nxp_set_ind_reset(struct hci_dev *hdev, void *data)
 static int nxp_setup(struct hci_dev *hdev)
 {
 	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
-	int err = 0;
+	int i, err = 0;
 
 	if (nxp_check_boot_sign(nxpdev)) {
 		bt_dev_dbg(hdev, "Need FW Download.");
@@ -1171,12 +1172,20 @@  static int nxp_setup(struct hci_dev *hdev)
 		clear_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
 	}
 
-	serdev_device_set_baudrate(nxpdev->serdev, nxpdev->fw_init_baudrate);
-	nxpdev->current_baudrate = nxpdev->fw_init_baudrate;
+	for (i = 0; i < ARRAY_SIZE(nxpdev->fw_init_baudrate); i++) {
+		serdev_device_set_baudrate(nxpdev->serdev, nxpdev->fw_init_baudrate[i]);
+		nxpdev->current_baudrate = nxpdev->fw_init_baudrate[i];
+
+		if (nxpdev->current_baudrate != HCI_NXP_SEC_BAUDRATE) {
+			nxpdev->new_baudrate = HCI_NXP_SEC_BAUDRATE;
+			err = nxp_set_baudrate_cmd(hdev, NULL);
+			if (err)
+				continue;
+		}
 
-	if (nxpdev->current_baudrate != HCI_NXP_SEC_BAUDRATE) {
-		nxpdev->new_baudrate = HCI_NXP_SEC_BAUDRATE;
-		hci_cmd_sync_queue(hdev, nxp_set_baudrate_cmd, NULL, NULL);
+		nxpdev->fw_primary_baudrate = nxpdev->fw_init_baudrate[i];
+		bt_dev_dbg(hdev, "Using primary baudrate %d", nxpdev->fw_primary_baudrate);
+		break;
 	}
 
 	ps_init(hdev);
@@ -1454,6 +1463,10 @@  static int nxp_serdev_probe(struct serdev_device *serdev)
 {
 	struct hci_dev *hdev;
 	struct btnxpuart_dev *nxpdev;
+	struct property *prop;
+	const __be32 *cur;
+	u32 value;
+	int i = 0;
 
 	nxpdev = devm_kzalloc(&serdev->dev, sizeof(*nxpdev), GFP_KERNEL);
 	if (!nxpdev)
@@ -1472,10 +1485,13 @@  static int nxp_serdev_probe(struct serdev_device *serdev)
 	init_waitqueue_head(&nxpdev->fw_dnld_done_wait_q);
 	init_waitqueue_head(&nxpdev->check_boot_sign_wait_q);
 
-	device_property_read_u32(&nxpdev->serdev->dev, "fw-init-baudrate",
-				 &nxpdev->fw_init_baudrate);
-	if (!nxpdev->fw_init_baudrate)
-		nxpdev->fw_init_baudrate = FW_INIT_BAUDRATE;
+	nxpdev->fw_init_baudrate[0] = FW_INIT_BAUDRATE;
+	of_property_for_each_u32(dev_of_node(&nxpdev->serdev->dev),
+				 "fw-init-baudrate", prop, cur, value) {
+		nxpdev->fw_init_baudrate[i] = value;
+		if (++i == ARRAY_SIZE(nxpdev->fw_init_baudrate))
+			break;
+	}
 
 	set_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
 
@@ -1525,12 +1541,13 @@  static void nxp_serdev_remove(struct serdev_device *serdev)
 		wake_up_interruptible(&nxpdev->check_boot_sign_wait_q);
 		wake_up_interruptible(&nxpdev->fw_dnld_done_wait_q);
 	} else {
-		/* Restore FW baudrate to fw_init_baudrate if changed.
+		/* Restore FW baudrate to fw_primary_baudrate if changed.
 		 * This will ensure FW baudrate is in sync with
 		 * driver baudrate in case this driver is re-inserted.
 		 */
-		if (nxpdev->current_baudrate != nxpdev->fw_init_baudrate) {
-			nxpdev->new_baudrate = nxpdev->fw_init_baudrate;
+		if (nxpdev->fw_primary_baudrate &&
+		    (nxpdev->current_baudrate != nxpdev->fw_primary_baudrate)) {
+			nxpdev->new_baudrate = nxpdev->fw_primary_baudrate;
 			nxp_set_baudrate_cmd(hdev, NULL);
 		}
 	}