linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC
@ 2018-06-27  5:43 sean.wang
  2018-06-27  5:43 ` [PATCH v4 1/7] dt-bindings: net: bluetooth: Add mediatek-bluetooth sean.wang
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

v4 and changes since v3:
 - refine patch 2 based on commit 919b7308fcc4 to allow that
   dev_pm_domain_attach() will return better error codes. 

v3 and changes since v2
* all changes happen on patch 6
 - fix up SPDX license style for btmtkuart.h.
 - change firmware download from in ACL data to in HCI commands
   and then remove unused mtk_acl_wmt_sync and related code.
 - add a workaround replacing bad vendor event id 0xe4 with 0xff every
   vendor should use.
 - add a sanity check for mtk_hci_wmt_sync to verifying if
   input parameters are valid.
 - add an atomic_inc(&bdev->hdev->cmd_cnt) for __hci_cmd_sync_ev.
 - be changed to use firmware with a header called mt7622pr2h.bin.

v2 and changes since v1
 - Dropped patches already being applied
 - Rewirte the whole driver using btuart [1], and add slight extension
   of btuart to fit into btmtkuart driver. Beware that [1] is also pulled
   into one part of the series for avoiding any breakage when the patchset
   is being compiled.

[1] btuart 
	https://www.spinics.net/lists/linux-bluetooth/msg74918.html

v1:

Hi,

This patchset introduces built-in Bluetooth support on MT7622 SoC.
And, it should be simple to make an extension to support other
MediaTek SoCs with adjusting a few of changes on the initialization
sequence of the device.

Before the main driver is being introduced, a few of things about
power-domain management should be re-worked for serdev core and MediaTek
SCPSYS to allow the Bluetooth to properly power up.

Patch 2: add a generic way attaching power domain to serdev
Patch 3 and 4: add cleanups with reuse APIs from Linux core
Patch 5: fix a limitation about power enablement Bluetooth depends on
Patch 1, 6 and 7: the major part of adding Bluetooth support to MT7622
	
	Sean

Marcel Holtmann (1):
  Bluetooth: Add new serdev based driver for UART attached controllers

Sean Wang (6):
  dt-bindings: net: bluetooth: Add mediatek-bluetooth
  serdev: add dev_pm_domain_attach|detach()
  Bluetooth: Add new quirk for non-persistent setup settings
  Bluetooth: Extend btuart driver for join more vendor devices
  Bluetooth: mediatek: Add protocol support for MediaTek serial devices
  MAINTAINERS: add an entry for MediaTek Bluetooth driver

 .../devicetree/bindings/net/mediatek-bluetooth.txt |  35 ++
 MAINTAINERS                                        |   8 +
 drivers/bluetooth/Kconfig                          |  23 +
 drivers/bluetooth/Makefile                         |   3 +
 drivers/bluetooth/btmtkuart.c                      | 355 ++++++++++++++
 drivers/bluetooth/btmtkuart.h                      | 119 +++++
 drivers/bluetooth/btuart.c                         | 527 +++++++++++++++++++++
 drivers/bluetooth/btuart.h                         |  30 ++
 drivers/tty/serdev/core.c                          |  15 +-
 include/net/bluetooth/hci.h                        |   9 +
 net/bluetooth/hci_core.c                           |   3 +-
 11 files changed, 1125 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
 create mode 100644 drivers/bluetooth/btmtkuart.c
 create mode 100644 drivers/bluetooth/btmtkuart.h
 create mode 100644 drivers/bluetooth/btuart.c
 create mode 100644 drivers/bluetooth/btuart.h

-- 
2.7.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 1/7] dt-bindings: net: bluetooth: Add mediatek-bluetooth
  2018-06-27  5:43 [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC sean.wang
@ 2018-06-27  5:43 ` sean.wang
  2018-06-27  5:43 ` [PATCH v4 2/7] serdev: add dev_pm_domain_attach|detach() sean.wang
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

Add binding document for a SoC built-in device using MediaTek protocol.
Which could be found on MT7622 SoC or other similar MediaTek SoCs.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/net/mediatek-bluetooth.txt | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/mediatek-bluetooth.txt

diff --git a/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
new file mode 100644
index 0000000..1335429
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
@@ -0,0 +1,35 @@
+MediaTek SoC built-in Bluetooth Devices
+==================================
+
+This device is a serial attached device to BTIF device and thus it must be a
+child node of the serial node with BTIF. The dt-bindings details for BTIF
+device can be known via Documentation/devicetree/bindings/serial/8250.txt.
+
+Required properties:
+
+- compatible:	Must be one of
+		  "mediatek,mt7622-bluetooth"": for MT7622 SoC
+- clocks:	Should be the clock specifiers corresponding to the entry in
+		clock-names property.
+- clock-names:	Should contain "ref" entries.
+- power-domains: Phandle to the power domain that the device is part of
+
+Example:
+
+	btif: serial@1100c000 {
+		compatible = "mediatek,mt7622-btif",
+			     "mediatek,mtk-btif";
+		reg = <0 0x1100c000 0 0x1000>;
+		interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&pericfg CLK_PERI_BTIF_PD>;
+		clock-names = "main";
+		reg-shift = <2>;
+		reg-io-width = <4>;
+
+		bluetooth {
+			compatible = "mediatek,mt7622-bluetooth";
+			power-domains = <&scpsys MT7622_POWER_DOMAIN_WB>;
+			clocks = <&clk25m>;
+			clock-names = "ref";
+		};
+	};
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 2/7] serdev: add dev_pm_domain_attach|detach()
  2018-06-27  5:43 [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC sean.wang
  2018-06-27  5:43 ` [PATCH v4 1/7] dt-bindings: net: bluetooth: Add mediatek-bluetooth sean.wang
