Add baudrate management for Bluetooth UART

PATCH Superseded Frederic Danis 2 participants 12 messages May 28, 2015 – Jun 6, 2015 Bluetooth drivers

Share & tools

Frederic Danis

author

2015-05-28 09:25 UTC #0 lore raw

v7 cover Add baudrate management for Bluetooth UART

This patch set introduce baudrate management for Bluetooth UART Controller setup and a first implementation for Broadcom BCM4324B3 used in Asus T100.

v6 -> v7:
- Fix setup when firmware is missing by moving request/release_firmware() out of btbcm_patchram().

v2 -> v6:
- Fixes for __hci_cmd_sync() has been integrated (patch v5)
- In btbcm_check_bdaddr(), merge controller address tests
- Fix declaration of speeds to use "unsigned int" type
- Rename default_speed to init_speed
- Rename operational_speed to oper_speed
- In case of error during vendor specific set_baudrate(), do not change host baudrate and continue without changing speed
- Discard changes to bcm_setup_patchram() and in btusb.c
- Introduce helper functions btbcm_initialize() and btbcm_finalize() to be used by the UART side of things.

Frederic Danis (6):
Bluetooth: btbcm: Move request/release_firmware()
Bluetooth: btbcm: Add BCM4324B3 UART device
Bluetooth: hci_uart: Support operational speed during setup
Bluetooth: btbcm: Add helper functions for UART setup
Bluetooth: hci_uart: Update Broadcom UART setup
Bluetooth: hci_uart: Add bcm_set_baudrate()

 drivers/bluetooth/btbcm.c     | 135 +++++++++++++++++++++++++++++++++---------
 drivers/bluetooth/btbcm.h     |  20 ++++++-
 drivers/bluetooth/hci_bcm.c   |  86 ++++++++++++++++++++++++++-
 drivers/bluetooth/hci_ldisc.c |  26 ++++++++
 drivers/bluetooth/hci_uart.h  |   4 ++
 5 files changed, 240 insertions(+), 31 deletions(-)

Signature · 1 line

1.9.1

Frederic Danis

author

2015-05-28 09:25 UTC #1 lore raw

PATCH v7 1/6 Bluetooth: btbcm: Move request/release_firmware()

Move request/release_firmware() out of btbcm_patchram(). This allows a better error management, if request_firmware() returns an error then the controller will be used without firmware loading and 0 is returned.
This will imply to change btbcm_patchram() to accept a firmware instead of firmware name.

Signed-off-by Frederic Danis <[email protected]>

---
 drivers/bluetooth/btbcm.c | 26 ++++++++++++--------------
 drivers/bluetooth/btbcm.h |  6 ++++--
 2 files changed, 16 insertions(+), 16 deletions(-)

drivers/bluetooth/btbcm.c +12 ~~-14~~

index 728fce3..c7aec97 100644
@@ -89,21 +89,14 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
 }
 EXPORT_SYMBOL_GPL(btbcm_set_bdaddr);
 
-int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
+int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
 {
 	const struct hci_command_hdr *cmd;
-	const struct firmware *fw;
 	const u8 *fw_ptr;
 	size_t fw_size;
 	struct sk_buff *skb;
 	u16 opcode;
-	int err;
-
-	err = request_firmware(&fw, firmware, &hdev->dev);
-	if (err < 0) {
-		BT_INFO("%s: BCM: Patch %s not found", hdev->name, firmware);
-		return err;
-	}
+	int err = 0;
 
 	/* Start Download */
 	skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT);

@@ -129,8 +122,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
 		fw_size -= sizeof(*cmd);
 
 		if (fw_size < cmd->plen) {
-			BT_ERR("%s: BCM: Patch %s is corrupted", hdev->name,
-			       firmware);
+			BT_ERR("%s: BCM: Patch is corrupted", hdev->name);
 			err = -EINVAL;
 			goto done;
 		}

@@ -156,7 +148,6 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
 	msleep(250);
 
 done:
-	release_firmware(fw);
 	return err;
 }
 EXPORT_SYMBOL(btbcm_patchram);

