Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-13 Thread Marcel Holtmann
Hi Sean,

>>> This adds the support of enabling MT7668U Bluetooth function running
>>> on the top of btusb driver. The patch also adds a newly created file
>>> mtkbt.c able to be reused independently from the transport type such
>>> as UART, USB and SDIO.
>> 
>> Include /sys/kernel/debug/usb/device portion in the commit message.
>> 
> 
> okay, I will have an inclusion for this.
> 
>>> 
>>> Signed-off-by: Sean Wang 
>>> ---
>>> drivers/bluetooth/Kconfig  |  16 +++
>>> drivers/bluetooth/Makefile |   1 +
>>> drivers/bluetooth/btmtk.c  | 308 
>>> +
>>> drivers/bluetooth/btmtk.h  |  99 +++
>>> drivers/bluetooth/btusb.c  | 174 +
>>> 5 files changed, 598 insertions(+)
>>> create mode 100644 drivers/bluetooth/btmtk.c
>>> create mode 100644 drivers/bluetooth/btmtk.h
>>> 
>>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>>> index 07e55cd..2788498 100644
>>> --- a/drivers/bluetooth/Kconfig
>>> +++ b/drivers/bluetooth/Kconfig
>>> @@ -11,6 +11,10 @@ config BT_BCM
>>> tristate
>>> select FW_LOADER
>>> 
>>> +config BT_MTK
>>> +   tristate
>>> +   select FW_LOADER
>>> +
>>> config BT_RTL
>>> tristate
>>> select FW_LOADER
>>> @@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
>>> 
>>>   Say Y here to compile support for Broadcom protocol.
>>> 
>>> +config BT_HCIBTUSB_MTK
>>> +   bool "MediaTek protocol support"
>>> +   depends on BT_HCIBTUSB
>>> +   select BT_MTK
>>> +   default y
>> 
>> This one has to be n since MediaTek hardware was never supported in the 
>> first place. The existing y here are only for legacy hardware that was 
>> somehow supported in a generic fashion.
>> 
> 
> okay, I will turn it into n
> 
>>> +   help
>>> + The MediaTek protocol support enables firmware download
>>> + support and chip initialization for MediaTek Bluetooth
>>> + USB controllers.
>>> +
>>> + Say Y here to compile support for MediaTek protocol.
>>> +
>>> config BT_HCIBTUSB_RTL
>>> bool "Realtek protocol support"
>>> depends on BT_HCIBTUSB
>>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
>>> index 4e4e44d..bc23724 100644
>>> --- a/drivers/bluetooth/Makefile
>>> +++ b/drivers/bluetooth/Makefile
>>> @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)+= btmrvl_sdio.o
>>> obj-$(CONFIG_BT_WILINK) += btwilink.o
>>> obj-$(CONFIG_BT_QCOMSMD)+= btqcomsmd.o
>>> obj-$(CONFIG_BT_BCM)+= btbcm.o
>>> +obj-$(CONFIG_BT_MTK)   += btmtk.o
>>> obj-$(CONFIG_BT_RTL)+= btrtl.o
>>> obj-$(CONFIG_BT_QCA)+= btqca.o
>>> 
>>> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
>>> new file mode 100644
>>> index 000..9e39a0a
>>> --- /dev/null
>>> +++ b/drivers/bluetooth/btmtk.c
>>> @@ -0,0 +1,308 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +// Copyright (c) 2018 MediaTek Inc.
>>> +
>>> +/*
>>> + * Common operations for MediaTek Bluetooth devices
>>> + * with the UART, USB and SDIO transport
>>> + *
>>> + * Author: Sean Wang 
>>> + *
>>> + */
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include 
>>> +#include 
>>> +
>>> +#include "btmtk.h"
>>> +
>>> +#define VERSION "0.1"
>>> +
>>> +int
>>> +btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params 
>>> *params)
>>> +{
>>> +   struct btmtk_hci_wmt_evt_funcc *evt_funcc;
>>> +   u32 hlen, status = BTMTK_WMT_INVALID;
>>> +   struct btmtk_wmt_hdr *hdr, *ehdr;
>>> +   struct btmtk_hci_wmt_cmd wc;
>>> +   struct sk_buff *skb;
>>> +   int err = 0;
>>> +
>>> +   hlen = sizeof(*hdr) + params->dlen;
>>> +   if (hlen > 255)
>>> +   return -EINVAL;
>>> +
>>> +   hdr = (struct btmtk_wmt_hdr *)
>>> +   hdr->dir = 1;
>>> +   hdr->op = params->op;
>>> +   hdr->dlen = cpu_to_le16(params->dlen + 1);
>>> +   hdr->flag = params->flag;
>>> +   memcpy(wc.data, params->data, params->dlen);
>>> +
>>> +   /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
>>> +* instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
>>> +*/
>>> +   atomic_inc(>cmd_cnt);
>>> +
>>> +   skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
>>> +HCI_INIT_TIMEOUT);
>> 
>> So first of all HCI_VENDOR_PKT is for H:4 pkt_type 0xff and not a vendor 
>> event. Use HCI_EV_VENDOR instead. Second __hci_cmd_sync_ev is not the right 
>> way doing this. Use __hci_cmd_send and handle the vendor event. See how we 
>> do it for the Intel firmware loading.
>> 
> 
> I didn't use __hci_cmd_send her before because I didn't know much about how 
> to do with skb the returned event from __hci_cmd_send in order to parse the 
> event content.

The __hci_cmd_send does not return anything. It is the same usage as within 
btmtkuart.c since it just queues the HCI command into the core for processing. 
You can see its usage also in btqca.c for patch downloading.

> I can have a confirmation first about whether 

Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-13 Thread Marcel Holtmann
Hi Sean,

