On Wed, 2018-06-27 at 20:04 +0300, Andy Shevchenko wrote:

On Wed, Jun 27, 2018 at 7:59 PM, Andy Shevchenko
[off-list ref] wrote:

quoted

On Wed, Jun 27, 2018 at 8:43 AM,  [off-list ref] wrote:

quoted

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

quoted

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

quoted

+       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

quoted

quoted

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

quoted

quoted

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

quoted

quoted

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

quoted

quoted

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

quoted

quoted

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

quoted

quoted

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

quoted

quoted

+       __u8 dlen2:8;

u8 already 8 bit.

the superfluous :8 for dlen2 will be removed

quoted

quoted

+       __u8 cs;
+} __packed;