Skip to contentSkip to conversation

TI Bluetooth serdev support

PATCH In review Rob Herring 8 participants 14 messages Apr 13, 2017 – Apr 19, 2017 7 messages not in the archive

Subsystems (5)
lore
Share & tools
lore

Rob Herring

author maintainer
#0 lore raw

v3 cover TI Bluetooth serdev support

This series adds serdev support to the HCI LL protocol used on TI BT modules and enables support on HiKey board with with the WL1835 module. With this the custom TI UIM daemon and btattach are no longer needed.

The series is available on this git branch[1]. This version is rebased on bluetooth-next tree containing its dependencies.

Rob

[1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git ti-bluetooth

Rob Herring (4):
dt-bindings: net: Add TI WiLink shared transport binding
bluetooth: hci_uart: remove unused hci_uart_init_tty
bluetooth: hci_uart: add LL protocol serdev driver support
arm64: dts: hikey: add WL1835 Bluetooth device node

 .../devicetree/bindings/net/ti,wilink-st.txt       |  35 +++
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts     |   5 +
 drivers/bluetooth/hci_ldisc.c                      |  19 --
 drivers/bluetooth/hci_ll.c                         | 262 ++++++++++++++++++++-
 drivers/bluetooth/hci_uart.h                       |   1 -
 5 files changed, 301 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/ti,wilink-st.txt
Signature · 1 line
2.11.0
author maintainer
#1 lore raw

PATCH v3 1/4 dt-bindings: net: Add TI WiLink shared transport binding

Add serial slave device binding for the TI WiLink series of Bluetooth/FM/GPS devices.

Cc Mark Rutland <[email protected]>
Cc netdev at vger.kernel.org
Cc devicetree at vger.kernel.org

---
v3:
- rebase on bluetooth-next

 .../devicetree/bindings/net/ti,wilink-st.txt       | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,wilink-st.txt
Documentation/devicetree/bindings/net/ti,wilink-st.txt new file +35 -0
new file mode 100644
index 000000000000..cbad73a84ac4
@@ -0,0 +1,35 @@
+TI WiLink 7/8 (wl12xx/wl18xx) Shared Transport BT/FM/GPS devices
+
+TI WiLink devices have a UART interface for providing Bluetooth, FM radio,
+and GPS over what's called "shared transport". The shared transport is
+standard BT HCI protocol with additional channels for the other functions.
+
+These devices also have a separate WiFi interface as described in
+wireless/ti,wlcore.txt.
+
+This bindings follows the UART slave device binding in
+../serial/slave-device.txt.
+
+Required properties:
+ - compatible: should be one of the following:
+    "ti,wl1271-st"
+    "ti,wl1273-st"
+    "ti,wl1831-st"
+    "ti,wl1835-st"
+    "ti,wl1837-st"
+
+Optional properties:
+ - enable-gpios : GPIO signal controlling enabling of BT. Active high.
+ - vio-supply : Vio input supply (1.8V)
+ - vbat-supply : Vbat input supply (2.9-4.8V)
+
+Example:
+
+&serial0 {
+	compatible = "ns16550a";
+	...
+	bluetooth {
+		compatible = "ti,wl1835-st";
+		enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+	};
+};
Signature · 1 line
2.11.0

A message that is not in the archive (sent off-list or not yet received): 20170413150353.7389-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

Reviewed-by
#3 lore raw

Re: [PATCH v3 1/4] dt-bindings: net: Add TI WiLink shared transport binding

Hi,

Quoted 46 lines · Rob Herring

Add serial slave device binding for the TI WiLink series of Bluetooth/FM/GPS devices.

Cc Mark Rutland <[email protected]>
Cc netdev at vger.kernel.org
Cc devicetree at vger.kernel.org

---
v3:
- rebase on bluetooth-next

 .../devicetree/bindings/net/ti,wilink-st.txt       | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,wilink-st.txt
Documentation/devicetree/bindings/net/ti,wilink-st.txt new file +25 -0
new file mode 100644
index 000000000000..cbad73a84ac4
@@ -0,0 +1,35 @@
+TI WiLink 7/8 (wl12xx/wl18xx) Shared Transport BT/FM/GPS devices
+
+TI WiLink devices have a UART interface for providing Bluetooth, FM radio,
+and GPS over what's called "shared transport". The shared transport is
+standard BT HCI protocol with additional channels for the other functions.
+
+These devices also have a separate WiFi interface as described in
+wireless/ti,wlcore.txt.
+
+This bindings follows the UART slave device binding in
+../serial/slave-device.txt.
+
+Required properties:
+ - compatible: should be one of the following:
+    "ti,wl1271-st"
+    "ti,wl1273-st"
+    "ti,wl1831-st"
+    "ti,wl1835-st"
+    "ti,wl1837-st"
+
+Optional properties:
+ - enable-gpios : GPIO signal controlling enabling of BT. Active high.
+ - vio-supply : Vio input supply (1.8V)
+ - vbat-supply : Vbat input supply (2.9-4.8V)
+

Documentation/devicetree/bindings/net/ti,wilink-st.txt line 35

Quoted 10 lines
Documentation/devicetree/bindings/net/ti,wilink-st.txt new file +10 -0
+Example:
+
+&serial0 {
+	compatible = "ns16550a";
+	...
+	bluetooth {
+		compatible = "ti,wl1835-st";
+		enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+	};
+};

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170415/5a3a5a04/attachment.sig>

A message that is not in the archive (sent off-list or not yet received): CAHCN7x+HhwmXGxZn_31LuzdkEdoAnOSxukAdXULrGbSeKmrtRQ@mail.gmail.com

A message that is not in the archive (sent off-list or not yet received): CAHCN7xLHZmS_3+JygQgRxi_tX0mXL22D1Nj9T8jkrMz3-8eQDg@mail.gmail.com

#6 lore raw

Re: [PATCH v3 1/4] dt-bindings: net: Add TI WiLink shared transport binding

On Apr 13, 2017 10:04 AM, "Rob Herring" <[email protected]> wrote:

Add serial slave device binding for the TI WiLink series of Bluetooth/FM/GPS devices.

---
v3:
- rebase on bluetooth-next

 .../devicetree/bindings/net/ti,wilink-st.txt       | 35
++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,wilink-st.txt
(quoted diff) +0 -0

b/Documentation/devicetree/bindings/net/ti,wilink-st.txt new file mode 100644
index 000000000000..cbad73a84ac4

Documentation/devicetree/bindings/net/ti,wilink-st.txt new file +20 -0
@@ -0,0 +1,35 @@
+TI WiLink 7/8 (wl12xx/wl18xx) Shared Transport BT/FM/GPS devices
+
+TI WiLink devices have a UART interface for providing Bluetooth, FM radio,
+and GPS over what's called "shared transport". The shared transport is
+standard BT HCI protocol with additional channels for the other functions.
+
+These devices also have a separate WiFi interface as described in
+wireless/ti,wlcore.txt.
+
+This bindings follows the UART slave device binding in
+../serial/slave-device.txt.
+
+Required properties:
+ - compatible: should be one of the following:
+    "ti,wl1271-st"
+    "ti,wl1273-st"
+    "ti,wl1831-st"
+    "ti,wl1835-st"
+    "ti,wl1837-st"
+

Would you expect the wl1283 chipset too?

I can help test this if you like after the holiday weekend. I have a board with WL1283 and currently using pdata-quirks to support it.

Adam

+Optional properties:
+ - enable-gpios : GPIO signal controlling enabling of BT. Active high.
+ - vio-supply : Vio input supply (1.8V)
+ - vbat-supply : Vbat input supply (2.9-4.8V)
+
+Example:
+
+&serial0 {

+       compatible = "ns16550a";
+       ...
+       bluetooth {

+ compatible = "ti,wl1835-st";
+ enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+ };
+};
--
2.11.0

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

A message that is not in the archive (sent off-list or not yet received): CAHCN7x+G_jbQTDfyPBZ4Er8vz46tEojrWf29Eztq56c9zBnMeA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#8 lore raw

Re: [PATCH v3 1/4] dt-bindings: net: Add TI WiLink shared transport binding

Quoted 49 lines · Adam Ford

On Apr 13, 2017 10:04 AM, "Rob Herring" <[email protected]> wrote:

Add serial slave device binding for the TI WiLink series of Bluetooth/FM/GPS devices.

Cc Mark Rutland <[email protected]>
Cc netdev at vger.kernel.org
Cc devicetree at vger.kernel.org

---
v3:
- rebase on bluetooth-next

 .../devicetree/bindings/net/ti,wilink-st.txt       | 35
++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,wilink-st.txt
Documentation/devicetree/bindings/net/ti,wilink-st.txt new file +20 -0
new file mode 100644
index 000000000000..cbad73a84ac4
@@ -0,0 +1,35 @@
+TI WiLink 7/8 (wl12xx/wl18xx) Shared Transport BT/FM/GPS devices
+
+TI WiLink devices have a UART interface for providing Bluetooth, FM radio,
+and GPS over what's called "shared transport". The shared transport is
+standard BT HCI protocol with additional channels for the other functions.
+
+These devices also have a separate WiFi interface as described in
+wireless/ti,wlcore.txt.
+
+This bindings follows the UART slave device binding in
+../serial/slave-device.txt.
+
+Required properties:
+ - compatible: should be one of the following:
+    "ti,wl1271-st"
+    "ti,wl1273-st"
+    "ti,wl1831-st"
+    "ti,wl1835-st"
+    "ti,wl1837-st"
+
Quoted 1 line

Would you expect the wl1283 chipset too?

Probably, but I left it out as there's no public information.

Quoted 2 lines

I can help test this if you like after the holiday weekend. I have a board with WL1283 and currently using pdata-quirks to support it.

author maintainer
#9 lore raw

PATCH v3 2/4 bluetooth: hci_uart: remove unused hci_uart_init_tty

In reply to Rob Herring

There are no users of hci_uart_init_tty, so remove it.

Cc Marcel Holtmann <[email protected]>
Cc Gustavo Padovan <[email protected]>
Cc Johan Hedberg <[email protected]>
Cc linux-bluetooth at vger.kernel.org

---
v3:
- rebase on bluetooth-next

 drivers/bluetooth/hci_ldisc.c | 19 -------------------
 drivers/bluetooth/hci_uart.h  |  1 -
 2 files changed, 20 deletions(-)
drivers/bluetooth/hci_ldisc.c +0 -19
index 17bcbc13623f..cec4438ede01 100644
@@ -319,25 +319,6 @@ void hci_uart_set_speeds(struct hci_uart *hu, unsigned int init_speed,
 	hu->oper_speed = oper_speed;
 }
 
-void hci_uart_init_tty(struct hci_uart *hu)
-{
-	struct tty_struct *tty = hu->tty;
-	struct ktermios ktermios;
-
-	/* Bring the UART into a known 8 bits no parity hw fc state */
-	ktermios = tty->termios;
-	ktermios.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP |
-			      INLCR | IGNCR | ICRNL | IXON);
-	ktermios.c_oflag &= ~OPOST;
-	ktermios.c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);
-	ktermios.c_cflag &= ~(CSIZE | PARENB);
-	ktermios.c_cflag |= CS8;
-	ktermios.c_cflag |= CRTSCTS;
-
-	/* tty_set_termios() return not checked as it is always 0 */
-	tty_set_termios(tty, &ktermios);
-}
-
 void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed)
 {
 	struct tty_struct *tty = hu->tty;
drivers/bluetooth/hci_uart.h +0 -1
index 1b41c661bbb8..2b05e557fad0 100644
@@ -114,7 +114,6 @@ int hci_uart_register_device(struct hci_uart *hu, 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_init_tty(struct hci_uart *hu);
 void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed);
 void hci_uart_set_flow_control(struct hci_uart *hu, bool enable);
 void hci_uart_set_speeds(struct hci_uart *hu, unsigned int init_speed,
Signature · 1 line
2.11.0
author maintainer
#10 lore raw

PATCH v3 3/4 bluetooth: hci_uart: add LL protocol serdev driver support

In reply to Rob Herring

Turns out that the LL protocol and the TI-ST are the same thing AFAICT. The TI-ST adds firmware loading, GPIO control, and shared access for NFC, FM radio, etc. For now, we're only implementing what is needed for BT. This mirrors other drivers like BCM and Intel, but uses the new serdev bus.

The firmware loading is greatly simplified by using existing infrastructure to send commands. It may be a bit slower than the original code using synchronous functions, but the real bottleneck is likely doing firmware load at 115.2kbps.

Cc Marcel Holtmann <[email protected]>
Cc Gustavo Padovan <[email protected]>
Cc Johan Hedberg <[email protected]>
Cc linux-bluetooth at vger.kernel.org

---
v3:
- rebase on bluetooth-next
- Add explicit of.h include
v2:
- Use IS_ENABLED() to fix module build

 drivers/bluetooth/hci_ll.c | 262 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 261 insertions(+), 1 deletion(-)
drivers/bluetooth/hci_ll.c +261 -1
index 02692fe30279..485e8eb04542 100644
@@ -34,20 +34,24 @@
 #include <linux/sched.h>
 #include <linux/types.h>
 #include <linux/fcntl.h>
+#include <linux/firmware.h>
 #include <linux/interrupt.h>
 #include <linux/ptrace.h>
 #include <linux/poll.h>
 
 #include <linux/slab.h>
-#include <linux/tty.h>
 #include <linux/errno.h>
 #include <linux/string.h>
 #include <linux/signal.h>
 #include <linux/ioctl.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
 #include <linux/skbuff.h>
+#include <linux/ti_wilink_st.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
+#include <linux/gpio/consumer.h>
 
 #include "hci_uart.h"
 
@@ -76,6 +80,12 @@ struct hcill_cmd {
 	u8 cmd;
 } __packed;
 
+struct ll_device {
+	struct hci_uart hu;
+	struct serdev_device *serdev;
+	struct gpio_desc *enable_gpio;
+};
+
 struct ll_struct {
 	unsigned long rx_state;
 	unsigned long rx_count;
@@ -136,6 +146,9 @@ static int ll_open(struct hci_uart *hu)
 
 	hu->priv = ll;
 
+	if (hu->serdev)
+		serdev_device_open(hu->serdev);
+
 	return 0;
 }
 
@@ -164,6 +177,13 @@ static int ll_close(struct hci_uart *hu)
 
 	kfree_skb(ll->rx_skb);
 
+	if (hu->serdev) {
+		struct ll_device *lldev = serdev_device_get_drvdata(hu->serdev);
+		gpiod_set_value_cansleep(lldev->enable_gpio, 0);
+
+		serdev_device_close(hu->serdev);
+	}
+
 	hu->priv = NULL;
 
 	kfree(ll);
@@ -505,9 +525,245 @@ static struct sk_buff *ll_dequeue(struct hci_uart *hu)
 	return skb_dequeue(&ll->txq);
 }
 
+#if IS_ENABLED(CONFIG_SERIAL_DEV_BUS)
+static int read_local_version(struct hci_dev *hdev)
+{
+	int err = 0;
+	unsigned short version = 0;
+	struct sk_buff *skb;
+	struct hci_rp_read_local_version *ver;
+
+	skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Reading TI version information failed (%ld)",
+			   PTR_ERR(skb));
+		err = PTR_ERR(skb);
+		goto out;
+	}
+	if (skb->len != sizeof(*ver)) {
+		err = -EILSEQ;
+		goto out;
+	}
+
+	ver = (struct hci_rp_read_local_version *)skb->data;
+	if (le16_to_cpu(ver->manufacturer) != 13) {
+		err = -ENODEV;
+		goto out;
+	}
+
+	version = le16_to_cpu(ver->lmp_subver);
+
+out:
+	if (err) bt_dev_err(hdev, "Failed to read TI version info: %d", err);
+	kfree_skb(skb);
+	return err ? err : version;
+}
+
+/**
+ * download_firmware -
+ *	internal function which parses through the .bts firmware
+ *	script file intreprets SEND, DELAY actions only as of now
+ */
+static int download_firmware(struct ll_device *lldev)
+{
+	unsigned short chip, min_ver, maj_ver;
+	int version, err, len;
+	unsigned char *ptr, *action_ptr;
+	unsigned char bts_scr_name[40];	/* 40 char long bts scr name? */
+	const struct firmware *fw;
+	struct sk_buff *skb;
+	struct hci_command *cmd;
+
+	version = read_local_version(lldev->hu.hdev);
+	if (version < 0)
+		return version;
+
+	chip = (version & 0x7C00) >> 10;
+	min_ver = (version & 0x007F);
+	maj_ver = (version & 0x0380) >> 7;
+	if (version & 0x8000)
+		maj_ver |= 0x0008;
+
+	snprintf(bts_scr_name, sizeof(bts_scr_name),
+		 "ti-connectivity/TIInit_%d.%d.%d.bts",
+		 chip, maj_ver, min_ver);
+
+	err = request_firmware(&fw, bts_scr_name, &lldev->serdev->dev);
+	if (err || !fw->data || !fw->size) {
+		bt_dev_err(lldev->hu.hdev, "request_firmware failed(errno %d) for %s",
+			   err, bts_scr_name);
+		return -EINVAL;
+	}
+	ptr = (void *)fw->data;
+	len = fw->size;
+	/* bts_header to remove out magic number and
+	 * version
+	 */
+	ptr += sizeof(struct bts_header);
+	len -= sizeof(struct bts_header);
+
+	while (len > 0 && ptr) {
+		bt_dev_dbg(lldev->hu.hdev, " action size %d, type %d ",
+			   ((struct bts_action *)ptr)->size,
+			   ((struct bts_action *)ptr)->type);
+
+		action_ptr = &(((struct bts_action *)ptr)->data[0]);
+
+		switch (((struct bts_action *)ptr)->type) {
+		case ACTION_SEND_COMMAND:	/* action send */
+			bt_dev_dbg(lldev->hu.hdev, "S");
+			cmd = (struct hci_command *)action_ptr;
+			if (cmd->opcode == 0xff36) {
+				/* ignore remote change
+				 * baud rate HCI VS command */
+				bt_dev_warn(lldev->hu.hdev, "change remote baud rate command in firmware");
+				break;
+			}
+			if (cmd->prefix != 1)
+				bt_dev_dbg(lldev->hu.hdev, "command type %d\n", cmd->prefix);
+
+			skb = __hci_cmd_sync(lldev->hu.hdev, cmd->opcode, cmd->plen, &cmd->speed, HCI_INIT_TIMEOUT);
+			if (IS_ERR(skb)) {
+				bt_dev_err(lldev->hu.hdev, "send command failed\n");
+				goto out_rel_fw;
+			}
+			kfree_skb(skb);
+			break;
+		case ACTION_WAIT_EVENT:  /* wait */
+			/* no need to wait as command was synchronous */
+			bt_dev_dbg(lldev->hu.hdev, "W");
+			break;
+		case ACTION_DELAY:	/* sleep */
+			bt_dev_info(lldev->hu.hdev, "sleep command in scr");
+			mdelay(((struct bts_action_delay *)action_ptr)->msec);
+			break;
+		}
+		len -= (sizeof(struct bts_action) +
+			((struct bts_action *)ptr)->size);
+		ptr += sizeof(struct bts_action) +
+			((struct bts_action *)ptr)->size;
+	}
+
+out_rel_fw:
+	/* fw download complete */
+	release_firmware(fw);
+	return err;
+}
+
+static int ll_setup(struct hci_uart *hu)
+{
+	int err, retry = 3;
+	struct ll_device *lldev;
+	struct serdev_device *serdev = hu->serdev;
+	u32 speed;
+
+	if (!serdev)
+		return 0;
+
+	lldev = serdev_device_get_drvdata(serdev);
+
+	serdev_device_set_flow_control(serdev, true);
+
+	do {
+		/* Configure BT_EN to HIGH state */
+		gpiod_set_value_cansleep(lldev->enable_gpio, 0);
+		msleep(5);
+		gpiod_set_value_cansleep(lldev->enable_gpio, 1);
+		msleep(100);
+
+		err = download_firmware(lldev);
+		if (!err)
+			break;
+
+		/* Toggle BT_EN and retry */
+		bt_dev_err(hu->hdev, "download firmware failed, retrying...");
+	} while (retry--);
+
+	if (err)
+		return err;
+
+	/* Operational speed if any */
+	if (hu->oper_speed)
+		speed = hu->oper_speed;
+	else if (hu->proto->oper_speed)
+		speed = hu->proto->oper_speed;
+	else
+		speed = 0;
+
+	if (speed) {
+		struct sk_buff *skb = __hci_cmd_sync(hu->hdev, 0xff36, sizeof(speed), &speed, HCI_INIT_TIMEOUT);
+		if (!IS_ERR(skb)) {
+			kfree_skb(skb);
+			serdev_device_set_baudrate(serdev, speed);
+		}
+	}
+
+	return 0;
+}
+
+static const struct hci_uart_proto llp;
+
+static int hci_ti_probe(struct serdev_device *serdev)
+{
+	struct hci_uart *hu;
+	struct ll_device *lldev;
+	u32 max_speed = 3000000;
+
+	lldev = devm_kzalloc(&serdev->dev, sizeof(struct ll_device), GFP_KERNEL);
+	if (!lldev)
+		return -ENOMEM;
+	hu = &lldev->hu;
+
+	serdev_device_set_drvdata(serdev, lldev);
+	lldev->serdev = hu->serdev = serdev;
+
+	lldev->enable_gpio = devm_gpiod_get_optional(&serdev->dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(lldev->enable_gpio))
+		return PTR_ERR(lldev->enable_gpio);
+
+	of_property_read_u32(serdev->dev.of_node, "max-speed", &max_speed);
+	hci_uart_set_speeds(hu, 115200, max_speed);
+
+	return hci_uart_register_device(hu, &llp);
+}
+
+static void hci_ti_remove(struct serdev_device *serdev)
+{
+	struct ll_device *lldev = serdev_device_get_drvdata(serdev);
+	struct hci_uart *hu = &lldev->hu;
+	struct hci_dev *hdev = hu->hdev;
+
+	cancel_work_sync(&hu->write_work);
+
+	hci_unregister_dev(hdev);
+	hci_free_dev(hdev);
+	hu->proto->close(hu);
+}
+
+static const struct of_device_id hci_ti_of_match[] = {
+	{ .compatible = "ti,wl1831-st" },
+	{ .compatible = "ti,wl1835-st" },
+	{ .compatible = "ti,wl1837-st" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hci_ti_of_match);
+
+static struct serdev_device_driver hci_ti_drv = {
+	.driver		= {
+		.name	= "hci-ti",
+		.of_match_table = of_match_ptr(hci_ti_of_match),
+	},
+	.probe	= hci_ti_probe,
+	.remove	= hci_ti_remove,
+};
+#else
+#define ll_setup NULL
+#endif
+
 static const struct hci_uart_proto llp = {
 	.id		= HCI_UART_LL,
 	.name		= "LL",
+	.setup		= ll_setup,
 	.open		= ll_open,
 	.close		= ll_close,
 	.recv		= ll_recv,
@@ -518,10 +774,14 @@ static const struct hci_uart_proto llp = {
 
 int __init ll_init(void)
 {
+	serdev_device_driver_register(&hci_ti_drv);
+
 	return hci_uart_register_proto(&llp);
 }
 
 int __exit ll_deinit(void)
 {
+	serdev_device_driver_unregister(&hci_ti_drv);
+
 	return hci_uart_unregister_proto(&llp);
 }
Signature · 1 line
2.11.0

A message that is not in the archive (sent off-list or not yet received): 20170413150353.7389-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#12 lore raw

Re: [PATCH v3 3/4] bluetooth: hci_uart: add LL protocol serdev driver support

Hi,

drivers/bluetooth/hci_ll.c static struct sk_buff *ll_dequeue(struct hci_uart *hu) · line 541

Quoted 13 lines · Rob Herring
drivers/bluetooth/hci_ll.c +13 -0
+static int read_local_version(struct hci_dev *hdev)
+{
+	int err = 0;
+	unsigned short version = 0;
+	struct sk_buff *skb;
+	struct hci_rp_read_local_version *ver;
+
+	skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Reading TI version information failed (%ld)",
+			   PTR_ERR(skb));
+		err = PTR_ERR(skb);
+		goto out;

If __hci_cmd_sync() fails the code tries to kfree_skb() an error pointer resulting in NULL pointer dereference warning + strack trace. This can just return err instead.

Quoted 9 lines
drivers/bluetooth/hci_ll.c +9 -0
+	}
+	if (skb->len != sizeof(*ver)) {
+		err = -EILSEQ;
+		goto out;
+	}
+
+	ver = (struct hci_rp_read_local_version *)skb->data;
+	if (le16_to_cpu(ver->manufacturer) != 13) {
+		err = -ENODEV;

drivers/bluetooth/hci_ll.c static struct sk_buff *ll_dequeue(struct hci_uart *hu) · line 560

Quoted 10 lines
drivers/bluetooth/hci_ll.c +10 -0
+		goto out;
+	}
+
+	version = le16_to_cpu(ver->lmp_subver);
+
+out:
+	if (err) bt_dev_err(hdev, "Failed to read TI version info: %d", err);
+	kfree_skb(skb);
+	return err ? err : version;
+}

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170415/ab9753e4/attachment.sig>

#13 lore raw

PATCH 1/1 Bluetooth: hci_ll: Fix NULL pointer deref on FW upload failure

Avoid NULL pointer dereference occurring due to freeing skb containing an error pointer. It can easily be triggered by using the driver with broken uart (i.e. due to misconfigured pinmuxing).

Fixes 371805522f87 ("bluetooth: hci_uart: add LL protocol serdev driver support")
---
 drivers/bluetooth/hci_ll.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/bluetooth/hci_ll.c +1 -2
index 485e8eb04542..adc444f309a3 100644
@@ -537,8 +537,7 @@ static int read_local_version(struct hci_dev *hdev)
 	if (IS_ERR(skb)) {
 		bt_dev_err(hdev, "Reading TI version information failed (%ld)",
 			   PTR_ERR(skb));
-		err = PTR_ERR(skb);
-		goto out;
+		return PTR_ERR(skb);
 	}
 	if (skb->len != sizeof(*ver)) {
 		err = -EILSEQ;
Signature · 1 line
2.11.0
maintainer
#14 lore raw

Re: [PATCH] Bluetooth: hci_ll: Fix NULL pointer deref on FW upload failure

Hi Sebastian,

Quoted 10 lines

Avoid NULL pointer dereference occurring due to freeing skb containing an error pointer. It can easily be triggered by using the driver with broken uart (i.e. due to misconfigured pinmuxing).

Fixes 371805522f87 ("bluetooth: hci_uart: add LL protocol serdev driver support")

---
drivers/bluetooth/hci_ll.c | 3 +--

1 file changed, 1 insertion(+), 2 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

#15 lore raw

Re: [PATCH v3 3/4] bluetooth: hci_uart: add LL protocol serdev driver support

In reply to a message not in the archive
Quoted 10 lines · Rob Herring

Turns out that the LL protocol and the TI-ST are the same thing AFAICT. The TI-ST adds firmware loading, GPIO control, and shared access for NFC, FM radio, etc. For now, we're only implementing what is needed for BT. This mirrors other drivers like BCM and Intel, but uses the new serdev bus.

The firmware loading is greatly simplified by using existing infrastructure to send commands. It may be a bit slower than the original code using synchronous functions, but the real bottleneck is likely doing firmware load at 115.2kbps.

I am using pdata-quirks to drive my wl1283 Bluetooth on a DM3730. I have the Bluetooth set to 3000000 baud in pdata quirks. Looking at the binding, I don't see an option to set the baudrate. Is there (or will there) be a way to set the baud rate of the Bluetooth?

adam

Quoted 352 lines
Cc Marcel Holtmann <[email protected]>
Cc Gustavo Padovan <[email protected]>
Cc Johan Hedberg <[email protected]>
Cc linux-bluetooth at vger.kernel.org

---
v3:
- rebase on bluetooth-next
- Add explicit of.h include
v2:
- Use IS_ENABLED() to fix module build

 drivers/bluetooth/hci_ll.c | 262 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 261 insertions(+), 1 deletion(-)
drivers/bluetooth/hci_ll.c +261 -2
index 02692fe30279..485e8eb04542 100644
@@ -34,20 +34,24 @@
 #include <linux/sched.h>
 #include <linux/types.h>
 #include <linux/fcntl.h>
+#include <linux/firmware.h>
 #include <linux/interrupt.h>
 #include <linux/ptrace.h>
 #include <linux/poll.h>
 
 #include <linux/slab.h>
-#include <linux/tty.h>
 #include <linux/errno.h>
 #include <linux/string.h>
 #include <linux/signal.h>
 #include <linux/ioctl.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
 #include <linux/skbuff.h>
+#include <linux/ti_wilink_st.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
+#include <linux/gpio/consumer.h>
 
 #include "hci_uart.h"
 
@@ -76,6 +80,12 @@ struct hcill_cmd {
        u8 cmd;
 } __packed;
 
+struct ll_device {
+       struct hci_uart hu;
+       struct serdev_device *serdev;
+       struct gpio_desc *enable_gpio;
+};
+
 struct ll_struct {
        unsigned long rx_state;
        unsigned long rx_count;
@@ -136,6 +146,9 @@ static int ll_open(struct hci_uart *hu)
 
        hu->priv = ll;
 
+       if (hu->serdev)
+               serdev_device_open(hu->serdev);
+
        return 0;
 }
 
@@ -164,6 +177,13 @@ static int ll_close(struct hci_uart *hu)
 
        kfree_skb(ll->rx_skb);
 
+       if (hu->serdev) {
+               struct ll_device *lldev = serdev_device_get_drvdata(hu->serdev);
+               gpiod_set_value_cansleep(lldev->enable_gpio, 0);
+
+               serdev_device_close(hu->serdev);
+       }
+
        hu->priv = NULL;
 
        kfree(ll);
@@ -505,9 +525,245 @@ static struct sk_buff *ll_dequeue(struct hci_uart *hu)
        return skb_dequeue(&ll->txq);
 }
 
+#if IS_ENABLED(CONFIG_SERIAL_DEV_BUS)
+static int read_local_version(struct hci_dev *hdev)
+{
+       int err = 0;
+       unsigned short version = 0;
+       struct sk_buff *skb;
+       struct hci_rp_read_local_version *ver;
+
+       skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL, HCI_INIT_TIMEOUT);
+       if (IS_ERR(skb)) {
+               bt_dev_err(hdev, "Reading TI version information failed (%ld)",
+                          PTR_ERR(skb));
+               err = PTR_ERR(skb);
+               goto out;
+       }
+       if (skb->len != sizeof(*ver)) {
+               err = -EILSEQ;
+               goto out;
+       }
+
+       ver = (struct hci_rp_read_local_version *)skb->data;
+       if (le16_to_cpu(ver->manufacturer) != 13) {
+               err = -ENODEV;
+               goto out;
+       }
+
+       version = le16_to_cpu(ver->lmp_subver);
+
+out:
+       if (err) bt_dev_err(hdev, "Failed to read TI version info: %d", err);
+       kfree_skb(skb);
+       return err ? err : version;
+}
+
+/**
+ * download_firmware -
+ *     internal function which parses through the .bts firmware
+ *     script file intreprets SEND, DELAY actions only as of now
+ */
+static int download_firmware(struct ll_device *lldev)
+{
+       unsigned short chip, min_ver, maj_ver;
+       int version, err, len;
+       unsigned char *ptr, *action_ptr;
+       unsigned char bts_scr_name[40]; /* 40 char long bts scr name? */
+       const struct firmware *fw;
+       struct sk_buff *skb;
+       struct hci_command *cmd;
+
+       version = read_local_version(lldev->hu.hdev);
+       if (version < 0)
+               return version;
+
+       chip = (version & 0x7C00) >> 10;
+       min_ver = (version & 0x007F);
+       maj_ver = (version & 0x0380) >> 7;
+       if (version & 0x8000)
+               maj_ver |= 0x0008;
+
+       snprintf(bts_scr_name, sizeof(bts_scr_name),
+                "ti-connectivity/TIInit_%d.%d.%d.bts",
+                chip, maj_ver, min_ver);
+
+       err = request_firmware(&fw, bts_scr_name, &lldev->serdev->dev);
+       if (err || !fw->data || !fw->size) {
+               bt_dev_err(lldev->hu.hdev, "request_firmware failed(errno %d) for %s",
+                          err, bts_scr_name);
+               return -EINVAL;
+       }
+       ptr = (void *)fw->data;
+       len = fw->size;
+       /* bts_header to remove out magic number and
+        * version
+        */
+       ptr += sizeof(struct bts_header);
+       len -= sizeof(struct bts_header);
+
+       while (len > 0 && ptr) {
+               bt_dev_dbg(lldev->hu.hdev, " action size %d, type %d ",
+                          ((struct bts_action *)ptr)->size,
+                          ((struct bts_action *)ptr)->type);
+
+               action_ptr = &(((struct bts_action *)ptr)->data[0]);
+
+               switch (((struct bts_action *)ptr)->type) {
+               case ACTION_SEND_COMMAND:       /* action send */
+                       bt_dev_dbg(lldev->hu.hdev, "S");
+                       cmd = (struct hci_command *)action_ptr;
+                       if (cmd->opcode == 0xff36) {
+                               /* ignore remote change
+                                * baud rate HCI VS command */
+                               bt_dev_warn(lldev->hu.hdev, "change remote baud rate command in firmware");
+                               break;
+                       }
+                       if (cmd->prefix != 1)
+                               bt_dev_dbg(lldev->hu.hdev, "command type %d\n", cmd->prefix);
+
+                       skb = __hci_cmd_sync(lldev->hu.hdev, cmd->opcode, cmd->plen, &cmd->speed, HCI_INIT_TIMEOUT);
+                       if (IS_ERR(skb)) {
+                               bt_dev_err(lldev->hu.hdev, "send command failed\n");
+                               goto out_rel_fw;
+                       }
+                       kfree_skb(skb);
+                       break;
+               case ACTION_WAIT_EVENT:  /* wait */
+                       /* no need to wait as command was synchronous */
+                       bt_dev_dbg(lldev->hu.hdev, "W");
+                       break;
+               case ACTION_DELAY:      /* sleep */
+                       bt_dev_info(lldev->hu.hdev, "sleep command in scr");
+                       mdelay(((struct bts_action_delay *)action_ptr)->msec);
+                       break;
+               }
+               len -= (sizeof(struct bts_action) +
+                       ((struct bts_action *)ptr)->size);
+               ptr += sizeof(struct bts_action) +
+                       ((struct bts_action *)ptr)->size;
+       }
+
+out_rel_fw:
+       /* fw download complete */
+       release_firmware(fw);
+       return err;
+}
+
+static int ll_setup(struct hci_uart *hu)
+{
+       int err, retry = 3;
+       struct ll_device *lldev;
+       struct serdev_device *serdev = hu->serdev;
+       u32 speed;
+
+       if (!serdev)
+               return 0;
+
+       lldev = serdev_device_get_drvdata(serdev);
+
+       serdev_device_set_flow_control(serdev, true);
+
+       do {
+               /* Configure BT_EN to HIGH state */
+               gpiod_set_value_cansleep(lldev->enable_gpio, 0);
+               msleep(5);
+               gpiod_set_value_cansleep(lldev->enable_gpio, 1);
+               msleep(100);
+
+               err = download_firmware(lldev);
+               if (!err)
+                       break;
+
+               /* Toggle BT_EN and retry */
+               bt_dev_err(hu->hdev, "download firmware failed, retrying...");
+       } while (retry--);
+
+       if (err)
+               return err;
+
+       /* Operational speed if any */
+       if (hu->oper_speed)
+               speed = hu->oper_speed;
+       else if (hu->proto->oper_speed)
+               speed = hu->proto->oper_speed;
+       else
+               speed = 0;
+
+       if (speed) {
+               struct sk_buff *skb = __hci_cmd_sync(hu->hdev, 0xff36, sizeof(speed), &speed, HCI_INIT_TIMEOUT);
+               if (!IS_ERR(skb)) {
+                       kfree_skb(skb);
+                       serdev_device_set_baudrate(serdev, speed);
+               }
+       }
+
+       return 0;
+}
+
+static const struct hci_uart_proto llp;
+
+static int hci_ti_probe(struct serdev_device *serdev)
+{
+       struct hci_uart *hu;
+       struct ll_device *lldev;
+       u32 max_speed = 3000000;
+
+       lldev = devm_kzalloc(&serdev->dev, sizeof(struct ll_device), GFP_KERNEL);
+       if (!lldev)
+               return -ENOMEM;
+       hu = &lldev->hu;
+
+       serdev_device_set_drvdata(serdev, lldev);
+       lldev->serdev = hu->serdev = serdev;
+
+       lldev->enable_gpio = devm_gpiod_get_optional(&serdev->dev, "enable", GPIOD_OUT_LOW);
+       if (IS_ERR(lldev->enable_gpio))
+               return PTR_ERR(lldev->enable_gpio);
+
+       of_property_read_u32(serdev->dev.of_node, "max-speed", &max_speed);
+       hci_uart_set_speeds(hu, 115200, max_speed);
+
+       return hci_uart_register_device(hu, &llp);
+}
+
+static void hci_ti_remove(struct serdev_device *serdev)
+{
+       struct ll_device *lldev = serdev_device_get_drvdata(serdev);
+       struct hci_uart *hu = &lldev->hu;
+       struct hci_dev *hdev = hu->hdev;
+
+       cancel_work_sync(&hu->write_work);
+
+       hci_unregister_dev(hdev);
+       hci_free_dev(hdev);
+       hu->proto->close(hu);
+}
+
+static const struct of_device_id hci_ti_of_match[] = {
+       { .compatible = "ti,wl1831-st" },
+       { .compatible = "ti,wl1835-st" },
+       { .compatible = "ti,wl1837-st" },
+       {},
+};
+MODULE_DEVICE_TABLE(of, hci_ti_of_match);
+
+static struct serdev_device_driver hci_ti_drv = {
+       .driver         = {
+               .name   = "hci-ti",
+               .of_match_table = of_match_ptr(hci_ti_of_match),
+       },
+       .probe  = hci_ti_probe,
+       .remove = hci_ti_remove,
+};
+#else
+#define ll_setup NULL
+#endif
+
 static const struct hci_uart_proto llp = {
        .id             = HCI_UART_LL,
        .name           = "LL",
+       .setup          = ll_setup,
        .open           = ll_open,
        .close          = ll_close,
        .recv           = ll_recv,
@@ -518,10 +774,14 @@ static const struct hci_uart_proto llp = {
 
 int __init ll_init(void)
 {
+       serdev_device_driver_register(&hci_ti_drv);
+
        return hci_uart_register_proto(&llp);
 }
 
 int __exit ll_deinit(void)
 {
+       serdev_device_driver_unregister(&hci_ti_drv);
+
        return hci_uart_unregister_proto(&llp);
 }
--

2.11.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

A message that is not in the archive (sent off-list or not yet received): CAHCN7xLNcD1rmifEOyPhG2rpB_C81hsyzV9W2q6kCQkKa69sZg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#17 lore raw

Re: [PATCH v3 3/4] bluetooth: hci_uart: add LL protocol serdev driver support

Quoted 12 lines · Adam Ford
Quoted 10 lines · Rob Herring

Turns out that the LL protocol and the TI-ST are the same thing AFAICT. The TI-ST adds firmware loading, GPIO control, and shared access for NFC, FM radio, etc. For now, we're only implementing what is needed for BT. This mirrors other drivers like BCM and Intel, but uses the new serdev bus.

The firmware loading is greatly simplified by using existing infrastructure to send commands. It may be a bit slower than the original code using synchronous functions, but the real bottleneck is likely doing firmware load at 115.2kbps.

Quoted 4 lines

I am using pdata-quirks to drive my wl1283 Bluetooth on a DM3730. I have the Bluetooth set to 3000000 baud in pdata quirks. Looking at the binding, I don't see an option to set the baudrate. Is there (or will there) be a way to set the baud rate of the Bluetooth?

If you read hci_ti_probe, you will see it is already there. The default is 3Mbps and the DT can override that with "max-speed". The intent is that 3Mbps is the max the device can do and max-speed is only for board or host limitations. Though I just checked the datasheet on the 1835 and it can go up to 4364kbps. No datasheets for wl1283, so I have no idea what the max is.

Quoted 5 lines
Quoted 5 lines
drivers/bluetooth/hci_ll.c +5 -0
+static int hci_ti_probe(struct serdev_device *serdev)
+{
+       struct hci_uart *hu;
+       struct ll_device *lldev;
+       u32 max_speed = 3000000;

[...]

Quoted 2 lines
Quoted 2 lines
drivers/bluetooth/hci_ll.c +2 -0
+       of_property_read_u32(serdev->dev.of_node, "max-speed", &max_speed);
+       hci_uart_set_speeds(hu, 115200, max_speed);

A message that is not in the archive (sent off-list or not yet received): 20170413150353.7389-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#19 lore raw

PATCH v3 4/4 arm64: dts: hikey: add WL1835 Bluetooth device node

This adds the serial slave device for the WL1835 Bluetooth interface.

---
v3:
- rebase on bluetooth-next

 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 5 +++++
 1 file changed, 5 insertions(+)
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts +5 -0
index dba3c131c62c..9b4ba7169210 100644
@@ -98,6 +98,11 @@
 			assigned-clocks = <&sys_ctrl HI6220_UART1_SRC>;
 			assigned-clock-rates = <150000000>;
 			status = "ok";
+
+			bluetooth {
+				compatible = "ti,wl1835-st";
+				enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+			};
 		};
 
 		uart2: uart at f7112000 {
Signature · 1 line
2.11.0
#20 lore raw
In reply to a message not in the archive

Hi Rob,

Quoted 17 lines

This series adds serdev support to the HCI LL protocol used on TI BT modules and enables support on HiKey board with with the WL1835 module. With this the custom TI UIM daemon and btattach are no longer needed.

The series is available on this git branch[1]. This version is rebased on bluetooth-next tree containing its dependencies.

Rob

[1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git ti-bluetooth

Rob Herring (4):
dt-bindings: net: Add TI WiLink shared transport binding
bluetooth: hci_uart: remove unused hci_uart_init_tty
bluetooth: hci_uart: add LL protocol serdev driver support
arm64: dts: hikey: add WL1835 Bluetooth device node

Quoted 7 lines
.../devicetree/bindings/net/ti,wilink-st.txt       |  35 +++
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts     |   5 +
drivers/bluetooth/hci_ldisc.c                      |  19 --
drivers/bluetooth/hci_ll.c                         | 262 ++++++++++++++++++++-
drivers/bluetooth/hci_uart.h                       |   1 -
5 files changed, 301 insertions(+), 21 deletions(-)

create mode 100644 Documentation/devicetree/bindings/net/ti,wilink-st.txt

all 4 patches have been applied to bluetooth-next tree.

Regards

Marcel