@@ -265,6 +256,7 @@ static const struct {
 int btbcm_setup_patchram(struct hci_dev *hdev)
 {
 	char fw_name[64];
+	const struct firmware *fw;
 	u16 subver, rev, pid, vid;
 	const char *hw_name = NULL;
 	struct sk_buff *skb;

@@ -335,9 +327,15 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 		hw_name ? : "BCM", (subver & 0x7000) >> 13,
 		(subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
 
-	err = btbcm_patchram(hdev, fw_name);
-	if (err == -ENOENT)
+	err = request_firmware(&fw, fw_name, &hdev->dev);
+	if (err < 0) {
+		BT_INFO("%s: BCM: Patch %s not found", hdev->name, fw_name);
 		return 0;
+	}
+
+	btbcm_patchram(hdev, fw);
+
+	release_firmware(fw);
 
 	/* Reset */
 	err = btbcm_reset(hdev);

drivers/bluetooth/btbcm.h +4 -2

index eb6ab5f..de11ace 100644
@@ -21,11 +21,13 @@
  *
  */
 
+#include <linux/firmware.h>
+
 #if IS_ENABLED(CONFIG_BT_BCM)
 
 int btbcm_check_bdaddr(struct hci_dev *hdev);
 int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
-int btbcm_patchram(struct hci_dev *hdev, const char *firmware);
+int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw);
 
 int btbcm_setup_patchram(struct hci_dev *hdev);
 int btbcm_setup_apple(struct hci_dev *hdev);

@@ -42,7 +44,7 @@ static inline int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
 	return -EOPNOTSUPP;
 }
 
-static inline int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
+static inline int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
 {
 	return -EOPNOTSUPP;
 }

Signature · 1 line

1.9.1

Marcel Holtmann

maintainer

2015-06-06 05:41 UTC #2 lore raw

Re: [PATCH v7 1/6] Bluetooth: btbcm: Move request/release_firmware()

Hi Fred,

Quoted 90 lines

Signed-off-by Frederic Danis <[email protected]>

---
drivers/bluetooth/btbcm.c | 26 ++++++++++++-------------- drivers/bluetooth/btbcm.h | 6 ++++--

2 files changed, 16 insertions(+), 16 deletions(-)

drivers/bluetooth/btbcm.c +3 ~~-11~~

index 728fce3..c7aec97 100644
@@ -89,21 +89,14 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
 }
 EXPORT_SYMBOL_GPL(btbcm_set_bdaddr);
 
-int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
+int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
 {
 	const struct hci_command_hdr *cmd;
-	const struct firmware *fw;
 	const u8 *fw_ptr;
 	size_t fw_size;
 	struct sk_buff *skb;
 	u16 opcode;
-	int err;
-
-	err = request_firmware(&fw, firmware, &hdev->dev);
-	if (err < 0) {
-		BT_INFO("%s: BCM: Patch %s not found", hdev->name, firmware);
-		return err;
-	}
+	int err = 0;
 
 	/* Start Download */
 	skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT);

@@ -129,8 +122,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
 		fw_size -= sizeof(*cmd);
 
 		if (fw_size < cmd->plen) {
-			BT_ERR("%s: BCM: Patch %s is corrupted", hdev->name,
-			       firmware);
+			BT_ERR("%s: BCM: Patch is corrupted", hdev->name);
 			err = -EINVAL;
 			goto done;
 		}

@@ -156,7 +148,6 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
 	msleep(250);

done:

drivers/bluetooth/btbcm.c +0 -1

-	release_firmware(fw);

	return err;
}

EXPORT_SYMBOL(btbcm_patchram);

drivers/bluetooth/btbcm.c +9 -2