@ 2018-06-27  5:43 ` sean.wang
  2018-06-27  8:00   ` Ulf Hansson
  2018-06-27  5:43 ` [PATCH v4 3/7] Bluetooth: Add new serdev based driver for UART attached controllers sean.wang
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel, Sean Wang, Rob Herring, Ulf Hansson,
	Greg Kroah-Hartman, Jiri Slaby, linux-serial

From: Sean Wang <sean.wang@mediatek.com>

In order to open up the required power gate before any operation can be
effectively performed over the serial bus between CPU and serdev, it's
clearly essential to add common attach functions for PM domains to serdev
at the probe phase.

Similarly, the relevant dettach function for the PM domains should be
properly and reversely added at the remove phase.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: linux-serial@vger.kernel.org
---
 drivers/tty/serdev/core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index df93b72..8096138 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/pm_domain.h>
 #include <linux/serdev.h>
 #include <linux/slab.h>
 
@@ -330,8 +331,17 @@ EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
 static int serdev_drv_probe(struct device *dev)
 {
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
+	int ret;
 
-	return sdrv->probe(to_serdev_device(dev));
+	ret = dev_pm_domain_attach(dev, true);
+	if (ret)
+		return ret;
+
+	ret = sdrv->probe(to_serdev_device(dev));
+	if (ret)
+		dev_pm_domain_detach(dev, true);
+
+	return ret;
 }
 
 static int serdev_drv_remove(struct device *dev)
@@ -339,6 +349,9 @@ static int serdev_drv_remove(struct device *dev)
 	const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
 	if (sdrv->remove)
 		sdrv->remove(to_serdev_device(dev));
+
+	dev_pm_domain_detach(dev, true);
+
 	return 0;
 }
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 3/7] Bluetooth: Add new serdev based driver for UART attached controllers
  2018-06-27  5:43 [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC sean.wang
  2018-06-27  5:43 ` [PATCH v4 1/7] dt-bindings: net: bluetooth: Add mediatek-bluetooth sean.wang
  2018-06-27  5:43 ` [PATCH v4 2/7] serdev: add dev_pm_domain_attach|detach() sean.wang
@ 2018-06-27  5:43 ` sean.wang
  2018-06-27  5:43 ` [PATCH v4 4/7] Bluetooth: Add new quirk for non-persistent setup settings sean.wang
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel

From: Marcel Holtmann <marcel@holtmann.org>

This is a from scratch written driver to run H:4 on serdev based system
with a Bluetooth controller attached via an UART. It is currently tested
on RPi3 and it has Broadcom integration. It is DT only and is missing
GPIO and runtime power management integration. Also Apple or ACPI
support is currently not added.

To integrate with controllers from Intel and Qualcomm, similar handling
like with btusb.c has to be done. A simple abstraction for that has been
provided to make it similar to hci_uart.

The goal is to run individual drivers on serdev capable systems so that
we can retire hci_uart on these system and continue with a lot simpler
and easier to maintain driver. It seems that hci_uart has too many race
conditions due to handling TTY and line disciplines. And fixes for that
are not really related to serdev based drivers. In a serdev only world
it makes sense to remove any of the complex code.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 drivers/bluetooth/Kconfig  |  11 +
 drivers/bluetooth/Makefile |   1 +
 drivers/bluetooth/btuart.c | 506 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 518 insertions(+)
 create mode 100644 drivers/bluetooth/btuart.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index f3c643a..00fdf5f 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -74,6 +74,17 @@ config BT_HCIBTSDIO
 	  Say Y here to compile support for Bluetooth SDIO devices into the
 	  kernel or say M to compile it as module (btsdio).
 
+config BT_HCIBTUART
+	tristate "HCI UART driver"
+	depends on SERIAL_DEV_BUS
+	help
+	  Bluetooth HCI UART driver.
+	  This driver is required if you want to use Bluetooth device with
+	  UART interface.
+
+	  Say Y here to compile support for Bluetooth UART devices into the
+	  kernel or say M to compile it as module (btuart).
+
 config BT_HCIUART
 	tristate "HCI UART driver"
 	depends on SERIAL_DEV_BUS || !SERIAL_DEV_BUS
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index ec16c55..60a19cb 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_BT_HCIBLUECARD)	+= bluecard_cs.o
 
 obj-$(CONFIG_BT_HCIBTUSB)	+= btusb.o
 obj-$(CONFIG_BT_HCIBTSDIO)	+= btsdio.o
+obj-$(CONFIG_BT_HCIBTUART)	+= btuart.o
 
 obj-$(CONFIG_BT_INTEL)		+= btintel.o
 obj-$(CONFIG_BT_ATH3K)		+= ath3k.o
diff --git a/drivers/bluetooth/btuart.c b/drivers/bluetooth/btuart.c
new file mode 100644
index 0000000..03e980f
--- /dev/null
+++ b/drivers/bluetooth/btuart.c
@@ -0,0 +1,506 @@
+/*
+ *
+ *  Generic Bluetooth HCI UART driver
+ *
+ *  Copyright (C) 2015-2018  Intel Corporation
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/serdev.h>
+#include <linux/of.h>
+#include <linux/firmware.h>
+#include <asm/unaligned.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "h4_recv.h"
+#include "btbcm.h"
+
+#define VERSION "1.0"
+
+struct btuart_vnd {
+	const struct h4_recv_pkt *recv_pkts;
+	int recv_pkts_cnt;
+	unsigned int manufacturer;
+	int (*open)(struct hci_dev *hdev);
+	int (*close)(struct hci_dev *hdev);
+	int (*setup)(struct hci_dev *hdev);
+};
+
+struct btuart_dev {
+	struct hci_dev *hdev;
+	struct serdev_device *serdev;
+
+	struct work_struct tx_work;
+	unsigned long tx_state;
+	struct sk_buff_head txq;
+
+	struct sk_buff *rx_skb;
+
+	const struct btuart_vnd *vnd;
+};
+
+#define BTUART_TX_STATE_ACTIVE	1
+#define BTUART_TX_STATE_WAKEUP	2
+
+static void btuart_tx_work(struct work_struct *work)
+{
+	struct btuart_dev *bdev = container_of(work, struct btuart_dev,
+					       tx_work);
+	struct serdev_device *serdev = bdev->serdev;
+	struct hci_dev *hdev = bdev->hdev;
+
+	while (1) {
+		clear_bit(BTUART_TX_STATE_WAKEUP, &bdev->tx_state);
+
+		while (1) {
+			struct sk_buff *skb = skb_dequeue(&bdev->txq);
+			int len;
+
+			if (!skb)
+				break;
+
+			len = serdev_device_write_buf(serdev, skb->data,
+						      skb->len);
+			hdev->stat.byte_tx += len;
+
+			skb_pull(skb, len);
+			if (skb->len > 0) {
+				skb_queue_head(&bdev->txq, skb);
+				break;
+			}
+
+			switch (hci_skb_pkt_type(skb)) {
+			case HCI_COMMAND_PKT:
+				hdev->stat.cmd_tx++;
+				break;
+			case HCI_ACLDATA_PKT:
+				hdev->stat.acl_tx++;
+				break;
+			case HCI_SCODATA_PKT:
+				hdev->stat.sco_tx++;
+				break;
+			}
+
+			kfree_skb(skb);
+		}
+
+		if (!test_bit(BTUART_TX_STATE_WAKEUP, &bdev->tx_state))
+			break;
+	}
+
+	clear_bit(BTUART_TX_STATE_ACTIVE, &bdev->tx_state);
+}
+
+static int btuart_tx_wakeup(struct btuart_dev *bdev)
+{
+	if (test_and_set_bit(BTUART_TX_STATE_ACTIVE, &bdev->tx_state)) {
+		set_bit(BTUART_TX_STATE_WAKEUP, &bdev->tx_state);
+		return 0;
+	}
+
+	schedule_work(&bdev->tx_work);
+	return 0;
+}
+
+static int btuart_open(struct hci_dev *hdev)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+	int err;
+
+	err = serdev_device_open(bdev->serdev);
+	if (err) {
+		bt_dev_err(hdev, "Unable to open UART device %s",
+			   dev_name(&bdev->serdev->dev));
+		return err;
+	}
+
+	if (bdev->vnd->open) {
+		err = bdev->vnd->open(hdev);
+		if (err) {
+			serdev_device_close(bdev->serdev);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int btuart_close(struct hci_dev *hdev)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+	int err;
+
+	if (bdev->vnd->close) {
+		err = bdev->vnd->close(hdev);
+		if (err)
+			return err;
+	}
+
+	serdev_device_close(bdev->serdev);
+
+	return 0;
+}
+
+static int btuart_flush(struct hci_dev *hdev)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+
+	/* Flush any pending characters */
+	serdev_device_write_flush(bdev->serdev);
+	skb_queue_purge(&bdev->txq);
+
+	cancel_work_sync(&bdev->tx_work);
+
+	kfree_skb(bdev->rx_skb);
+	bdev->rx_skb = NULL;
+
+	return 0;
+}
+
+static int btuart_setup(struct hci_dev *hdev)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+
+	if (bdev->vnd->setup)
+		return bdev->vnd->setup(hdev);
+
+	return 0;
+}
+
+static int btuart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
+	skb_queue_tail(&bdev->txq, skb);
+
+	btuart_tx_wakeup(bdev);
+	return 0;
+}
+
+static int btuart_receive_buf(struct serdev_device *serdev, const u8 *data,
+			      size_t count)
+{
+	struct btuart_dev *bdev = serdev_device_get_drvdata(serdev);
+	const struct btuart_vnd *vnd = bdev->vnd;
+
+	bdev->rx_skb = h4_recv_buf(bdev->hdev, bdev->rx_skb, data, count,
+				   vnd->recv_pkts, vnd->recv_pkts_cnt);
+	if (IS_ERR(bdev->rx_skb)) {
+		int err = PTR_ERR(bdev->rx_skb);
+		bt_dev_err(bdev->hdev, "Frame reassembly failed (%d)", err);
+		bdev->rx_skb = NULL;
+		return err;
+	}
+
+	bdev->hdev->stat.byte_rx += count;
+
+	return count;
+}
+
+static void btuart_write_wakeup(struct serdev_device *serdev)
+{
+	struct btuart_dev *bdev = serdev_device_get_drvdata(serdev);
+
+	btuart_tx_wakeup(bdev);
+}
+
+static const struct serdev_device_ops btuart_client_ops = {
+	.receive_buf = btuart_receive_buf,
+	.write_wakeup = btuart_write_wakeup,
+};
+
+#define BCM_NULL_PKT 0x00
+#define BCM_NULL_SIZE 0
+
+#define BCM_LM_DIAG_PKT 0x07
+#define BCM_LM_DIAG_SIZE 63
+
+#define BCM_RECV_LM_DIAG \
+	.type = BCM_LM_DIAG_PKT, \
+	.hlen = BCM_LM_DIAG_SIZE, \
+	.loff = 0, \
+	.lsize = 0, \
+	.maxlen = BCM_LM_DIAG_SIZE
+
+#define BCM_RECV_NULL \
+	.type = BCM_NULL_PKT, \
+	.hlen = BCM_NULL_SIZE, \
+	.loff = 0, \
+	.lsize = 0, \
+	.maxlen = BCM_NULL_SIZE
+
+static int bcm_set_diag(struct hci_dev *hdev, bool enable)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+	struct sk_buff *skb;
+
+	if (!test_bit(HCI_RUNNING, &hdev->flags))
+		return -ENETDOWN;
+
+	skb = bt_skb_alloc(3, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	skb_put_u8(skb, BCM_LM_DIAG_PKT);
+	skb_put_u8(skb, 0xf0);
+	skb_put_u8(skb, enable);
+
+	skb_queue_tail(&bdev->txq, skb);
+	btuart_tx_wakeup(bdev);
+
+	return 0;
+}
+
+static int bcm_set_baudrate(struct btuart_dev *bdev, unsigned int speed)
+{
+	struct hci_dev *hdev = bdev->hdev;
+	struct sk_buff *skb;
+	struct bcm_update_uart_baud_rate param;
+
+	if (speed > 3000000) {
+		struct bcm_write_uart_clock_setting clock;
+
+		clock.type = BCM_UART_CLOCK_48MHZ;
+
+		bt_dev_dbg(hdev, "Set Controller clock (%d)", clock.type);
+
+		/* This Broadcom specific command changes the UART's controller
+		 * clock for baud rate > 3000000.
+		 */
+		skb = __hci_cmd_sync(hdev, 0xfc45, 1, &clock, HCI_INIT_TIMEOUT);
+		if (IS_ERR(skb)) {
+			int err = PTR_ERR(skb);
+			bt_dev_err(hdev, "Failed to write clock (%d)", err);
+			return err;
+		}
+
+		kfree_skb(skb);
+	}
+
+	bt_dev_dbg(hdev, "Set Controller UART speed to %d bit/s", speed);
+
+	param.zero = cpu_to_le16(0);
+	param.baud_rate = cpu_to_le32(speed);
+
+	/* This Broadcom specific command changes the UART's controller baud
+	 * rate.
+	 */
+	skb = __hci_cmd_sync(hdev, 0xfc18, sizeof(param), &param,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		int err = PTR_ERR(skb);
+		bt_dev_err(hdev, "Failed to write update baudrate (%d)", err);
+		return err;
+	}
+
+	kfree_skb(skb);
+
+	return 0;
+}
+
+static int bcm_setup(struct hci_dev *hdev)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+	char fw_name[64];
+	const struct firmware *fw;
+	unsigned int speed;
+	int err;
+
+	hdev->set_diag = bcm_set_diag;
+	hdev->set_bdaddr = btbcm_set_bdaddr;
+
+	/* Init speed if any */
+	speed = 115200;
+
+	if (speed)
+		serdev_device_set_baudrate(bdev->serdev, speed);
+
+	/* Operational speed if any */
+	speed = 115200;
+
+	if (speed) {
+		err = bcm_set_baudrate(bdev, speed);
+		if (err)
+			bt_dev_err(hdev, "Failed to set baudrate");
+		else
+			serdev_device_set_baudrate(bdev->serdev, speed);
+	}
+
+	err = btbcm_initialize(hdev, fw_name, sizeof(fw_name));
+	if (err)
+		return err;
+
+	err = request_firmware(&fw, fw_name, &hdev->dev);
+	if (err < 0) {
+		bt_dev_warn(hdev, "Patch %s not found", fw_name);
+		return 0;
+	}
+
+	err = btbcm_patchram(bdev->hdev, fw);
+	if (err) {
+		bt_dev_err(hdev, "Patching failed (%d)", err);
+		goto finalize;
+	}
+
+	/* Init speed if any */
+	speed = 115200;
+
+	if (speed)
+		serdev_device_set_baudrate(bdev->serdev, speed);
+
+	/* Operational speed if any */
+	speed = 115200;
+
+	if (speed) {
+		err = bcm_set_baudrate(bdev, speed);
+		if (!err)
+			serdev_device_set_baudrate(bdev->serdev, speed);
+	}
+
+finalize:
+	release_firmware(fw);
+
+	err = btbcm_finalize(hdev);
+	if (err)
+		return err;
+
+	return err;
+}
+
+static const struct h4_recv_pkt bcm_recv_pkts[] = {
+	{ H4_RECV_ACL,      .recv = hci_recv_frame },
+	{ H4_RECV_SCO,      .recv = hci_recv_frame },
+	{ H4_RECV_EVENT,    .recv = hci_recv_frame },
+	{ BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
+	{ BCM_RECV_NULL,    .recv = hci_recv_diag  },
+};
+
+static const struct btuart_vnd bcm_vnd = {
+	.recv_pkts	= bcm_recv_pkts,
+	.recv_pkts_cnt	= ARRAY_SIZE(bcm_recv_pkts),
+	.manufacturer	= 15,
+	.setup		= bcm_setup,
+};
+
+static const struct h4_recv_pkt default_recv_pkts[] = {
+	{ H4_RECV_ACL,      .recv = hci_recv_frame },
+	{ H4_RECV_SCO,      .recv = hci_recv_frame },
+	{ H4_RECV_EVENT,    .recv = hci_recv_frame },
+};
+
+static const struct btuart_vnd default_vnd = {
+	.recv_pkts	= default_recv_pkts,
+	.recv_pkts_cnt	= ARRAY_SIZE(default_recv_pkts),
+};
+
+static int btuart_probe(struct serdev_device *serdev)
+{
+	struct btuart_dev *bdev;
+	struct hci_dev *hdev;
+
+	bdev = devm_kzalloc(&serdev->dev, sizeof(*bdev), GFP_KERNEL);
+	if (!bdev)
+		return -ENOMEM;
+
+	/* Request the vendor specific data and callbacks */
+	bdev->vnd = device_get_match_data(&serdev->dev);
+	if (!bdev->vnd)
+		bdev->vnd = &default_vnd;
+
+	bdev->serdev = serdev;
+	serdev_device_set_drvdata(serdev, bdev);
+
+	serdev_device_set_client_ops(serdev, &btuart_client_ops);
+
+	INIT_WORK(&bdev->tx_work, btuart_tx_work);
+	skb_queue_head_init(&bdev->txq);
+
+	/* Initialize and register HCI device */
+	hdev = hci_alloc_dev();
+	if (!hdev) {
+		dev_err(&serdev->dev, "Can't allocate HCI device\n");
+		return -ENOMEM;
+	}
+
+	bdev->hdev = hdev;
+
+	hdev->bus = HCI_UART;
+	hci_set_drvdata(hdev, bdev);
+
+	/* Only when vendor specific setup callback is provided, consider
+	 * the manufacturer information valid. This avoids filling in the
+	 * value for Ericsson when nothing is specified.
+	 */
+	if (bdev->vnd->setup)
+		hdev->manufacturer = bdev->vnd->manufacturer;
+
+	hdev->open  = btuart_open;
+	hdev->close = btuart_close;
+	hdev->flush = btuart_flush;
+	hdev->setup = btuart_setup;
+	hdev->send  = btuart_send_frame;
+	SET_HCIDEV_DEV(hdev, &serdev->dev);
+
+	if (hci_register_dev(hdev) < 0) {
+		dev_err(&serdev->dev, "Can't register HCI device\n");
+		hci_free_dev(hdev);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static void btuart_remove(struct serdev_device *serdev)
+{
+	struct btuart_dev *bdev = serdev_device_get_drvdata(serdev);
+	struct hci_dev *hdev = bdev->hdev;
+
+	hci_unregister_dev(hdev);
+	hci_free_dev(hdev);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id btuart_of_match_table[] = {
+	{ .compatible = "brcm,bcm43438-bt", .data = &bcm_vnd },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, btuart_of_match_table);
+#endif
+
+static struct serdev_device_driver btuart_driver = {
+	.probe = btuart_probe,
+	.remove = btuart_remove,
+	.driver = {
+		.name = "btuart",
+		.of_match_table = of_match_ptr(btuart_of_match_table),
+	},
+};
+
+module_serdev_device_driver(btuart_driver);
+
+MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
+MODULE_DESCRIPTION("Generic Bluetooth UART driver ver " VERSION);
+MODULE_VERSION(VERSION);
+MODULE_LICENSE("GPL");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 4/7] Bluetooth: Add new quirk for non-persistent setup settings
  2018-06-27  5:43 [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC sean.wang
                   ` (2 preceding siblings ...)
  2018-06-27  5:43 ` [PATCH v4 3/7] Bluetooth: Add new serdev based driver for UART attached controllers sean.wang
@ 2018-06-27  5:43 ` sean.wang
  2018-06-27  5:43 ` [PATCH v4 5/7] Bluetooth: Extend btuart driver for join more vendor devices sean.wang
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

Add a new quirk HCI_QUIRK_NON_PERSISTENT_SETUP allowing that a quirk that
runs setup() after every open() and not just after the first open().

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 include/net/bluetooth/hci.h | 9 +++++++++
 net/bluetooth/hci_core.c    | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 1668211..b37d973 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -183,6 +183,15 @@ enum {
 	 * during the hdev->setup vendor callback.
 	 */
 	HCI_QUIRK_NON_PERSISTENT_DIAG,
+
+	/* When this quirk is set, setup() would be run after every
+	 * open() and not just after the first open().
+	 *
+	 * This quirk can be set before hci_register_dev is called or
+	 * during the hdev->setup vendor callback.
+	 *
+	 */
+	HCI_QUIRK_NON_PERSISTENT_SETUP,
 };
 
 /* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index ee8ef12..f2bf0c6 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1368,7 +1368,8 @@ static int hci_dev_do_open(struct hci_dev *hdev)
 	atomic_set(&hdev->cmd_cnt, 1);
 	set_bit(HCI_INIT, &hdev->flags);
 
-	if (hci_dev_test_flag(hdev, HCI_SETUP)) {
+	if (hci_dev_test_flag(hdev, HCI_SETUP) ||
+	    test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
 		hci_sock_dev_event(hdev, HCI_DEV_SETUP);
 
 		if (hdev->setup)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 5/7] Bluetooth: Extend btuart driver for join more vendor devices
  2018-06-27  5:43 [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC sean.wang
                   ` (3 preceding siblings ...)
  2018-06-27  5:43 ` [PATCH v4 4/7] Bluetooth: Add new quirk for non-persistent setup settings sean.wang
@ 2018-06-27  5:43 ` sean.wang
  2018-06-27  5:43 ` [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices sean.wang
  2018-06-27  5:43 ` [PATCH v4 7/7] MAINTAINERS: add an entry for MediaTek Bluetooth driver sean.wang
  6 siblings, 0 replies; 15+ messages in thread
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