>>> This adds the support of enabling MT7668U Bluetooth function running
>>> on the top of btusb driver. The patch also adds a newly created file
>>> mtkbt.c able to be reused independently from the transport type such
>>> as UART, USB and SDIO.
>> 
>> Include /sys/kernel/debug/usb/device portion in the commit message.
>> 
> 
> okay, I will have an inclusion for this.
> 
>>> 
>>> Signed-off-by: Sean Wang 
>>> ---
>>> drivers/bluetooth/Kconfig  |  16 +++
>>> drivers/bluetooth/Makefile |   1 +
>>> drivers/bluetooth/btmtk.c  | 308 
>>> +
>>> drivers/bluetooth/btmtk.h  |  99 +++
>>> drivers/bluetooth/btusb.c  | 174 +
>>> 5 files changed, 598 insertions(+)
>>> create mode 100644 drivers/bluetooth/btmtk.c
>>> create mode 100644 drivers/bluetooth/btmtk.h
>>> 
>>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>>> index 07e55cd..2788498 100644
>>> --- a/drivers/bluetooth/Kconfig
>>> +++ b/drivers/bluetooth/Kconfig
>>> @@ -11,6 +11,10 @@ config BT_BCM
>>> tristate
>>> select FW_LOADER
>>> 
>>> +config BT_MTK
>>> +   tristate
>>> +   select FW_LOADER
>>> +
>>> config BT_RTL
>>> tristate
>>> select FW_LOADER
>>> @@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
>>> 
>>>   Say Y here to compile support for Broadcom protocol.
>>> 
>>> +config BT_HCIBTUSB_MTK
>>> +   bool "MediaTek protocol support"
>>> +   depends on BT_HCIBTUSB
>>> +   select BT_MTK
>>> +   default y
>> 
>> This one has to be n since MediaTek hardware was never supported in the 
>> first place. The existing y here are only for legacy hardware that was 
>> somehow supported in a generic fashion.
>> 
> 
> okay, I will turn it into n
> 
>>> +   help
>>> + The MediaTek protocol support enables firmware download
>>> + support and chip initialization for MediaTek Bluetooth
>>> + USB controllers.
>>> +
>>> + Say Y here to compile support for MediaTek protocol.
>>> +
>>> config BT_HCIBTUSB_RTL
>>> bool "Realtek protocol support"
>>> depends on BT_HCIBTUSB
>>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
>>> index 4e4e44d..bc23724 100644
>>> --- a/drivers/bluetooth/Makefile
>>> +++ b/drivers/bluetooth/Makefile
>>> @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)+= btmrvl_sdio.o
>>> obj-$(CONFIG_BT_WILINK) += btwilink.o
>>> obj-$(CONFIG_BT_QCOMSMD)+= btqcomsmd.o
>>> obj-$(CONFIG_BT_BCM)+= btbcm.o
>>> +obj-$(CONFIG_BT_MTK)   += btmtk.o
>>> obj-$(CONFIG_BT_RTL)+= btrtl.o
>>> obj-$(CONFIG_BT_QCA)+= btqca.o
>>> 
>>> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
>>> new file mode 100644
>>> index 000..9e39a0a
>>> --- /dev/null
>>> +++ b/drivers/bluetooth/btmtk.c
>>> @@ -0,0 +1,308 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +// Copyright (c) 2018 MediaTek Inc.
>>> +
>>> +/*
>>> + * Common operations for MediaTek Bluetooth devices
>>> + * with the UART, USB and SDIO transport
>>> + *
>>> + * Author: Sean Wang 
>>> + *
>>> + */
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include 
>>> +#include 
>>> +
>>> +#include "btmtk.h"
>>> +
>>> +#define VERSION "0.1"
>>> +
>>> +int
>>> +btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params 
>>> *params)
>>> +{
>>> +   struct btmtk_hci_wmt_evt_funcc *evt_funcc;
>>> +   u32 hlen, status = BTMTK_WMT_INVALID;
>>> +   struct btmtk_wmt_hdr *hdr, *ehdr;
>>> +   struct btmtk_hci_wmt_cmd wc;
>>> +   struct sk_buff *skb;
>>> +   int err = 0;
>>> +
>>> +   hlen = sizeof(*hdr) + params->dlen;
>>> +   if (hlen > 255)
>>> +   return -EINVAL;
>>> +
>>> +   hdr = (struct btmtk_wmt_hdr *)
>>> +   hdr->dir = 1;
>>> +   hdr->op = params->op;
>>> +   hdr->dlen = cpu_to_le16(params->dlen + 1);
>>> +   hdr->flag = params->flag;
>>> +   memcpy(wc.data, params->data, params->dlen);
>>> +
>>> +   /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
>>> +* instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
>>> +*/
>>> +   atomic_inc(>cmd_cnt);
>>> +
>>> +   skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
>>> +HCI_INIT_TIMEOUT);
>> 
>> So first of all HCI_VENDOR_PKT is for H:4 pkt_type 0xff and not a vendor 
>> event. Use HCI_EV_VENDOR instead. Second __hci_cmd_sync_ev is not the right 
>> way doing this. Use __hci_cmd_send and handle the vendor event. See how we 
>> do it for the Intel firmware loading.
>> 
> 
> I didn't use __hci_cmd_send her before because I didn't know much about how 
> to do with skb the returned event from __hci_cmd_send in order to parse the 
> event content.

The __hci_cmd_send does not return anything. It is the same usage as within 
btmtkuart.c since it just queues the HCI command into the core for processing. 
You can see its usage also in btqca.c for patch downloading.

