[PATCH 0/2] Bluetooth: hci_bcm: Autobaud mode support

[PATCH 0/2] Bluetooth: hci_bcm: Autobaud mode support

[Hakan Jansson]

Some devices (e.g. CYW5557x) require autobaud mode to enable FW loading.
Autobaud mode can also be required on some boards where the controller device
is using a non-standard baud rate when first powered on.

Only a limited subset of HCI commands are supported in autobaud mode.

These patches add a DT property, "brcm,uses-autobaud-mode", to control autobaud
mode selection.

Hakan Jansson (2):
  dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode
  Bluetooth: hci_bcm: Add support for FW loading in autobaud mode

 .../bindings/net/broadcom-bluetooth.yaml      |  7 +++++
 drivers/bluetooth/btbcm.c                     | 31 ++++++++++++++-----
 drivers/bluetooth/btbcm.h                     |  8 ++---
 drivers/bluetooth/hci_bcm.c                   | 15 +++++++--
 4 files changed, 46 insertions(+), 15 deletions(-)


base-commit: 48b57999e38745b707abe233019786cc097df3c9
-- 
2.25.1

[PATCH 1/2] dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode

[Hakan Jansson]

Some devices (e.g. CYW5557x) require autobaud mode to enable FW loading.
Autobaud mode can also be required on some boards where the controller
device is using a non-standard baud rate when first powered on.

This patch adds a property, "brcm,uses-autobaud-mode", to enable autobaud
mode selection.

Signed-off-by: Hakan Jansson <hakan.jansson@infineon.com>
---
 .../devicetree/bindings/net/broadcom-bluetooth.yaml        | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
index 5aac094fd217..26bba3f1c935 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml
@@ -92,6 +92,13 @@ properties:
        pcm-sync-mode: slave, master
        pcm-clock-mode: slave, master
 
+  brcm,uses-autobaud-mode:
+    type: boolean
+    description: >
+      The controller should be started in autobaud mode by asserting
+      BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported
+      in autobaud mode should be used until patch FW has been loaded.
+
   interrupts:
     items:
       - description: Handle to the line HOST_WAKE used to wake
-- 
2.25.1

[PATCH 2/2] Bluetooth: hci_bcm: Add support for FW loading in autobaud mode

[Hakan Jansson]

Some devices (e.g. CYW5557x) require autobaud mode to enable FW loading.
Autobaud mode can also be required on some boards where the controller
device is using a non-standard baud rate when first powered on.

Only a limited subset of HCI commands are supported in autobaud mode.

This patch looks for a DT property, "brcm,uses-autobaud-mode", to enable
autobaud mode selection. If the property is present, the device is started
in autobaud mode by keeping RTS asserted while powering on the device. The
patch also prevents the use of unsupported commands for devices started in
autobaud mode.

Signed-off-by: Hakan Jansson <hakan.jansson@infineon.com>
---
 drivers/bluetooth/btbcm.c   | 31 +++++++++++++++++++++++--------
 drivers/bluetooth/btbcm.h   |  8 ++++----
 drivers/bluetooth/hci_bcm.c | 15 ++++++++++++---
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 92a2b7e81757..0c0958030c0a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -403,6 +403,13 @@ static int btbcm_read_info(struct hci_dev *hdev)
 	bt_dev_info(hdev, "BCM: chip id %u", skb->data[1]);
 	kfree_skb(skb);
 
+	return 0;
+}
+
+static int btbcm_print_controller_features(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+
 	/* Read Controller Features */
 	skb = btbcm_read_controller_features(hdev);
 	if (IS_ERR(skb))
@@ -513,7 +520,7 @@ static const char *btbcm_get_board_name(struct device *dev)
 #endif
 }
 
-int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done)
+int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done, bool use_autobaud_mode)
 {
 	u16 subver, rev, pid, vid;
 	struct sk_buff *skb;
@@ -550,9 +557,16 @@ int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done)
 		if (err)
 			return err;
 	}