Adding an independent btuart.h header allows these essential definitions
can be reused in vendor driver. Also, struct btuart_vnd is extended with
additional callbacks such as .init initializing vendor data, .shtudown,
.recv and .send supporting SoC specific framing for that btuart can
simply adapt to various Bluetooth uart-based devices.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/bluetooth/btuart.c | 73 ++++++++++++++++++++++++----------------------
 drivers/bluetooth/btuart.h | 30 +++++++++++++++++++
 2 files changed, 68 insertions(+), 35 deletions(-)
 create mode 100644 drivers/bluetooth/btuart.h

diff --git a/drivers/bluetooth/btuart.c b/drivers/bluetooth/btuart.c
index 03e980f..ab7f836 100644
--- a/drivers/bluetooth/btuart.c
+++ b/drivers/bluetooth/btuart.c
@@ -33,35 +33,11 @@
 #include <net/bluetooth/hci_core.h>
 
 #include "h4_recv.h"
+#include "btuart.h"
 #include "btbcm.h"
 
 #define VERSION "1.0"
 
-struct btuart_vnd {
-	const struct h4_recv_pkt *recv_pkts;
-	int recv_pkts_cnt;
-	unsigned int manufacturer;
-	int (*open)(struct hci_dev *hdev);
-	int (*close)(struct hci_dev *hdev);
-	int (*setup)(struct hci_dev *hdev);
-};
-
-struct btuart_dev {
-	struct hci_dev *hdev;
-	struct serdev_device *serdev;
-
-	struct work_struct tx_work;
-	unsigned long tx_state;
-	struct sk_buff_head txq;
-
-	struct sk_buff *rx_skb;
-
-	const struct btuart_vnd *vnd;
-};
-
-#define BTUART_TX_STATE_ACTIVE	1
-#define BTUART_TX_STATE_WAKEUP	2
-
 static void btuart_tx_work(struct work_struct *work)
 {
 	struct btuart_dev *bdev = container_of(work, struct btuart_dev,
@@ -187,13 +163,27 @@ static int btuart_setup(struct hci_dev *hdev)
 	return 0;
 }
 
+static int btuart_shutdown(struct hci_dev *hdev)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+
+	if (bdev->vnd->shutdown)
+		return bdev->vnd->shutdown(hdev);
+
+	return 0;
+}
+
 static int btuart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct btuart_dev *bdev = hci_get_drvdata(hdev);
 
-	/* Prepend skb with frame type */
-	memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
-	skb_queue_tail(&bdev->txq, skb);
+	if (bdev->vnd->send) {
+		bdev->vnd->send(hdev, skb);
+	} else {
+		/* Prepend skb with frame type */
+		memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
+		skb_queue_tail(&bdev->txq, skb);
+	}
 
 	btuart_tx_wakeup(bdev);
 	return 0;
