dt-bindings: net: broadcom-bluetooth: Add CYW4373A0 DT binding

PATCH Superseded Marek Vasut 5 participants 12 messages Oct 31, 2022 – Nov 2, 2022

Subsystems (3)

Network drivers Device tree bindings Bluetooth drivers

lore

Share & tools

lore

Marek Vasut

author

2022-10-31 20:48 UTC #0 lore raw

PATCH 1/2 dt-bindings: net: broadcom-bluetooth: Add CYW4373A0 DT binding

CYW4373A0 is a Wi-Fi + Bluetooth combo device from Cypress. This chip is present e.g. on muRata 2AE module. Extend the binding with its DT compatible.

Signed-off-by Marek Vasut <[email protected]>

---
Cc: Hakan Jansson <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Luiz Augusto von Dentz <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
To: [email protected]

---
 Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml | 1 +
 1 file changed, 1 insertion(+)

Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml +1 -0

index 445b2a5536259..e0c8abc274134 100644
@@ -24,6 +24,7 @@ properties:
       - brcm,bcm43540-bt
       - brcm,bcm4335a0
       - brcm,bcm4349-bt
+      - cypress,cyw4373a0-bt
       - infineon,cyw55572-bt
 
   shutdown-gpios:

Signature · 1 line

2.35.1

Marek Vasut

author

2022-10-31 20:48 UTC #1 lore raw

PATCH 2/2 Bluetooth: hci_bcm: Add CYW4373A0 support

CYW4373A0 is a Wi-Fi + Bluetooth combo device from Cypress. This chip is present e.g. on muRata 2AE module.

This chip has additional quirk where the HCI command 0xfc45, used on older chips to switch UART clock from 24 MHz to 48 MHz, to support baudrates over 3 Mbdps, is no longer recognized by this newer chip. This newer chip can configure the 4 Mbdps baudrate without the need to issue HCI command 0xfc45, so add flag to indicate this and do not issue the command on this chip to avoid failure to set 4 Mbdps baud rate.

It is not clear whether there is a way to determine which chip does and which chip does not support the HCI command 0xfc45, other than trial and error.

Signed-off-by Marek Vasut <[email protected]>

---
 drivers/bluetooth/hci_bcm.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

drivers/bluetooth/hci_bcm.c +12 -1

index d7e0b75db8a60..29da04815763f 100644
@@ -53,11 +53,13 @@
  * struct bcm_device_data - device specific data
  * @no_early_set_baudrate: Disallow set baudrate before driver setup()
  * @drive_rts_on_open: drive RTS signal on ->open() when platform requires it
+ * @no_uart_clock_set: UART clock set command for >3Mbps mode is unavailable
  * @max_autobaud_speed: max baudrate supported by device in autobaud mode
  */
 struct bcm_device_data {
 	bool	no_early_set_baudrate;
 	bool	drive_rts_on_open;
+	bool	no_uart_clock_set;
 	u32	max_autobaud_speed;
 };