-	err = btbcm_print_local_name(hdev);
-	if (err)
-		return err;
+
+	if (!use_autobaud_mode) {
+		err = btbcm_print_controller_features(hdev);
+		if (err)
+			return err;
+
+		err = btbcm_print_local_name(hdev);
+		if (err)
+			return err;
+	}
 
 	bcm_subver_table = (hdev->bus == HCI_USB) ? bcm_usb_subver_table :
 						    bcm_uart_subver_table;
@@ -635,13 +649,13 @@ int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done)
 }
 EXPORT_SYMBOL_GPL(btbcm_initialize);
 
-int btbcm_finalize(struct hci_dev *hdev, bool *fw_load_done)
+int btbcm_finalize(struct hci_dev *hdev, bool *fw_load_done, bool use_autobaud_mode)
 {
 	int err;
 
 	/* Re-initialize if necessary */
 	if (*fw_load_done) {
-		err = btbcm_initialize(hdev, fw_load_done);
+		err = btbcm_initialize(hdev, fw_load_done, use_autobaud_mode);
 		if (err)
 			return err;
 	}
@@ -657,15 +671,16 @@ EXPORT_SYMBOL_GPL(btbcm_finalize);
 int btbcm_setup_patchram(struct hci_dev *hdev)
 {
 	bool fw_load_done = false;
+	bool use_autobaud_mode = false;
 	int err;
 
 	/* Initialize */
-	err = btbcm_initialize(hdev, &fw_load_done);
+	err = btbcm_initialize(hdev, &fw_load_done, use_autobaud_mode);
 	if (err)
 		return err;
 
 	/* Re-initialize after loading Patch */
-	return btbcm_finalize(hdev, &fw_load_done);
+	return btbcm_finalize(hdev, &fw_load_done, use_autobaud_mode);
 }
 EXPORT_SYMBOL_GPL(btbcm_setup_patchram);
 
diff --git a/drivers/bluetooth/btbcm.h b/drivers/bluetooth/btbcm.h
index 8bf01565fdfc..b4cb24231a20 100644
--- a/drivers/bluetooth/btbcm.h
+++ b/drivers/bluetooth/btbcm.h
@@ -62,8 +62,8 @@ int btbcm_write_pcm_int_params(struct hci_dev *hdev,
 int btbcm_setup_patchram(struct hci_dev *hdev);
 int btbcm_setup_apple(struct hci_dev *hdev);
 
-int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done);
-int btbcm_finalize(struct hci_dev *hdev, bool *fw_load_done);
+int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done, bool use_autobaud_mode);
+int btbcm_finalize(struct hci_dev *hdev, bool *fw_load_done, bool use_autobaud_mode);
 
 #else
 
@@ -104,12 +104,12 @@ static inline int btbcm_setup_apple(struct hci_dev *hdev)
 	return 0;
 }
 
-static inline int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done)
+static inline int btbcm_initialize(struct hci_dev *hdev, bool *fw_load_done, bool use_autobaud_mode)
 {
 	return 0;
 }
 
