RE: [PATCH v10 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
Hi Alan, > From: One Thousand Gnomes [mailto:gno...@lxorguk.ukuu.org.uk] > Sent: Thursday, May 05, 2016 8:53 PM > To: Amitkumar Karwar > Cc: linux-blueto...@vger.kernel.org; linux-kernel@vger.kernel.org; > Ganapathi Bhat > Subject: Re: [PATCH v10 3/3] Bluetooth: hci_uart: Support firmware > download for Marvell > > > +/* Send ACK/NAK to the device */ > > +static void mrvl_send_ack(struct hci_uart *hu, unsigned char ack) { > > + struct tty_struct *tty = hu->tty; > > + > > + tty->ops->write(tty, , sizeof(ack)); } > > You don't know if the device has a write method, and it should be > locked. > This should go via your ldisc not directly. > Thanks for review. We will take care of this. Regards, Amitkumar
RE: [PATCH v10 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
Hi Alan, > From: One Thousand Gnomes [mailto:gno...@lxorguk.ukuu.org.uk] > Sent: Thursday, May 05, 2016 8:53 PM > To: Amitkumar Karwar > Cc: linux-blueto...@vger.kernel.org; linux-kernel@vger.kernel.org; > Ganapathi Bhat > Subject: Re: [PATCH v10 3/3] Bluetooth: hci_uart: Support firmware > download for Marvell > > > +/* Send ACK/NAK to the device */ > > +static void mrvl_send_ack(struct hci_uart *hu, unsigned char ack) { > > + struct tty_struct *tty = hu->tty; > > + > > + tty->ops->write(tty, , sizeof(ack)); } > > You don't know if the device has a write method, and it should be > locked. > This should go via your ldisc not directly. > Thanks for review. We will take care of this. Regards, Amitkumar
Re: [PATCH v10 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
> +/* Send ACK/NAK to the device */ > +static void mrvl_send_ack(struct hci_uart *hu, unsigned char ack) > +{ > + struct tty_struct *tty = hu->tty; > + > + tty->ops->write(tty, , sizeof(ack)); > +} You don't know if the device has a write method, and it should be locked. This should go via your ldisc not directly. Alan
Re: [PATCH v10 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
> +/* Send ACK/NAK to the device */ > +static void mrvl_send_ack(struct hci_uart *hu, unsigned char ack) > +{ > + struct tty_struct *tty = hu->tty; > + > + tty->ops->write(tty, , sizeof(ack)); > +} You don't know if the device has a write method, and it should be locked. This should go via your ldisc not directly. Alan
[PATCH v10 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
From: Ganapathi BhatThis patch implement firmware download feature for Marvell Bluetooth devices. If firmware is already downloaded, it will skip downloading. Signed-off-by: Ganapathi Bhat Signed-off-by: Amitkumar Karwar --- v2: Fixed compilation warning reported by kbuild test robot v3: Addressed review comments from Marcel Holtmann a) Removed vendor specific code from hci_ldisc.c b) Get rid of static forward declaration c) Removed unnecessary heavy nesting d) Git rid of module parameter and global variables e) Add logic to pick right firmware image v4: Addresses review comments from Alan a) Use existing kernel helper APIs instead of writing own. b) Replace mdelay() with msleep() v5: Addresses review comments from Loic Poulain a) Use bt_dev_err/warn/dbg helpers insted of BT_ERR/WARN/DBG b) Used static functions where required and removed forward delcarations c) Edited comments for the function hci_uart_recv_data d) Made HCI_UART_DNLD_FW flag a part of driver private data v6: Addresses review comments from Loic Poulain a) Used skb instead of array to store firmware data during download b) Used hci_uart_tx_wakeup and enqueued packets instead of tty write c) Used GFP_KERNEL instead of GFP_ATOMIC v7: Edited Kconfig to add dependency for BT_HCIUART_H4. The change resolves errors reported by kbuild test robot. v8: Addressed review comments from Marcel Holtmann a) Removed unnecessary memory allocation failure messages b) Get rid of btmrvl.h header file and add definitions in hci_mrvl.c file v9: Addressed review comments from Marcel Holtmann a) Moved firmware download code from setup to prepare handler. b) Change messages from bt_dev_*->BT_*, as hdev isn't available during firmware download. v10: Addressed review comments from Marcel Holtmann a) Added new callback recv_for_prepare to receive data from device during prepare phase b) Avoided using private flags (HCI_UART_DNLD_FW) as new receive callback is added for the same purpose c) Used kernel API to handle unaligned data d) Moved mrvl_set_baud functionality inside setup callback --- drivers/bluetooth/Kconfig | 11 + drivers/bluetooth/Makefile| 1 + drivers/bluetooth/hci_ldisc.c | 6 + drivers/bluetooth/hci_mrvl.c | 535 ++ drivers/bluetooth/hci_uart.h | 8 +- 5 files changed, 560 insertions(+), 1 deletion(-) create mode 100644 drivers/bluetooth/hci_mrvl.c diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig index cf50fd2..daafd0c 100644 --- a/drivers/bluetooth/Kconfig +++ b/drivers/bluetooth/Kconfig @@ -180,6 +180,17 @@ config BT_HCIUART_AG6XX Say Y here to compile support for Intel AG6XX protocol. +config BT_HCIUART_MRVL + bool "Marvell protocol support" + depends on BT_HCIUART + select BT_HCIUART_H4 + help + Marvell is serial protocol for communication between Bluetooth + device and host. This protocol is required for most Marvell Bluetooth + devices with UART interface. + + Say Y here to compile support for HCI MRVL protocol. + config BT_HCIBCM203X tristate "HCI BCM203x USB driver" depends on USB diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile index 9c18939..364dbb6 100644 --- a/drivers/bluetooth/Makefile +++ b/drivers/bluetooth/Makefile @@ -37,6 +37,7 @@ hci_uart-$(CONFIG_BT_HCIUART_INTEL) += hci_intel.o hci_uart-$(CONFIG_BT_HCIUART_BCM) += hci_bcm.o hci_uart-$(CONFIG_BT_HCIUART_QCA) += hci_qca.o hci_uart-$(CONFIG_BT_HCIUART_AG6XX)+= hci_ag6xx.o +hci_uart-$(CONFIG_BT_HCIUART_MRVL) += hci_mrvl.o hci_uart-objs := $(hci_uart-y) ccflags-y += -D__CHECK_ENDIAN__ diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index 047e786..4896b6f 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -821,6 +821,9 @@ static int __init hci_uart_init(void) #ifdef CONFIG_BT_HCIUART_AG6XX ag6xx_init(); #endif +#ifdef CONFIG_BT_HCIUART_MRVL + mrvl_init(); +#endif return 0; } @@ -856,6 +859,9 @@ static void __exit hci_uart_exit(void) #ifdef CONFIG_BT_HCIUART_AG6XX ag6xx_deinit(); #endif +#ifdef CONFIG_BT_HCIUART_MRVL + mrvl_deinit(); +#endif /* Release tty registration of line discipline */ err = tty_unregister_ldisc(N_HCI); diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c new file mode 100644 index 000..8416014 --- /dev/null +++ b/drivers/bluetooth/hci_mrvl.c @@ -0,0 +1,535 @@ +/* Bluetooth HCI UART driver for Marvell devices + * + * Copyright (C) 2016, Marvell International Ltd. + * + * Acknowledgements: + * This file is based on hci_h4.c, which was written + * by Maxim Krasnyansky and Marcel Holtmann. + * + * This software file (the "File") is distributed by Marvell International
[PATCH v10 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
From: Ganapathi Bhat This patch implement firmware download feature for Marvell Bluetooth devices. If firmware is already downloaded, it will skip downloading. Signed-off-by: Ganapathi Bhat Signed-off-by: Amitkumar Karwar --- v2: Fixed compilation warning reported by kbuild test robot v3: Addressed review comments from Marcel Holtmann a) Removed vendor specific code from hci_ldisc.c b) Get rid of static forward declaration c) Removed unnecessary heavy nesting d) Git rid of module parameter and global variables e) Add logic to pick right firmware image v4: Addresses review comments from Alan a) Use existing kernel helper APIs instead of writing own. b) Replace mdelay() with msleep() v5: Addresses review comments from Loic Poulain a) Use bt_dev_err/warn/dbg helpers insted of BT_ERR/WARN/DBG b) Used static functions where required and removed forward delcarations c) Edited comments for the function hci_uart_recv_data d) Made HCI_UART_DNLD_FW flag a part of driver private data v6: Addresses review comments from Loic Poulain a) Used skb instead of array to store firmware data during download b) Used hci_uart_tx_wakeup and enqueued packets instead of tty write c) Used GFP_KERNEL instead of GFP_ATOMIC v7: Edited Kconfig to add dependency for BT_HCIUART_H4. The change resolves errors reported by kbuild test robot. v8: Addressed review comments from Marcel Holtmann a) Removed unnecessary memory allocation failure messages b) Get rid of btmrvl.h header file and add definitions in hci_mrvl.c file v9: Addressed review comments from Marcel Holtmann a) Moved firmware download code from setup to prepare handler. b) Change messages from bt_dev_*->BT_*, as hdev isn't available during firmware download. v10: Addressed review comments from Marcel Holtmann a) Added new callback recv_for_prepare to receive data from device during prepare phase b) Avoided using private flags (HCI_UART_DNLD_FW) as new receive callback is added for the same purpose c) Used kernel API to handle unaligned data d) Moved mrvl_set_baud functionality inside setup callback --- drivers/bluetooth/Kconfig | 11 + drivers/bluetooth/Makefile| 1 + drivers/bluetooth/hci_ldisc.c | 6 + drivers/bluetooth/hci_mrvl.c | 535 ++ drivers/bluetooth/hci_uart.h | 8 +- 5 files changed, 560 insertions(+), 1 deletion(-) create mode 100644 drivers/bluetooth/hci_mrvl.c diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig index cf50fd2..daafd0c 100644 --- a/drivers/bluetooth/Kconfig +++ b/drivers/bluetooth/Kconfig @@ -180,6 +180,17 @@ config BT_HCIUART_AG6XX Say Y here to compile support for Intel AG6XX protocol. +config BT_HCIUART_MRVL + bool "Marvell protocol support" + depends on BT_HCIUART + select BT_HCIUART_H4 + help + Marvell is serial protocol for communication between Bluetooth + device and host. This protocol is required for most Marvell Bluetooth + devices with UART interface. + + Say Y here to compile support for HCI MRVL protocol. + config BT_HCIBCM203X tristate "HCI BCM203x USB driver" depends on USB diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile index 9c18939..364dbb6 100644 --- a/drivers/bluetooth/Makefile +++ b/drivers/bluetooth/Makefile @@ -37,6 +37,7 @@ hci_uart-$(CONFIG_BT_HCIUART_INTEL) += hci_intel.o hci_uart-$(CONFIG_BT_HCIUART_BCM) += hci_bcm.o hci_uart-$(CONFIG_BT_HCIUART_QCA) += hci_qca.o hci_uart-$(CONFIG_BT_HCIUART_AG6XX)+= hci_ag6xx.o +hci_uart-$(CONFIG_BT_HCIUART_MRVL) += hci_mrvl.o hci_uart-objs := $(hci_uart-y) ccflags-y += -D__CHECK_ENDIAN__ diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index 047e786..4896b6f 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -821,6 +821,9 @@ static int __init hci_uart_init(void) #ifdef CONFIG_BT_HCIUART_AG6XX ag6xx_init(); #endif +#ifdef CONFIG_BT_HCIUART_MRVL + mrvl_init(); +#endif return 0; } @@ -856,6 +859,9 @@ static void __exit hci_uart_exit(void) #ifdef CONFIG_BT_HCIUART_AG6XX ag6xx_deinit(); #endif +#ifdef CONFIG_BT_HCIUART_MRVL + mrvl_deinit(); +#endif /* Release tty registration of line discipline */ err = tty_unregister_ldisc(N_HCI); diff --git a/drivers/bluetooth/hci_mrvl.c b/drivers/bluetooth/hci_mrvl.c new file mode 100644 index 000..8416014 --- /dev/null +++ b/drivers/bluetooth/hci_mrvl.c @@ -0,0 +1,535 @@ +/* Bluetooth HCI UART driver for Marvell devices + * + * Copyright (C) 2016, Marvell International Ltd. + * + * Acknowledgements: + * This file is based on hci_h4.c, which was written + * by Maxim Krasnyansky and Marcel Holtmann. + * + * This software file (the "File") is distributed by Marvell International + * Ltd. under the terms of the GNU General Public License