@@ -100,6 +102,7 @@ struct bcm_device_data {
  * @is_suspended: whether flow control is currently disabled
  * @no_early_set_baudrate: don't set_baudrate before setup()
  * @drive_rts_on_open: drive RTS signal on ->open() when platform requires it
+ * @no_uart_clock_set: UART clock set command for >3Mbps mode is unavailable
  * @pcm_int_params: keep the initial PCM configuration
  * @use_autobaud_mode: start Bluetooth device in autobaud mode
  * @max_autobaud_speed: max baudrate supported by device in autobaud mode

@@ -140,6 +143,7 @@ struct bcm_device {
 #endif
 	bool			no_early_set_baudrate;
 	bool			drive_rts_on_open;
+	bool			no_uart_clock_set;
 	bool			use_autobaud_mode;
 	u8			pcm_int_params[5];
 	u32			max_autobaud_speed;

@@ -172,10 +176,11 @@ static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed)
 static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
 {
 	struct hci_dev *hdev = hu->hdev;
+	struct bcm_data *bcm = hu->priv;
 	struct sk_buff *skb;
 	struct bcm_update_uart_baud_rate param;
 
-	if (speed > 3000000) {
+	if (speed > 3000000 && !bcm->dev->no_uart_clock_set) {
 		struct bcm_write_uart_clock_setting clock;
 
 		clock.type = BCM_UART_CLOCK_48MHZ;

@@ -1529,6 +1534,7 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
 		bcmdev->max_autobaud_speed = data->max_autobaud_speed;
 		bcmdev->no_early_set_baudrate = data->no_early_set_baudrate;
 		bcmdev->drive_rts_on_open = data->drive_rts_on_open;
+		bcmdev->no_uart_clock_set = data->no_uart_clock_set;
 	}
 
 	return hci_uart_register_device(&bcmdev->serdev_hu, &bcm_proto);

@@ -1550,6 +1556,10 @@ static struct bcm_device_data bcm43438_device_data = {
 	.drive_rts_on_open = true,
 };
 
+static struct bcm_device_data bcm4373a0_device_data = {
+	.no_uart_clock_set = true,
+};
+
 static struct bcm_device_data cyw55572_device_data = {
 	.max_autobaud_speed = 921600,
 };

@@ -1566,6 +1576,7 @@ static const struct of_device_id bcm_bluetooth_of_match[] = {
 	{ .compatible = "brcm,bcm4349-bt", .data = &bcm43438_device_data },
 	{ .compatible = "brcm,bcm43540-bt", .data = &bcm4354_device_data },
 	{ .compatible = "brcm,bcm4335a0" },
+	{ .compatible = "cypress,cyw4373a0-bt", .data = &bcm4373a0_device_data },
 	{ .compatible = "infineon,cyw55572-bt", .data = &cyw55572_device_data },
 	{ },
 };

Signature · 1 line

2.35.1

Linus Walleij

Reviewed-by on v1 2/2

2022-11-01 22:24 UTC #2 lore raw

Re: [PATCH 2/2] Bluetooth: hci_bcm: Add CYW4373A0 support

Quoted 16 lines · Marek Vasut

CYW4373A0 is a Wi-Fi + Bluetooth combo device from Cypress. This chip is present e.g. on muRata 2AE module.
This chip has additional quirk where the HCI command 0xfc45, used on older chips to switch UART clock from 24 MHz to 48 MHz, to support baudrates over 3 Mbdps, is no longer recognized by this newer chip. This newer chip can configure the 4 Mbdps baudrate without the need to issue HCI command 0xfc45, so add flag to indicate this and do not issue the command on this chip to avoid failure to set 4 Mbdps baud rate.
It is not clear whether there is a way to determine which chip does and which chip does not support the HCI command 0xfc45, other than trial and error.
Signed-off-by Marek Vasut <[email protected]>

(...)

drivers/bluetooth/hci_bcm.c static struct bcm_device_data bcm43438_device_data = { · line 1561

Quoted 3 lines

drivers/bluetooth/hci_bcm.c +3 -0

+static struct bcm_device_data bcm4373a0_device_data = {
+       .no_uart_clock_set = true,
+};

Shouldn't this be named cyw4373a0_device_data like the compatible string and the device below it?

With that fixed:

Reviewed-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

[email protected]

2022-10-31 22:07 UTC #3 lore raw

In reply to Marek Vasut

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=690631

---Test result---

Test Summary:

CheckPatch                    PASS      2.99 seconds
GitLint                       PASS      1.48 seconds
SubjectPrefix                 FAIL      0.58 seconds
BuildKernel                   PASS      34.02 seconds
BuildKernel32                 PASS      30.96 seconds
Incremental Build with patchesPASS      54.02 seconds
TestRunner: Setup             PASS      513.84 seconds
TestRunner: l2cap-tester      PASS      17.53 seconds
TestRunner: iso-tester        PASS      16.75 seconds
TestRunner: bnep-tester       PASS      6.65 seconds
TestRunner: mgmt-tester       PASS      106.19 seconds
TestRunner: rfcomm-tester     PASS      10.48 seconds
TestRunner: sco-tester        PASS      9.85 seconds
TestRunner: ioctl-tester      PASS      11.30 seconds
TestRunner: mesh-tester       PASS      8.02 seconds
TestRunner: smp-tester        PASS      9.87 seconds
TestRunner: userchan-tester   PASS      6.82 seconds

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

---
Regards,
Linux Bluetooth

Marek Vasut

author

2022-11-01 22:39 UTC #4 lore raw

Quoted 8 lines · On 10/31/22 23:07, [email protected] wrote:

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=690631

Quoted 6 lines

---Test result---

Test Summary:

CheckPatch                    PASS      2.99 seconds
GitLint                       PASS      1.48 seconds
SubjectPrefix                 FAIL      0.58 seconds

Should the DT bindings really have Bluetooth: prefix/tag too ? git log on prior art indicates they shouldn't .

Luiz Augusto von Dentz

maintainer

2022-11-01 22:45 UTC #5 lore raw

Hi Marek,

Quoted 12 lines · Marek Vasut

Quoted 10 lines · On 10/31/22 23:07, [email protected] wrote:
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=690631
---Test result---

Quoted 7 lines

Quoted 4 lines
Test Summary:
CheckPatch                    PASS      2.99 seconds
GitLint                       PASS      1.48 seconds
SubjectPrefix                 FAIL      0.58 seconds
Should the DT bindings really have Bluetooth: prefix/tag too ? git log on prior art indicates they shouldn't .

If it is meant for bluetooth-next then yes it shall contain it since the CI does attempt to check its presence, in the other hand we could perhaps use the prefix [bluetooth] to avoid having the CI run on patches that are not meant for bluetooth-next but I don't think other subsystem do require this so it sort of hard to enforce proper prefixing.

Signature · 1 line

Luiz Augusto von Dentz

Marek Vasut

author

2022-11-01 22:53 UTC #6 lore raw

Quoted 1 line · Luiz Augusto von Dentz

Hi Marek,

Hi,

Quoted 21 lines · Marek Vasut

Quoted 12 lines · On 10/31/22 23:07, [email protected] wrote:
Quoted 10 lines
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=690631
---Test result---
Quoted 7 lines
Quoted 4 lines
Test Summary:
CheckPatch                    PASS      2.99 seconds
GitLint                       PASS      1.48 seconds
SubjectPrefix                 FAIL      0.58 seconds
Should the DT bindings really have Bluetooth: prefix/tag too ? git log on prior art indicates they shouldn't .

Quoted 6 lines

If it is meant for bluetooth-next then yes it shall contain it since the CI does attempt to check its presence, in the other hand we could perhaps use the prefix [bluetooth] to avoid having the CI run on patches that are not meant for bluetooth-next but I don't think other subsystem do require this so it sort of hard to enforce proper prefixing.

Linux Documentation/devicetree/bindings seems to start with dt-bindings: prefix always, so maybe we should keep it that way ?

This binding document seems to follow the scheme now too:

next$ git log --oneline --follow Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml c8ce64900db47 dt-bindings: net: broadcom-bluetooth: Add conditional constraints cba6164f7c5e3 dt-bindings: net: broadcom-bluetooth: Add CYW55572 DT binding 88b65887aa1b7 dt-bindings: bluetooth: broadcom: Add BCM4349B1 DT binding 0b4de2523f281 dt-bindings: net: broadcom-bluetooth: Add property for autobaud mode 88ffadce9d4cc dt-bindings: bluetooth: broadcom: Fix clocks check
71793730ebfdb Bluetooth: btbcm: Add BCM4334 DT binding
7820ee1c4757d Bluetooth: btbcm: Rewrite bindings in YAML and add reset

Rob Herring

maintainer

2022-11-02 16:39 UTC #7 lore raw

Quoted 33 lines · Marek Vasut

Quoted 1 line · Luiz Augusto von Dentz
Hi Marek,
Hi,
Quoted 21 lines · Marek Vasut
Quoted 12 lines · On 10/31/22 23:07, [email protected] wrote:
Quoted 10 lines
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=690631
---Test result---
Quoted 7 lines
Quoted 4 lines
Test Summary:
CheckPatch                    PASS      2.99 seconds
GitLint                       PASS      1.48 seconds
SubjectPrefix                 FAIL      0.58 seconds
Should the DT bindings really have Bluetooth: prefix/tag too ? git log on prior art indicates they shouldn't .
Quoted 6 lines
If it is meant for bluetooth-next then yes it shall contain it since the CI does attempt to check its presence, in the other hand we could perhaps use the prefix [bluetooth] to avoid having the CI run on patches that are not meant for bluetooth-next but I don't think other subsystem do require this so it sort of hard to enforce proper prefixing.

Quoted 2 lines

Linux Documentation/devicetree/bindings seems to start with dt-bindings:
prefix always, so maybe we should keep it that way ?

Yes.

Linus Walleij

Reviewed-by on v1 1/2

2022-11-01 22:20 UTC #8 lore raw

In reply to Marek Vasut

Quoted 5 lines · Marek Vasut

CYW4373A0 is a Wi-Fi + Bluetooth combo device from Cypress. This chip is present e.g. on muRata 2AE module. Extend the binding with its DT compatible.
Signed-off-by Marek Vasut <[email protected]>

Reviewed-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

Luiz Augusto von Dentz

maintainer

2022-11-01 22:39 UTC #9 lore raw

Hi Linus,

Quoted 10 lines · Linus Walleij

Quoted 5 lines · Marek Vasut
CYW4373A0 is a Wi-Fi + Bluetooth combo device from Cypress. This chip is present e.g. on muRata 2AE module. Extend the binding with its DT compatible.
Signed-off-by Marek Vasut <[email protected]>
Reviewed-by Linus Walleij <[email protected]>

Should we have these changes in bluetooth-next or dt has a dedicated tree?

Signature · 1 line

Luiz Augusto von Dentz

Rob Herring

maintainer

2022-11-02 16:41 UTC #10 lore raw

Quoted 10 lines · Luiz Augusto von Dentz

Hi Linus,
Quoted 7 lines · Linus Walleij
Quoted 4 lines · Marek Vasut
CYW4373A0 is a Wi-Fi + Bluetooth combo device from Cypress. This chip is present e.g. on muRata 2AE module. Extend the binding with its DT compatible.

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
Signed-off-by Marek Vasut <[email protected]>
Reviewed-by Linus Walleij <[email protected]>
Should we have these changes in bluetooth-next or dt has a dedicated tree?

Most of the time, they go thru the subsystem trees like bluetooth-next.

Rob

Rob Herring

maintainer Acked-by on v1 1/2

2022-11-02 16:42 UTC #11 lore raw

In reply to Marek Vasut

Quoted 14 lines · Marek Vasut

CYW4373A0 is a Wi-Fi + Bluetooth combo device from Cypress. This chip is present e.g. on muRata 2AE module. Extend the binding with its DT compatible.
Signed-off-by Marek Vasut <[email protected]>
---
Cc: Hakan Jansson <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Luiz Augusto von Dentz <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: [email protected]
To: [email protected]

Quoted 4 lines

---
 Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml | 1 +
 1 file changed, 1 insertion(+)

Acked-by Rob Herring <[email protected]>