> I can have a confirmation first about whether 

Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-13 Thread Sean Wang
On Mon, 2018-08-13 at 18:20 +0800, Sean Wang wrote:
> On Mon, 2018-08-13 at 10:00 +0200, Marcel Holtmann wrote:
> > Hi Sean,
> > 
> > > This adds the support of enabling MT7668U Bluetooth function running
> > > on the top of btusb driver. The patch also adds a newly created file
> > > mtkbt.c able to be reused independently from the transport type such
> > > as UART, USB and SDIO.
> > 
> > Include /sys/kernel/debug/usb/device portion in the commit message.
> > 
> 
> okay, I will have an inclusion for this.
> 
> > > 
> > > Signed-off-by: Sean Wang 
> > > ---
> > > drivers/bluetooth/Kconfig  |  16 +++
> > > drivers/bluetooth/Makefile |   1 +
> > > drivers/bluetooth/btmtk.c  | 308 
> > > +
> > > drivers/bluetooth/btmtk.h  |  99 +++
> > > drivers/bluetooth/btusb.c  | 174 +
> > > 5 files changed, 598 insertions(+)
> > > create mode 100644 drivers/bluetooth/btmtk.c
> > > create mode 100644 drivers/bluetooth/btmtk.h
> > > 
> > > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > > index 07e55cd..2788498 100644
> > > --- a/drivers/bluetooth/Kconfig
> > > +++ b/drivers/bluetooth/Kconfig
> > > @@ -11,6 +11,10 @@ config BT_BCM
> > >   tristate
> > >   select FW_LOADER
> > > 
> > > +config BT_MTK
> > > + tristate
> > > + select FW_LOADER
> > > +
> > > config BT_RTL
> > >   tristate
> > >   select FW_LOADER
> > > @@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
> > > 
> > > Say Y here to compile support for Broadcom protocol.
> > > 
> > > +config BT_HCIBTUSB_MTK
> > > + bool "MediaTek protocol support"
> > > + depends on BT_HCIBTUSB
> > > + select BT_MTK
> > > + default y
> > 
> > This one has to be n since MediaTek hardware was never supported in the 
> > first place. The existing y here are only for legacy hardware that was 
> > somehow supported in a generic fashion.
> > 
> 
> okay, I will turn it into n
> 
> > > + help
> > > +   The MediaTek protocol support enables firmware download
> > > +   support and chip initialization for MediaTek Bluetooth
> > > +   USB controllers.
> > > +
> > > +   Say Y here to compile support for MediaTek protocol.
> > > +
> > > config BT_HCIBTUSB_RTL
> > >   bool "Realtek protocol support"
> > >   depends on BT_HCIBTUSB
> > > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> > > index 4e4e44d..bc23724 100644
> > > --- a/drivers/bluetooth/Makefile
> > > +++ b/drivers/bluetooth/Makefile
> > > @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)  += btmrvl_sdio.o
> > > obj-$(CONFIG_BT_WILINK)   += btwilink.o
> > > obj-$(CONFIG_BT_QCOMSMD)  += btqcomsmd.o
> > > obj-$(CONFIG_BT_BCM)  += btbcm.o
> > > +obj-$(CONFIG_BT_MTK) += btmtk.o
> > > obj-$(CONFIG_BT_RTL)  += btrtl.o
> > > obj-$(CONFIG_BT_QCA)  += btqca.o
> > > 
> > > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > > new file mode 100644
> > > index 000..9e39a0a
> > > --- /dev/null
> > > +++ b/drivers/bluetooth/btmtk.c
> > > @@ -0,0 +1,308 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// Copyright (c) 2018 MediaTek Inc.
> > > +
> > > +/*
> > > + * Common operations for MediaTek Bluetooth devices
> > > + * with the UART, USB and SDIO transport
> > > + *
> > > + * Author: Sean Wang 
> > > + *
> > > + */
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +#include "btmtk.h"
> > > +
> > > +#define VERSION "0.1"
> > > +
> > > +int
> > > +btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params 
> > > *params)
> > > +{
> > > + struct btmtk_hci_wmt_evt_funcc *evt_funcc;
> > > + u32 hlen, status = BTMTK_WMT_INVALID;
> > > + struct btmtk_wmt_hdr *hdr, *ehdr;
> > > + struct btmtk_hci_wmt_cmd wc;
> > > + struct sk_buff *skb;
> > > + int err = 0;
> > > +
> > > + hlen = sizeof(*hdr) + params->dlen;
> > > + if (hlen > 255)
> > > + return -EINVAL;
> > > +
> > > + hdr = (struct btmtk_wmt_hdr *)
> > > + hdr->dir = 1;
> > > + hdr->op = params->op;
> > > + hdr->dlen = cpu_to_le16(params->dlen + 1);
> > > + hdr->flag = params->flag;
> > > + memcpy(wc.data, params->data, params->dlen);
> > > +
> > > + /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
> > > +  * instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
> > > +  */
> > > + atomic_inc(>cmd_cnt);
> > > +
> > > + skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
> > > +  HCI_INIT_TIMEOUT);
> > 
> > So first of all HCI_VENDOR_PKT is for H:4 pkt_type 0xff and not a vendor 
> > event. Use HCI_EV_VENDOR instead. Second __hci_cmd_sync_ev is not the right 
> > way doing this. Use __hci_cmd_send and handle the vendor event. See how we 
> > do it for the Intel firmware loading.
> > 
> 
> I didn't use __hci_cmd_send her before because I didn't know much about how 
> to do with skb the returned event from __hci_cmd_send in order to parse the 
> 

Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-13 Thread Sean Wang
On Mon, 2018-08-13 at 18:20 +0800, Sean Wang wrote:
> On Mon, 2018-08-13 at 10:00 +0200, Marcel Holtmann wrote:
> > Hi Sean,
> > 
> > > This adds the support of enabling MT7668U Bluetooth function running
> > > on the top of btusb driver. The patch also adds a newly created file
> > > mtkbt.c able to be reused independently from the transport type such
> > > as UART, USB and SDIO.
> > 
> > Include /sys/kernel/debug/usb/device portion in the commit message.
> > 
> 
> okay, I will have an inclusion for this.
> 
> > > 
> > > Signed-off-by: Sean Wang 
> > > ---
> > > drivers/bluetooth/Kconfig  |  16 +++
> > > drivers/bluetooth/Makefile |   1 +
> > > drivers/bluetooth/btmtk.c  | 308 
> > > +
> > > drivers/bluetooth/btmtk.h  |  99 +++
> > > drivers/bluetooth/btusb.c  | 174 +
> > > 5 files changed, 598 insertions(+)
> > > create mode 100644 drivers/bluetooth/btmtk.c
> > > create mode 100644 drivers/bluetooth/btmtk.h
> > > 
> > > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > > index 07e55cd..2788498 100644
> > > --- a/drivers/bluetooth/Kconfig
> > > +++ b/drivers/bluetooth/Kconfig
> > > @@ -11,6 +11,10 @@ config BT_BCM
> > >   tristate
> > >   select FW_LOADER
> > > 
> > > +config BT_MTK
> > > + tristate
> > > + select FW_LOADER
> > > +
> > > config BT_RTL
> > >   tristate
> > >   select FW_LOADER
> > > @@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
> > > 
> > > Say Y here to compile support for Broadcom protocol.
> > > 
> > > +config BT_HCIBTUSB_MTK
> > > + bool "MediaTek protocol support"
> > > + depends on BT_HCIBTUSB
> > > + select BT_MTK
> > > + default y
> > 
> > This one has to be n since MediaTek hardware was never supported in the 
> > first place. The existing y here are only for legacy hardware that was 
> > somehow supported in a generic fashion.
> > 
> 
> okay, I will turn it into n
> 
> > > + help
> > > +   The MediaTek protocol support enables firmware download
> > > +   support and chip initialization for MediaTek Bluetooth
> > > +   USB controllers.
> > > +
> > > +   Say Y here to compile support for MediaTek protocol.
> > > +
> > > config BT_HCIBTUSB_RTL
> > >   bool "Realtek protocol support"
> > >   depends on BT_HCIBTUSB
> > > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> > > index 4e4e44d..bc23724 100644
> > > --- a/drivers/bluetooth/Makefile
> > > +++ b/drivers/bluetooth/Makefile
> > > @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)  += btmrvl_sdio.o
> > > obj-$(CONFIG_BT_WILINK)   += btwilink.o
> > > obj-$(CONFIG_BT_QCOMSMD)  += btqcomsmd.o
> > > obj-$(CONFIG_BT_BCM)  += btbcm.o
> > > +obj-$(CONFIG_BT_MTK) += btmtk.o
> > > obj-$(CONFIG_BT_RTL)  += btrtl.o
> > > obj-$(CONFIG_BT_QCA)  += btqca.o
> > > 
> > > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > > new file mode 100644
> > > index 000..9e39a0a
> > > --- /dev/null
> > > +++ b/drivers/bluetooth/btmtk.c
> > > @@ -0,0 +1,308 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// Copyright (c) 2018 MediaTek Inc.
> > > +
> > > +/*
> > > + * Common operations for MediaTek Bluetooth devices
> > > + * with the UART, USB and SDIO transport
> > > + *
> > > + * Author: Sean Wang 
> > > + *
> > > + */
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +#include "btmtk.h"
> > > +
> > > +#define VERSION "0.1"
> > > +
> > > +int
> > > +btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params 
> > > *params)
> > > +{
> > > + struct btmtk_hci_wmt_evt_funcc *evt_funcc;
> > > + u32 hlen, status = BTMTK_WMT_INVALID;
> > > + struct btmtk_wmt_hdr *hdr, *ehdr;
> > > + struct btmtk_hci_wmt_cmd wc;
> > > + struct sk_buff *skb;
> > > + int err = 0;
> > > +
> > > + hlen = sizeof(*hdr) + params->dlen;
> > > + if (hlen > 255)
> > > + return -EINVAL;
> > > +
> > > + hdr = (struct btmtk_wmt_hdr *)
> > > + hdr->dir = 1;
> > > + hdr->op = params->op;
> > > + hdr->dlen = cpu_to_le16(params->dlen + 1);
> > > + hdr->flag = params->flag;
> > > + memcpy(wc.data, params->data, params->dlen);
> > > +
> > > + /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
> > > +  * instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
> > > +  */
> > > + atomic_inc(>cmd_cnt);
> > > +
> > > + skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
> > > +  HCI_INIT_TIMEOUT);
> > 
> > So first of all HCI_VENDOR_PKT is for H:4 pkt_type 0xff and not a vendor 
> > event. Use HCI_EV_VENDOR instead. Second __hci_cmd_sync_ev is not the right 
> > way doing this. Use __hci_cmd_send and handle the vendor event. See how we 
> > do it for the Intel firmware loading.
> > 
> 
> I didn't use __hci_cmd_send her before because I didn't know much about how 
> to do with skb the returned event from __hci_cmd_send in order to parse the 
> 

Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-13 Thread Sean Wang
On Mon, 2018-08-13 at 10:00 +0200, Marcel Holtmann wrote:
> Hi Sean,
> 
> > This adds the support of enabling MT7668U Bluetooth function running
> > on the top of btusb driver. The patch also adds a newly created file
> > mtkbt.c able to be reused independently from the transport type such
> > as UART, USB and SDIO.
> 
> Include /sys/kernel/debug/usb/device portion in the commit message.
> 

