add support for Bluetooth on MT7622 SoC

From: Sean Wang <[email protected]>

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 <[email protected]>

Cc Rob Herring <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Jiri Slaby <[email protected]>

Cc linux-serial at vger.kernel.org

---
 drivers/tty/serdev/core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

drivers/tty/serdev/core.c +14 -2

index df93b72..8096138 100644
@@ -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

drivers/bluetooth/btmtkuart.c new file +345 -0

+#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 <[email protected]>");
+MODULE_DESCRIPTION("Bluetooth Support for MediaTek Serial Devices");
+MODULE_LICENSE("GPL v2");

drivers/bluetooth/btmtkuart.h new file +119 -0

new file mode 100644
index 0000000..e76ab23e
@@ -0,0 +1,119 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ *
+ * Bluetooth support for MediaTek serial devices
+ *
+ * Author: Sean Wang <[email protected]>
+ *
+ */
+
+#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

drivers/bluetooth/btuart.c +18 -1

index ab7f836..169bf1a 100644
@@ -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

drivers/bluetooth/Kconfig config BT_HCIBTUART · line 91

Quoted 8 lines · On Wed, Jun 27, 2018 at 8:43 AM, <[email protected]> wrote:

From: Sean Wang <[email protected]>

drivers/bluetooth/Kconfig +4 -0

+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.

drivers/bluetooth/btmtkuart.c line 17

Quoted 7 lines

drivers/bluetooth/btmtkuart.c new file +7 -0

+#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>

drivers/bluetooth/btmtkuart.c line 298

Quoted 9 lines

drivers/bluetooth/btmtkuart.c new file +7 -0

+#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;

drivers/bluetooth/btmtkuart.c

Quoted 7 lines

drivers/bluetooth/btmtkuart.c new file +6 -0

+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)

drivers/bluetooth/btmtkuart.h line 29

Quoted 6 lines

drivers/bluetooth/btmtkuart.h new file +5 -0

+struct mtk_stp_hdr {
+       __u8 prefix;
+       __u8 dlen1:4;
+       __u8 type:4;
 
+       __u8 dlen2:8;

Quoted 2 lines

(quoted diff) +2 -0

+       __u8 cs;
+} __packed;

Quoted 8 lines · Andy Shevchenko

Quoted 6 lines · On Wed, Jun 27, 2018 at 8:43 AM, <[email protected]> wrote:

From: Sean Wang <[email protected]>

drivers/bluetooth/Kconfig +3 -0

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

Quoted 5 lines

Quoted 1 line

+ default y

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

Quoted 8 lines

drivers/bluetooth/btmtkuart.c line 17

Quoted 8 lines

drivers/bluetooth/btmtkuart.c new file +7 -0

+#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.

Quoted 9 lines

drivers/bluetooth/btmtkuart.c line 298

Quoted 9 lines

drivers/bluetooth/btmtkuart.c new file +7 -0

+#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.

Quoted 4 lines

Quoted 4 lines

drivers/bluetooth/btmtkuart.c new file +4 -0

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

Quoted 2 lines

drivers/bluetooth/btmtkuart.h line 17

Quoted 2 lines

drivers/bluetooth/btmtkuart.h new file +2 -0

+#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)

Quoted 6 lines

drivers/bluetooth/btmtkuart.h line 29

Quoted 6 lines

drivers/bluetooth/btmtkuart.h new file +5 -0

+struct mtk_stp_hdr {
+       __u8 prefix;
+       __u8 dlen1:4;
+       __u8 type:4;
 
+       __u8 dlen2:8;

u8 already 8 bit.

Quoted 2 lines

Quoted 2 lines

(quoted diff) +2 -0

+       __u8 cs;
+} __packed;

Quoted 13 lines · Andy Shevchenko

Quoted 11 lines · Andy Shevchenko

Quoted 10 lines · On Wed, Jun 27, 2018 at 8:43 AM, <[email protected]> wrote:

From: Sean Wang <[email protected]>

drivers/bluetooth/btmtkuart.c new file +7 -0

+#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;

Quoted 4 lines

Quoted 4 lines

drivers/bluetooth/btmtkuart.c line 328

Quoted 4 lines

drivers/bluetooth/btmtkuart.c new file +4 -0

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

Quoted 8 lines

Quoted 8 lines

drivers/bluetooth/btmtkuart.h line 31

Quoted 8 lines

drivers/bluetooth/btmtkuart.h new file +7 -0

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

Quoted 15 lines · Sean Wang

Quoted 13 lines · Andy Shevchenko

Quoted 11 lines · Andy Shevchenko

Quoted 10 lines · On Wed, Jun 27, 2018 at 8:43 AM, <[email protected]> wrote:

From: Sean Wang <[email protected]>

drivers/bluetooth/btmtkuart.c new file +7 -0

+#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;

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

drivers/bluetooth/btmtkuart.c line 328

Quoted 4 lines

drivers/bluetooth/btmtkuart.c new file +4 -0

+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.

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

drivers/bluetooth/btmtkuart.h new file +4 -0

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

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

drivers/bluetooth/btmtkuart.h line 31

Quoted 3 lines

drivers/bluetooth/btmtkuart.h new file +3 -0

+       __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

Quoted 2 lines · On Wed, Jun 27, 2018 at 8:43 AM, <[email protected]> wrote:

From: Sean Wang <[email protected]>

drivers/bluetooth/Kconfig config BT_HCIBTUART · line 91

Quoted 5 lines

drivers/bluetooth/Kconfig +4 -0

+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.

Quoted 8 lines

drivers/bluetooth/btmtkuart.c new file +7 -0

+#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>

Quoted 4 lines

MODULE_DEVICE_TABLE(of, btuart_of_match_table);
--
2.7.4