[PATCH 0/3] Marvell HCI fixes and serdev support

[PATCH 0/3] Marvell HCI fixes and serdev support

[Sascha Hauer]

First two patches are a fix for the Marvell HCI driver which fails to
properly upload the firmware. Third patch adds simple serdev support
to the driver.

Sascha

Sascha Hauer (3):
  Bluetooth: hci_ldisc: Add function to wait for characters to be sent
  Bluetooth: hci_mrvl: Wait for final ack before switching baudrate
  Bluetooth: hci_mrvl: Add serdev support

 .../bindings/net/marvell-bluetooth.txt        | 25 +++++++
 drivers/bluetooth/Kconfig                     |  1 +
 drivers/bluetooth/hci_ldisc.c                 |  8 +++
 drivers/bluetooth/hci_mrvl.c                  | 72 ++++++++++++++++++-
 drivers/bluetooth/hci_uart.h                  |  1 +
 5 files changed, 106 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell-bluetooth.txt

-- 
2.20.1

[PATCH 1/3] Bluetooth: hci_ldisc: Add function to wait for characters to be sent

[Sascha Hauer]

The hci UART line discipline sends its characters in a workqueue. Some
devices like the Marvell Bluetooth chips need to make sure that all
queued characters are sent before switching the baudrate. This adds
a function to synchronize with the workqueue.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/bluetooth/hci_ldisc.c | 8 ++++++++
 drivers/bluetooth/hci_uart.h  | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index c84f985f348d..8950e07889fe 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -178,6 +178,7 @@ static void hci_uart_write_work(struct work_struct *work)
 		goto restart;
 
 	clear_bit(HCI_UART_SENDING, &hu->tx_state);
+	wake_up_bit(&hu->tx_state, HCI_UART_SENDING);
 }
 
 void hci_uart_init_work(struct work_struct *work)
@@ -213,6 +214,13 @@ int hci_uart_init_ready(struct hci_uart *hu)
 	return 0;
 }
 
+int hci_uart_wait_until_sent(struct hci_uart *hu)
+{
+	return wait_on_bit_timeout(&hu->tx_state, HCI_UART_SENDING,
+				   TASK_INTERRUPTIBLE,
+				   msecs_to_jiffies(2000));
+}
+
 /* ------- Interface to HCI layer ------ */
 /* Reset device */
 static int hci_uart_flush(struct hci_dev *hdev)
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index d8cf005e3c5d..f11af3912ce6 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -100,6 +100,7 @@ int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p
 void hci_uart_unregister_device(struct hci_uart *hu);
 
 int hci_uart_tx_wakeup(struct hci_uart *hu);
+int hci_uart_wait_until_sent(struct hci_uart *hu);
 int hci_uart_init_ready(struct hci_uart *hu);
 void hci_uart_init_work(struct work_struct *work);
 void hci_uart_set_baudrate(struct hci_uart *hu, unsigned int speed);
-- 
2.20.1

[PATCH 2/3] Bluetooth: hci_mrvl: Wait for final ack before switching baudrate

[Sascha Hauer]

For the Marvell HCI UART we have to upload two firmware files. The first
one is only for switching the baudrate of the device to a higher
baudrate. After the baudrate switching firmware has been uploaded the
device waits for a final ack (0x5a) before actually switching the
baudrate. To send this final ack with the old baudrate give the hci
ldisc workqueue a chance to run before switching the baudrate. Without
this the final ack will never be received by the device and firmware
upload fails.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/bluetooth/hci_mrvl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c
index 50212ac629e3..a0a74362455e 100644
--- a/drivers/bluetooth/hci_mrvl.c
+++ b/drivers/bluetooth/hci_mrvl.c
@@ -339,6 +339,9 @@ static int mrvl_setup(struct hci_uart *hu)
 		return -EINVAL;
 	}
 
+	/* Let the final ack go out before switching the baudrate */
+	hci_uart_wait_until_sent(hu);
+
 	hci_uart_set_baudrate(hu, 3000000);
 	hci_uart_set_flow_control(hu, false);
 