okay, I will have an inclusion for this.

> > 
> > Signed-off-by: Sean Wang 
> > ---
> > drivers/bluetooth/Kconfig  |  16 +++
> > drivers/bluetooth/Makefile |   1 +
> > drivers/bluetooth/btmtk.c  | 308 
> > +
> > drivers/bluetooth/btmtk.h  |  99 +++
> > drivers/bluetooth/btusb.c  | 174 +
> > 5 files changed, 598 insertions(+)
> > create mode 100644 drivers/bluetooth/btmtk.c
> > create mode 100644 drivers/bluetooth/btmtk.h
> > 
> > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > index 07e55cd..2788498 100644
> > --- a/drivers/bluetooth/Kconfig
> > +++ b/drivers/bluetooth/Kconfig
> > @@ -11,6 +11,10 @@ config BT_BCM
> > tristate
> > select FW_LOADER
> > 
> > +config BT_MTK
> > +   tristate
> > +   select FW_LOADER
> > +
> > config BT_RTL
> > tristate
> > select FW_LOADER
> > @@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
> > 
> >   Say Y here to compile support for Broadcom protocol.
> > 
> > +config BT_HCIBTUSB_MTK
> > +   bool "MediaTek protocol support"
> > +   depends on BT_HCIBTUSB
> > +   select BT_MTK
> > +   default y
> 
> This one has to be n since MediaTek hardware was never supported in the first 
> place. The existing y here are only for legacy hardware that was somehow 
> supported in a generic fashion.
> 

okay, I will turn it into n

> > +   help
> > + The MediaTek protocol support enables firmware download
> > + support and chip initialization for MediaTek Bluetooth
> > + USB controllers.
> > +
> > + Say Y here to compile support for MediaTek protocol.
> > +
> > config BT_HCIBTUSB_RTL
> > bool "Realtek protocol support"
> > depends on BT_HCIBTUSB
> > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> > index 4e4e44d..bc23724 100644
> > --- a/drivers/bluetooth/Makefile
> > +++ b/drivers/bluetooth/Makefile
> > @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)+= btmrvl_sdio.o
> > obj-$(CONFIG_BT_WILINK) += btwilink.o
> > obj-$(CONFIG_BT_QCOMSMD)+= btqcomsmd.o
> > obj-$(CONFIG_BT_BCM)+= btbcm.o
> > +obj-$(CONFIG_BT_MTK)   += btmtk.o
> > obj-$(CONFIG_BT_RTL)+= btrtl.o
> > obj-$(CONFIG_BT_QCA)+= btqca.o
> > 
> > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > new file mode 100644
> > index 000..9e39a0a
> > --- /dev/null
> > +++ b/drivers/bluetooth/btmtk.c
> > @@ -0,0 +1,308 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2018 MediaTek Inc.
> > +
> > +/*
> > + * Common operations for MediaTek Bluetooth devices
> > + * with the UART, USB and SDIO transport
> > + *
> > + * Author: Sean Wang 
> > + *
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +#include "btmtk.h"
> > +
> > +#define VERSION "0.1"
> > +
> > +int
> > +btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params 
> > *params)
> > +{
> > +   struct btmtk_hci_wmt_evt_funcc *evt_funcc;
> > +   u32 hlen, status = BTMTK_WMT_INVALID;
> > +   struct btmtk_wmt_hdr *hdr, *ehdr;
> > +   struct btmtk_hci_wmt_cmd wc;
> > +   struct sk_buff *skb;
> > +   int err = 0;
> > +
> > +   hlen = sizeof(*hdr) + params->dlen;
> > +   if (hlen > 255)
> > +   return -EINVAL;
> > +
> > +   hdr = (struct btmtk_wmt_hdr *)
> > +   hdr->dir = 1;
> > +   hdr->op = params->op;
> > +   hdr->dlen = cpu_to_le16(params->dlen + 1);
> > +   hdr->flag = params->flag;
> > +   memcpy(wc.data, params->data, params->dlen);
> > +
> > +   /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
> > +* instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
> > +*/
> > +   atomic_inc(>cmd_cnt);
> > +
> > +   skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
> > +HCI_INIT_TIMEOUT);
> 
> So first of all HCI_VENDOR_PKT is for H:4 pkt_type 0xff and not a vendor 
> event. Use HCI_EV_VENDOR instead. Second __hci_cmd_sync_ev is not the right 
> way doing this. Use __hci_cmd_send and handle the vendor event. See how we do 
> it for the Intel firmware loading.
> 