@@ -204,14 +194,23 @@ static int btuart_receive_buf(struct serdev_device *serdev, const u8 *data,
 {
 	struct btuart_dev *bdev = serdev_device_get_drvdata(serdev);
 	const struct btuart_vnd *vnd = bdev->vnd;
+	int err;
 
-	bdev->rx_skb = h4_recv_buf(bdev->hdev, bdev->rx_skb, data, count,
-				   vnd->recv_pkts, vnd->recv_pkts_cnt);
-	if (IS_ERR(bdev->rx_skb)) {
-		int err = PTR_ERR(bdev->rx_skb);
-		bt_dev_err(bdev->hdev, "Frame reassembly failed (%d)", err);
-		bdev->rx_skb = NULL;
-		return err;
+	if (bdev->vnd->recv) {
+		err = bdev->vnd->recv(bdev->hdev, data, count);
+		if (err < 0)
+			return err;
+	} else {
+		bdev->rx_skb = h4_recv_buf(bdev->hdev, bdev->rx_skb,
+					   data, count, vnd->recv_pkts,
+					   vnd->recv_pkts_cnt);
+		if (IS_ERR(bdev->rx_skb)) {
+			err = PTR_ERR(bdev->rx_skb);
+			bt_dev_err(bdev->hdev,
+				   "Frame reassembly failed (%d)", err);
+			bdev->rx_skb = NULL;
+			return err;
+		}
 	}
 
 	bdev->hdev->stat.byte_rx += count;
@@ -429,6 +428,9 @@ static int btuart_probe(struct serdev_device *serdev)
 	if (!bdev->vnd)
 		bdev->vnd = &default_vnd;
 
+	if (bdev->vnd->init)
+		bdev->data = bdev->vnd->init(&serdev->dev);
+
 	bdev->serdev = serdev;
 	serdev_device_set_drvdata(serdev, bdev);
 
@@ -460,6 +462,7 @@ static int btuart_probe(struct serdev_device *serdev)
 	hdev->close = btuart_close;
 	hdev->flush = btuart_flush;
 	hdev->setup = btuart_setup;
+	hdev->shutdown = btuart_shutdown;
 	hdev->send  = btuart_send_frame;
 	SET_HCIDEV_DEV(hdev, &serdev->dev);
 
diff --git a/drivers/bluetooth/btuart.h b/drivers/bluetooth/btuart.h
new file mode 100644
index 0000000..6c1fe31
--- /dev/null
+++ b/drivers/bluetooth/btuart.h
@@ -0,0 +1,30 @@
+struct btuart_vnd {
+	const struct h4_recv_pkt *recv_pkts;
+	int recv_pkts_cnt;
+	unsigned int manufacturer;
+	void *(*init)(struct device *dev);
+
+	int (*open)(struct hci_dev *hdev);
+	int (*close)(struct hci_dev *hdev);
+	int (*setup)(struct hci_dev *hdev);
+	int (*shutdown)(struct hci_dev *hdev);
+	int (*send)(struct hci_dev *hdev, struct sk_buff *skb);
+	int (*recv)(struct hci_dev *hdev, const u8 *data, size_t count);
+};
+
+struct btuart_dev {
+	struct hci_dev *hdev;
+	struct serdev_device *serdev;
+
+	struct work_struct tx_work;
+	unsigned long tx_state;
+	struct sk_buff_head txq;
+
+	struct sk_buff *rx_skb;
+
+	const struct btuart_vnd *vnd;
+	void *data;
+};
+
+#define BTUART_TX_STATE_ACTIVE	1
+#define BTUART_TX_STATE_WAKEUP	2
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices
  2018-06-27  5:43 [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC sean.wang
                   ` (4 preceding siblings ...)
  2018-06-27  5:43 ` [PATCH v4 5/7] Bluetooth: Extend btuart driver for join more vendor devices sean.wang
@ 2018-06-27  5:43 ` sean.wang
  2018-06-27 16:59   ` Andy Shevchenko
  2018-06-27  5:43 ` [PATCH v4 7/7] MAINTAINERS: add an entry for MediaTek Bluetooth driver sean.wang
  6 siblings, 1 reply; 15+ messages in thread
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

This adds a driver to run on the top of btuart driver for the MediaTek
serial protocol based on running H:4, which can enable the built-in
Bluetooth device inside MT7622 SoC.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/bluetooth/Kconfig     |  12 ++
 drivers/bluetooth/Makefile    |   2 +
 drivers/bluetooth/btmtkuart.c | 355 ++++++++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/btmtkuart.h | 119 ++++++++++++++
 drivers/bluetooth/btuart.c    |  18 +++
 5 files changed, 506 insertions(+)
 create mode 100644 drivers/bluetooth/btmtkuart.c
 create mode 100644 drivers/bluetooth/btmtkuart.h

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 00fdf5f..1a44afd 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -85,6 +85,18 @@ config BT_HCIBTUART
 	  Say Y here to compile support for Bluetooth UART devices into the
 	  kernel or say M to compile it as module (btuart).
 
+config BT_HCIBTUART_MTK
+	tristate "MediaTek HCI UART driver"
+	depends on BT_HCIBTUART
+	default y
+	help
+	  MediaTek Bluetooth HCI UART driver.
+	  This driver is required if you want to use MediaTek Bluetooth
+	  with serial interface.
+
+	  Say Y here to compile support for MediaTek Bluetooth UART devices
+	  into the kernel or say M to compile it as module (btmtkuart).
+
 config BT_HCIUART
 	tristate "HCI UART driver"
 	depends on SERIAL_DEV_BUS || !SERIAL_DEV_BUS
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 60a19cb..c9a8926 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -26,6 +26,8 @@ obj-$(CONFIG_BT_BCM)		+= btbcm.o
 obj-$(CONFIG_BT_RTL)		+= btrtl.o
 obj-$(CONFIG_BT_QCA)		+= btqca.o
 
+obj-$(CONFIG_BT_HCIBTUART_MTK)	+= btmtkuart.o
+
 obj-$(CONFIG_BT_HCIUART_NOKIA)	+= hci_nokia.o
 
 obj-$(CONFIG_BT_HCIRSI)		+= btrsi.o
diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c
new file mode 100644
index 0000000..3118f61
--- /dev/null
+++ b/drivers/bluetooth/btmtkuart.c
@@ -0,0 +1,355 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 MediaTek Inc.
+
+/*
+ * Bluetooth support for MediaTek serial devices
+ *
+ * Author: Sean Wang <sean.wang@mediatek.com>
+ *
+ */
+
+#include <asm/unaligned.h>
+#include <linux/atomic.h>
+#include <linux/clk.h>
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/serdev.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "h4_recv.h"
+#include "btuart.h"
+#include "btmtkuart.h"
+
+static void mtk_stp_reset(struct mtk_stp_splitter *sp)
+{
+	sp->cursor = 2;
+	sp->dlen = 0;
+}
+
+static const unsigned char *
+mtk_stp_split(struct btuart_dev *bdev, struct mtk_stp_splitter *sp,
+	      const unsigned char *data, int count, int *sz_h4)
+{
+	struct mtk_stp_hdr *shdr;
+
+	/* The cursor is reset when all the data of STP is consumed out. */
+	if (!sp->dlen && sp->cursor >= 6)
+		sp->cursor = 0;
+
+	/* Filling pad until all STP info is obtained. */
+	while (sp->cursor < 6 && count > 0) {
+		sp->pad[sp->cursor] = *data;
+		sp->cursor++;
+		data++;
+		count--;
+	}
+
+	/* Retrieve STP info and have a sanity check. */
+	if (!sp->dlen && sp->cursor >= 6) {
+		shdr = (struct mtk_stp_hdr *)&sp->pad[2];
+		sp->dlen = shdr->dlen1 << 8 | shdr->dlen2;
+
+		/* Resync STP when unexpected data is being read. */
+		if (shdr->prefix != 0x80 || sp->dlen > 2048) {
+			bt_dev_err(bdev->hdev, "stp format unexpect (%d, %d)",
+				   shdr->prefix, sp->dlen);
+			mtk_stp_reset(sp);
+		}
+	}
+
+	/* Directly quit when there's no data found for H4 can process. */
+	if (count <= 0)
+		return NULL;
+
+	/* Tranlate to how much the size of data H4 can handle so far. */
+	*sz_h4 = min_t(int, count, sp->dlen);
+	/* Update the remaining size of STP packet. */
+	sp->dlen -= *sz_h4;
+
+	/* Data points to STP payload which can be handled by H4. */
+	return data;
+}
+
+static int mtk_stp_send(struct btuart_dev *bdev, struct sk_buff *skb)
+{
+	struct mtk_stp_hdr *shdr;
+	struct sk_buff *new_skb;
+	int dlen;
+
+	memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
+	dlen = skb->len;
+
+	/* Make sure of STP header at least has 4-bytes free space to fill. */
+	if (unlikely(skb_headroom(skb) < MTK_STP_HDR_SIZE)) {
+		new_skb = skb_realloc_headroom(skb, MTK_STP_HDR_SIZE);
+		kfree_skb(skb);
+		skb = new_skb;
+	}
+
+	/* Build for STP packet format. */
+	shdr = skb_push(skb, MTK_STP_HDR_SIZE);
+	mtk_make_stp_hdr(shdr, 0, dlen);
+	skb_put_zero(skb, MTK_STP_TLR_SIZE);
+
+	skb_queue_tail(&bdev->txq, skb);
+
+	return 0;
+}
+
+static int mtk_hci_wmt_sync(struct btuart_dev *bdev, u8 opcode, u8 flag,
+			    u16 plen, const void *param)
+{
+	struct mtk_hci_wmt_cmd wc;
+	struct mtk_wmt_hdr *hdr;
+	struct sk_buff *skb;
+	u32 hlen;
+
+	hlen = sizeof(*hdr) + plen;
+	if (hlen > 255)
+		return -EINVAL;
+
+	hdr = (struct mtk_wmt_hdr *)&wc;
+	mtk_make_wmt_hdr(hdr, opcode, plen, flag);
+	memcpy(wc.data, param, plen);
+
+	atomic_inc(&bdev->hdev->cmd_cnt);
+
+	skb =  __hci_cmd_sync_ev(bdev->hdev, 0xfc6f, hlen, &wc, HCI_VENDOR_PKT,
+				 HCI_INIT_TIMEOUT);
+
+	if (IS_ERR(skb)) {
+		int err = PTR_ERR(skb);
+
+		bt_dev_err(bdev->hdev, "Failed to send wmt cmd (%d)\n", err);
+		return err;
+	}
+
+	kfree_skb(skb);
+
+	return 0;
+}
+
+static int mtk_setup_fw(struct btuart_dev *bdev)
+{
+	const struct firmware *fw;
+	struct device *dev;
+	const char *fwname;
+	const u8 *fw_ptr;
+	size_t fw_size;
+	int err, dlen;
+	u8 flag;
+
+	dev = &bdev->serdev->dev;
+	fwname = FIRMWARE_MT7622;
+
+	err = request_firmware(&fw, fwname, dev);
+	if (err < 0) {
+		bt_dev_err(bdev->hdev, "Failed to load firmware file (%d)",
+			   err);
+		return err;
+	}
+
+	fw_ptr = fw->data;
+	fw_size = fw->size;
+
+	/* The size of patch header is 30 bytes, should be skip. */
+	if (fw_size < 30)
+		return -EINVAL;
+
+	fw_size -= 30;
+	fw_ptr += 30;
+
+	while (fw_size > 0) {
+		dlen = min_t(int, 250, fw_size);
+
+		/* Tell deivice the position in sequence. */
+		flag = (fw_size - dlen <= 0) ? 3 :
+		       (fw_size < fw->size - 30) ? 2 : 1;
+
+		err = mtk_hci_wmt_sync(bdev, MTK_WMT_PATCH_DWNLD, flag, dlen,
+				       fw_ptr);
+		if (err < 0)
+			break;
+
+		fw_size -= dlen;
+		fw_ptr += dlen;
+	}
+
+	release_firmware(fw);
+
+	return err;
+}
+
+void *mtk_btuart_init(struct device *dev)
+{
+	struct mtk_bt_dev *soc;
+
+	soc = devm_kzalloc(dev, sizeof(*soc), GFP_KERNEL);
+	if (!soc)
+		return ERR_PTR(-ENOMEM);
+
+	soc->sp = devm_kzalloc(dev, sizeof(*soc->sp), GFP_KERNEL);
+	if (!soc->sp)
+		return ERR_PTR(-ENOMEM);
+
+	soc->clk = devm_clk_get(dev, "ref");
+	if (IS_ERR(soc->clk))
+		return ERR_CAST(soc->clk);
+
+	return soc;
+}
+EXPORT_SYMBOL_GPL(mtk_btuart_init);
+
+int mtk_btuart_send(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+
+	return mtk_stp_send(bdev, skb);
+}
+EXPORT_SYMBOL_GPL(mtk_btuart_send);
+
+int mtk_btuart_hci_frame(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_event_hdr *hdr = (void *)skb->data;
+
+	/* Fix up the vendor event id with HCI_VENDOR_PKT instead of
+	 * 0xe4 so that btmon can parse the kind of vendor event properly.
+	 */
+	if (hdr->evt == 0xe4)
+		hdr->evt = HCI_VENDOR_PKT;
+
+	/* Each HCI event would go through the core. */
+	return hci_recv_frame(hdev, skb);
+}
+EXPORT_SYMBOL_GPL(mtk_btuart_hci_frame);
+
+int mtk_btuart_recv(struct hci_dev *hdev, const u8 *data, size_t count)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+	const unsigned char *p_left = data, *p_h4;
+	const struct btuart_vnd *vnd = bdev->vnd;
+	struct mtk_bt_dev *soc = bdev->data;
+	int sz_left = count, sz_h4, adv;
+	struct device *dev;
+	int err;
+
+	dev = &bdev->serdev->dev;
+
+	while (sz_left > 0) {
+		/*  The serial data received from MT7622 BT controller is
+		 *  at all time padded around with the STP header and tailer.
+		 *
+		 *  A full STP packet is looking like
+		 *   -----------------------------------
+		 *  | STP header  |  H:4   | STP tailer |
+		 *   -----------------------------------
+		 *  but it don't guarantee to contain a full H:4 packet which
+		 *  means that it's possible for multiple STP packets forms a
+		 *  full H:4 packet and whose length recorded in STP header can
+		 *  shows up the most length the H:4 engine can handle in one
+		 *  time.
+		 */
+
+		p_h4 = mtk_stp_split(bdev, soc->sp, p_left, sz_left, &sz_h4);
+		if (!p_h4)
+			break;
+
+		adv = p_h4 - p_left;
+		sz_left -= adv;
+		p_left += adv;
+
+		bdev->rx_skb = h4_recv_buf(bdev->hdev, bdev->rx_skb, p_h4,
+					   sz_h4, vnd->recv_pkts,
+					   vnd->recv_pkts_cnt);
+		if (IS_ERR(bdev->rx_skb)) {
+			err = PTR_ERR(bdev->rx_skb);
+			bt_dev_err(bdev->hdev,
+				   "Frame reassembly failed (%d)", err);
+			bdev->rx_skb = NULL;
+			return err;
+		}
+
+		sz_left -= sz_h4;
+		p_left += sz_h4;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_btuart_recv);
+
+int mtk_btuart_setup(struct hci_dev *hdev)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+	struct mtk_bt_dev *soc = bdev->data;
+	struct device *dev;
+	u8 param = 0x1;
+	int err = 0;
+
+	dev = &bdev->serdev->dev;
+
+	mtk_stp_reset(soc->sp);
+
+	/* Enable the power domain and clock the device requires. */
+	pm_runtime_enable(dev);
+	err = pm_runtime_get_sync(dev);
+	if (err < 0)
+		goto err_pm2;
+
+	err = clk_prepare_enable(soc->clk);
+	if (err < 0)
+		goto err_pm1;
+
+	/* Setup a firmware which the device definitely requires. */
+	err = mtk_setup_fw(bdev);
+	if (err < 0)
+		goto err_clk;
+
+	/* Activate funciton the firmware providing to. */
+	err = mtk_hci_wmt_sync(bdev, MTK_WMT_RST, 0x4, 0, 0);
+	if (err < 0)
+		goto err_clk;
+
+	/* Enable Bluetooth protocol. */
+	err = mtk_hci_wmt_sync(bdev, MTK_WMT_FUNC_CTRL, 0x0, sizeof(param),
+			       &param);
+	if (err < 0)
+		goto err_clk;
+
+	set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
+
+	return 0;
+err_clk:
+	clk_disable_unprepare(soc->clk);
+err_pm1:
+	pm_runtime_put_sync(dev);
+err_pm2:
+	pm_runtime_disable(dev);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(mtk_btuart_setup);
+
+int mtk_btuart_shutdown(struct hci_dev *hdev)
+{
+	struct btuart_dev *bdev = hci_get_drvdata(hdev);
+	struct device *dev = &bdev->serdev->dev;
+	struct mtk_bt_dev *soc = bdev->data;
+	u8 param = 0x0;
+
+	/* Disable the device. */
+	mtk_hci_wmt_sync(bdev, MTK_WMT_FUNC_CTRL, 0x0, sizeof(param), &param);
+
+	/* Shutdown the clock and power domain the device requires. */
+	clk_disable_unprepare(soc->clk);
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_btuart_shutdown);
+
+MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
+MODULE_DESCRIPTION("Bluetooth Support for MediaTek Serial Devices");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/bluetooth/btmtkuart.h b/drivers/bluetooth/btmtkuart.h
new file mode 100644
index 0000000..e76ab23e
--- /dev/null
+++ b/drivers/bluetooth/btmtkuart.h
@@ -0,0 +1,119 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ *
+ * Bluetooth support for MediaTek serial devices
+ *
+ * Author: Sean Wang <sean.wang@mediatek.com>
+ *
+ */
+
+#define FIRMWARE_MT7622		"mediatek/mt7622pr2h.bin"
+
+#define MTK_STP_HDR_SIZE	4
+#define MTK_STP_TLR_SIZE	2
+#define MTK_WMT_HDR_SIZE	5
+#define MTK_WMT_CMD_SIZE	(MTK_WMT_HDR_SIZE + MTK_STP_HDR_SIZE + \
+				 MTK_STP_TLR_SIZE + HCI_ACL_HDR_SIZE)
+
+enum {
+	MTK_WMT_PATCH_DWNLD = 0x1,
+	MTK_WMT_FUNC_CTRL = 0x6,
+	MTK_WMT_RST = 0x7
+};
+
+struct mtk_stp_hdr {
+	__u8 prefix;
+	__u8 dlen1:4;
+	__u8 type:4;
+	__u8 dlen2:8;
+	__u8 cs;
+} __packed;
+
+struct mtk_wmt_hdr {
+	__u8	dir;
+	__u8	op;
+	__le16	dlen;
+	__u8	flag;
+} __packed;
+
+struct mtk_hci_wmt_cmd {
+	struct mtk_wmt_hdr hdr;
+	__u8 data[256];
+} __packed;
+
+struct mtk_stp_splitter {
+	u8	pad[6];
+	u8	cursor;
+	u16	dlen;
+};
+
+struct mtk_bt_dev {
+	struct clk *clk;
+	struct completion wmt_cmd;
+	struct mtk_stp_splitter *sp;
+};
+
+static inline void mtk_make_stp_hdr(struct mtk_stp_hdr *hdr, u8 type, u32 dlen)
+{
+	__u8 *p = (__u8 *)hdr;
+
+	hdr->prefix = 0x80;
+	hdr->dlen1 = (dlen & 0xf00) >> 8;
+	hdr->type = type;
+	hdr->dlen2 = dlen & 0xff;
+	hdr->cs = p[0] + p[1] + p[2];
+}
+
+static inline void mtk_make_wmt_hdr(struct mtk_wmt_hdr *hdr, u8 op, u16 plen,
+				    u8 flag)
+{
+	hdr->dir = 1;
+	hdr->op = op;
+	hdr->dlen = cpu_to_le16(plen + 1);
+	hdr->flag = flag;
+}
+
+#if IS_ENABLED(CONFIG_BT_HCIBTUART_MTK)
+
+void *mtk_btuart_init(struct device *dev);
+int mtk_btuart_setup(struct hci_dev *hdev);
+int mtk_btuart_shutdown(struct hci_dev *hdev);
+int mtk_btuart_send(struct hci_dev *hdev, struct sk_buff *skb);
+int mtk_btuart_hci_frame(struct hci_dev *hdev, struct sk_buff *skb);
+int mtk_btuart_recv(struct hci_dev *hdev, const u8 *data, size_t count);
+
+#else
+
+static void *mtk_btuart_init(struct device *dev)
+{
+	return 0;
+}
+
+static inline int mtk_btuart_setup(struct hci_dev *hdev)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int mtk_btuart_shutdown(struct hci_dev *hdev)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int mtk_btuart_send(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	return -EOPNOTSUPP;
+}
+
+static int mtk_btuart_hci_frame(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int mtk_btuart_recv(struct hci_dev *hdev, const u8 *data,
+				  size_t count)
+{
+	return -EOPNOTSUPP;
+}
+
+#endif
diff --git a/drivers/bluetooth/btuart.c b/drivers/bluetooth/btuart.c
index ab7f836..169bf1a 100644
--- a/drivers/bluetooth/btuart.c
+++ b/drivers/bluetooth/btuart.c
@@ -35,6 +35,7 @@
 #include "h4_recv.h"
 #include "btuart.h"
 #include "btbcm.h"
+#include "btmtkuart.h"
 
 #define VERSION "1.0"
 
@@ -396,6 +397,12 @@ static const struct h4_recv_pkt bcm_recv_pkts[] = {
 	{ BCM_RECV_NULL,    .recv = hci_recv_diag  },
 };
 
+static const struct h4_recv_pkt mtk_recv_pkts[] = {
+	{ H4_RECV_ACL,      .recv = hci_recv_frame },
+	{ H4_RECV_SCO,      .recv = hci_recv_frame },
+	{ H4_RECV_EVENT,    .recv = mtk_btuart_hci_frame },
+};
+
 static const struct btuart_vnd bcm_vnd = {
 	.recv_pkts	= bcm_recv_pkts,
 	.recv_pkts_cnt	= ARRAY_SIZE(bcm_recv_pkts),
@@ -403,6 +410,16 @@ static const struct btuart_vnd bcm_vnd = {
 	.setup		= bcm_setup,
 };
 
+static const struct btuart_vnd mtk_vnd = {
+	.recv_pkts	= mtk_recv_pkts,
+	.recv_pkts_cnt	= ARRAY_SIZE(mtk_recv_pkts),
+	.init		= mtk_btuart_init,
+	.setup		= mtk_btuart_setup,
+	.shutdown	= mtk_btuart_shutdown,
+	.send		= mtk_btuart_send,
+	.recv		= mtk_btuart_recv,
+};
+
 static const struct h4_recv_pkt default_recv_pkts[] = {
 	{ H4_RECV_ACL,      .recv = hci_recv_frame },
 	{ H4_RECV_SCO,      .recv = hci_recv_frame },
@@ -487,6 +504,7 @@ static void btuart_remove(struct serdev_device *serdev)
 #ifdef CONFIG_OF
 static const struct of_device_id btuart_of_match_table[] = {
 	{ .compatible = "brcm,bcm43438-bt", .data = &bcm_vnd },
+	{ .compatible = "mediatek,mt7622-bluetooth", .data = &mtk_vnd },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, btuart_of_match_table);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 7/7] MAINTAINERS: add an entry for MediaTek Bluetooth driver
  2018-06-27  5:43 [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC sean.wang
                   ` (5 preceding siblings ...)
  2018-06-27  5:43 ` [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices sean.wang
@ 2018-06-27  5:43 ` sean.wang
  6 siblings, 0 replies; 15+ messages in thread
From: sean.wang @ 2018-06-27  5:43 UTC (permalink / raw)
  To: robh+dt, mark.rutland, marcel, johan.hedberg
  Cc: devicetree, linux-bluetooth, linux-arm-kernel, linux-mediatek,
	linux-kernel, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

Add an entry for the MediaTek Bluetooth driver.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d5eeff..b06e38d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8987,6 +8987,14 @@ F:	include/uapi/linux/meye.h
 F:	include/uapi/linux/ivtv*
 F:	include/uapi/linux/uvcvideo.h
 
+MEDIATEK BLUETOOTH DRIVER
+M:	Sean Wang <sean.wang@mediatek.com>
+L:	linux-bluetooth@vger.kernel.org
+L:	linux-mediatek@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/mediatek-bluetooth.txt
+F:	drivers/bluetooth/btmtkuart.c
+
 MEDIATEK CIR DRIVER
 M:	Sean Wang <sean.wang@mediatek.com>
 S:	Maintained
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 2/7] serdev: add dev_pm_domain_attach|detach()
  2018-06-27  5:43 ` [PATCH v4 2/7] serdev: add dev_pm_domain_attach|detach() sean.wang
@ 2018-06-27  8:00   ` Ulf Hansson
  0 siblings, 0 replies; 15+ messages in thread
From: Ulf Hansson @ 2018-06-27  8:00 UTC (permalink / raw)
  To: sean.wang
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Johan Hedberg,
	devicetree, linux-bluetooth, Linux ARM, linux-mediatek,
	Linux Kernel Mailing List, Rob Herring, Greg Kroah-Hartman,
	Jiri Slaby, linux-serial

On 27 June 2018 at 07:43,  <sean.wang@mediatek.com> wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> In order to open up the required power gate before any operation can be
> effectively performed over the serial bus between CPU and serdev, it's
> clearly essential to add common attach functions for PM domains to serdev
> at the probe phase.
>
> Similarly, the relevant dettach function for the PM domains should be
> properly and reversely added at the remove phase.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: linux-serial@vger.kernel.org

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/tty/serdev/core.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index df93b72..8096138 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -13,6 +13,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/serdev.h>
>  #include <linux/slab.h>
>
> @@ -330,8 +331,17 @@ EXPORT_SYMBOL_GPL(serdev_device_set_tiocm);
>  static int serdev_drv_probe(struct device *dev)
>  {
>         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
> +       int ret;
>
> -       return sdrv->probe(to_serdev_device(dev));
> +       ret = dev_pm_domain_attach(dev, true);
> +       if (ret)
> +               return ret;
> +
> +       ret = sdrv->probe(to_serdev_device(dev));
> +       if (ret)
> +               dev_pm_domain_detach(dev, true);
> +
> +       return ret;
>  }
>
>  static int serdev_drv_remove(struct device *dev)
> @@ -339,6 +349,9 @@ static int serdev_drv_remove(struct device *dev)
>         const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver);
>         if (sdrv->remove)
>                 sdrv->remove(to_serdev_device(dev));
> +
> +       dev_pm_domain_detach(dev, true);
> +
>         return 0;
>  }
>
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices
  2018-06-27  5:43 ` [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices sean.wang
@ 2018-06-27 16:59   ` Andy Shevchenko
  2018-06-27 17:04     ` Andy Shevchenko
  2018-06-28  2:54     ` Sean Wang
  0 siblings, 2 replies; 15+ messages in thread
From: Andy Shevchenko @ 2018-06-27 16:59 UTC (permalink / raw)
  To: sean.wang
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Johan Hedberg,
	devicetree, linux-bluetooth, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support,
	Linux Kernel Mailing List

On Wed, Jun 27, 2018 at 8:43 AM,  <sean.wang@mediatek.com> wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>

> +config BT_HCIBTUART_MTK
> +       tristate "MediaTek HCI UART driver"
> +       depends on BT_HCIBTUART

> +       default y

Perhaps it's an overkill for users which would like to have less
amount on stuff in kernel.


> +#include <asm/unaligned.h>
> +#include <linux/atomic.h>
> +#include <linux/clk.h>
> +#include <linux/firmware.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/serdev.h>

> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include "h4_recv.h"
> +#include "btuart.h"
> +#include "btmtkuart.h"
> +
> +static void mtk_stp_reset(struct mtk_stp_splitter *sp)
> +{
> +       sp->cursor = 2;
> +       sp->dlen = 0;
> +}
> +
> +static const unsigned char *
> +mtk_stp_split(struct btuart_dev *bdev, struct mtk_stp_splitter *sp,
> +             const unsigned char *data, int count, int *sz_h4)
> +{
> +       struct mtk_stp_hdr *shdr;
> +
> +       /* The cursor is reset when all the data of STP is consumed out. */
> +       if (!sp->dlen && sp->cursor >= 6)
> +               sp->cursor = 0;
> +
> +       /* Filling pad until all STP info is obtained. */
> +       while (sp->cursor < 6 && count > 0) {
> +               sp->pad[sp->cursor] = *data;
> +               sp->cursor++;
> +               data++;
> +               count--;
> +       }
> +
> +       /* Retrieve STP info and have a sanity check. */
> +       if (!sp->dlen && sp->cursor >= 6) {
> +               shdr = (struct mtk_stp_hdr *)&sp->pad[2];
> +               sp->dlen = shdr->dlen1 << 8 | shdr->dlen2;
> +
> +               /* Resync STP when unexpected data is being read. */
> +               if (shdr->prefix != 0x80 || sp->dlen > 2048) {
> +                       bt_dev_err(bdev->hdev, "stp format unexpect (%d, %d)",
> +                                  shdr->prefix, sp->dlen);
> +                       mtk_stp_reset(sp);
> +               }
> +       }
> +
> +       /* Directly quit when there's no data found for H4 can process. */
> +       if (count <= 0)
> +               return NULL;
> +
> +       /* Tranlate to how much the size of data H4 can handle so far. */
> +       *sz_h4 = min_t(int, count, sp->dlen);
> +       /* Update the remaining size of STP packet. */
> +       sp->dlen -= *sz_h4;
> +
> +       /* Data points to STP payload which can be handled by H4. */
> +       return data;
> +}
> +
> +static int mtk_stp_send(struct btuart_dev *bdev, struct sk_buff *skb)
> +{
> +       struct mtk_stp_hdr *shdr;
> +       struct sk_buff *new_skb;
> +       int dlen;
> +
> +       memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
> +       dlen = skb->len;
> +
> +       /* Make sure of STP header at least has 4-bytes free space to fill. */
> +       if (unlikely(skb_headroom(skb) < MTK_STP_HDR_SIZE)) {
> +               new_skb = skb_realloc_headroom(skb, MTK_STP_HDR_SIZE);
> +               kfree_skb(skb);
> +               skb = new_skb;
> +       }
> +
> +       /* Build for STP packet format. */
> +       shdr = skb_push(skb, MTK_STP_HDR_SIZE);
> +       mtk_make_stp_hdr(shdr, 0, dlen);
> +       skb_put_zero(skb, MTK_STP_TLR_SIZE);
> +
> +       skb_queue_tail(&bdev->txq, skb);
> +
> +       return 0;
> +}
> +
> +static int mtk_hci_wmt_sync(struct btuart_dev *bdev, u8 opcode, u8 flag,
> +                           u16 plen, const void *param)
> +{
> +       struct mtk_hci_wmt_cmd wc;
> +       struct mtk_wmt_hdr *hdr;
> +       struct sk_buff *skb;
> +       u32 hlen;
> +
> +       hlen = sizeof(*hdr) + plen;
> +       if (hlen > 255)
> +               return -EINVAL;
> +
> +       hdr = (struct mtk_wmt_hdr *)&wc;
> +       mtk_make_wmt_hdr(hdr, opcode, plen, flag);
> +       memcpy(wc.data, param, plen);
> +
> +       atomic_inc(&bdev->hdev->cmd_cnt);
> +
> +       skb =  __hci_cmd_sync_ev(bdev->hdev, 0xfc6f, hlen, &wc, HCI_VENDOR_PKT,
> +                                HCI_INIT_TIMEOUT);
> +
> +       if (IS_ERR(skb)) {
> +               int err = PTR_ERR(skb);
> +
> +               bt_dev_err(bdev->hdev, "Failed to send wmt cmd (%d)\n", err);
> +               return err;
> +       }
> +
> +       kfree_skb(skb);
> +
> +       return 0;
> +}
> +
> +static int mtk_setup_fw(struct btuart_dev *bdev)
> +{
> +       const struct firmware *fw;
> +       struct device *dev;
> +       const char *fwname;
> +       const u8 *fw_ptr;
> +       size_t fw_size;
> +       int err, dlen;
> +       u8 flag;
> +
> +       dev = &bdev->serdev->dev;
> +       fwname = FIRMWARE_MT7622;
> +
> +       err = request_firmware(&fw, fwname, dev);
> +       if (err < 0) {
> +               bt_dev_err(bdev->hdev, "Failed to load firmware file (%d)",
> +                          err);
> +               return err;
> +       }
> +
> +       fw_ptr = fw->data;
> +       fw_size = fw->size;
> +
> +       /* The size of patch header is 30 bytes, should be skip. */
> +       if (fw_size < 30)
> +               return -EINVAL;
> +
> +       fw_size -= 30;
> +       fw_ptr += 30;
> +
> +       while (fw_size > 0) {
> +               dlen = min_t(int, 250, fw_size);
> +
> +               /* Tell deivice the position in sequence. */
> +               flag = (fw_size - dlen <= 0) ? 3 :
> +                      (fw_size < fw->size - 30) ? 2 : 1;
> +
> +               err = mtk_hci_wmt_sync(bdev, MTK_WMT_PATCH_DWNLD, flag, dlen,
> +                                      fw_ptr);
> +               if (err < 0)
> +                       break;
> +
> +               fw_size -= dlen;
> +               fw_ptr += dlen;
> +       }
> +
> +       release_firmware(fw);
> +
> +       return err;
> +}
> +
> +void *mtk_btuart_init(struct device *dev)
> +{
> +       struct mtk_bt_dev *soc;
> +
> +       soc = devm_kzalloc(dev, sizeof(*soc), GFP_KERNEL);
> +       if (!soc)
> +               return ERR_PTR(-ENOMEM);
> +
> +       soc->sp = devm_kzalloc(dev, sizeof(*soc->sp), GFP_KERNEL);
> +       if (!soc->sp)
> +               return ERR_PTR(-ENOMEM);
> +
> +       soc->clk = devm_clk_get(dev, "ref");
> +       if (IS_ERR(soc->clk))
> +               return ERR_CAST(soc->clk);
> +
> +       return soc;
> +}
> +EXPORT_SYMBOL_GPL(mtk_btuart_init);
> +
> +int mtk_btuart_send(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       struct btuart_dev *bdev = hci_get_drvdata(hdev);
> +
> +       return mtk_stp_send(bdev, skb);
> +}
> +EXPORT_SYMBOL_GPL(mtk_btuart_send);
> +
> +int mtk_btuart_hci_frame(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       struct hci_event_hdr *hdr = (void *)skb->data;
> +
> +       /* Fix up the vendor event id with HCI_VENDOR_PKT instead of
> +        * 0xe4 so that btmon can parse the kind of vendor event properly.
> +        */
> +       if (hdr->evt == 0xe4)
> +               hdr->evt = HCI_VENDOR_PKT;
> +
> +       /* Each HCI event would go through the core. */
> +       return hci_recv_frame(hdev, skb);
> +}
> +EXPORT_SYMBOL_GPL(mtk_btuart_hci_frame);
> +
> +int mtk_btuart_recv(struct hci_dev *hdev, const u8 *data, size_t count)
> +{
> +       struct btuart_dev *bdev = hci_get_drvdata(hdev);
> +       const unsigned char *p_left = data, *p_h4;
> +       const struct btuart_vnd *vnd = bdev->vnd;
> +       struct mtk_bt_dev *soc = bdev->data;
> +       int sz_left = count, sz_h4, adv;
> +       struct device *dev;
> +       int err;
> +
> +       dev = &bdev->serdev->dev;
> +
> +       while (sz_left > 0) {
> +               /*  The serial data received from MT7622 BT controller is
> +                *  at all time padded around with the STP header and tailer.
> +                *
> +                *  A full STP packet is looking like
> +                *   -----------------------------------
> +                *  | STP header  |  H:4   | STP tailer |
> +                *   -----------------------------------
> +                *  but it don't guarantee to contain a full H:4 packet which
> +                *  means that it's possible for multiple STP packets forms a
> +                *  full H:4 packet and whose length recorded in STP header can
> +                *  shows up the most length the H:4 engine can handle in one
> +                *  time.
> +                */
> +
> +               p_h4 = mtk_stp_split(bdev, soc->sp, p_left, sz_left, &sz_h4);
> +               if (!p_h4)
> +                       break;
> +
> +               adv = p_h4 - p_left;
> +               sz_left -= adv;
> +               p_left += adv;
> +
> +               bdev->rx_skb = h4_recv_buf(bdev->hdev, bdev->rx_skb, p_h4,
> +                                          sz_h4, vnd->recv_pkts,
> +                                          vnd->recv_pkts_cnt);
> +               if (IS_ERR(bdev->rx_skb)) {
> +                       err = PTR_ERR(bdev->rx_skb);
> +                       bt_dev_err(bdev->hdev,
> +                                  "Frame reassembly failed (%d)", err);
> +                       bdev->rx_skb = NULL;
> +                       return err;
> +               }
> +
> +               sz_left -= sz_h4;
> +               p_left += sz_h4;
> +       }
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(mtk_btuart_recv);
> +
> +int mtk_btuart_setup(struct hci_dev *hdev)
> +{
> +       struct btuart_dev *bdev = hci_get_drvdata(hdev);
> +       struct mtk_bt_dev *soc = bdev->data;
> +       struct device *dev;
> +       u8 param = 0x1;
> +       int err = 0;
> +
> +       dev = &bdev->serdev->dev;
> +
> +       mtk_stp_reset(soc->sp);
> +
> +       /* Enable the power domain and clock the device requires. */
> +       pm_runtime_enable(dev);
> +       err = pm_runtime_get_sync(dev);
> +       if (err < 0)
> +               goto err_pm2;
> +
> +       err = clk_prepare_enable(soc->clk);
> +       if (err < 0)
> +               goto err_pm1;
> +
> +       /* Setup a firmware which the device definitely requires. */
> +       err = mtk_setup_fw(bdev);
> +       if (err < 0)
> +               goto err_clk;
> +
> +       /* Activate funciton the firmware providing to. */
> +       err = mtk_hci_wmt_sync(bdev, MTK_WMT_RST, 0x4, 0, 0);
> +       if (err < 0)
> +               goto err_clk;
> +
> +       /* Enable Bluetooth protocol. */
> +       err = mtk_hci_wmt_sync(bdev, MTK_WMT_FUNC_CTRL, 0x0, sizeof(param),
> +                              &param);
> +       if (err < 0)
> +               goto err_clk;
> +
> +       set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
> +
> +       return 0;
> +err_clk:
> +       clk_disable_unprepare(soc->clk);
> +err_pm1:
> +       pm_runtime_put_sync(dev);
> +err_pm2:
> +       pm_runtime_disable(dev);
> +
> +       return err;
> +}
> +EXPORT_SYMBOL_GPL(mtk_btuart_setup);
> +
> +int mtk_btuart_shutdown(struct hci_dev *hdev)
> +{
> +       struct btuart_dev *bdev = hci_get_drvdata(hdev);
> +       struct device *dev = &bdev->serdev->dev;
> +       struct mtk_bt_dev *soc = bdev->data;
> +       u8 param = 0x0;
> +
> +       /* Disable the device. */
> +       mtk_hci_wmt_sync(bdev, MTK_WMT_FUNC_CTRL, 0x0, sizeof(param), &param);
> +
> +       /* Shutdown the clock and power domain the device requires. */
> +       clk_disable_unprepare(soc->clk);
> +       pm_runtime_put_sync(dev);
> +       pm_runtime_disable(dev);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(mtk_btuart_shutdown);
> +
> +MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
> +MODULE_DESCRIPTION("Bluetooth Support for MediaTek Serial Devices");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/bluetooth/btmtkuart.h b/drivers/bluetooth/btmtkuart.h
> new file mode 100644
> index 0000000..e76ab23e
> --- /dev/null
> +++ b/drivers/bluetooth/btmtkuart.h
> @@ -0,0 +1,119 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + *
> + * Bluetooth support for MediaTek serial devices
> + *
> + * Author: Sean Wang <sean.wang@mediatek.com>
> + *
> + */
> +
> +#define FIRMWARE_MT7622                "mediatek/mt7622pr2h.bin"
> +
> +#define MTK_STP_HDR_SIZE       4
> +#define MTK_STP_TLR_SIZE       2
> +#define MTK_WMT_HDR_SIZE       5
> +#define MTK_WMT_CMD_SIZE       (MTK_WMT_HDR_SIZE + MTK_STP_HDR_SIZE + \
> +                                MTK_STP_TLR_SIZE + HCI_ACL_HDR_SIZE)
> +
> +enum {
> +       MTK_WMT_PATCH_DWNLD = 0x1,
> +       MTK_WMT_FUNC_CTRL = 0x6,
> +       MTK_WMT_RST = 0x7
> +};
> +
> +struct mtk_stp_hdr {
> +       __u8 prefix;
> +       __u8 dlen1:4;
> +       __u8 type:4;
> +       __u8 dlen2:8;
> +       __u8 cs;
> +} __packed;
> +
> +struct mtk_wmt_hdr {
> +       __u8    dir;
> +       __u8    op;
> +       __le16  dlen;
> +       __u8    flag;
> +} __packed;
> +
> +struct mtk_hci_wmt_cmd {
> +       struct mtk_wmt_hdr hdr;
> +       __u8 data[256];
> +} __packed;
> +
> +struct mtk_stp_splitter {
> +       u8      pad[6];
> +       u8      cursor;
> +       u16     dlen;
> +};
> +
> +struct mtk_bt_dev {
> +       struct clk *clk;
> +       struct completion wmt_cmd;
> +       struct mtk_stp_splitter *sp;
> +};
> +
> +static inline void mtk_make_stp_hdr(struct mtk_stp_hdr *hdr, u8 type, u32 dlen)
> +{
> +       __u8 *p = (__u8 *)hdr;
> +
> +       hdr->prefix = 0x80;
> +       hdr->dlen1 = (dlen & 0xf00) >> 8;
> +       hdr->type = type;
> +       hdr->dlen2 = dlen & 0xff;
> +       hdr->cs = p[0] + p[1] + p[2];
> +}
> +
> +static inline void mtk_make_wmt_hdr(struct mtk_wmt_hdr *hdr, u8 op, u16 plen,
> +                                   u8 flag)
> +{
> +       hdr->dir = 1;
> +       hdr->op = op;
> +       hdr->dlen = cpu_to_le16(plen + 1);
> +       hdr->flag = flag;
> +}
> +
> +#if IS_ENABLED(CONFIG_BT_HCIBTUART_MTK)
> +
> +void *mtk_btuart_init(struct device *dev);
> +int mtk_btuart_setup(struct hci_dev *hdev);
> +int mtk_btuart_shutdown(struct hci_dev *hdev);
> +int mtk_btuart_send(struct hci_dev *hdev, struct sk_buff *skb);
> +int mtk_btuart_hci_frame(struct hci_dev *hdev, struct sk_buff *skb);
> +int mtk_btuart_recv(struct hci_dev *hdev, const u8 *data, size_t count);
> +
> +#else
> +
> +static void *mtk_btuart_init(struct device *dev)
> +{
> +       return 0;
> +}
> +
> +static inline int mtk_btuart_setup(struct hci_dev *hdev)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline int mtk_btuart_shutdown(struct hci_dev *hdev)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline int mtk_btuart_send(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static int mtk_btuart_hci_frame(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static inline int mtk_btuart_recv(struct hci_dev *hdev, const u8 *data,
> +                                 size_t count)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +#endif
> diff --git a/drivers/bluetooth/btuart.c b/drivers/bluetooth/btuart.c
> index ab7f836..169bf1a 100644
> --- a/drivers/bluetooth/btuart.c
> +++ b/drivers/bluetooth/btuart.c
> @@ -35,6 +35,7 @@
>  #include "h4_recv.h"
>  #include "btuart.h"
>  #include "btbcm.h"
> +#include "btmtkuart.h"
>
>  #define VERSION "1.0"
>
> @@ -396,6 +397,12 @@ static const struct h4_recv_pkt bcm_recv_pkts[] = {
>         { BCM_RECV_NULL,    .recv = hci_recv_diag  },
>  };
>
> +static const struct h4_recv_pkt mtk_recv_pkts[] = {
> +       { H4_RECV_ACL,      .recv = hci_recv_frame },
> +       { H4_RECV_SCO,      .recv = hci_recv_frame },
> +       { H4_RECV_EVENT,    .recv = mtk_btuart_hci_frame },
> +};
> +
>  static const struct btuart_vnd bcm_vnd = {
>         .recv_pkts      = bcm_recv_pkts,
>         .recv_pkts_cnt  = ARRAY_SIZE(bcm_recv_pkts),
> @@ -403,6 +410,16 @@ static const struct btuart_vnd bcm_vnd = {
>         .setup          = bcm_setup,
>  };
>
> +static const struct btuart_vnd mtk_vnd = {
> +       .recv_pkts      = mtk_recv_pkts,
> +       .recv_pkts_cnt  = ARRAY_SIZE(mtk_recv_pkts),
> +       .init           = mtk_btuart_init,
> +       .setup          = mtk_btuart_setup,
> +       .shutdown       = mtk_btuart_shutdown,
> +       .send           = mtk_btuart_send,
> +       .recv           = mtk_btuart_recv,
> +};
> +
>  static const struct h4_recv_pkt default_recv_pkts[] = {
>         { H4_RECV_ACL,      .recv = hci_recv_frame },
>         { H4_RECV_SCO,      .recv = hci_recv_frame },
> @@ -487,6 +504,7 @@ static void btuart_remove(struct serdev_device *serdev)
>  #ifdef CONFIG_OF
>  static const struct of_device_id btuart_of_match_table[] = {
>         { .compatible = "brcm,bcm43438-bt", .data = &bcm_vnd },
> +       { .compatible = "mediatek,mt7622-bluetooth", .data = &mtk_vnd },
>         { }
>  };
>  MODULE_DEVICE_TABLE(of, btuart_of_match_table);
> --
> 2.7.4
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices
  2018-06-27 16:59   ` Andy Shevchenko
@ 2018-06-27 17:04     ` Andy Shevchenko
  2018-06-28  3:06       ` Sean Wang
  2018-06-28  2:54     ` Sean Wang
  1 sibling, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2018-06-27 17:04 UTC (permalink / raw)
  To: sean.wang
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Johan Hedberg,
	devicetree, linux-bluetooth, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support,
	Linux Kernel Mailing List

On Wed, Jun 27, 2018 at 7:59 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Jun 27, 2018 at 8:43 AM,  <sean.wang@mediatek.com> wrote:
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>
>> +config BT_HCIBTUART_MTK
>> +       tristate "MediaTek HCI UART driver"
>> +       depends on BT_HCIBTUART
>
>> +       default y
>
> Perhaps it's an overkill for users which would like to have less
> amount on stuff in kernel.
>
>
>> +#include <asm/unaligned.h>
>> +#include <linux/atomic.h>
>> +#include <linux/clk.h>
>> +#include <linux/firmware.h>
>> +#include <linux/module.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/serdev.h>

Perhaps alphabetically ordered?

+ blank line.

>
>> +#include <net/bluetooth/bluetooth.h>
>> +#include <net/bluetooth/hci_core.h>

>> +       /* Enable the power domain and clock the device requires. */
>> +       pm_runtime_enable(dev);
>> +       err = pm_runtime_get_sync(dev);
>> +       if (err < 0)
>> +               goto err_pm2;

Should be err_pm1 here.
Yes, that's correct.

>> +err_pm1:
>> +       pm_runtime_put_sync(dev);
>> +err_pm2:
>> +       pm_runtime_disable(dev);

>> +#define MTK_WMT_CMD_SIZE       (MTK_WMT_HDR_SIZE + MTK_STP_HDR_SIZE + \
>> +                                MTK_STP_TLR_SIZE + HCI_ACL_HDR_SIZE)

It would look slightly better if you start on new line like
#define FOO \
 (BAR + BAZ)

>> +struct mtk_stp_hdr {
>> +       __u8 prefix;
>> +       __u8 dlen1:4;
>> +       __u8 type:4;

>> +       __u8 dlen2:8;

u8 already 8 bit.

>> +       __u8 cs;
>> +} __packed;

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices
  2018-06-27 16:59   ` Andy Shevchenko
  2018-06-27 17:04     ` Andy Shevchenko
@ 2018-06-28  2:54     ` Sean Wang
  1 sibling, 0 replies; 15+ messages in thread
From: Sean Wang @ 2018-06-28  2:54 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Johan Hedberg,
	devicetree, linux-bluetooth, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support,
	Linux Kernel Mailing List

On Wed, 2018-06-27 at 19:59 +0300, Andy Shevchenko wrote:
> On Wed, Jun 27, 2018 at 8:43 AM,  <sean.wang@mediatek.com> wrote:
> > From: Sean Wang <sean.wang@mediatek.com>
> >
> 
> > +config BT_HCIBTUART_MTK
> > +       tristate "MediaTek HCI UART driver"
> > +       depends on BT_HCIBTUART
> 
> > +       default y
> 
> Perhaps it's an overkill for users which would like to have less
> amount on stuff in kernel.

sure, the default y will be removed

> 
> > +#include <asm/unaligned.h>
> > +#include <linux/atomic.h>
> > +#include <linux/clk.h>
> > +#include <linux/firmware.h>
> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/serdev.h>
> 

[ ... ]

> >  MODULE_DEVICE_TABLE(of, btuart_of_match_table);
> > --
> > 2.7.4
> >
> 
> 
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices
  2018-06-27 17:04     ` Andy Shevchenko
@ 2018-06-28  3:06       ` Sean Wang
  2018-06-28  5:19         ` Johan Hovold
  0 siblings, 1 reply; 15+ messages in thread
From: Sean Wang @ 2018-06-28  3:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Johan Hedberg,
	devicetree, linux-bluetooth, linux-arm Mailing List,
	moderated list:ARM/Mediatek SoC support,
	Linux Kernel Mailing List

On Wed, 2018-06-27 at 20:04 +0300, Andy Shevchenko wrote:
> On Wed, Jun 27, 2018 at 7:59 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Jun 27, 2018 at 8:43 AM,  <sean.wang@mediatek.com> wrote:
> >> From: Sean Wang <sean.wang@mediatek.com>
> >>
> >
> >> +config BT_HCIBTUART_MTK
> >> +       tristate "MediaTek HCI UART driver"
> >> +       depends on BT_HCIBTUART
> >
> >> +       default y
> >
> > Perhaps it's an overkill for users which would like to have less
> > amount on stuff in kernel.
> >

Sure, the default y will be removed

> >
> >> +#include <asm/unaligned.h>
> >> +#include <linux/atomic.h>
> >> +#include <linux/clk.h>
> >> +#include <linux/firmware.h>
> >> +#include <linux/module.h>
> >> +#include <linux/pm_runtime.h>
> >> +#include <linux/serdev.h>
> 
> Perhaps alphabetically ordered?
> 

They seem already in alphabetically ordered

> + blank line.
> 

A blank line will be added here

> >
> >> +#include <net/bluetooth/bluetooth.h>
> >> +#include <net/bluetooth/hci_core.h>
> 
> >> +       /* Enable the power domain and clock the device requires. */
> >> +       pm_runtime_enable(dev);
> >> +       err = pm_runtime_get_sync(dev);
> >> +       if (err < 0)
> >> +               goto err_pm2;
> 
> Should be err_pm1 here.

Label err_pm1 and err_pm2 can be swapped for the readability and this
doesn't have any effect on the logic.

> Yes, that's correct.
> 
> >> +err_pm1:
> >> +       pm_runtime_put_sync(dev);
> >> +err_pm2:
> >> +       pm_runtime_disable(dev);
> 
> >> +#define MTK_WMT_CMD_SIZE       (MTK_WMT_HDR_SIZE + MTK_STP_HDR_SIZE + \
> >> +                                MTK_STP_TLR_SIZE + HCI_ACL_HDR_SIZE)
> 
> It would look slightly better if you start on new line like
> #define FOO \
>  (BAR + BAZ)
> 

Thanks for your pointing out. I also found that the macro is not being
used by anyone, it can be removed freely.

> >> +struct mtk_stp_hdr {
> >> +       __u8 prefix;
> >> +       __u8 dlen1:4;
> >> +       __u8 type:4;
> 
> >> +       __u8 dlen2:8;
> 
> u8 already 8 bit.
> 

the superfluous :8 for dlen2 will be removed

> >> +       __u8 cs;
> >> +} __packed;
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices
  2018-06-28  3:06       ` Sean Wang
@ 2018-06-28  5:19         ` Johan Hovold
  2018-06-29  9:47           ` Sean Wang
  0 siblings, 1 reply; 15+ messages in thread
From: Johan Hovold @ 2018-06-28  5:19 UTC (permalink / raw)
  To: Sean Wang
  Cc: Andy Shevchenko, Mark Rutland, devicetree, Johan Hedberg,
	Marcel Holtmann, Linux Kernel Mailing List, linux-bluetooth,
	Rob Herring, moderated list:ARM/Mediatek SoC support,
	linux-arm Mailing List

On Thu, Jun 28, 2018 at 11:06:13AM +0800, Sean Wang wrote:
> On Wed, 2018-06-27 at 20:04 +0300, Andy Shevchenko wrote:
> > On Wed, Jun 27, 2018 at 7:59 PM, Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Wed, Jun 27, 2018 at 8:43 AM,  <sean.wang@mediatek.com> wrote:
> > >> From: Sean Wang <sean.wang@mediatek.com>

> > >> +#include <net/bluetooth/bluetooth.h>
> > >> +#include <net/bluetooth/hci_core.h>
> > 
> > >> +       /* Enable the power domain and clock the device requires. */
> > >> +       pm_runtime_enable(dev);
> > >> +       err = pm_runtime_get_sync(dev);
> > >> +       if (err < 0)
> > >> +               goto err_pm2;

> > >> +err_pm1:
> > >> +       pm_runtime_put_sync(dev);
> > >> +err_pm2:
> > >> +       pm_runtime_disable(dev);

Please name error labels after what they do, not using numbers (see
CodingStyle). Here you could use err_disable_rpm instead of err_pm2, for
example.

Also, if you really want to undo pm_runtime_get_sync() failing above,
you still need a pm_runtime_put_noidle() to balance the usage count.

> > >> +struct mtk_stp_hdr {
> > >> +       __u8 prefix;
> > >> +       __u8 dlen1:4;
> > >> +       __u8 type:4;
> > 
> > >> +       __u8 dlen2:8;
> > >> +       __u8 cs;
> > >> +} __packed;

Perhaps too much context have been lost here, but unless you're sharing
this struct with user space, you should be using u8 (without __) above
(and elsewhere).

Johan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices
  2018-06-28  5:19         ` Johan Hovold
@ 2018-06-29  9:47           ` Sean Wang
  0 siblings, 0 replies; 15+ messages in thread
From: Sean Wang @ 2018-06-29  9:47 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Andy Shevchenko, Mark Rutland, devicetree, Johan Hedberg,
	Marcel Holtmann, Linux Kernel Mailing List, linux-bluetooth,
	Rob Herring, moderated list:ARM/Mediatek SoC support,
	linux-arm Mailing List

On Thu, 2018-06-28 at 07:19 +0200, Johan Hovold wrote:
> On Thu, Jun 28, 2018 at 11:06:13AM +0800, Sean Wang wrote:
> > On Wed, 2018-06-27 at 20:04 +0300, Andy Shevchenko wrote:
> > > On Wed, Jun 27, 2018 at 7:59 PM, Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Wed, Jun 27, 2018 at 8:43 AM,  <sean.wang@mediatek.com> wrote:
> > > >> From: Sean Wang <sean.wang@mediatek.com>
> 
> > > >> +#include <net/bluetooth/bluetooth.h>
> > > >> +#include <net/bluetooth/hci_core.h>
> > > 
> > > >> +       /* Enable the power domain and clock the device requires. */
> > > >> +       pm_runtime_enable(dev);
> > > >> +       err = pm_runtime_get_sync(dev);
> > > >> +       if (err < 0)
> > > >> +               goto err_pm2;
> 
> > > >> +err_pm1:
> > > >> +       pm_runtime_put_sync(dev);
> > > >> +err_pm2:
> > > >> +       pm_runtime_disable(dev);
> 
> Please name error labels after what they do, not using numbers (see
> CodingStyle). Here you could use err_disable_rpm instead of err_pm2, for
> example.
> 
> Also, if you really want to undo pm_runtime_get_sync() failing above,
> you still need a pm_runtime_put_noidle() to balance the usage count.
> 

I'll give a reasonable naming for these labels and add a
pm_runtime_put_noidle() in the path undoing failing
pm_runtime_get_sync().

> > > >> +struct mtk_stp_hdr {
> > > >> +       __u8 prefix;
> > > >> +       __u8 dlen1:4;
> > > >> +       __u8 type:4;
> > > 
> > > >> +       __u8 dlen2:8;
> > > >> +       __u8 cs;
> > > >> +} __packed;
> 
> Perhaps too much context have been lost here, but unless you're sharing
> this struct with user space, you should be using u8 (without __) above
> (and elsewhere).
> 

These struct don't be shard with user space so I will turn __u8 into u8.

Thanks so much for all suggestions. 

> Johan



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-06-29  9:47 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-27  5:43 [PATCH v4 0/7] add support for Bluetooth on MT7622 SoC sean.wang
2018-06-27  5:43 ` [PATCH v4 1/7] dt-bindings: net: bluetooth: Add mediatek-bluetooth sean.wang
2018-06-27  5:43 ` [PATCH v4 2/7] serdev: add dev_pm_domain_attach|detach() sean.wang
2018-06-27  8:00   ` Ulf Hansson
2018-06-27  5:43 ` [PATCH v4 3/7] Bluetooth: Add new serdev based driver for UART attached controllers sean.wang
2018-06-27  5:43 ` [PATCH v4 4/7] Bluetooth: Add new quirk for non-persistent setup settings sean.wang
2018-06-27  5:43 ` [PATCH v4 5/7] Bluetooth: Extend btuart driver for join more vendor devices sean.wang
2018-06-27  5:43 ` [PATCH v4 6/7] Bluetooth: mediatek: Add protocol support for MediaTek serial devices sean.wang
2018-06-27 16:59   ` Andy Shevchenko
2018-06-27 17:04     ` Andy Shevchenko
2018-06-28  3:06       ` Sean Wang
2018-06-28  5:19         ` Johan Hovold
2018-06-29  9:47           ` Sean Wang
2018-06-28  2:54     ` Sean Wang
2018-06-27  5:43 ` [PATCH v4 7/7] MAINTAINERS: add an entry for MediaTek Bluetooth driver sean.wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).