-static inline int btbcm_finalize(struct hci_dev *hdev, bool *fw_load_done)
+static inline int btbcm_finalize(struct hci_dev *hdev, bool *fw_load_done, bool use_autobaud_mode)
 {
 	return 0;
 }
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 785f445dd60d..0230cfcc8e3e 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -99,6 +99,7 @@ struct bcm_device_data {
  * @no_early_set_baudrate: don't set_baudrate before setup()
  * @drive_rts_on_open: drive RTS signal on ->open() when platform requires it
  * @pcm_int_params: keep the initial PCM configuration
+ * @use_autobaud_mode: start Bluetooth device in autobaud mode
  */
 struct bcm_device {
 	/* Must be the first member, hci_serdev.c expects this. */
@@ -136,6 +137,7 @@ struct bcm_device {
 #endif
 	bool			no_early_set_baudrate;
 	bool			drive_rts_on_open;
+	bool			use_autobaud_mode;
 	u8			pcm_int_params[5];
 };
 
@@ -472,7 +474,9 @@ static int bcm_open(struct hci_uart *hu)
 
 out:
 	if (bcm->dev) {
-		if (bcm->dev->drive_rts_on_open)
+		if (bcm->dev->use_autobaud_mode)
+			hci_uart_set_flow_control(hu, false);	/* Assert BT_UART_CTS_N */
+		else if (bcm->dev->drive_rts_on_open)
 			hci_uart_set_flow_control(hu, true);
 
 		hu->init_speed = bcm->dev->init_speed;
@@ -564,6 +568,7 @@ static int bcm_setup(struct hci_uart *hu)
 {
 	struct bcm_data *bcm = hu->priv;
 	bool fw_load_done = false;
+	bool use_autobaud_mode = (bcm->dev ? bcm->dev->use_autobaud_mode : 0);
 	unsigned int speed;
 	int err;
 
@@ -572,7 +577,7 @@ static int bcm_setup(struct hci_uart *hu)
 	hu->hdev->set_diag = bcm_set_diag;
 	hu->hdev->set_bdaddr = btbcm_set_bdaddr;
 
-	err = btbcm_initialize(hu->hdev, &fw_load_done);
+	err = btbcm_initialize(hu->hdev, &fw_load_done, use_autobaud_mode);
 	if (err)
 		return err;
 
@@ -616,7 +621,7 @@ static int bcm_setup(struct hci_uart *hu)
 		btbcm_write_pcm_int_params(hu->hdev, &params);
 	}
 
-	err = btbcm_finalize(hu->hdev, &fw_load_done);
+	err = btbcm_finalize(hu->hdev, &fw_load_done, use_autobaud_mode);
 	if (err)
 		return err;
 
@@ -1197,6 +1202,10 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 
 static int bcm_of_probe(struct bcm_device *bdev)
 {
+	bdev->use_autobaud_mode = device_property_read_bool(bdev->dev, "brcm,uses-autobaud-mode");
+	if (bdev->use_autobaud_mode)
+		bdev->no_early_set_baudrate = true;
+
 	device_property_read_u32(bdev->dev, "max-speed", &bdev->oper_speed);
 	device_property_read_u8_array(bdev->dev, "brcm,bt-pcm-int-params",
 				      bdev->pcm_int_params, 5);
-- 
2.25.1

RE: Bluetooth: hci_bcm: Autobaud mode support

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=638213

---Test result---

Test Summary:
CheckPatch                    PASS      3.55 seconds
GitLint                       PASS      2.05 seconds
SubjectPrefix                 FAIL      0.89 seconds
BuildKernel                   PASS      31.78 seconds
BuildKernel32                 PASS      28.33 seconds
Incremental Build with patchesPASS      46.11 seconds
TestRunner: Setup             PASS      478.79 seconds
TestRunner: l2cap-tester      PASS      17.45 seconds
TestRunner: bnep-tester       PASS      6.18 seconds
TestRunner: mgmt-tester       PASS      101.56 seconds
TestRunner: rfcomm-tester     PASS      9.85 seconds
TestRunner: sco-tester        PASS      9.53 seconds
TestRunner: smp-tester        PASS      9.57 seconds
TestRunner: userchan-tester   PASS      6.40 seconds

Details
##############################
Test: SubjectPrefix - FAIL - 0.89 seconds
Check subject contains "Bluetooth" prefix
"Bluetooth: " is not specified in the subject



---
Regards,
Linux Bluetooth

Re: [PATCH 1/2] dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode

[Linus Walleij]

On Wed, May 4, 2022 at 11:04 AM Hakan Jansson
<hakan.jansson@infineon.com> wrote:

> +  brcm,uses-autobaud-mode:
> +    type: boolean
> +    description: >
> +      The controller should be started in autobaud mode by asserting
> +      BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported
> +      in autobaud mode should be used until patch FW has been loaded.

Things like gnss uses the current-speed attribute to set a serial link speed,
maybe also Bluetooth?

Would it make sense to use

current-speed-auto;

As a flag for these things in general?

Yours,
Linus Walleij

Re: [PATCH 1/2] dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode

[Hakan Jansson]

Hi Linus,

Thanks for responding.

On 5/5/2022 12:15 AM, Linus Walleij wrote:
> On Wed, May 4, 2022 at 11:04 AM Hakan Jansson
> <hakan.jansson@infineon.com> wrote:
>
>> +  brcm,uses-autobaud-mode:
>> +    type: boolean
>> +    description: >
>> +      The controller should be started in autobaud mode by asserting
>> +      BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported
>> +      in autobaud mode should be used until patch FW has been loaded.
> Things like gnss uses the current-speed attribute to set a serial link speed,
> maybe also Bluetooth?
As far as I can tell, not many Bluetooth drivers use the current-speed 
attribute yet but perhaps it would be a good idea to start using it more 
broadly in the future to set the initial serial link speed.

>
> Would it make sense to use
>
> current-speed-auto;
>
> As a flag for these things in general?
I suppose a general flag could be useful but to be honest I don't know 
if any other devices besides the ones using the Broadcom driver has any 
use for it. You would probably also still want to be able to use 
current-speed to set the link speed and end up using both 
current-speed=x and current-speed-auto at the same time, which might 
look a little confusing?

Please let me know if you'd still prefer "current-speed-auto" over 
"brcm,uses-autobaud-mode" and I'll revise the patch and rename it!

>
> Yours,
> Linus Walleij

Thanks,
Håkan

Re: [PATCH 1/2] dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode

[Linus Walleij]

On Thu, May 5, 2022 at 3:11 PM Hakan Jansson <hakan.jansson@infineon.com> wrote:

> I suppose a general flag could be useful but to be honest I don't know
> if any other devices besides the ones using the Broadcom driver has any
> use for it. You would probably also still want to be able to use
> current-speed to set the link speed and end up using both
> current-speed=x and current-speed-auto at the same time, which might
> look a little confusing?

I do not think it is more confusing than being able to use
current-speed and brcm,uses-autobaud-mode at the same time.

> Please let me know if you'd still prefer "current-speed-auto" over
> "brcm,uses-autobaud-mode" and I'll revise the patch and rename it!

It actually depends a bit.

This:

> >> +      The controller should be started in autobaud mode by asserting
> >> +      BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported
> >> +      in autobaud mode should be used until patch FW has been loaded.

sounds a bit vague?

Does it mean that CTS is asserted, then you send a bit (CTS then goes low)
and then CTS is asserted again when the device is ready to receieve more
data? i.e is this some kind of one-bit mode, because it doesn't sound like
it is using CTS as it was used in legacy modems.

Some more explanation of this mode is needed so we can understand if
this is something generic or a BRCM-only thing.

Yours,
Linus Walleij

Re: [PATCH 1/2] dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode

[Hakan Jansson]

On 5/5/2022 4:13 PM, Linus Walleij wrote:
> I suppose a general flag could be useful but to be honest I don't know
>> if any other devices besides the ones using the Broadcom driver has any
>> use for it. You would probably also still want to be able to use
>> current-speed to set the link speed and end up using both
>> current-speed=x and current-speed-auto at the same time, which might
>> look a little confusing?
> I do not think it is more confusing than being able to use
> current-speed and brcm,uses-autobaud-mode at the same time.
>
>> Please let me know if you'd still prefer "current-speed-auto" over
>> "brcm,uses-autobaud-mode" and I'll revise the patch and rename it!
> It actually depends a bit.
>
> This:
>
>>>> +      The controller should be started in autobaud mode by asserting
>>>> +      BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI commands supported
>>>> +      in autobaud mode should be used until patch FW has been loaded.
> sounds a bit vague?

Yes, perhaps. I was thinking the details could be helpful but I can see 
how they might be perceived as vague and confusing. Maybe it would be 
better to just leave it at "The controller should be started in autobaud 
mode"?

>
> Does it mean that CTS is asserted, then you send a bit (CTS then goes low)
> and then CTS is asserted again when the device is ready to receieve more
> data? i.e is this some kind of one-bit mode, because it doesn't sound like
> it is using CTS as it was used in legacy modems.

CTS and RTS are actually used in the normal way during communication. 
The host will assert its RTS to indicate being ready to receive data 
from the controller. This flag just controls whether this happens before 
or after the controller is powered on. The controller will check the 
initial state of its BT_UART_CTS_N pin (connected to host's RTS) when 
starting up. It will enter autobaud mode if the signal is already asserted.

> Some more explanation of this mode is needed so we can understand if
> this is something generic or a BRCM-only thing.
>
> Yours,
> Linus Walleij

Thanks,
Håkan

Re: [PATCH 1/2] dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode

[Hakan Jansson]

Hi Linus,

I checked the state of this patch on Devicetree Bindings Patchwork and 
it's marked "Changes Requested". I'd be happy to revise the patch but 
it's not clear to me what changes are requested. Could you please help 
guide me how to proceed?

Sorry if I'm missing something obvious here.

Thanks,
Håkan

On 5/5/2022 6:16 PM, Hakan Jansson wrote:
>
>
> On 5/5/2022 4:13 PM, Linus Walleij wrote:
>> I suppose a general flag could be useful but to be honest I don't know
>>> if any other devices besides the ones using the Broadcom driver has any
>>> use for it. You would probably also still want to be able to use
>>> current-speed to set the link speed and end up using both
>>> current-speed=x and current-speed-auto at the same time, which might
>>> look a little confusing?
>> I do not think it is more confusing than being able to use
>> current-speed and brcm,uses-autobaud-mode at the same time.
>>
>>> Please let me know if you'd still prefer "current-speed-auto" over
>>> "brcm,uses-autobaud-mode" and I'll revise the patch and rename it!
>> It actually depends a bit.
>>
>> This:
>>
>>>>> +      The controller should be started in autobaud mode by asserting
>>>>> +      BT_UART_CTS_N (i.e. host RTS) during startup. Only HCI 
>>>>> commands supported
>>>>> +      in autobaud mode should be used until patch FW has been 
>>>>> loaded.
>> sounds a bit vague?
>
> Yes, perhaps. I was thinking the details could be helpful but I can 
> see how they might be perceived as vague and confusing. Maybe it would 
> be better to just leave it at "The controller should be started in 
> autobaud mode"?
>
>>
>> Does it mean that CTS is asserted, then you send a bit (CTS then goes 
>> low)
>> and then CTS is asserted again when the device is ready to receieve more
>> data? i.e is this some kind of one-bit mode, because it doesn't sound 
>> like
>> it is using CTS as it was used in legacy modems.
>
> CTS and RTS are actually used in the normal way during communication. 
> The host will assert its RTS to indicate being ready to receive data 
> from the controller. This flag just controls whether this happens 
> before or after the controller is powered on. The controller will 
> check the initial state of its BT_UART_CTS_N pin (connected to host's 
> RTS) when starting up. It will enter autobaud mode if the signal is 
> already asserted.
>
>> Some more explanation of this mode is needed so we can understand if
>> this is something generic or a BRCM-only thing.
>>
>> Yours,
>> Linus Walleij
>
> Thanks,
> Håkan

Re: [PATCH 1/2] dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode

[Linus Walleij]

On Thu, May 19, 2022 at 4:04 PM Hakan Jansson
<hakan.jansson@infineon.com> wrote:

> I checked the state of this patch on Devicetree Bindings Patchwork and
> it's marked "Changes Requested". I'd be happy to revise the patch but
> it's not clear to me what changes are requested. Could you please help
> guide me how to proceed?

Sorry if it's not clear but I'm essentially requesting that the document
describe how autobaud mode actually works. The binding documentation
must help DT authors to know what they should do with this property.

Yours,
Linus Walleij