I didn't use __hci_cmd_send her before because I didn't know much about how to 
do with skb the returned event from __hci_cmd_send in order to parse the event 
content.

I can have a confirmation first about whether the Intel firmware download have 
the similar logic wants to handle.

> > +   if (IS_ERR(skb)) {
> > +   err = PTR_ERR(skb);
> > +
> > +   bt_dev_err(hdev, "Failed to send wmt 

Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-13 Thread Sean Wang
On Mon, 2018-08-13 at 10:00 +0200, Marcel Holtmann wrote:
> Hi Sean,
> 
> > This adds the support of enabling MT7668U Bluetooth function running
> > on the top of btusb driver. The patch also adds a newly created file
> > mtkbt.c able to be reused independently from the transport type such
> > as UART, USB and SDIO.
> 
> Include /sys/kernel/debug/usb/device portion in the commit message.
> 

okay, I will have an inclusion for this.

> > 
> > Signed-off-by: Sean Wang 
> > ---
> > drivers/bluetooth/Kconfig  |  16 +++
> > drivers/bluetooth/Makefile |   1 +
> > drivers/bluetooth/btmtk.c  | 308 
> > +
> > drivers/bluetooth/btmtk.h  |  99 +++
> > drivers/bluetooth/btusb.c  | 174 +
> > 5 files changed, 598 insertions(+)
> > create mode 100644 drivers/bluetooth/btmtk.c
> > create mode 100644 drivers/bluetooth/btmtk.h
> > 
> > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > index 07e55cd..2788498 100644
> > --- a/drivers/bluetooth/Kconfig
> > +++ b/drivers/bluetooth/Kconfig
> > @@ -11,6 +11,10 @@ config BT_BCM
> > tristate
> > select FW_LOADER
> > 
> > +config BT_MTK
> > +   tristate
> > +   select FW_LOADER
> > +
> > config BT_RTL
> > tristate
> > select FW_LOADER
> > @@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
> > 
> >   Say Y here to compile support for Broadcom protocol.
> > 
> > +config BT_HCIBTUSB_MTK
> > +   bool "MediaTek protocol support"
> > +   depends on BT_HCIBTUSB
> > +   select BT_MTK
> > +   default y
> 
> This one has to be n since MediaTek hardware was never supported in the first 
> place. The existing y here are only for legacy hardware that was somehow 
> supported in a generic fashion.
> 

okay, I will turn it into n

> > +   help
> > + The MediaTek protocol support enables firmware download
> > + support and chip initialization for MediaTek Bluetooth
> > + USB controllers.
> > +
> > + Say Y here to compile support for MediaTek protocol.
> > +
> > config BT_HCIBTUSB_RTL
> > bool "Realtek protocol support"
> > depends on BT_HCIBTUSB
> > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> > index 4e4e44d..bc23724 100644
> > --- a/drivers/bluetooth/Makefile
> > +++ b/drivers/bluetooth/Makefile
> > @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)+= btmrvl_sdio.o
> > obj-$(CONFIG_BT_WILINK) += btwilink.o
> > obj-$(CONFIG_BT_QCOMSMD)+= btqcomsmd.o
> > obj-$(CONFIG_BT_BCM)+= btbcm.o
> > +obj-$(CONFIG_BT_MTK)   += btmtk.o
> > obj-$(CONFIG_BT_RTL)+= btrtl.o
> > obj-$(CONFIG_BT_QCA)+= btqca.o
> > 
> > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > new file mode 100644
> > index 000..9e39a0a
> > --- /dev/null
> > +++ b/drivers/bluetooth/btmtk.c
> > @@ -0,0 +1,308 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2018 MediaTek Inc.
> > +
> > +/*
> > + * Common operations for MediaTek Bluetooth devices
> > + * with the UART, USB and SDIO transport
> > + *
> > + * Author: Sean Wang 
> > + *
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +#include "btmtk.h"
> > +
> > +#define VERSION "0.1"
> > +
> > +int
> > +btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params 
> > *params)
> > +{
> > +   struct btmtk_hci_wmt_evt_funcc *evt_funcc;
> > +   u32 hlen, status = BTMTK_WMT_INVALID;
> > +   struct btmtk_wmt_hdr *hdr, *ehdr;
> > +   struct btmtk_hci_wmt_cmd wc;
> > +   struct sk_buff *skb;
> > +   int err = 0;
> > +
> > +   hlen = sizeof(*hdr) + params->dlen;
> > +   if (hlen > 255)
> > +   return -EINVAL;
> > +
> > +   hdr = (struct btmtk_wmt_hdr *)
> > +   hdr->dir = 1;
> > +   hdr->op = params->op;
> > +   hdr->dlen = cpu_to_le16(params->dlen + 1);
> > +   hdr->flag = params->flag;
> > +   memcpy(wc.data, params->data, params->dlen);
> > +
> > +   /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
> > +* instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
> > +*/
> > +   atomic_inc(>cmd_cnt);
> > +
> > +   skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
> > +HCI_INIT_TIMEOUT);
> 
> So first of all HCI_VENDOR_PKT is for H:4 pkt_type 0xff and not a vendor 
> event. Use HCI_EV_VENDOR instead. Second __hci_cmd_sync_ev is not the right 
> way doing this. Use __hci_cmd_send and handle the vendor event. See how we do 
> it for the Intel firmware loading.
> 

I didn't use __hci_cmd_send her before because I didn't know much about how to 
do with skb the returned event from __hci_cmd_send in order to parse the event 
content.

I can have a confirmation first about whether the Intel firmware download have 
the similar logic wants to handle.