-- 
2.20.1

[PATCH 3/3] Bluetooth: hci_mrvl: Add serdev support

[Sascha Hauer]

This adds serdev support to the Marvell hci uart driver. Only basic
serdev support, none of the fancier features like regulator or enable
GPIO support is added for now.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../bindings/net/marvell-bluetooth.txt        | 25 +++++++
 drivers/bluetooth/Kconfig                     |  1 +
 drivers/bluetooth/hci_mrvl.c                  | 69 ++++++++++++++++++-
 3 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell-bluetooth.txt

diff --git a/Documentation/devicetree/bindings/net/marvell-bluetooth.txt b/Documentation/devicetree/bindings/net/marvell-bluetooth.txt
new file mode 100644
index 000000000000..0e2842296032
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell-bluetooth.txt
@@ -0,0 +1,25 @@
+Marvell Bluetooth Chips
+-----------------------
+
+This documents the binding structure and common properties for serial
+attached Marvell Bluetooth devices. The following chips are included in
+this binding:
+
+* Marvell 88W8897 Bluetooth devices
+
+Required properties:
+ - compatible: should be:
+    "mrvl,88w8897"
+
+Optional properties:
+None so far
+
+Example:
+
+&serial0 {
+	compatible = "ns16550a";
+	...
+	bluetooth {
+		compatible = "mrvl,88w8897";
+	};
+};
diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index b9c34ff9a0d3..a3fafd781aa1 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -237,6 +237,7 @@ config BT_HCIUART_AG6XX
 config BT_HCIUART_MRVL
 	bool "Marvell protocol support"
 	depends on BT_HCIUART
+	depends on BT_HCIUART_SERDEV
 	select BT_HCIUART_H4
 	help
 	  Marvell is serial protocol for communication between Bluetooth
diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c
index a0a74362455e..f98e5cc343b2 100644
--- a/drivers/bluetooth/hci_mrvl.c
+++ b/drivers/bluetooth/hci_mrvl.c
@@ -13,6 +13,8 @@
 #include <linux/firmware.h>
 #include <linux/module.h>
 #include <linux/tty.h>
+#include <linux/of.h>
+#include <linux/serdev.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -40,6 +42,10 @@ struct mrvl_data {
 	u8 id, rev;
 };
 