@@ -265,6 +256,7 @@ static const struct {
 int btbcm_setup_patchram(struct hci_dev *hdev)
 {
 	char fw_name[64];
+	const struct firmware *fw;
 	u16 subver, rev, pid, vid;
 	const char *hw_name = NULL;
 	struct sk_buff *skb;

@@ -335,9 +327,15 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 		hw_name ? : "BCM", (subver & 0x7000) >> 13,
 		(subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
 
-	err = btbcm_patchram(hdev, fw_name);
-	if (err == -ENOENT)
+	err = request_firmware(&fw, fw_name, &hdev->dev);
+	if (err < 0) {
+		BT_INFO("%s: BCM: Patch %s not found", hdev->name, fw_name);
 		return 0;
+	}
+
+	btbcm_patchram(hdev, fw);
+
+	release_firmware(fw);
 
 	/* Reset */
 	err = btbcm_reset(hdev);

drivers/bluetooth/btbcm.h +0 -0

index eb6ab5f..de11ace 100644

drivers/bluetooth/btbcm.h line 25

Quoted 5 lines

drivers/bluetooth/btbcm.h +2 -0

@@ -21,11 +21,13 @@
 *
 */
 
+#include <linux/firmware.h>
+

I removed this include and put it into the files that reference this.

Regards

Marcel

Frederic Danis

author

2015-05-28 09:25 UTC #3 lore raw

PATCH v7 2/6 Bluetooth: btbcm: Add BCM4324B3 UART device

In reply to Frederic Danis

Add "waiting for configuration" address.
Add lmp_subver and firmware name for BCM4324B3 controller.

Signed-off-by Frederic Danis <[email protected]>

---
 drivers/bluetooth/btbcm.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

drivers/bluetooth/btbcm.c +7 -3

index c7aec97..e90e6b8 100644
@@ -33,6 +33,7 @@
 #define VERSION "0.1"
 
 #define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
+#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43}})
 
 int btbcm_check_bdaddr(struct hci_dev *hdev)
 {

@@ -56,10 +57,11 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
 
 	bda = (struct hci_rp_read_bd_addr *)skb->data;
 
-	/* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller
-	 * with no configured address.
+	/* Check if the address indicates a controller with no configured
+	 * address.
 	 */
-	if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0)) {
+	if (!bacmp(&bda->bdaddr, BDADDR_BCM20702A0) ||
+	    !bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
 		BT_INFO("%s: BCM: Using default device address (%pMR)",
 			hdev->name, &bda->bdaddr);
 		set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);

@@ -233,6 +235,7 @@ static const struct {
 	const char *name;
 } bcm_uart_subver_table[] = {
 	{ 0x410e, "BCM43341B0"	},	/* 002.001.014 */
+	{ 0x4406, "BCM4324B3"	},	/* 002.004.006 */
 	{ }
 };

@@ -288,6 +291,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 
 	switch ((rev & 0xf000) >> 12) {
 	case 0:
+	case 3:
 		for (i = 0; bcm_uart_subver_table[i].name; i++) {
 			if (subver == bcm_uart_subver_table[i].subver) {
 				hw_name = bcm_uart_subver_table[i].name;

Signature · 1 line

1.9.1

Marcel Holtmann

maintainer

2015-06-06 05:41 UTC #4 lore raw

Re: [PATCH v7 2/6] Bluetooth: btbcm: Add BCM4324B3 UART device

Hi Fred,

Quoted 21 lines

Add "waiting for configuration" address.
Add lmp_subver and firmware name for BCM4324B3 controller.
Signed-off-by Frederic Danis <[email protected]>
---
drivers/bluetooth/btbcm.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
drivers/bluetooth/btbcm.c +1 -0
index c7aec97..e90e6b8 100644
@@ -33,6 +33,7 @@
 #define VERSION "0.1"
 
 #define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
+#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43}})
 
 int btbcm_check_bdaddr(struct hci_dev *hdev)
 {

drivers/bluetooth/btbcm.c int btbcm_check_bdaddr(struct hci_dev *hdev) · line 62

Quoted 8 lines

drivers/bluetooth/btbcm.c +2 -2

@@ -56,10 +57,11 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
 
 	bda = (struct hci_rp_read_bd_addr *)skb->data;
 
-	/* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller
-	 * with no configured address.
+	/* Check if the address indicates a controller with no configured
+	 * address.
 	 */

I did like the comment that shows the address in human readable format. So I re-phrased this and mentioned both addresses. It is just a lot easier to grep for in case anybody ever wants to find this.

Regards

Marcel

Frederic Danis

author

2015-05-28 09:25 UTC #5 lore raw

PATCH v7 3/6 Bluetooth: hci_uart: Support operational speed during setup

In reply to Frederic Danis

Add initial and operational speeds.
Change to operational speed as soon as possible. If controller set_baudrate() fails, continue at initial speed.

Signed-off-by Frederic Danis <[email protected]>

---
 drivers/bluetooth/hci_ldisc.c | 26 ++++++++++++++++++++++++++
 drivers/bluetooth/hci_uart.h  |  4 ++++
 2 files changed, 30 insertions(+)

drivers/bluetooth/hci_ldisc.c +26 -0

index 5c9a73f..a39e1ab 100644
@@ -265,11 +265,37 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 	return 0;
 }
 
+void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed)
+{
+	struct tty_struct *tty = hu->tty;
+	struct ktermios ktermios;
+
+	ktermios = tty->termios;
+	ktermios.c_cflag &= ~CBAUD;
+	ktermios.c_cflag |= BOTHER;
+	tty_termios_encode_baud_rate(&ktermios, speed, speed);
+
+	/* tty_set_termios() return not checked as it is always 0 */
+	tty_set_termios(tty, &ktermios);
+
+	BT_DBG("%s: New tty speed: %d", hu->hdev->name, tty->termios.c_ispeed);
+}
+
 static int hci_uart_setup(struct hci_dev *hdev)
 {
 	struct hci_uart *hu = hci_get_drvdata(hdev);
 	struct hci_rp_read_local_version *ver;
 	struct sk_buff *skb;
+	int err;
+
+	if (hu->proto->init_speed)
+		hci_uart_set_baudrate(hu, hu->proto->init_speed);
+
+	if (hu->proto->set_baudrate && hu->proto->oper_speed) {
+		err = hu->proto->set_baudrate(hu, hu->proto->oper_speed);
+		if (!err)
+			hci_uart_set_baudrate(hu, hu->proto->oper_speed);
+	}
 
 	if (hu->proto->setup)
 		return hu->proto->setup(hu);

drivers/bluetooth/hci_uart.h +4 -0

index 72120a5..e9f970c 100644
@@ -58,10 +58,13 @@ struct hci_uart;
 struct hci_uart_proto {
 	unsigned int id;
 	const char *name;
+	unsigned int init_speed;
+	unsigned int oper_speed;
 	int (*open)(struct hci_uart *hu);
 	int (*close)(struct hci_uart *hu);
 	int (*flush)(struct hci_uart *hu);
 	int (*setup)(struct hci_uart *hu);
+	int (*set_baudrate)(struct hci_uart *hu, unsigned int speed);
 	int (*recv)(struct hci_uart *hu, const void *data, int len);
 	int (*enqueue)(struct hci_uart *hu, struct sk_buff *skb);
 	struct sk_buff *(*dequeue)(struct hci_uart *hu);

@@ -96,6 +99,7 @@ int hci_uart_register_proto(const struct hci_uart_proto *p);
 int hci_uart_unregister_proto(const struct hci_uart_proto *p);
 int hci_uart_tx_wakeup(struct hci_uart *hu);
 int hci_uart_init_ready(struct hci_uart *hu);
+void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed);
 
 #ifdef CONFIG_BT_HCIUART_H4
 int h4_init(void);

Signature · 1 line

1.9.1

Frederic Danis

author

2015-05-28 09:25 UTC #6 lore raw

PATCH v7 4/6 Bluetooth: btbcm: Add helper functions for UART setup

In reply to Frederic Danis

Firmware loading may reset the controller UART speed and needs to set host UART speed back to init speed.

UART drivers setup is split in 3 parts:
- btbcm_initialize() resets the controller and returns the firmware name based on controller revision and sub_version.
- btbtcm_patchram() (already existing and public), which takes the firmware name as parameter, requests the firmware and loads it to the controller.
- btbcm_finalize() which resets the controller, reads local version and checks if the controller address is a default one or not.

Remove firmware name retrieval for UART controllers from btbcm_setup_patchram().

Signed-off-by Frederic Danis <[email protected]>

---
 drivers/bluetooth/btbcm.c | 101 ++++++++++++++++++++++++++++++++++++++++------
 drivers/bluetooth/btbcm.h |  14 +++++++
 2 files changed, 103 insertions(+), 12 deletions(-)

drivers/bluetooth/btbcm.c +89 ~~-12~~

index e90e6b8..d16878e 100644
@@ -239,6 +239,95 @@ static const struct {
 	{ }
 };
 
+int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len)
+{
+	u16 subver, rev;
+	const char *hw_name = NULL;
+	struct sk_buff *skb;
+	struct hci_rp_read_local_version *ver;
+	int i, err;
+
+	/* Reset */
+	err = btbcm_reset(hdev);
+	if (err)
+		return err;
+
+	/* Read Local Version Info */
+	skb = btbcm_read_local_version(hdev);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	ver = (struct hci_rp_read_local_version *)skb->data;
+	rev = le16_to_cpu(ver->hci_rev);
+	subver = le16_to_cpu(ver->lmp_subver);
+	kfree_skb(skb);
+
+	/* Read Verbose Config Version Info */
+	skb = btbcm_read_verbose_config(hdev);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
+	kfree_skb(skb);
+
+	switch ((rev & 0xf000) >> 12) {
+	case 0:
+	case 3:
+		for (i = 0; bcm_uart_subver_table[i].name; i++) {
+			if (subver == bcm_uart_subver_table[i].subver) {
+				hw_name = bcm_uart_subver_table[i].name;
+				break;
+			}
+		}
+
+		snprintf(fw_name, len, "brcm/%s.hcd", hw_name ? : "BCM");
+		break;
+	default:
+		return 0;
+	}
+
+	BT_INFO("%s: %s (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
+		hw_name ? : "BCM", (subver & 0x7000) >> 13,
+		(subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(btbcm_initialize);
+
+int btbcm_finalize(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	struct hci_rp_read_local_version *ver;
+	u16 subver, rev;
+	int err;
+
+	/* Reset */
+	err = btbcm_reset(hdev);
+	if (err)
+		return err;
+
+	/* Read Local Version Info */
+	skb = btbcm_read_local_version(hdev);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	ver = (struct hci_rp_read_local_version *)skb->data;
+	rev = le16_to_cpu(ver->hci_rev);
+	subver = le16_to_cpu(ver->lmp_subver);
+	kfree_skb(skb);
+
+	BT_INFO("%s: BCM (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
+		(subver & 0x7000) >> 13, (subver & 0x1f00) >> 8,
+		(subver & 0x00ff), rev & 0x0fff);
+
+	btbcm_check_bdaddr(hdev);
+
+	set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(btbcm_finalize);
+
 static const struct {
 	u16 subver;
 	const char *name;

@@ -290,18 +379,6 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 	kfree_skb(skb);
 
 	switch ((rev & 0xf000) >> 12) {
-	case 0:
-	case 3:
-		for (i = 0; bcm_uart_subver_table[i].name; i++) {
-			if (subver == bcm_uart_subver_table[i].subver) {
-				hw_name = bcm_uart_subver_table[i].name;
-				break;
-			}
-		}
-
-		snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
-			 hw_name ? : "BCM");
-		break;
 	case 1:
 	case 2:
 		/* Read USB Product Info */

drivers/bluetooth/btbcm.h +14 -0

index de11ace..6f1e418 100644
@@ -32,6 +32,9 @@ int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw);
 int btbcm_setup_patchram(struct hci_dev *hdev);
 int btbcm_setup_apple(struct hci_dev *hdev);
 
+int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len);
+int btbcm_finalize(struct hci_dev *hdev);
+
 #else
 
 static inline int btbcm_check_bdaddr(struct hci_dev *hdev)

@@ -59,4 +62,15 @@ static inline int btbcm_setup_apple(struct hci_dev *hdev)
 	return 0;
 }
 
+static inline int btbcm_initialize(struct hci_dev *hdev, char *fw_name,
+				   size_t len)
+{
+	return 0;
+}
+
+static inline int btbcm_finalize(struct hci_dev *hdev)
+{
+	return 0;
+}
+
 #endif

Signature · 1 line

1.9.1

Marcel Holtmann

maintainer

2015-06-06 05:41 UTC #7 lore raw

Re: [PATCH v7 4/6] Bluetooth: btbcm: Add helper functions for UART setup

Hi Fred,

Quoted 127 lines

Firmware loading may reset the controller UART speed and needs to set host UART speed back to init speed.

Remove firmware name retrieval for UART controllers from btbcm_setup_patchram().

Signed-off-by Frederic Danis <[email protected]>

---
drivers/bluetooth/btbcm.c | 101 ++++++++++++++++++++++++++++++++++++++++------ drivers/bluetooth/btbcm.h | 14 +++++++

2 files changed, 103 insertions(+), 12 deletions(-)

drivers/bluetooth/btbcm.c +89 -2

index e90e6b8..d16878e 100644
@@ -239,6 +239,95 @@ static const struct {
 	{ }
 };
 
+int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len)
+{
+	u16 subver, rev;
+	const char *hw_name = NULL;
+	struct sk_buff *skb;
+	struct hci_rp_read_local_version *ver;
+	int i, err;
+
+	/* Reset */
+	err = btbcm_reset(hdev);
+	if (err)
+		return err;
+
+	/* Read Local Version Info */
+	skb = btbcm_read_local_version(hdev);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	ver = (struct hci_rp_read_local_version *)skb->data;
+	rev = le16_to_cpu(ver->hci_rev);
+	subver = le16_to_cpu(ver->lmp_subver);
+	kfree_skb(skb);
+
+	/* Read Verbose Config Version Info */
+	skb = btbcm_read_verbose_config(hdev);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	BT_INFO("%s: BCM: chip id %u", hdev->name, skb->data[1]);
+	kfree_skb(skb);
+
+	switch ((rev & 0xf000) >> 12) {
+	case 0:
+	case 3:
+		for (i = 0; bcm_uart_subver_table[i].name; i++) {
+			if (subver == bcm_uart_subver_table[i].subver) {
+				hw_name = bcm_uart_subver_table[i].name;
+				break;
+			}
+		}
+
+		snprintf(fw_name, len, "brcm/%s.hcd", hw_name ? : "BCM");
+		break;
+	default:
+		return 0;
+	}
+
+	BT_INFO("%s: %s (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
+		hw_name ? : "BCM", (subver & 0x7000) >> 13,
+		(subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(btbcm_initialize);
+
+int btbcm_finalize(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	struct hci_rp_read_local_version *ver;
+	u16 subver, rev;
+	int err;
+
+	/* Reset */
+	err = btbcm_reset(hdev);
+	if (err)
+		return err;
+
+	/* Read Local Version Info */
+	skb = btbcm_read_local_version(hdev);
+	if (IS_ERR(skb))
+		return PTR_ERR(skb);
+
+	ver = (struct hci_rp_read_local_version *)skb->data;
+	rev = le16_to_cpu(ver->hci_rev);
+	subver = le16_to_cpu(ver->lmp_subver);
+	kfree_skb(skb);
+
+	BT_INFO("%s: BCM (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
+		(subver & 0x7000) >> 13, (subver & 0x1f00) >> 8,
+		(subver & 0x00ff), rev & 0x0fff);
+
+	btbcm_check_bdaddr(hdev);
+
+	set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(btbcm_finalize);
+
 static const struct {
 	u16 subver;
 	const char *name;

@@ -290,18 +379,6 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 	kfree_skb(skb);
 
 	switch ((rev & 0xf000) >> 12) {
-	case 0:
-	case 3:

drivers/bluetooth/btbcm.c int btbcm_setup_patchram(struct hci_dev *hdev) · line 304

Quoted 10 lines

drivers/bluetooth/btbcm.c +0 ~~-10~~

-		for (i = 0; bcm_uart_subver_table[i].name; i++) {
-			if (subver == bcm_uart_subver_table[i].subver) {
-				hw_name = bcm_uart_subver_table[i].name;
-				break;
-			}
-		}
-
-		snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
-			 hw_name ? : "BCM");
-		break;

I took this hunk out since I am pretty sure I found an USB dongle that has this gotten wrong. Long term we will move over to a manifest file that will list modalias and firmware name. That way this becomes a detail of the linux-firmware tree and no longer a driver problem.

Regards

Marcel

Frederic Danis

author

2015-05-28 09:25 UTC #8 lore raw

PATCH v7 5/6 Bluetooth: hci_uart: Update Broadcom UART setup

In reply to Frederic Danis

Use btbcm helpers to perform controller setup.
Perform host UART reset to init speed between btbcm_patchram() and btbcm_finalize(). This may be need because firmware loading may have reseted controller UART to init speed.

Signed-off-by Frederic Danis <[email protected]>

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

drivers/bluetooth/hci_bcm.c +29 -1

index 1ec0b4a..6c34135 100644
@@ -79,11 +79,39 @@ static int bcm_flush(struct hci_uart *hu)
 
 static int bcm_setup(struct hci_uart *hu)
 {
+	char fw_name[64];
+	const struct firmware *fw;
+	int err;
+
 	BT_DBG("hu %p", hu);
 
 	hu->hdev->set_bdaddr = btbcm_set_bdaddr;
 
-	return btbcm_setup_patchram(hu->hdev);
+	err = btbcm_initialize(hu->hdev, fw_name, sizeof(fw_name));
+	if (err)
+		return err;
+
+	err = request_firmware(&fw, fw_name, &hu->hdev->dev);
+	if (err < 0) {
+		BT_INFO("%s: BCM: Patch %s not found", hu->hdev->name, fw_name);
+		return 0;
+	}
+
+	err = btbcm_patchram(hu->hdev, fw);
+	if (err) {
+		BT_INFO("%s: BCM: Patch failed (%d)", hu->hdev->name, err);
+		goto finalize;
+	}
+
+	if (hu->proto->init_speed)
+		hci_uart_set_baudrate(hu, hu->proto->init_speed);
+
+finalize:
+	release_firmware(fw);
+
+	err = btbcm_finalize(hu->hdev);
+
+	return err;
 }
 
 static const struct h4_recv_pkt bcm_recv_pkts[] = {

Signature · 1 line

1.9.1

Frederic Danis

author

2015-05-28 09:25 UTC #9 lore raw

PATCH v7 6/6 Bluetooth: hci_uart: Add bcm_set_baudrate()

In reply to Frederic Danis

Add vendor specific command to change controller device speed.

Signed-off-by Frederic Danis <[email protected]>

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

drivers/bluetooth/hci_bcm.c +56 -0

index 6c34135..0cbf9d4 100644
@@ -24,6 +24,7 @@
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/skbuff.h>
+#include <linux/tty.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>

@@ -31,11 +32,55 @@
 #include "btbcm.h"
 #include "hci_uart.h"
 
+#define BCM43XX_CLOCK_48 1
+#define BCM43XX_CLOCK_24 2
+
 struct bcm_data {
 	struct sk_buff *rx_skb;
 	struct sk_buff_head txq;
 };
 
+struct hci_cp_bcm_set_speed {
+	__le16   dummy;
+	__le32   speed;
+} __packed;
+
+static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
+{
+	struct hci_dev *hdev = hu->hdev;
+	struct sk_buff *skb;
+	struct hci_cp_bcm_set_speed param = { 0, cpu_to_le32(speed) };
+
+	if (speed > 3000000) {
+		u8 clock = BCM43XX_CLOCK_48;
+
+		BT_DBG("%s: Set Controller clock (%d)", hdev->name, clock);
+
+		skb = __hci_cmd_sync(hdev, 0xfc45, 1, &clock, HCI_INIT_TIMEOUT);
+		if (IS_ERR(skb)) {
+			BT_ERR("%s: failed to write update clock command (%ld)",
+			       hdev->name, PTR_ERR(skb));
+			return PTR_ERR(skb);
+		}
+
+		kfree_skb(skb);
+	}
+
+	BT_DBG("%s: Set Controller UART speed to %d bit/s", hdev->name, speed);
+
+	skb = __hci_cmd_sync(hdev, 0xfc18, sizeof(param), &param,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s: failed to write update baudrate command (%ld)",
+		       hdev->name, PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+
+	kfree_skb(skb);
+
+	return 0;
+}
+
 static int bcm_open(struct hci_uart *hu)
 {
 	struct bcm_data *bcm;

@@ -106,6 +151,14 @@ static int bcm_setup(struct hci_uart *hu)
 	if (hu->proto->init_speed)
 		hci_uart_set_baudrate(hu, hu->proto->init_speed);
 
+	if (hu->proto->oper_speed) {
+		err = bcm_set_baudrate(hu, hu->proto->oper_speed);
+		if (!err)
+			/* hci_uart_set_baudrate() has no return value as
+			   tty_set_termios() return is always 0 */
+			hci_uart_set_baudrate(hu, hu->proto->oper_speed);
+	}
+
 finalize:
 	release_firmware(fw);

@@ -161,10 +214,13 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
 static const struct hci_uart_proto bcm_proto = {
 	.id		= HCI_UART_BCM,
 	.name		= "BCM",
+	.init_speed	= 115200,
+	.oper_speed	= 4000000,
 	.open		= bcm_open,
 	.close		= bcm_close,
 	.flush		= bcm_flush,
 	.setup		= bcm_setup,
+	.set_baudrate	= bcm_set_baudrate,
 	.recv		= bcm_recv,
 	.enqueue	= bcm_enqueue,
 	.dequeue	= bcm_dequeue,

Signature · 1 line

1.9.1

Marcel Holtmann

maintainer

2015-06-06 05:47 UTC #10 lore raw

Re: [PATCH v7 6/6] Bluetooth: hci_uart: Add bcm_set_baudrate()

Hi Fred,

Quoted 25 lines

Add vendor specific command to change controller device speed.
Signed-off-by Frederic Danis <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
drivers/bluetooth/hci_bcm.c +3 -0
index 6c34135..0cbf9d4 100644
@@ -24,6 +24,7 @@
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/skbuff.h>
+#include <linux/tty.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -31,11 +32,55 @@
 #include "btbcm.h"
 #include "hci_uart.h"
 
+#define BCM43XX_CLOCK_48 1
+#define BCM43XX_CLOCK_24 2

drivers/bluetooth/hci_bcm.c line 46

Quoted 10 lines

drivers/bluetooth/hci_bcm.c +5 -0

+
 struct bcm_data {
 	struct sk_buff *rx_skb;
 	struct sk_buff_head txq;
 };
 
+struct hci_cp_bcm_set_speed {
+	__le16   dummy;
+	__le32   speed;
+} __packed;

I think this data structure should go into btbcm.h with a little bit better name. This should be similar to the btintel.h where I am slowly trying to get the proper data structures put in place.

drivers/bluetooth/hci_bcm.c line 52

Quoted 6 lines

drivers/bluetooth/hci_bcm.c +6 -0

+
+static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
+{
+	struct hci_dev *hdev = hu->hdev;
+	struct sk_buff *skb;
+	struct hci_cp_bcm_set_speed param = { 0, cpu_to_le32(speed) };

I would prefer if we assign this one by one and use cpu_lo_le16(0) here as well. Just to remember that the value is actually in little endian.

drivers/bluetooth/hci_bcm.c line 58

Quoted 6 lines

drivers/bluetooth/hci_bcm.c +6 -0

+
+	if (speed > 3000000) {
+		u8 clock = BCM43XX_CLOCK_48;
+
+		BT_DBG("%s: Set Controller clock (%d)", hdev->name, clock);
+

I prefer having a comment above the vendor commands explain on what they do and why this is correct. That is as close as it gets for someone with the proper documentation to figure out why we are doing things. I think the Intel support is a good example on how far I have gone with this.

drivers/bluetooth/hci_bcm.c

Quoted 3 lines

drivers/bluetooth/hci_bcm.c +3 -0

+		skb = __hci_cmd_sync(hdev, 0xfc45, 1, &clock, HCI_INIT_TIMEOUT);
+		if (IS_ERR(skb)) {
+			BT_ERR("%s: failed to write update clock command (%ld)”,

Lets try to be a bit consistent with the error messages. I think we prefixed most of it with BCM: as well. Overall, I try to make this really consistent.

drivers/bluetooth/hci_bcm.c line 70

Quoted 9 lines

drivers/bluetooth/hci_bcm.c +9 -0

+			       hdev->name, PTR_ERR(skb));
+			return PTR_ERR(skb);
+		}
+
+		kfree_skb(skb);
+	}
+
+	BT_DBG("%s: Set Controller UART speed to %d bit/s", hdev->name, speed);
+

Same here. Comments should be put above and assignment of the values to param maybe better places above here so you know exactly where they are coming from.

drivers/bluetooth/hci_bcm.c

Quoted 4 lines

drivers/bluetooth/hci_bcm.c +4 -0

+	skb = __hci_cmd_sync(hdev, 0xfc18, sizeof(param), &param,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		BT_ERR("%s: failed to write update baudrate command (%ld)”,

Same as the comment above with the BCM: prefix.

Quoted 13 lines

drivers/bluetooth/hci_bcm.c +9 -0

+		       hdev->name, PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+
+	kfree_skb(skb);
+
+	return 0;
+}
+
 static int bcm_open(struct hci_uart *hu)
 {
 	struct bcm_data *bcm;

drivers/bluetooth/hci_bcm.c static int bcm_setup(struct hci_uart *hu) · line 158

Quoted 8 lines

drivers/bluetooth/hci_bcm.c +5 -0

@@ -106,6 +151,14 @@ static int bcm_setup(struct hci_uart *hu)
 	if (hu->proto->init_speed)
 		hci_uart_set_baudrate(hu, hu->proto->init_speed);
 
+	if (hu->proto->oper_speed) {
+		err = bcm_set_baudrate(hu, hu->proto->oper_speed);
+		if (!err)
+			/* hci_uart_set_baudrate() has no return value as
+			   tty_set_termios() return is always 0 */

This comment is just duplicating it. Especially since above you do not comment it. So in this case I would remove it.

Quoted 10 lines

drivers/bluetooth/hci_bcm.c +3 -0

+			hci_uart_set_baudrate(hu, hu->proto->oper_speed);
+	}
+

finalize:

	release_firmware(fw);

drivers/bluetooth/hci_bcm.c +0 -0

@@ -161,10 +214,13 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
 static const struct hci_uart_proto bcm_proto = {
 	.id		= HCI_UART_BCM,
 	.name		= "BCM",

drivers/bluetooth/hci_bcm.c static struct sk_buff *bcm_dequeue(struct hci_uart *hu) · line 226

Quoted 10 lines

drivers/bluetooth/hci_bcm.c +3 -0

+	.init_speed	= 115200,
+	.oper_speed	= 4000000,
 	.open		= bcm_open,
 	.close		= bcm_close,
 	.flush		= bcm_flush,
 	.setup		= bcm_setup,
+	.set_baudrate	= bcm_set_baudrate,
 	.recv		= bcm_recv,
 	.enqueue	= bcm_enqueue,
 	.dequeue	= bcm_dequeue,

Regards

Marcel

Marcel Holtmann

maintainer

2015-06-06 05:48 UTC #11 lore raw

In reply to Frederic Danis

Hi Fred,

Quoted 28 lines

This patch set introduce baudrate management for Bluetooth UART Controller setup and a first implementation for Broadcom BCM4324B3 used in Asus T100.
v6 -> v7:
- Fix setup when firmware is missing by moving request/release_firmware() out of btbcm_patchram().
v2 -> v6:
- Fixes for __hci_cmd_sync() has been integrated (patch v5)
- In btbcm_check_bdaddr(), merge controller address tests
- Fix declaration of speeds to use "unsigned int" type
- Rename default_speed to init_speed
- Rename operational_speed to oper_speed
- In case of error during vendor specific set_baudrate(), do not change host baudrate and continue without changing speed
- Discard changes to bcm_setup_patchram() and in btusb.c
- Introduce helper functions btbcm_initialize() and btbcm_finalize() to be used by the UART side of things.
Frederic Danis (6):
Bluetooth: btbcm: Move request/release_firmware()
Bluetooth: btbcm: Add BCM4324B3 UART device
Bluetooth: hci_uart: Support operational speed during setup
Bluetooth: btbcm: Add helper functions for UART setup
Bluetooth: hci_uart: Update Broadcom UART setup
Bluetooth: hci_uart: Add bcm_set_baudrate()

Quoted 6 lines

drivers/bluetooth/btbcm.c     | 135 +++++++++++++++++++++++++++++++++---------
drivers/bluetooth/btbcm.h     |  20 ++++++-
drivers/bluetooth/hci_bcm.c   |  86 ++++++++++++++++++++++++++-
drivers/bluetooth/hci_ldisc.c |  26 ++++++++
drivers/bluetooth/hci_uart.h  |   4 ++
5 files changed, 240 insertions(+), 31 deletions(-)

patches 1-5 have been applied to bluetooth-next tree.

However I did a few minor modifications as mentioned in the separate emails. They were mainly cosmetic and easy to do inline so that you do not have to worry about redoing the whole series. However patch 6 needs a bit of extra work.

Regards

Marcel