> > +   if (IS_ERR(skb)) {
> > +   err = PTR_ERR(skb);
> > +
> > +   bt_dev_err(hdev, "Failed to send wmt 

Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-13 Thread Marcel Holtmann
Hi Sean,

> This adds the support of enabling MT7668U Bluetooth function running
> on the top of btusb driver. The patch also adds a newly created file
> mtkbt.c able to be reused independently from the transport type such
> as UART, USB and SDIO.

Include /sys/kernel/debug/usb/device portion in the commit message.

> 
> Signed-off-by: Sean Wang 
> ---
> drivers/bluetooth/Kconfig  |  16 +++
> drivers/bluetooth/Makefile |   1 +
> drivers/bluetooth/btmtk.c  | 308 +
> drivers/bluetooth/btmtk.h  |  99 +++
> drivers/bluetooth/btusb.c  | 174 +
> 5 files changed, 598 insertions(+)
> create mode 100644 drivers/bluetooth/btmtk.c
> create mode 100644 drivers/bluetooth/btmtk.h
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 07e55cd..2788498 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -11,6 +11,10 @@ config BT_BCM
>   tristate
>   select FW_LOADER
> 
> +config BT_MTK
> + tristate
> + select FW_LOADER
> +
> config BT_RTL
>   tristate
>   select FW_LOADER
> @@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
> 
> Say Y here to compile support for Broadcom protocol.
> 
> +config BT_HCIBTUSB_MTK
> + bool "MediaTek protocol support"
> + depends on BT_HCIBTUSB
> + select BT_MTK
> + default y

This one has to be n since MediaTek hardware was never supported in the first 
place. The existing y here are only for legacy hardware that was somehow 
supported in a generic fashion.

> + help
> +   The MediaTek protocol support enables firmware download
> +   support and chip initialization for MediaTek Bluetooth
> +   USB controllers.
> +
> +   Say Y here to compile support for MediaTek protocol.
> +
> config BT_HCIBTUSB_RTL
>   bool "Realtek protocol support"
>   depends on BT_HCIBTUSB
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 4e4e44d..bc23724 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)  += btmrvl_sdio.o
> obj-$(CONFIG_BT_WILINK)   += btwilink.o
> obj-$(CONFIG_BT_QCOMSMD)  += btqcomsmd.o
> obj-$(CONFIG_BT_BCM)  += btbcm.o
> +obj-$(CONFIG_BT_MTK) += btmtk.o
> obj-$(CONFIG_BT_RTL)  += btrtl.o
> obj-$(CONFIG_BT_QCA)  += btqca.o
> 
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> new file mode 100644
> index 000..9e39a0a
> --- /dev/null
> +++ b/drivers/bluetooth/btmtk.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 MediaTek Inc.
> +
> +/*
> + * Common operations for MediaTek Bluetooth devices
> + * with the UART, USB and SDIO transport
> + *
> + * Author: Sean Wang 
> + *
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "btmtk.h"
> +
> +#define VERSION "0.1"
> +
> +int
> +btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params *params)
> +{
> + struct btmtk_hci_wmt_evt_funcc *evt_funcc;
> + u32 hlen, status = BTMTK_WMT_INVALID;
> + struct btmtk_wmt_hdr *hdr, *ehdr;
> + struct btmtk_hci_wmt_cmd wc;
> + struct sk_buff *skb;
> + int err = 0;
> +
> + hlen = sizeof(*hdr) + params->dlen;
> + if (hlen > 255)
> + return -EINVAL;
> +
> + hdr = (struct btmtk_wmt_hdr *)
> + hdr->dir = 1;
> + hdr->op = params->op;
> + hdr->dlen = cpu_to_le16(params->dlen + 1);
> + hdr->flag = params->flag;
> + memcpy(wc.data, params->data, params->dlen);
> +
> + /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
> +  * instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
> +  */
> + atomic_inc(>cmd_cnt);
> +
> + skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
> +  HCI_INIT_TIMEOUT);

So first of all HCI_VENDOR_PKT is for H:4 pkt_type 0xff and not a vendor event. 
Use HCI_EV_VENDOR instead. Second __hci_cmd_sync_ev is not the right way doing 
this. Use __hci_cmd_send and handle the vendor event. See how we do it for the 
Intel firmware loading.

> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> +
> + bt_dev_err(hdev, "Failed to send wmt cmd (%d)\n", err);
> +
> + print_hex_dump(KERN_ERR, "failed cmd: ",
> +DUMP_PREFIX_ADDRESS, 16, 1, ,
> +hlen > 16 ? 16 : hlen, true);

Scrap this.

> + return err;
> + }
> +
> + ehdr = (struct btmtk_wmt_hdr *)skb->data;
> + if (ehdr->op != hdr->op) {
> + bt_dev_err(hdev, "Wrong op received %d expected %d",
> +ehdr->op, hdr->op);
> + err = -EIO;
> + goto err_free_skb;
> + }
> +
> + switch (ehdr->op) {
> + case BTMTK_WMT_SEMAPHORE:
> + if (ehdr->flag == 2)
> +   

Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-13 Thread Marcel Holtmann
Hi Sean,

> This adds the support of enabling MT7668U Bluetooth function running
> on the top of btusb driver. The patch also adds a newly created file
> mtkbt.c able to be reused independently from the transport type such
> as UART, USB and SDIO.

Include /sys/kernel/debug/usb/device portion in the commit message.

> 
> Signed-off-by: Sean Wang 
> ---
> drivers/bluetooth/Kconfig  |  16 +++
> drivers/bluetooth/Makefile |   1 +
> drivers/bluetooth/btmtk.c  | 308 +
> drivers/bluetooth/btmtk.h  |  99 +++
> drivers/bluetooth/btusb.c  | 174 +
> 5 files changed, 598 insertions(+)
> create mode 100644 drivers/bluetooth/btmtk.c
> create mode 100644 drivers/bluetooth/btmtk.h
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 07e55cd..2788498 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -11,6 +11,10 @@ config BT_BCM
>   tristate
>   select FW_LOADER
> 
> +config BT_MTK
> + tristate
> + select FW_LOADER
> +
> config BT_RTL
>   tristate
>   select FW_LOADER
> @@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
> 
> Say Y here to compile support for Broadcom protocol.
> 
> +config BT_HCIBTUSB_MTK
> + bool "MediaTek protocol support"
> + depends on BT_HCIBTUSB
> + select BT_MTK
> + default y

This one has to be n since MediaTek hardware was never supported in the first 
place. The existing y here are only for legacy hardware that was somehow 
supported in a generic fashion.

> + help
> +   The MediaTek protocol support enables firmware download
> +   support and chip initialization for MediaTek Bluetooth
> +   USB controllers.
> +
> +   Say Y here to compile support for MediaTek protocol.
> +
> config BT_HCIBTUSB_RTL
>   bool "Realtek protocol support"
>   depends on BT_HCIBTUSB
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 4e4e44d..bc23724 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)  += btmrvl_sdio.o
> obj-$(CONFIG_BT_WILINK)   += btwilink.o
> obj-$(CONFIG_BT_QCOMSMD)  += btqcomsmd.o
> obj-$(CONFIG_BT_BCM)  += btbcm.o
> +obj-$(CONFIG_BT_MTK) += btmtk.o
> obj-$(CONFIG_BT_RTL)  += btrtl.o
> obj-$(CONFIG_BT_QCA)  += btqca.o
> 
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> new file mode 100644
> index 000..9e39a0a
> --- /dev/null
> +++ b/drivers/bluetooth/btmtk.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 MediaTek Inc.
> +
> +/*
> + * Common operations for MediaTek Bluetooth devices
> + * with the UART, USB and SDIO transport
> + *
> + * Author: Sean Wang 
> + *
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "btmtk.h"
> +
> +#define VERSION "0.1"
> +
> +int
> +btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params *params)
> +{
> + struct btmtk_hci_wmt_evt_funcc *evt_funcc;
> + u32 hlen, status = BTMTK_WMT_INVALID;
> + struct btmtk_wmt_hdr *hdr, *ehdr;
> + struct btmtk_hci_wmt_cmd wc;
> + struct sk_buff *skb;
> + int err = 0;
> +
> + hlen = sizeof(*hdr) + params->dlen;
> + if (hlen > 255)
> + return -EINVAL;
> +
> + hdr = (struct btmtk_wmt_hdr *)
> + hdr->dir = 1;
> + hdr->op = params->op;
> + hdr->dlen = cpu_to_le16(params->dlen + 1);
> + hdr->flag = params->flag;
> + memcpy(wc.data, params->data, params->dlen);
> +
> + /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
> +  * instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
> +  */
> + atomic_inc(>cmd_cnt);
> +
> + skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
> +  HCI_INIT_TIMEOUT);