+struct mrvl_serdev {
+	struct hci_uart hu;
+};
+
 struct hci_mrvl_pkt {
 	__le16 lhs;
 	__le16 rhs;
@@ -49,6 +55,7 @@ struct hci_mrvl_pkt {
 static int mrvl_open(struct hci_uart *hu)
 {
 	struct mrvl_data *mrvl;
+	int ret;
 
 	BT_DBG("hu %p", hu);
 
@@ -62,7 +69,18 @@ static int mrvl_open(struct hci_uart *hu)
 	set_bit(STATE_CHIP_VER_PENDING, &mrvl->flags);
 
 	hu->priv = mrvl;
+
+	if (hu->serdev) {
+		ret = serdev_device_open(hu->serdev);
+		if (ret)
+			goto err;
+	}
+
 	return 0;
+err:
+	kfree(mrvl);
+
+	return ret;
 }
 
 static int mrvl_close(struct hci_uart *hu)
@@ -71,6 +89,9 @@ static int mrvl_close(struct hci_uart *hu)
 
 	BT_DBG("hu %p", hu);
 
+	if (hu->serdev)
+		serdev_device_close(hu->serdev);
+
 	skb_queue_purge(&mrvl->txq);
 	skb_queue_purge(&mrvl->rawq);
 	kfree_skb(mrvl->rx_skb);
@@ -342,7 +363,11 @@ static int mrvl_setup(struct hci_uart *hu)
 	/* Let the final ack go out before switching the baudrate */
 	hci_uart_wait_until_sent(hu);
 
-	hci_uart_set_baudrate(hu, 3000000);
+	if (hu->serdev)
+		serdev_device_set_baudrate(hu->serdev, 3000000);
+	else
+		hci_uart_set_baudrate(hu, 3000000);
+
 	hci_uart_set_flow_control(hu, false);
 
 	err = mrvl_load_firmware(hu->hdev, "mrvl/uart8897_bt.bin");
@@ -365,12 +390,54 @@ static const struct hci_uart_proto mrvl_proto = {
 	.dequeue	= mrvl_dequeue,
 };
 
+static int mrvl_serdev_probe(struct serdev_device *serdev)
+{
+	struct mrvl_serdev *mrvldev;
+
+	mrvldev = devm_kzalloc(&serdev->dev, sizeof(*mrvldev), GFP_KERNEL);
+	if (!mrvldev)
+		return -ENOMEM;
+
+	mrvldev->hu.serdev = serdev;
+	serdev_device_set_drvdata(serdev, mrvldev);
+
+	return hci_uart_register_device(&mrvldev->hu, &mrvl_proto);
+}
+
+static void mrvl_serdev_remove(struct serdev_device *serdev)
+{
+	struct mrvl_serdev *mrvldev = serdev_device_get_drvdata(serdev);
+
+	hci_uart_unregister_device(&mrvldev->hu);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id mrvl_bluetooth_of_match[] = {
+	{ .compatible = "mrvl,88w8897" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, mrvl_bluetooth_of_match);
+#endif
+
+static struct serdev_device_driver mrvl_serdev_driver = {
+	.probe = mrvl_serdev_probe,
+	.remove = mrvl_serdev_remove,
+	.driver = {
+		.name = "hci_uart_mrvl",
+		.of_match_table = of_match_ptr(mrvl_bluetooth_of_match),
+	},
+};
+
 int __init mrvl_init(void)
 {
+	serdev_device_driver_register(&mrvl_serdev_driver);
+
 	return hci_uart_register_proto(&mrvl_proto);
 }
 
 int __exit mrvl_deinit(void)
 {
+	serdev_device_driver_unregister(&mrvl_serdev_driver);
+
 	return hci_uart_unregister_proto(&mrvl_proto);
 }
-- 
2.20.1

Re: [PATCH 0/3] Marvell HCI fixes and serdev support

[Sascha Hauer]

Hi,

On Fri, Jun 14, 2019 at 09:23:48AM +0200, Sascha Hauer wrote:
> First two patches are a fix for the Marvell HCI driver which fails to
> properly upload the firmware. Third patch adds simple serdev support
> to the driver.
> 
> Sascha
> 
> Sascha Hauer (3):
>   Bluetooth: hci_ldisc: Add function to wait for characters to be sent
>   Bluetooth: hci_mrvl: Wait for final ack before switching baudrate
>   Bluetooth: hci_mrvl: Add serdev support

Any comments to this series?

There are more issues with the firmware upload in the hci_mrvl driver.
The firmware upload is done in multiple threads and only works on an
idle system and even then only most of the time. I'd like to address
these issues soon, so be prepared for more patches ;)

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/3] Marvell HCI fixes and serdev support

[Marcel Holtmann]

Hallo Sascha,

> First two patches are a fix for the Marvell HCI driver which fails to
> properly upload the firmware. Third patch adds simple serdev support
> to the driver.
> 
> Sascha
> 
> Sascha Hauer (3):
>  Bluetooth: hci_ldisc: Add function to wait for characters to be sent
>  Bluetooth: hci_mrvl: Wait for final ack before switching baudrate
>  Bluetooth: hci_mrvl: Add serdev support
> 
> .../bindings/net/marvell-bluetooth.txt        | 25 +++++++
> drivers/bluetooth/Kconfig                     |  1 +
> drivers/bluetooth/hci_ldisc.c                 |  8 +++
> drivers/bluetooth/hci_mrvl.c                  | 72 ++++++++++++++++++-
> drivers/bluetooth/hci_uart.h                  |  1 +
> 5 files changed, 106 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/net/marvell-bluetooth.txt

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

Regards

Marcel