So first of all HCI_VENDOR_PKT is for H:4 pkt_type 0xff and not a vendor event. 
Use HCI_EV_VENDOR instead. Second __hci_cmd_sync_ev is not the right way doing 
this. Use __hci_cmd_send and handle the vendor event. See how we do it for the 
Intel firmware loading.

> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> +
> + bt_dev_err(hdev, "Failed to send wmt cmd (%d)\n", err);
> +
> + print_hex_dump(KERN_ERR, "failed cmd: ",
> +DUMP_PREFIX_ADDRESS, 16, 1, ,
> +hlen > 16 ? 16 : hlen, true);

Scrap this.

> + return err;
> + }
> +
> + ehdr = (struct btmtk_wmt_hdr *)skb->data;
> + if (ehdr->op != hdr->op) {
> + bt_dev_err(hdev, "Wrong op received %d expected %d",
> +ehdr->op, hdr->op);
> + err = -EIO;
> + goto err_free_skb;
> + }
> +
> + switch (ehdr->op) {
> + case BTMTK_WMT_SEMAPHORE:
> + if (ehdr->flag == 2)
> +   

Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-12 Thread kbuild test robot
Hi Sean,

I love your patch! Perhaps something to improve:

[auto build test WARNING on bluetooth/master]
[also build test WARNING on v4.18 next-20180810]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/linux-kernel-owner-vger-kernel-org/Bluetooth-mediatek-Add-protocol-support-for-MediaTek-MT7668U-USB-devices/20180813-043802
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git 
master


coccinelle warnings: (new ones prefixed by >>)

>> drivers/bluetooth/btmtk.c:86:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


Re: [PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-12 Thread kbuild test robot
Hi Sean,

I love your patch! Perhaps something to improve:

[auto build test WARNING on bluetooth/master]
[also build test WARNING on v4.18 next-20180810]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/linux-kernel-owner-vger-kernel-org/Bluetooth-mediatek-Add-protocol-support-for-MediaTek-MT7668U-USB-devices/20180813-043802
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git 
master


coccinelle warnings: (new ones prefixed by >>)

>> drivers/bluetooth/btmtk.c:86:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


[PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-12 Thread sean.wang
From: Sean Wang 

This adds the support of enabling MT7668U Bluetooth function running
on the top of btusb driver. The patch also adds a newly created file
mtkbt.c able to be reused independently from the transport type such
as UART, USB and SDIO.

Signed-off-by: Sean Wang 
---
 drivers/bluetooth/Kconfig  |  16 +++
 drivers/bluetooth/Makefile |   1 +
 drivers/bluetooth/btmtk.c  | 308 +
 drivers/bluetooth/btmtk.h  |  99 +++
 drivers/bluetooth/btusb.c  | 174 +
 5 files changed, 598 insertions(+)
 create mode 100644 drivers/bluetooth/btmtk.c
 create mode 100644 drivers/bluetooth/btmtk.h

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 07e55cd..2788498 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -11,6 +11,10 @@ config BT_BCM
tristate
select FW_LOADER
 
+config BT_MTK
+   tristate
+   select FW_LOADER
+
 config BT_RTL
tristate
select FW_LOADER
@@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
 
  Say Y here to compile support for Broadcom protocol.
 
+config BT_HCIBTUSB_MTK
+   bool "MediaTek protocol support"
+   depends on BT_HCIBTUSB
+   select BT_MTK
+   default y
+   help
+ The MediaTek protocol support enables firmware download
+ support and chip initialization for MediaTek Bluetooth
+ USB controllers.
+
+ Say Y here to compile support for MediaTek protocol.
+
 config BT_HCIBTUSB_RTL
bool "Realtek protocol support"
depends on BT_HCIBTUSB
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 4e4e44d..bc23724 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)+= btmrvl_sdio.o
 obj-$(CONFIG_BT_WILINK)+= btwilink.o
 obj-$(CONFIG_BT_QCOMSMD)   += btqcomsmd.o
 obj-$(CONFIG_BT_BCM)   += btbcm.o
+obj-$(CONFIG_BT_MTK)   += btmtk.o
 obj-$(CONFIG_BT_RTL)   += btrtl.o
 obj-$(CONFIG_BT_QCA)   += btqca.o
 
diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
new file mode 100644
index 000..9e39a0a
--- /dev/null
+++ b/drivers/bluetooth/btmtk.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 MediaTek Inc.
+
+/*
+ * Common operations for MediaTek Bluetooth devices
+ * with the UART, USB and SDIO transport
+ *
+ * Author: Sean Wang 
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "btmtk.h"
+
+#define VERSION "0.1"
+
+int
+btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params *params)
+{
+   struct btmtk_hci_wmt_evt_funcc *evt_funcc;
+   u32 hlen, status = BTMTK_WMT_INVALID;
+   struct btmtk_wmt_hdr *hdr, *ehdr;
+   struct btmtk_hci_wmt_cmd wc;
+   struct sk_buff *skb;
+   int err = 0;
+
+   hlen = sizeof(*hdr) + params->dlen;
+   if (hlen > 255)
+   return -EINVAL;
+
+   hdr = (struct btmtk_wmt_hdr *)
+   hdr->dir = 1;
+   hdr->op = params->op;
+   hdr->dlen = cpu_to_le16(params->dlen + 1);
+   hdr->flag = params->flag;
+   memcpy(wc.data, params->data, params->dlen);
+
+   /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
+* instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
+*/
+   atomic_inc(>cmd_cnt);
+
+   skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
+HCI_INIT_TIMEOUT);
+   if (IS_ERR(skb)) {
+   err = PTR_ERR(skb);
+
+   bt_dev_err(hdev, "Failed to send wmt cmd (%d)\n", err);
+
+   print_hex_dump(KERN_ERR, "failed cmd: ",
+  DUMP_PREFIX_ADDRESS, 16, 1, ,
+  hlen > 16 ? 16 : hlen, true);
+   return err;
+   }
+
+   ehdr = (struct btmtk_wmt_hdr *)skb->data;
+   if (ehdr->op != hdr->op) {
+   bt_dev_err(hdev, "Wrong op received %d expected %d",
+  ehdr->op, hdr->op);
+   err = -EIO;
+   goto err_free_skb;
+   }
+
+   switch (ehdr->op) {
+   case BTMTK_WMT_SEMAPHORE:
+   if (ehdr->flag == 2)
+   status = BTMTK_WMT_PATCH_UNDONE;
+   else
+   status = BTMTK_WMT_PATCH_DONE;
+   break;
+   case BTMTK_WMT_FUNC_CTRL:
+   evt_funcc = (struct btmtk_hci_wmt_evt_funcc *)ehdr;
+   if (be16_to_cpu(evt_funcc->status) == 4)
+   status = BTMTK_WMT_ON_DONE;
+   else if (be16_to_cpu(evt_funcc->status) == 32)
+   status = BTMTK_WMT_ON_PROGRESS;
+   else
+   status = BTMTK_WMT_ON_UNDONE;
+   break;
+   };
+
+   if (params->status)
+   *params->status = status;
+

[PATCH v1 1/2] Bluetooth: mediatek: Add protocol support for MediaTek MT7668U USB devices

2018-08-12 Thread sean.wang
From: Sean Wang 

This adds the support of enabling MT7668U Bluetooth function running
on the top of btusb driver. The patch also adds a newly created file
mtkbt.c able to be reused independently from the transport type such
as UART, USB and SDIO.

Signed-off-by: Sean Wang 
---
 drivers/bluetooth/Kconfig  |  16 +++
 drivers/bluetooth/Makefile |   1 +
 drivers/bluetooth/btmtk.c  | 308 +
 drivers/bluetooth/btmtk.h  |  99 +++
 drivers/bluetooth/btusb.c  | 174 +
 5 files changed, 598 insertions(+)
 create mode 100644 drivers/bluetooth/btmtk.c
 create mode 100644 drivers/bluetooth/btmtk.h

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 07e55cd..2788498 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -11,6 +11,10 @@ config BT_BCM
tristate
select FW_LOADER
 
+config BT_MTK
+   tristate
+   select FW_LOADER
+
 config BT_RTL
tristate
select FW_LOADER
@@ -52,6 +56,18 @@ config BT_HCIBTUSB_BCM
 
  Say Y here to compile support for Broadcom protocol.
 
+config BT_HCIBTUSB_MTK
+   bool "MediaTek protocol support"
+   depends on BT_HCIBTUSB
+   select BT_MTK
+   default y
+   help
+ The MediaTek protocol support enables firmware download
+ support and chip initialization for MediaTek Bluetooth
+ USB controllers.
+
+ Say Y here to compile support for MediaTek protocol.
+
 config BT_HCIBTUSB_RTL
bool "Realtek protocol support"
depends on BT_HCIBTUSB
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 4e4e44d..bc23724 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_BT_MRVL_SDIO)+= btmrvl_sdio.o
 obj-$(CONFIG_BT_WILINK)+= btwilink.o
 obj-$(CONFIG_BT_QCOMSMD)   += btqcomsmd.o
 obj-$(CONFIG_BT_BCM)   += btbcm.o
+obj-$(CONFIG_BT_MTK)   += btmtk.o
 obj-$(CONFIG_BT_RTL)   += btrtl.o
 obj-$(CONFIG_BT_QCA)   += btqca.o
 
diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
new file mode 100644
index 000..9e39a0a
--- /dev/null
+++ b/drivers/bluetooth/btmtk.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 MediaTek Inc.
+
+/*
+ * Common operations for MediaTek Bluetooth devices
+ * with the UART, USB and SDIO transport
+ *
+ * Author: Sean Wang 
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "btmtk.h"
+
+#define VERSION "0.1"
+
+int
+btmtk_hci_wmt_sync(struct hci_dev *hdev, struct btmtk_hci_wmt_params *params)
+{
+   struct btmtk_hci_wmt_evt_funcc *evt_funcc;
+   u32 hlen, status = BTMTK_WMT_INVALID;
+   struct btmtk_wmt_hdr *hdr, *ehdr;
+   struct btmtk_hci_wmt_cmd wc;
+   struct sk_buff *skb;
+   int err = 0;
+
+   hlen = sizeof(*hdr) + params->dlen;
+   if (hlen > 255)
+   return -EINVAL;
+
+   hdr = (struct btmtk_wmt_hdr *)
+   hdr->dir = 1;
+   hdr->op = params->op;
+   hdr->dlen = cpu_to_le16(params->dlen + 1);
+   hdr->flag = params->flag;
+   memcpy(wc.data, params->data, params->dlen);
+
+   /* TODO: Add a fixup with __hci_raw_sync_ev that uses the hdev->raw_q
+* instead of the hack with __hci_cmd_sync_ev + atomic_inc on cmd_cnt.
+*/
+   atomic_inc(>cmd_cnt);
+
+   skb =  __hci_cmd_sync_ev(hdev, 0xfc6f, hlen, , HCI_VENDOR_PKT,
+HCI_INIT_TIMEOUT);
+   if (IS_ERR(skb)) {
+   err = PTR_ERR(skb);
+
+   bt_dev_err(hdev, "Failed to send wmt cmd (%d)\n", err);
+
+   print_hex_dump(KERN_ERR, "failed cmd: ",
+  DUMP_PREFIX_ADDRESS, 16, 1, ,
+  hlen > 16 ? 16 : hlen, true);
+   return err;
+   }
+
+   ehdr = (struct btmtk_wmt_hdr *)skb->data;
+   if (ehdr->op != hdr->op) {
+   bt_dev_err(hdev, "Wrong op received %d expected %d",
+  ehdr->op, hdr->op);
+   err = -EIO;
+   goto err_free_skb;
+   }
+
+   switch (ehdr->op) {
+   case BTMTK_WMT_SEMAPHORE:
+   if (ehdr->flag == 2)
+   status = BTMTK_WMT_PATCH_UNDONE;
+   else
+   status = BTMTK_WMT_PATCH_DONE;
+   break;
+   case BTMTK_WMT_FUNC_CTRL:
+   evt_funcc = (struct btmtk_hci_wmt_evt_funcc *)ehdr;
+   if (be16_to_cpu(evt_funcc->status) == 4)
+   status = BTMTK_WMT_ON_DONE;
+   else if (be16_to_cpu(evt_funcc->status) == 32)
+   status = BTMTK_WMT_ON_PROGRESS;
+   else
+   status = BTMTK_WMT_ON_UNDONE;
+   break;
+   };
+
+   if (params->status)
+   *params->status = status;
+