* RE: [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
2016-05-06 15:31 ` [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download for Marvell Amitkumar Karwar
@ 2016-05-10 13:59 ` Amitkumar Karwar
2016-05-23 6:06 ` Amitkumar Karwar
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Amitkumar Karwar @ 2016-05-10 13:59 UTC (permalink / raw)
To: linux-bluetooth; +Cc: linux-kernel, Ganapathi Bhat
Hi Marcel,
> From: Amitkumar Karwar [mailto:akarwar@marvell.com]
> Sent: Friday, May 06, 2016 9:02 PM
> To: linux-bluetooth@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Ganapathi Bhat; Amitkumar Karwar
> Subject: [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download
> for Marvell
>
> From: Ganapathi Bhat <gbhat@marvell.com>
>
> This patch implement firmware download feature for Marvell Bluetooth
> devices. If firmware is already downloaded, it will skip downloading.
>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> 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
> v11: Write data through ldisc in mrvl_send_ack() instead of directly
> calling
> write method(One Thousand Gnomes).
Could you please take this patch if there are no further comments/suggestions for improvement?
Regards,
Amitkumar
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
2016-05-06 15:31 ` [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download for Marvell Amitkumar Karwar
2016-05-10 13:59 ` Amitkumar Karwar
@ 2016-05-23 6:06 ` Amitkumar Karwar
2016-06-14 6:35 ` Amitkumar Karwar
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Amitkumar Karwar @ 2016-05-23 6:06 UTC (permalink / raw)
To: linux-bluetooth; +Cc: linux-kernel, Ganapathi Bhat, Cathy Luo
Hi Marcel,
> > From: Amitkumar Karwar [mailto:akarwar@marvell.com]
> > Sent: Friday, May 06, 2016 9:02 PM
> > To: linux-bluetooth@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org; Ganapathi Bhat; Amitkumar Karwar
> > Subject: [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware
> > download for Marvell
> >
> > From: Ganapathi Bhat <gbhat@marvell.com>
> >
> > This patch implement firmware download feature for Marvell Bluetooth
> > devices. If firmware is already downloaded, it will skip downloading.
> >
> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> > 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
> > v11: Write data through ldisc in mrvl_send_ack() instead of directly
> > calling
> > write method(One Thousand Gnomes).
>
Could you please take this patch?
Regards,
Amitkumar
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download for Marvell
2016-05-06 15:31 ` [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download for Marvell Amitkumar Karwar
2016-05-10 13:59 ` Amitkumar Karwar
2016-05-23 6:06 ` Amitkumar Karwar
@ 2016-06-14 6:35 ` Amitkumar Karwar
2016-06-21 3:47 ` [v11,3/3] " Caesar Wang
2016-06-24 6:02 ` Jeffy Chen
4 siblings, 0 replies; 13+ messages in thread
From: Amitkumar Karwar @ 2016-06-14 6:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: linux-kernel, Ganapathi Bhat, Cathy Luo
Hi Marcel,
> From: Amitkumar Karwar
> Sent: Monday, May 23, 2016 11:37 AM
> To: 'linux-bluetooth@vger.kernel.org'
> Cc: 'linux-kernel@vger.kernel.org'; Ganapathi Bhat; Cathy Luo
> Subject: RE: [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware
> download for Marvell
>
> Hi Marcel,
>
> > > From: Amitkumar Karwar [mailto:akarwar@marvell.com]
> > > Sent: Friday, May 06, 2016 9:02 PM
> > > To: linux-bluetooth@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org; Ganapathi Bhat; Amitkumar Karwar
> > > Subject: [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware
> > > download for Marvell
> > >
> > > From: Ganapathi Bhat <gbhat@marvell.com>
> > >
> > > This patch implement firmware download feature for Marvell Bluetooth
> > > devices. If firmware is already downloaded, it will skip
> downloading.
> > >
> > > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> > > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > > ---
> > > 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
> > > v11: Write data through ldisc in mrvl_send_ack() instead of directly
> > > calling
> > > write method(One Thousand Gnomes).
> >
>
> Could you please take this patch?
>
This has been pending for a while.
Please let me know your comment.
Regards,
Amitkumar
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v11,3/3] Bluetooth: hci_uart: Support firmware download for Marvell
2016-05-06 15:31 ` [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download for Marvell Amitkumar Karwar
` (2 preceding siblings ...)
2016-06-14 6:35 ` Amitkumar Karwar
@ 2016-06-21 3:47 ` Caesar Wang
2016-06-24 6:02 ` Jeffy Chen
4 siblings, 0 replies; 13+ messages in thread
From: Caesar Wang @ 2016-06-21 3:47 UTC (permalink / raw)
To: Amitkumar Karwar, linux-bluetooth; +Cc: linux-kernel, Ganapathi Bhat
On 2016年05月06日 23:31, Amitkumar Karwar wrote:
> From: Ganapathi Bhat <gbhat@marvell.com>
>
> This patch implement firmware download feature for
> Marvell Bluetooth devices. If firmware is already
> downloaded, it will skip downloading.
>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Tested-by: Caesar Wang <wxt@rock-chips.com>
Tested on chromeos4.4. so you can free add my test tag:
I don't find the cover-letter, so add it in here.
Cc: linux-rockchip@lists.infradead.org,
that's interesting in this series patches.
> ---
> 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
> v11: Write data through ldisc in mrvl_send_ack() instead of directly calling
> write method(One Thousand Gnomes).
> ---
> drivers/bluetooth/Kconfig | 11 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/hci_ldisc.c | 6 +
> drivers/bluetooth/hci_mrvl.c | 543 ++++++++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/hci_uart.h | 8 +-
> 5 files changed, 568 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 0000000..2686901
> --- /dev/null
> +++ b/drivers/bluetooth/hci_mrvl.c
> @@ -0,0 +1,543 @@
> +/* 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 Version 2, June 1991
> + * (the "License"). You may use, redistribute and/or modify this File in
> + * accordance with the terms and conditions of the License, a copy of which
> + * is available on the worldwide web at
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
> + *
> + * THE FILE IS DISTRIBUTED AS-IS, WITHOUT WARRANTY OF ANY KIND, AND THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE
> + * ARE EXPRESSLY DISCLAIMED. The License provides additional details about
> + * this warranty disclaimer.
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/tty.h>
> +#include <asm/unaligned.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include "hci_uart.h"
> +
> +struct fw_data {
> + wait_queue_head_t init_wait_q;
> + u8 wait_fw;
> + int next_len;
> + u8 five_bytes[5];
> + u8 next_index;
> + u8 last_ack;
> + u8 expected_ack;
> + struct ktermios old_termios;
> + u8 chip_id;
> + u8 chip_rev;
> + struct sk_buff *skb;
> +};
> +
> +#define MRVL_HELPER_NAME "mrvl/helper_uart_3000000.bin"
> +#define MRVL_8997_CHIP_ID 0x50
> +#define MRVL_8997_FW_NAME "mrvl/uart8997_bt.bin"
> +#define MRVL_MAX_FW_BLOCK_SIZE 1024
> +#define MRVL_MAX_RETRY_SEND 12
> +#define MRVL_DNLD_DELAY 100
> +#define MRVL_ACK 0x5A
> +#define MRVL_NAK 0xBF
> +#define MRVL_HDR_REQ_FW 0xA5
> +#define MRVL_HDR_CHIP_VER 0xAA
> +#define MRVL_HCI_OP_SET_BAUD 0xFC09
> +#define MRVL_FW_HDR_LEN 5
> +#define MRVL_WAIT_TIMEOUT msecs_to_jiffies(12000)
> +
> +struct mrvl_data {
> + struct sk_buff *rx_skb;
> + struct sk_buff_head txq;
> + struct fw_data *fwdata;
> +};
> +
> +static int get_cts(struct hci_uart *hu)
> +{
> + struct tty_struct *tty = hu->tty;
> + u32 state = tty->ops->tiocmget(tty);
> +
> + if (state & TIOCM_CTS) {
> + bt_dev_dbg(hu->hdev, "CTS is low");
> + return 1;
> + }
> + bt_dev_dbg(hu->hdev, "CTS is high");
> +
> + return 0;
> +}
> +
> +/* Initialize protocol */
> +static int mrvl_open(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl;
> +
> + bt_dev_dbg(hu->hdev, "hu %p", hu);
> +
> + mrvl = kzalloc(sizeof(*mrvl), GFP_KERNEL);
> + if (!mrvl)
> + return -ENOMEM;
> +
> + skb_queue_head_init(&mrvl->txq);
> + hu->priv = mrvl;
> +
> + return 0;
> +}
> +
> +/* Flush protocol data */
> +static int mrvl_flush(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + bt_dev_dbg(hu->hdev, "hu %p", hu);
> +
> + skb_queue_purge(&mrvl->txq);
> +
> + return 0;
> +}
> +
> +/* Close protocol */
> +static int mrvl_close(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + bt_dev_dbg(hu->hdev, "hu %p", hu);
> +
> + skb_queue_purge(&mrvl->txq);
> + kfree_skb(mrvl->rx_skb);
> + hu->priv = NULL;
> + kfree(mrvl);
> +
> + return 0;
> +}
> +
> +/* Enqueue frame for transmittion (padding, crc, etc) */
> +static int mrvl_enqueue(struct hci_uart *hu, struct sk_buff *skb)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + bt_dev_dbg(hu->hdev, "hu %p skb %p", hu, skb);
> +
> + /* Prepend skb with frame type */
> + memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
> + skb_queue_tail(&mrvl->txq, skb);
> +
> + return 0;
> +}
> +
> +static const struct h4_recv_pkt mrvl_recv_pkts[] = {
> + { H4_RECV_ACL, .recv = hci_recv_frame },
> + { H4_RECV_SCO, .recv = hci_recv_frame },
> + { H4_RECV_EVENT, .recv = hci_recv_frame },
> +};
> +
> +/* Send ACK/NAK to the device */
> +static void mrvl_send_ack(struct hci_uart *hu, unsigned char ack)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct sk_buff *skb;
> +
> + skb = bt_skb_alloc(sizeof(ack), GFP_KERNEL);
> + if (!skb)
> + return;
> +
> + memcpy(skb->data, &ack, sizeof(ack));
> + skb_put(skb, sizeof(ack));
> + skb_queue_head(&mrvl->txq, skb);
> + hci_uart_tx_wakeup(hu);
> +}
> +
> +/* Validate the feedback data from device */
> +static void mrvl_pkt_complete(struct hci_uart *hu, struct sk_buff *skb)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> + u16 lhs, rhs;
> +
> + lhs = get_unaligned_le16(skb->data + 1);
> + rhs = get_unaligned_le16(skb->data + 3);
> + if ((lhs ^ rhs) == 0xffff) {
> + mrvl_send_ack(hu, MRVL_ACK);
> + fw_data->wait_fw = 1;
> + fw_data->next_len = lhs;
> + /* Firmware download is done, send the last ack */
> + if (!lhs)
> + fw_data->last_ack = 1;
> +
> + if (fw_data->expected_ack == MRVL_HDR_CHIP_VER) {
> + fw_data->chip_id = skb->data[1];
> + fw_data->chip_rev = skb->data[2];
> + }
> + wake_up_interruptible(&fw_data->init_wait_q);
> + } else {
> + mrvl_send_ack(hu, MRVL_NAK);
> + }
> +}
> +
> +/* This function receives data from the uart device during firmware download.
> + * Driver expects 5 bytes of data as per the protocal in the below format:
> + * <HEADER><BYTE_1><BYTE_2><BYTE_3><BYTE_4>
> + * BYTE_3 and BYTE_4 are compliment of BYTE_1 an BYTE_2. Data can come in chunks
> + * of any length. If length received is < 5, accumulate the data in an array,
> + * until we have a sequence of 5 bytes, starting with the expected HEADER. If
> + * the length received is > 5 bytes, then get the first 5 bytes, starting with
> + * the HEADER and process the same, ignoring the rest of the bytes as per the
> + * protocal.
> + */
> +static struct sk_buff *mrvl_process_fw_data(struct hci_uart *hu,
> + struct sk_buff *skb,
> + u8 *buf, int count)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> + int i = 0, len;
> +
> + if (!skb) {
> + while (buf[i] != fw_data->expected_ack && i < count)
> + i++;
> + if (i == count)
> + return ERR_PTR(-EILSEQ);
> +
> + skb = bt_skb_alloc(MRVL_FW_HDR_LEN, GFP_KERNEL);
> + }
> +
> + if (!skb)
> + return ERR_PTR(-ENOMEM);
> +
> + len = count - i;
> + memcpy(skb_put(skb, len), &buf[i], len);
> +
> + if (skb->len == MRVL_FW_HDR_LEN) {
> + mrvl_pkt_complete(hu, skb);
> + kfree_skb(skb);
> + skb = NULL;
> + }
> +
> + return skb;
> +}
> +
> +/* Receive firmware feedback data */
> +static int mrvl_recv_for_prepare(struct hci_uart *hu, const void *data,
> + int count)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + mrvl->fwdata->skb = mrvl_process_fw_data(hu, mrvl->fwdata->skb,
> + (u8 *)data, count);
> + if (IS_ERR(mrvl->fwdata->skb)) {
> + int err = PTR_ERR(mrvl->fwdata->skb);
> +
> + bt_dev_err(hu->hdev,
> + "Receive firmware data failed (%d)", err);
> + mrvl->fwdata->skb = NULL;
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +/* Receive data */
> +static int mrvl_recv(struct hci_uart *hu, const void *data, int count)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
> + return -EUNATCH;
> +
> + mrvl->rx_skb = h4_recv_buf(hu->hdev, mrvl->rx_skb, data, count,
> + mrvl_recv_pkts, ARRAY_SIZE(mrvl_recv_pkts));
> + if (IS_ERR(mrvl->rx_skb)) {
> + int err = PTR_ERR(mrvl->rx_skb);
> +
> + bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> + mrvl->rx_skb = NULL;
> + return err;
> + }
> +
> + return count;
> +}
> +
> +static struct sk_buff *mrvl_dequeue(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + return skb_dequeue(&mrvl->txq);
> +}
> +
> +static int mrvl_init_fw_data(struct hci_uart *hu)
> +{
> + struct fw_data *fwdata;
> + struct mrvl_data *mrvl = hu->priv;
> +
> + fwdata = kzalloc(sizeof(*fwdata), GFP_KERNEL);
> + if (!fwdata)
> + return -ENOMEM;
> +
> + mrvl->fwdata = fwdata;
> + init_waitqueue_head(&fwdata->init_wait_q);
> +
> + return 0;
> +}
> +
> +/* Wait for the header from device */
> +static int mrvl_wait_for_hdr(struct hci_uart *hu, u8 header)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> +
> + fw_data->expected_ack = header;
> + fw_data->wait_fw = 0;
> +
> + if (!wait_event_interruptible_timeout(fw_data->init_wait_q,
> + fw_data->wait_fw,
> + MRVL_WAIT_TIMEOUT)) {
> + BT_ERR("TIMEOUT, waiting for:0x%x", fw_data->expected_ack);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/* Send bytes to device */
> +static int mrvl_send_data(struct hci_uart *hu, struct sk_buff *skb)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + skb_queue_head(&mrvl->txq, skb);
> + hci_uart_tx_wakeup(hu);
> +
> + if (mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW) == -1)
> + return -1;
> +
> + return 0;
> +}
> +
> +/* Download firmware to the device */
> +static int mrvl_dnld_fw(struct hci_uart *hu, const char *file_name)
> +{
> + const struct firmware *fw = NULL;
> + struct sk_buff *skb = NULL;
> + int offset = 0;
> + int ret, tx_len;
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> +
> + ret = request_firmware(&fw, file_name, hu->tty->dev);
> + if (ret < 0) {
> + BT_ERR("request_firmware() failed");
> + return -1;
> + }
> +
> + BT_INFO("Downloading FW (%d bytes)", (u16)fw->size);
> +
> + fw_data->last_ack = 0;
> +
> + while (1) {
> + if ((offset >= fw->size) || (fw_data->last_ack))
> + break;
> + tx_len = fw_data->next_len;
> + if ((fw->size - offset) < tx_len)
> + tx_len = fw->size - offset;
> +
> + skb = bt_skb_alloc(MRVL_MAX_FW_BLOCK_SIZE, GFP_KERNEL);
> + if (!skb) {
> + ret = -1;
> + goto done;
> + }
> +
> + memcpy(skb->data, &fw->data[offset], tx_len);
> + skb_put(skb, tx_len);
> + if (mrvl_send_data(hu, skb) != 0) {
> + BT_ERR("Fail to download firmware");
> + ret = -1;
> + goto done;
> + }
> + offset += tx_len;
> + }
> +
> + BT_INFO("Downloaded %d byte firmware", offset);
> +done:
> + release_firmware(fw);
> +
> + return ret;
> +}
> +
> +/* Set the baud rate */
> +static int mrvl_set_dev_baud(struct hci_uart *hu)
> +{
> + struct hci_dev *hdev = hu->hdev;
> + struct sk_buff *skb;
> + static const u8 baud_param[] = { 0xc0, 0xc6, 0x2d, 0x00 };
> + int err;
> +
> + skb = __hci_cmd_sync(hdev, MRVL_HCI_OP_SET_BAUD, sizeof(baud_param),
> + baud_param, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + bt_dev_err(hu->hdev, "Set device baudrate failed (%d)", err);
> + return err;
> + }
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> +/* Reset device */
> +static int mrvl_reset(struct hci_uart *hu)
> +{
> + struct hci_dev *hdev = hu->hdev;
> + struct sk_buff *skb;
> + int err;
> +
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_CMD_TIMEOUT);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + bt_dev_err(hu->hdev, "Reset device failed (%d)", err);
> + return err;
> + }
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> +static int mrvl_get_fw_name(struct hci_uart *hu, char *fw_name)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> +
> + if (mrvl_wait_for_hdr(hu, MRVL_HDR_CHIP_VER) != 0) {
> + BT_ERR("Could not read chip id and revision");
> + return -1;
> + }
> +
> + BT_DBG("chip_id=0x%x, chip_rev=0x%x",
> + fw_data->chip_id, fw_data->chip_rev);
> +
> + switch (fw_data->chip_id) {
> + case MRVL_8997_CHIP_ID:
> + memcpy(fw_name, MRVL_8997_FW_NAME, sizeof(MRVL_8997_FW_NAME));
> + return 0;
> + default:
> + BT_ERR("Invalid chip id");
> + return -1;
> + }
> +}
> +
> +/* Download helper and firmare to device */
> +static int hci_uart_dnld_fw(struct hci_uart *hu)
> +{
> + struct tty_struct *tty = hu->tty;
> + struct ktermios new_termios;
> + struct ktermios old_termios;
> + char fw_name[128];
> + int ret;
> +
> + old_termios = tty->termios;
> +
> + if (get_cts(hu)) {
> + BT_INFO("fw is running");
> + return 0;
> + }
> +
> + hci_uart_set_baudrate(hu, 115200);
> + hci_uart_set_flow_control(hu, true);
> +
> + ret = mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW);
> + if (ret)
> + goto fail;
> +
> + ret = mrvl_dnld_fw(hu, MRVL_HELPER_NAME);
> + if (ret)
> + goto fail;
> +
> + msleep(MRVL_DNLD_DELAY);
> +
> + hci_uart_set_baudrate(hu, 3000000);
> + hci_uart_set_flow_control(hu, false);
> +
> + ret = mrvl_get_fw_name(hu, fw_name);
> + if (ret)
> + goto fail;
> +
> + ret = mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW);
> + if (ret)
> + goto fail;
> +
> + ret = mrvl_dnld_fw(hu, fw_name);
> + if (ret)
> + goto fail;
> +
> + msleep(MRVL_DNLD_DELAY);
> +fail:
> + /* restore uart settings */
> + new_termios = tty->termios;
> + tty->termios.c_cflag = old_termios.c_cflag;
> + tty_set_termios(tty, &new_termios);
> +
> + return ret;
> +}
> +
> +static int mrvl_setup(struct hci_uart *hu)
> +{
> + int err;
> +
> + hci_uart_set_baudrate(hu, 115200);
> + hci_uart_set_flow_control(hu, false);
> +
> + err = mrvl_reset(hu);
> + if (!err) {
> + err = mrvl_set_dev_baud(hu);
> + if (err)
> + return -1;
> + }
> +
> + hci_uart_set_baudrate(hu, 3000000);
> + hci_uart_set_flow_control(hu, false);
> + msleep(MRVL_DNLD_DELAY);
> +
> + return 0;
> +}
> +
> +static int mrvl_prepare(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + int err;
> +
> + err = mrvl_init_fw_data(hu);
> + if (!err)
> + err = hci_uart_dnld_fw(hu);
> +
> + kfree(mrvl->fwdata);
> + return err;
> +}
> +
> +static const struct hci_uart_proto mrvlp = {
> + .id = HCI_UART_MRVL,
> + .name = "MRVL",
> + .open = mrvl_open,
> + .close = mrvl_close,
> + .recv = mrvl_recv,
> + .enqueue = mrvl_enqueue,
> + .dequeue = mrvl_dequeue,
> + .flush = mrvl_flush,
> + .setup = mrvl_setup,
> + .prepare = mrvl_prepare,
> + .recv_for_prepare = mrvl_recv_for_prepare,
> +};
> +
> +int __init mrvl_init(void)
> +{
> + return hci_uart_register_proto(&mrvlp);
> +}
> +
> +int __exit mrvl_deinit(void)
> +{
> + return hci_uart_unregister_proto(&mrvlp);
> +}
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index 17ba3b4..8c53b50 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -35,7 +35,7 @@
> #define HCIUARTGETFLAGS _IOR('U', 204, int)
>
> /* UART protocols */
> -#define HCI_UART_MAX_PROTO 10
> +#define HCI_UART_MAX_PROTO 11
>
> #define HCI_UART_H4 0
> #define HCI_UART_BCSP 1
> @@ -47,6 +47,7 @@
> #define HCI_UART_BCM 7
> #define HCI_UART_QCA 8
> #define HCI_UART_AG6XX 9
> +#define HCI_UART_MRVL 10
>
> #define HCI_UART_RAW_DEVICE 0
> #define HCI_UART_RESET_ON_INIT 1
> @@ -192,3 +193,8 @@ int qca_deinit(void);
> int ag6xx_init(void);
> int ag6xx_deinit(void);
> #endif
> +
> +#ifdef CONFIG_BT_HCIUART_MRVL
> +int mrvl_init(void);
> +int mrvl_deinit(void);
> +#endif
>
>
--
caesar wang | software engineer | wxt@rock-chip.com
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [v11,3/3] Bluetooth: hci_uart: Support firmware download for Marvell
2016-05-06 15:31 ` [PATCH v11 3/3] Bluetooth: hci_uart: Support firmware download for Marvell Amitkumar Karwar
` (3 preceding siblings ...)
2016-06-21 3:47 ` [v11,3/3] " Caesar Wang
@ 2016-06-24 6:02 ` Jeffy Chen
2016-06-27 6:28 ` Amitkumar Karwar
4 siblings, 1 reply; 13+ messages in thread
From: Jeffy Chen @ 2016-06-24 6:02 UTC (permalink / raw)
To: Amitkumar Karwar, linux-bluetooth; +Cc: linux-kernel, Ganapathi Bhat
On 2016-5-6 23:31, Amitkumar Karwar wrote:
> From: Ganapathi Bhat <gbhat@marvell.com>
>
> This patch implement firmware download feature for
> Marvell Bluetooth devices. If firmware is already
> downloaded, it will skip downloading.
>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> 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
> v11: Write data through ldisc in mrvl_send_ack() instead of directly calling
> write method(One Thousand Gnomes).
> ---
> drivers/bluetooth/Kconfig | 11 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/hci_ldisc.c | 6 +
> drivers/bluetooth/hci_mrvl.c | 543 ++++++++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/hci_uart.h | 8 +-
> 5 files changed, 568 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 0000000..2686901
> --- /dev/null
> +++ b/drivers/bluetooth/hci_mrvl.c
> @@ -0,0 +1,543 @@
> +/* 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 Version 2, June 1991
> + * (the "License"). You may use, redistribute and/or modify this File in
> + * accordance with the terms and conditions of the License, a copy of which
> + * is available on the worldwide web at
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
> + *
> + * THE FILE IS DISTRIBUTED AS-IS, WITHOUT WARRANTY OF ANY KIND, AND THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE
> + * ARE EXPRESSLY DISCLAIMED. The License provides additional details about
> + * this warranty disclaimer.
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/tty.h>
> +#include <asm/unaligned.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include "hci_uart.h"
> +
> +struct fw_data {
> + wait_queue_head_t init_wait_q;
> + u8 wait_fw;
> + int next_len;
> + u8 five_bytes[5];
> + u8 next_index;
> + u8 last_ack;
> + u8 expected_ack;
> + struct ktermios old_termios;
> + u8 chip_id;
> + u8 chip_rev;
> + struct sk_buff *skb;
> +};
> +
> +#define MRVL_HELPER_NAME "mrvl/helper_uart_3000000.bin"
> +#define MRVL_8997_CHIP_ID 0x50
> +#define MRVL_8997_FW_NAME "mrvl/uart8997_bt.bin"
> +#define MRVL_MAX_FW_BLOCK_SIZE 1024
> +#define MRVL_MAX_RETRY_SEND 12
> +#define MRVL_DNLD_DELAY 100
> +#define MRVL_ACK 0x5A
> +#define MRVL_NAK 0xBF
> +#define MRVL_HDR_REQ_FW 0xA5
> +#define MRVL_HDR_CHIP_VER 0xAA
> +#define MRVL_HCI_OP_SET_BAUD 0xFC09
> +#define MRVL_FW_HDR_LEN 5
> +#define MRVL_WAIT_TIMEOUT msecs_to_jiffies(12000)
> +
> +struct mrvl_data {
> + struct sk_buff *rx_skb;
> + struct sk_buff_head txq;
> + struct fw_data *fwdata;
> +};
> +
> +static int get_cts(struct hci_uart *hu)
> +{
> + struct tty_struct *tty = hu->tty;
> + u32 state = tty->ops->tiocmget(tty);
> +
> + if (state & TIOCM_CTS) {
> + bt_dev_dbg(hu->hdev, "CTS is low");
> + return 1;
> + }
> + bt_dev_dbg(hu->hdev, "CTS is high");
> +
> + return 0;
> +}
> +
> +/* Initialize protocol */
> +static int mrvl_open(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl;
> +
> + bt_dev_dbg(hu->hdev, "hu %p", hu);
> +
> + mrvl = kzalloc(sizeof(*mrvl), GFP_KERNEL);
> + if (!mrvl)
> + return -ENOMEM;
> +
> + skb_queue_head_init(&mrvl->txq);
> + hu->priv = mrvl;
> +
> + return 0;
> +}
> +
> +/* Flush protocol data */
> +static int mrvl_flush(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + bt_dev_dbg(hu->hdev, "hu %p", hu);
> +
> + skb_queue_purge(&mrvl->txq);
> +
> + return 0;
> +}
> +
> +/* Close protocol */
> +static int mrvl_close(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + bt_dev_dbg(hu->hdev, "hu %p", hu);
> +
> + skb_queue_purge(&mrvl->txq);
> + kfree_skb(mrvl->rx_skb);
> + hu->priv = NULL;
> + kfree(mrvl);
> +
> + return 0;
> +}
> +
> +/* Enqueue frame for transmittion (padding, crc, etc) */
> +static int mrvl_enqueue(struct hci_uart *hu, struct sk_buff *skb)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + bt_dev_dbg(hu->hdev, "hu %p skb %p", hu, skb);
> +
> + /* Prepend skb with frame type */
> + memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
> + skb_queue_tail(&mrvl->txq, skb);
> +
> + return 0;
> +}
> +
> +static const struct h4_recv_pkt mrvl_recv_pkts[] = {
> + { H4_RECV_ACL, .recv = hci_recv_frame },
> + { H4_RECV_SCO, .recv = hci_recv_frame },
> + { H4_RECV_EVENT, .recv = hci_recv_frame },
> +};
> +
> +/* Send ACK/NAK to the device */
> +static void mrvl_send_ack(struct hci_uart *hu, unsigned char ack)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct sk_buff *skb;
> +
> + skb = bt_skb_alloc(sizeof(ack), GFP_KERNEL);
> + if (!skb)
> + return;
> +
> + memcpy(skb->data, &ack, sizeof(ack));
> + skb_put(skb, sizeof(ack));
> + skb_queue_head(&mrvl->txq, skb);
> + hci_uart_tx_wakeup(hu);
> +}
> +
> +/* Validate the feedback data from device */
> +static void mrvl_pkt_complete(struct hci_uart *hu, struct sk_buff *skb)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> + u16 lhs, rhs;
> +
> + lhs = get_unaligned_le16(skb->data + 1);
> + rhs = get_unaligned_le16(skb->data + 3);
> + if ((lhs ^ rhs) == 0xffff) {
> + mrvl_send_ack(hu, MRVL_ACK);
> + fw_data->wait_fw = 1;
> + fw_data->next_len = lhs;
> + /* Firmware download is done, send the last ack */
> + if (!lhs)
> + fw_data->last_ack = 1;
> +
> + if (fw_data->expected_ack == MRVL_HDR_CHIP_VER) {
> + fw_data->chip_id = skb->data[1];
> + fw_data->chip_rev = skb->data[2];
> + }
> + wake_up_interruptible(&fw_data->init_wait_q);
> + } else {
> + mrvl_send_ack(hu, MRVL_NAK);
> + }
> +}
> +
> +/* This function receives data from the uart device during firmware download.
> + * Driver expects 5 bytes of data as per the protocal in the below format:
> + * <HEADER><BYTE_1><BYTE_2><BYTE_3><BYTE_4>
> + * BYTE_3 and BYTE_4 are compliment of BYTE_1 an BYTE_2. Data can come in chunks
> + * of any length. If length received is < 5, accumulate the data in an array,
> + * until we have a sequence of 5 bytes, starting with the expected HEADER. If
> + * the length received is > 5 bytes, then get the first 5 bytes, starting with
> + * the HEADER and process the same, ignoring the rest of the bytes as per the
> + * protocal.
> + */
> +static struct sk_buff *mrvl_process_fw_data(struct hci_uart *hu,
> + struct sk_buff *skb,
> + u8 *buf, int count)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> + int i = 0, len;
> +
> + if (!skb) {
> + while (buf[i] != fw_data->expected_ack && i < count)
> + i++;
> + if (i == count)
> + return ERR_PTR(-EILSEQ);
> +
> + skb = bt_skb_alloc(MRVL_FW_HDR_LEN, GFP_KERNEL);
> + }
> +
> + if (!skb)
> + return ERR_PTR(-ENOMEM);
> +
> + len = count - i;
> + memcpy(skb_put(skb, len), &buf[i], len);
> +
> + if (skb->len == MRVL_FW_HDR_LEN) {
> + mrvl_pkt_complete(hu, skb);
> + kfree_skb(skb);
> + skb = NULL;
> + }
> +
> + return skb;
> +}
> +
> +/* Receive firmware feedback data */
> +static int mrvl_recv_for_prepare(struct hci_uart *hu, const void *data,
> + int count)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + mrvl->fwdata->skb = mrvl_process_fw_data(hu, mrvl->fwdata->skb,
> + (u8 *)data, count);
> + if (IS_ERR(mrvl->fwdata->skb)) {
> + int err = PTR_ERR(mrvl->fwdata->skb);
> +
> + bt_dev_err(hu->hdev,
> + "Receive firmware data failed (%d)", err);
> + mrvl->fwdata->skb = NULL;
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +/* Receive data */
> +static int mrvl_recv(struct hci_uart *hu, const void *data, int count)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
> + return -EUNATCH;
> +
> + mrvl->rx_skb = h4_recv_buf(hu->hdev, mrvl->rx_skb, data, count,
> + mrvl_recv_pkts, ARRAY_SIZE(mrvl_recv_pkts));
> + if (IS_ERR(mrvl->rx_skb)) {
> + int err = PTR_ERR(mrvl->rx_skb);
> +
> + bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> + mrvl->rx_skb = NULL;
> + return err;
> + }
> +
> + return count;
> +}
> +
> +static struct sk_buff *mrvl_dequeue(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + return skb_dequeue(&mrvl->txq);
> +}
> +
> +static int mrvl_init_fw_data(struct hci_uart *hu)
> +{
> + struct fw_data *fwdata;
> + struct mrvl_data *mrvl = hu->priv;
> +
> + fwdata = kzalloc(sizeof(*fwdata), GFP_KERNEL);
> + if (!fwdata)
> + return -ENOMEM;
> +
> + mrvl->fwdata = fwdata;
> + init_waitqueue_head(&fwdata->init_wait_q);
> +
> + return 0;
> +}
> +
> +/* Wait for the header from device */
> +static int mrvl_wait_for_hdr(struct hci_uart *hu, u8 header)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> +
> + fw_data->expected_ack = header;
> + fw_data->wait_fw = 0;
> +
> + if (!wait_event_interruptible_timeout(fw_data->init_wait_q,
> + fw_data->wait_fw,
> + MRVL_WAIT_TIMEOUT)) {
> + BT_ERR("TIMEOUT, waiting for:0x%x", fw_data->expected_ack);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/* Send bytes to device */
> +static int mrvl_send_data(struct hci_uart *hu, struct sk_buff *skb)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> +
> + skb_queue_head(&mrvl->txq, skb);
> + hci_uart_tx_wakeup(hu);
> +
> + if (mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW) == -1)
> + return -1;
> +
> + return 0;
> +}
> +
> +/* Download firmware to the device */
> +static int mrvl_dnld_fw(struct hci_uart *hu, const char *file_name)
> +{
> + const struct firmware *fw = NULL;
> + struct sk_buff *skb = NULL;
> + int offset = 0;
> + int ret, tx_len;
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> +
> + ret = request_firmware(&fw, file_name, hu->tty->dev);
> + if (ret < 0) {
> + BT_ERR("request_firmware() failed");
> + return -1;
> + }
> +
> + BT_INFO("Downloading FW (%d bytes)", (u16)fw->size);
> +
> + fw_data->last_ack = 0;
> +
> + while (1) {
> + if ((offset >= fw->size) || (fw_data->last_ack))
> + break;
> + tx_len = fw_data->next_len;
> + if ((fw->size - offset) < tx_len)
> + tx_len = fw->size - offset;
> +
> + skb = bt_skb_alloc(MRVL_MAX_FW_BLOCK_SIZE, GFP_KERNEL);
> + if (!skb) {
> + ret = -1;
> + goto done;
> + }
> +
> + memcpy(skb->data, &fw->data[offset], tx_len);
> + skb_put(skb, tx_len);
> + if (mrvl_send_data(hu, skb) != 0) {
> + BT_ERR("Fail to download firmware");
> + ret = -1;
> + goto done;
> + }
> + offset += tx_len;
> + }
> +
> + BT_INFO("Downloaded %d byte firmware", offset);
> +done:
> + release_firmware(fw);
> +
> + return ret;
> +}
> +
> +/* Set the baud rate */
> +static int mrvl_set_dev_baud(struct hci_uart *hu)
> +{
> + struct hci_dev *hdev = hu->hdev;
> + struct sk_buff *skb;
> + static const u8 baud_param[] = { 0xc0, 0xc6, 0x2d, 0x00 };
> + int err;
> +
> + skb = __hci_cmd_sync(hdev, MRVL_HCI_OP_SET_BAUD, sizeof(baud_param),
> + baud_param, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + bt_dev_err(hu->hdev, "Set device baudrate failed (%d)", err);
> + return err;
> + }
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> +/* Reset device */
> +static int mrvl_reset(struct hci_uart *hu)
> +{
> + struct hci_dev *hdev = hu->hdev;
> + struct sk_buff *skb;
> + int err;
> +
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_CMD_TIMEOUT);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + bt_dev_err(hu->hdev, "Reset device failed (%d)", err);
> + return err;
> + }
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> +static int mrvl_get_fw_name(struct hci_uart *hu, char *fw_name)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + struct fw_data *fw_data = mrvl->fwdata;
> +
> + if (mrvl_wait_for_hdr(hu, MRVL_HDR_CHIP_VER) != 0) {
> + BT_ERR("Could not read chip id and revision");
> + return -1;
> + }
> +
> + BT_DBG("chip_id=0x%x, chip_rev=0x%x",
> + fw_data->chip_id, fw_data->chip_rev);
> +
> + switch (fw_data->chip_id) {
> + case MRVL_8997_CHIP_ID:
> + memcpy(fw_name, MRVL_8997_FW_NAME, sizeof(MRVL_8997_FW_NAME));
> + return 0;
> + default:
> + BT_ERR("Invalid chip id");
> + return -1;
> + }
> +}
> +
> +/* Download helper and firmare to device */
> +static int hci_uart_dnld_fw(struct hci_uart *hu)
> +{
> + struct tty_struct *tty = hu->tty;
> + struct ktermios new_termios;
> + struct ktermios old_termios;
> + char fw_name[128];
> + int ret;
> +
> + old_termios = tty->termios;
> +
> + if (get_cts(hu)) {
> + BT_INFO("fw is running");
> + return 0;
> + }
> +
> + hci_uart_set_baudrate(hu, 115200);
> + hci_uart_set_flow_control(hu, true);
> +
> + ret = mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW);
> + if (ret)
> + goto fail;
> +
> + ret = mrvl_dnld_fw(hu, MRVL_HELPER_NAME);
> + if (ret)
> + goto fail;
> +
> + msleep(MRVL_DNLD_DELAY);
> +
> + hci_uart_set_baudrate(hu, 3000000);
> + hci_uart_set_flow_control(hu, false);
> +
> + ret = mrvl_get_fw_name(hu, fw_name);
> + if (ret)
> + goto fail;
> +
> + ret = mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW);
> + if (ret)
> + goto fail;
> +
> + ret = mrvl_dnld_fw(hu, fw_name);
> + if (ret)
> + goto fail;
> +
> + msleep(MRVL_DNLD_DELAY);
> +fail:
> + /* restore uart settings */
> + new_termios = tty->termios;
> + tty->termios.c_cflag = old_termios.c_cflag;
> + tty_set_termios(tty, &new_termios);
> +
> + return ret;
> +}
> +
> +static int mrvl_setup(struct hci_uart *hu)
> +{
> + int err;
> +
> + hci_uart_set_baudrate(hu, 115200);
> + hci_uart_set_flow_control(hu, false);
> +
> + err = mrvl_reset(hu);
> + if (!err) {
> + err = mrvl_set_dev_baud(hu);
> + if (err)
> + return -1;
> + }
> +
> + hci_uart_set_baudrate(hu, 3000000);
> + hci_uart_set_flow_control(hu, false);
> + msleep(MRVL_DNLD_DELAY);
> +
> + return 0;
> +}
> +
> +static int mrvl_prepare(struct hci_uart *hu)
> +{
> + struct mrvl_data *mrvl = hu->priv;
> + int err;
> +
> + err = mrvl_init_fw_data(hu);
> + if (!err)
> + err = hci_uart_dnld_fw(hu);
> +
> + kfree(mrvl->fwdata);
> + return err;
> +}
> +
> +static const struct hci_uart_proto mrvlp = {
> + .id = HCI_UART_MRVL,
> + .name = "MRVL",
> + .open = mrvl_open,
> + .close = mrvl_close,
> + .recv = mrvl_recv,
> + .enqueue = mrvl_enqueue,
> + .dequeue = mrvl_dequeue,
> + .flush = mrvl_flush,
> + .setup = mrvl_setup,
> + .prepare = mrvl_prepare,
> + .recv_for_prepare = mrvl_recv_for_prepare,
> +};
> +
> +int __init mrvl_init(void)
> +{
> + return hci_uart_register_proto(&mrvlp);
> +}
> +
> +int __exit mrvl_deinit(void)
> +{
> + return hci_uart_unregister_proto(&mrvlp);
> +}
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index 17ba3b4..8c53b50 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -35,7 +35,7 @@
> #define HCIUARTGETFLAGS _IOR('U', 204, int)
>
> /* UART protocols */
> -#define HCI_UART_MAX_PROTO 10
> +#define HCI_UART_MAX_PROTO 11
>
> #define HCI_UART_H4 0
> #define HCI_UART_BCSP 1
> @@ -47,6 +47,7 @@
> #define HCI_UART_BCM 7
> #define HCI_UART_QCA 8
> #define HCI_UART_AG6XX 9
> +#define HCI_UART_MRVL 10
>
> #define HCI_UART_RAW_DEVICE 0
> #define HCI_UART_RESET_ON_INIT 1
> @@ -192,3 +193,8 @@ int qca_deinit(void);
> int ag6xx_init(void);
> int ag6xx_deinit(void);
> #endif
> +
> +#ifdef CONFIG_BT_HCIUART_MRVL
> +int mrvl_init(void);
> +int mrvl_deinit(void);
> +#endif
>
>
This serial of patches work well on my device...
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [v11,3/3] Bluetooth: hci_uart: Support firmware download for Marvell
2016-06-24 6:02 ` Jeffy Chen
@ 2016-06-27 6:28 ` Amitkumar Karwar
2016-06-30 10:54 ` Loic Poulain
0 siblings, 1 reply; 13+ messages in thread
From: Amitkumar Karwar @ 2016-06-27 6:28 UTC (permalink / raw)
To: Jeffy Chen, linux-bluetooth; +Cc: linux-kernel, Ganapathi Bhat, Cathy Luo
Hi Marcel,
> From: Jeffy Chen [mailto:jeffy.chen@rock-chips.com]
> Sent: Friday, June 24, 2016 11:32 AM
> To: Amitkumar Karwar; linux-bluetooth@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Ganapathi Bhat
> Subject: Re: [v11,3/3] Bluetooth: hci_uart: Support firmware download
> for Marvell
>
> On 2016-5-6 23:31, Amitkumar Karwar wrote:
> > From: Ganapathi Bhat <gbhat@marvell.com>
> >
> > This patch implement firmware download feature for Marvell Bluetooth
> > devices. If firmware is already downloaded, it will skip downloading.
> >
> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> > 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
> > v11: Write data through ldisc in mrvl_send_ack() instead of directly
> calling
> > write method(One Thousand Gnomes).
> > ---
> > drivers/bluetooth/Kconfig | 11 +
> > drivers/bluetooth/Makefile | 1 +
> > drivers/bluetooth/hci_ldisc.c | 6 +
> > drivers/bluetooth/hci_mrvl.c | 543
> ++++++++++++++++++++++++++++++++++++++++++
> > drivers/bluetooth/hci_uart.h | 8 +-
> > 5 files changed, 568 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
> > 0000000..2686901
> > --- /dev/null
> > +++ b/drivers/bluetooth/hci_mrvl.c
> > @@ -0,0 +1,543 @@
> > +/* 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 Version 2,
> > +June 1991
> > + * (the "License"). You may use, redistribute and/or modify this
> > +File in
> > + * accordance with the terms and conditions of the License, a copy of
> > +which
> > + * is available on the worldwide web at
> > + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
> > + *
> > + * THE FILE IS DISTRIBUTED AS-IS, WITHOUT WARRANTY OF ANY KIND, AND
> > +THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR
> > +PURPOSE
> > + * ARE EXPRESSLY DISCLAIMED. The License provides additional details
> > +about
> > + * this warranty disclaimer.
> > + */
> > +
> > +#include <linux/firmware.h>
> > +#include <linux/tty.h>
> > +#include <asm/unaligned.h>
> > +#include <net/bluetooth/bluetooth.h>
> > +#include <net/bluetooth/hci_core.h>
> > +#include "hci_uart.h"
> > +
> > +struct fw_data {
> > + wait_queue_head_t init_wait_q;
> > + u8 wait_fw;
> > + int next_len;
> > + u8 five_bytes[5];
> > + u8 next_index;
> > + u8 last_ack;
> > + u8 expected_ack;
> > + struct ktermios old_termios;
> > + u8 chip_id;
> > + u8 chip_rev;
> > + struct sk_buff *skb;
> > +};
> > +
> > +#define MRVL_HELPER_NAME "mrvl/helper_uart_3000000.bin"
> > +#define MRVL_8997_CHIP_ID 0x50
> > +#define MRVL_8997_FW_NAME "mrvl/uart8997_bt.bin"
> > +#define MRVL_MAX_FW_BLOCK_SIZE 1024
> > +#define MRVL_MAX_RETRY_SEND 12
> > +#define MRVL_DNLD_DELAY 100
> > +#define MRVL_ACK 0x5A
> > +#define MRVL_NAK 0xBF
> > +#define MRVL_HDR_REQ_FW 0xA5
> > +#define MRVL_HDR_CHIP_VER 0xAA
> > +#define MRVL_HCI_OP_SET_BAUD 0xFC09
> > +#define MRVL_FW_HDR_LEN 5
> > +#define MRVL_WAIT_TIMEOUT msecs_to_jiffies(12000)
> > +
> > +struct mrvl_data {
> > + struct sk_buff *rx_skb;
> > + struct sk_buff_head txq;
> > + struct fw_data *fwdata;
> > +};
> > +
> > +static int get_cts(struct hci_uart *hu) {
> > + struct tty_struct *tty = hu->tty;
> > + u32 state = tty->ops->tiocmget(tty);
> > +
> > + if (state & TIOCM_CTS) {
> > + bt_dev_dbg(hu->hdev, "CTS is low");
> > + return 1;
> > + }
> > + bt_dev_dbg(hu->hdev, "CTS is high");
> > +
> > + return 0;
> > +}
> > +
> > +/* Initialize protocol */
> > +static int mrvl_open(struct hci_uart *hu) {
> > + struct mrvl_data *mrvl;
> > +
> > + bt_dev_dbg(hu->hdev, "hu %p", hu);
> > +
> > + mrvl = kzalloc(sizeof(*mrvl), GFP_KERNEL);
> > + if (!mrvl)
> > + return -ENOMEM;
> > +
> > + skb_queue_head_init(&mrvl->txq);
> > + hu->priv = mrvl;
> > +
> > + return 0;
> > +}
> > +
> > +/* Flush protocol data */
> > +static int mrvl_flush(struct hci_uart *hu) {
> > + struct mrvl_data *mrvl = hu->priv;
> > +
> > + bt_dev_dbg(hu->hdev, "hu %p", hu);
> > +
> > + skb_queue_purge(&mrvl->txq);
> > +
> > + return 0;
> > +}
> > +
> > +/* Close protocol */
> > +static int mrvl_close(struct hci_uart *hu) {
> > + struct mrvl_data *mrvl = hu->priv;
> > +
> > + bt_dev_dbg(hu->hdev, "hu %p", hu);
> > +
> > + skb_queue_purge(&mrvl->txq);
> > + kfree_skb(mrvl->rx_skb);
> > + hu->priv = NULL;
> > + kfree(mrvl);
> > +
> > + return 0;
> > +}
> > +
> > +/* Enqueue frame for transmittion (padding, crc, etc) */ static int
> > +mrvl_enqueue(struct hci_uart *hu, struct sk_buff *skb) {
> > + struct mrvl_data *mrvl = hu->priv;
> > +
> > + bt_dev_dbg(hu->hdev, "hu %p skb %p", hu, skb);
> > +
> > + /* Prepend skb with frame type */
> > + memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
> > + skb_queue_tail(&mrvl->txq, skb);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct h4_recv_pkt mrvl_recv_pkts[] = {
> > + { H4_RECV_ACL, .recv = hci_recv_frame },
> > + { H4_RECV_SCO, .recv = hci_recv_frame },
> > + { H4_RECV_EVENT, .recv = hci_recv_frame }, };
> > +
> > +/* Send ACK/NAK to the device */
> > +static void mrvl_send_ack(struct hci_uart *hu, unsigned char ack) {
> > + struct mrvl_data *mrvl = hu->priv;
> > + struct sk_buff *skb;
> > +
> > + skb = bt_skb_alloc(sizeof(ack), GFP_KERNEL);
> > + if (!skb)
> > + return;
> > +
> > + memcpy(skb->data, &ack, sizeof(ack));
> > + skb_put(skb, sizeof(ack));
> > + skb_queue_head(&mrvl->txq, skb);
> > + hci_uart_tx_wakeup(hu);
> > +}
> > +
> > +/* Validate the feedback data from device */ static void
> > +mrvl_pkt_complete(struct hci_uart *hu, struct sk_buff *skb) {
> > + struct mrvl_data *mrvl = hu->priv;
> > + struct fw_data *fw_data = mrvl->fwdata;
> > + u16 lhs, rhs;
> > +
> > + lhs = get_unaligned_le16(skb->data + 1);
> > + rhs = get_unaligned_le16(skb->data + 3);
> > + if ((lhs ^ rhs) == 0xffff) {
> > + mrvl_send_ack(hu, MRVL_ACK);
> > + fw_data->wait_fw = 1;
> > + fw_data->next_len = lhs;
> > + /* Firmware download is done, send the last ack */
> > + if (!lhs)
> > + fw_data->last_ack = 1;
> > +
> > + if (fw_data->expected_ack == MRVL_HDR_CHIP_VER) {
> > + fw_data->chip_id = skb->data[1];
> > + fw_data->chip_rev = skb->data[2];
> > + }
> > + wake_up_interruptible(&fw_data->init_wait_q);
> > + } else {
> > + mrvl_send_ack(hu, MRVL_NAK);
> > + }
> > +}
> > +
> > +/* This function receives data from the uart device during firmware
> download.
> > + * Driver expects 5 bytes of data as per the protocal in the below
> format:
> > + * <HEADER><BYTE_1><BYTE_2><BYTE_3><BYTE_4>
> > + * BYTE_3 and BYTE_4 are compliment of BYTE_1 an BYTE_2. Data can
> > +come in chunks
> > + * of any length. If length received is < 5, accumulate the data in
> > +an array,
> > + * until we have a sequence of 5 bytes, starting with the expected
> > +HEADER. If
> > + * the length received is > 5 bytes, then get the first 5 bytes,
> > +starting with
> > + * the HEADER and process the same, ignoring the rest of the bytes as
> > +per the
> > + * protocal.
> > + */
> > +static struct sk_buff *mrvl_process_fw_data(struct hci_uart *hu,
> > + struct sk_buff *skb,
> > + u8 *buf, int count)
> > +{
> > + struct mrvl_data *mrvl = hu->priv;
> > + struct fw_data *fw_data = mrvl->fwdata;
> > + int i = 0, len;
> > +
> > + if (!skb) {
> > + while (buf[i] != fw_data->expected_ack && i < count)
> > + i++;
> > + if (i == count)
> > + return ERR_PTR(-EILSEQ);
> > +
> > + skb = bt_skb_alloc(MRVL_FW_HDR_LEN, GFP_KERNEL);
> > + }
> > +
> > + if (!skb)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + len = count - i;
> > + memcpy(skb_put(skb, len), &buf[i], len);
> > +
> > + if (skb->len == MRVL_FW_HDR_LEN) {
> > + mrvl_pkt_complete(hu, skb);
> > + kfree_skb(skb);
> > + skb = NULL;
> > + }
> > +
> > + return skb;
> > +}
> > +
> > +/* Receive firmware feedback data */
> > +static int mrvl_recv_for_prepare(struct hci_uart *hu, const void
> *data,
> > + int count)
> > +{
> > + struct mrvl_data *mrvl = hu->priv;
> > +
> > + mrvl->fwdata->skb = mrvl_process_fw_data(hu, mrvl->fwdata->skb,
> > + (u8 *)data, count);
> > + if (IS_ERR(mrvl->fwdata->skb)) {
> > + int err = PTR_ERR(mrvl->fwdata->skb);
> > +
> > + bt_dev_err(hu->hdev,
> > + "Receive firmware data failed (%d)", err);
> > + mrvl->fwdata->skb = NULL;
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* Receive data */
> > +static int mrvl_recv(struct hci_uart *hu, const void *data, int
> > +count) {
> > + struct mrvl_data *mrvl = hu->priv;
> > +
> > + if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
> > + return -EUNATCH;
> > +
> > + mrvl->rx_skb = h4_recv_buf(hu->hdev, mrvl->rx_skb, data, count,
> > + mrvl_recv_pkts, ARRAY_SIZE(mrvl_recv_pkts));
> > + if (IS_ERR(mrvl->rx_skb)) {
> > + int err = PTR_ERR(mrvl->rx_skb);
> > +
> > + bt_dev_err(hu->hdev, "Frame reassembly failed (%d)", err);
> > + mrvl->rx_skb = NULL;
> > + return err;
> > + }
> > +
> > + return count;
> > +}
> > +
> > +static struct sk_buff *mrvl_dequeue(struct hci_uart *hu) {
> > + struct mrvl_data *mrvl = hu->priv;
> > +
> > + return skb_dequeue(&mrvl->txq);
> > +}
> > +
> > +static int mrvl_init_fw_data(struct hci_uart *hu) {
> > + struct fw_data *fwdata;
> > + struct mrvl_data *mrvl = hu->priv;
> > +
> > + fwdata = kzalloc(sizeof(*fwdata), GFP_KERNEL);
> > + if (!fwdata)
> > + return -ENOMEM;
> > +
> > + mrvl->fwdata = fwdata;
> > + init_waitqueue_head(&fwdata->init_wait_q);
> > +
> > + return 0;
> > +}
> > +
> > +/* Wait for the header from device */ static int
> > +mrvl_wait_for_hdr(struct hci_uart *hu, u8 header) {
> > + struct mrvl_data *mrvl = hu->priv;
> > + struct fw_data *fw_data = mrvl->fwdata;
> > +
> > + fw_data->expected_ack = header;
> > + fw_data->wait_fw = 0;
> > +
> > + if (!wait_event_interruptible_timeout(fw_data->init_wait_q,
> > + fw_data->wait_fw,
> > + MRVL_WAIT_TIMEOUT)) {
> > + BT_ERR("TIMEOUT, waiting for:0x%x", fw_data->expected_ack);
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* Send bytes to device */
> > +static int mrvl_send_data(struct hci_uart *hu, struct sk_buff *skb) {
> > + struct mrvl_data *mrvl = hu->priv;
> > +
> > + skb_queue_head(&mrvl->txq, skb);
> > + hci_uart_tx_wakeup(hu);
> > +
> > + if (mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW) == -1)
> > + return -1;
> > +
> > + return 0;
> > +}
> > +
> > +/* Download firmware to the device */ static int mrvl_dnld_fw(struct
> > +hci_uart *hu, const char *file_name) {
> > + const struct firmware *fw = NULL;
> > + struct sk_buff *skb = NULL;
> > + int offset = 0;
> > + int ret, tx_len;
> > + struct mrvl_data *mrvl = hu->priv;
> > + struct fw_data *fw_data = mrvl->fwdata;
> > +
> > + ret = request_firmware(&fw, file_name, hu->tty->dev);
> > + if (ret < 0) {
> > + BT_ERR("request_firmware() failed");
> > + return -1;
> > + }
> > +
> > + BT_INFO("Downloading FW (%d bytes)", (u16)fw->size);
> > +
> > + fw_data->last_ack = 0;
> > +
> > + while (1) {
> > + if ((offset >= fw->size) || (fw_data->last_ack))
> > + break;
> > + tx_len = fw_data->next_len;
> > + if ((fw->size - offset) < tx_len)
> > + tx_len = fw->size - offset;
> > +
> > + skb = bt_skb_alloc(MRVL_MAX_FW_BLOCK_SIZE, GFP_KERNEL);
> > + if (!skb) {
> > + ret = -1;
> > + goto done;
> > + }
> > +
> > + memcpy(skb->data, &fw->data[offset], tx_len);
> > + skb_put(skb, tx_len);
> > + if (mrvl_send_data(hu, skb) != 0) {
> > + BT_ERR("Fail to download firmware");
> > + ret = -1;
> > + goto done;
> > + }
> > + offset += tx_len;
> > + }
> > +
> > + BT_INFO("Downloaded %d byte firmware", offset);
> > +done:
> > + release_firmware(fw);
> > +
> > + return ret;
> > +}
> > +
> > +/* Set the baud rate */
> > +static int mrvl_set_dev_baud(struct hci_uart *hu) {
> > + struct hci_dev *hdev = hu->hdev;
> > + struct sk_buff *skb;
> > + static const u8 baud_param[] = { 0xc0, 0xc6, 0x2d, 0x00 };
> > + int err;
> > +
> > + skb = __hci_cmd_sync(hdev, MRVL_HCI_OP_SET_BAUD,
> sizeof(baud_param),
> > + baud_param, HCI_INIT_TIMEOUT);
> > + if (IS_ERR(skb)) {
> > + err = PTR_ERR(skb);
> > + bt_dev_err(hu->hdev, "Set device baudrate failed (%d)",
> err);
> > + return err;
> > + }
> > + kfree_skb(skb);
> > +
> > + return 0;
> > +}
> > +
> > +/* Reset device */
> > +static int mrvl_reset(struct hci_uart *hu) {
> > + struct hci_dev *hdev = hu->hdev;
> > + struct sk_buff *skb;
> > + int err;
> > +
> > + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
> HCI_CMD_TIMEOUT);
> > + if (IS_ERR(skb)) {
> > + err = PTR_ERR(skb);
> > + bt_dev_err(hu->hdev, "Reset device failed (%d)", err);
> > + return err;
> > + }
> > + kfree_skb(skb);
> > +
> > + return 0;
> > +}
> > +
> > +static int mrvl_get_fw_name(struct hci_uart *hu, char *fw_name) {
> > + struct mrvl_data *mrvl = hu->priv;
> > + struct fw_data *fw_data = mrvl->fwdata;
> > +
> > + if (mrvl_wait_for_hdr(hu, MRVL_HDR_CHIP_VER) != 0) {
> > + BT_ERR("Could not read chip id and revision");
> > + return -1;
> > + }
> > +
> > + BT_DBG("chip_id=0x%x, chip_rev=0x%x",
> > + fw_data->chip_id, fw_data->chip_rev);
> > +
> > + switch (fw_data->chip_id) {
> > + case MRVL_8997_CHIP_ID:
> > + memcpy(fw_name, MRVL_8997_FW_NAME,
> sizeof(MRVL_8997_FW_NAME));
> > + return 0;
> > + default:
> > + BT_ERR("Invalid chip id");
> > + return -1;
> > + }
> > +}
> > +
> > +/* Download helper and firmare to device */ static int
> > +hci_uart_dnld_fw(struct hci_uart *hu) {
> > + struct tty_struct *tty = hu->tty;
> > + struct ktermios new_termios;
> > + struct ktermios old_termios;
> > + char fw_name[128];
> > + int ret;
> > +
> > + old_termios = tty->termios;
> > +
> > + if (get_cts(hu)) {
> > + BT_INFO("fw is running");
> > + return 0;
> > + }
> > +
> > + hci_uart_set_baudrate(hu, 115200);
> > + hci_uart_set_flow_control(hu, true);
> > +
> > + ret = mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW);
> > + if (ret)
> > + goto fail;
> > +
> > + ret = mrvl_dnld_fw(hu, MRVL_HELPER_NAME);
> > + if (ret)
> > + goto fail;
> > +
> > + msleep(MRVL_DNLD_DELAY);
> > +
> > + hci_uart_set_baudrate(hu, 3000000);
> > + hci_uart_set_flow_control(hu, false);
> > +
> > + ret = mrvl_get_fw_name(hu, fw_name);
> > + if (ret)
> > + goto fail;
> > +
> > + ret = mrvl_wait_for_hdr(hu, MRVL_HDR_REQ_FW);
> > + if (ret)
> > + goto fail;
> > +
> > + ret = mrvl_dnld_fw(hu, fw_name);
> > + if (ret)
> > + goto fail;
> > +
> > + msleep(MRVL_DNLD_DELAY);
> > +fail:
> > + /* restore uart settings */
> > + new_termios = tty->termios;
> > + tty->termios.c_cflag = old_termios.c_cflag;
> > + tty_set_termios(tty, &new_termios);
> > +
> > + return ret;
> > +}
> > +
> > +static int mrvl_setup(struct hci_uart *hu) {
> > + int err;
> > +
> > + hci_uart_set_baudrate(hu, 115200);
> > + hci_uart_set_flow_control(hu, false);
> > +
> > + err = mrvl_reset(hu);
> > + if (!err) {
> > + err = mrvl_set_dev_baud(hu);
> > + if (err)
> > + return -1;
> > + }
> > +
> > + hci_uart_set_baudrate(hu, 3000000);
> > + hci_uart_set_flow_control(hu, false);
> > + msleep(MRVL_DNLD_DELAY);
> > +
> > + return 0;
> > +}
> > +
> > +static int mrvl_prepare(struct hci_uart *hu) {
> > + struct mrvl_data *mrvl = hu->priv;
> > + int err;
> > +
> > + err = mrvl_init_fw_data(hu);
> > + if (!err)
> > + err = hci_uart_dnld_fw(hu);
> > +
> > + kfree(mrvl->fwdata);
> > + return err;
> > +}
> > +
> > +static const struct hci_uart_proto mrvlp = {
> > + .id = HCI_UART_MRVL,
> > + .name = "MRVL",
> > + .open = mrvl_open,
> > + .close = mrvl_close,
> > + .recv = mrvl_recv,
> > + .enqueue = mrvl_enqueue,
> > + .dequeue = mrvl_dequeue,
> > + .flush = mrvl_flush,
> > + .setup = mrvl_setup,
> > + .prepare = mrvl_prepare,
> > + .recv_for_prepare = mrvl_recv_for_prepare,
> > +};
> > +
> > +int __init mrvl_init(void)
> > +{
> > + return hci_uart_register_proto(&mrvlp); }
> > +
> > +int __exit mrvl_deinit(void)
> > +{
> > + return hci_uart_unregister_proto(&mrvlp);
> > +}
> > diff --git a/drivers/bluetooth/hci_uart.h
> > b/drivers/bluetooth/hci_uart.h index 17ba3b4..8c53b50 100644
> > --- a/drivers/bluetooth/hci_uart.h
> > +++ b/drivers/bluetooth/hci_uart.h
> > @@ -35,7 +35,7 @@
> > #define HCIUARTGETFLAGS _IOR('U', 204, int)
> >
> > /* UART protocols */
> > -#define HCI_UART_MAX_PROTO 10
> > +#define HCI_UART_MAX_PROTO 11
> >
> > #define HCI_UART_H4 0
> > #define HCI_UART_BCSP 1
> > @@ -47,6 +47,7 @@
> > #define HCI_UART_BCM 7
> > #define HCI_UART_QCA 8
> > #define HCI_UART_AG6XX 9
> > +#define HCI_UART_MRVL 10
> >
> > #define HCI_UART_RAW_DEVICE 0
> > #define HCI_UART_RESET_ON_INIT 1
> > @@ -192,3 +193,8 @@ int qca_deinit(void);
> > int ag6xx_init(void);
> > int ag6xx_deinit(void);
> > #endif
> > +
> > +#ifdef CONFIG_BT_HCIUART_MRVL
> > +int mrvl_init(void);
> > +int mrvl_deinit(void);
> > +#endif
> >
> >
> This serial of patches work well on my device...
>
> Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Any further comments on this patch series? It’s been pending for long time.
Regards,
Amitkumar
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [v11,3/3] Bluetooth: hci_uart: Support firmware download for Marvell
2016-06-27 6:28 ` Amitkumar Karwar
@ 2016-06-30 10:54 ` Loic Poulain
2016-08-09 16:18 ` Amitkumar Karwar
0 siblings, 1 reply; 13+ messages in thread
From: Loic Poulain @ 2016-06-30 10:54 UTC (permalink / raw)
To: Amitkumar Karwar, Jeffy Chen, linux-bluetooth
Cc: linux-kernel, Ganapathi Bhat, Cathy Luo, Marcel Holtmann
Hi Amitkumar,
> Hi Marcel,
I suggest you to add Marcel as recipient of your patches.
>
>> From: Jeffy Chen [mailto:jeffy.chen@rock-chips.com]
>> Sent: Friday, June 24, 2016 11:32 AM
>> To: Amitkumar Karwar; linux-bluetooth@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org; Ganapathi Bhat
>> Subject: Re: [v11,3/3] Bluetooth: hci_uart: Support firmware download
>> for Marvell
>>
>> On 2016-5-6 23:31, Amitkumar Karwar wrote:
>>> From: Ganapathi Bhat <gbhat@marvell.com>
>>>
>>> This patch implement firmware download feature for Marvell Bluetooth
>>> devices. If firmware is already downloaded, it will skip downloading.
>>> +static struct sk_buff *mrvl_process_fw_data(struct hci_uart *hu,
>>> + struct sk_buff *skb,
>>> + u8 *buf, int count)
>>> +{
>>> + struct mrvl_data *mrvl = hu->priv;
>>> + struct fw_data *fw_data = mrvl->fwdata;
>>> + int i = 0, len;
>>> +
>>> + if (!skb) {
>>> + while (buf[i] != fw_data->expected_ack && i < count)
>>> + i++;
>>> + if (i == count)
>>> + return ERR_PTR(-EILSEQ);
>>> +
>>> + skb = bt_skb_alloc(MRVL_FW_HDR_LEN, GFP_KERNEL);
Why you don't test skb here.
>>> + }
>>> +
>>> + if (!skb)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + len = count - i;
>>> + memcpy(skb_put(skb, len), &buf[i], len);
You copy all the remaining data from buf into your skb, but what if buf
contains more than one packet ? out of skb.
Don't assume that buf contains a full packet as well as only one packet.
Regards,
Loic
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [v11,3/3] Bluetooth: hci_uart: Support firmware download for Marvell
2016-06-30 10:54 ` Loic Poulain
@ 2016-08-09 16:18 ` Amitkumar Karwar
0 siblings, 0 replies; 13+ messages in thread
From: Amitkumar Karwar @ 2016-08-09 16:18 UTC (permalink / raw)
To: Loic Poulain, Jeffy Chen, linux-bluetooth
Cc: linux-kernel, Ganapathi Bhat, Marcel Holtmann
Hi Loic,
> -----Original Message-----
> From: Loic Poulain [mailto:loic.poulain@intel.com]
> Sent: Thursday, June 30, 2016 4:24 PM
> To: Amitkumar Karwar; Jeffy Chen; linux-bluetooth@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Ganapathi Bhat; Cathy Luo; Marcel
> Holtmann
> Subject: Re: [v11,3/3] Bluetooth: hci_uart: Support firmware download
> for Marvell
>
> Hi Amitkumar,
>
>
> > Hi Marcel,
>
> I suggest you to add Marcel as recipient of your patches.
>
> >
> >> From: Jeffy Chen [mailto:jeffy.chen@rock-chips.com]
> >> Sent: Friday, June 24, 2016 11:32 AM
> >> To: Amitkumar Karwar; linux-bluetooth@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org; Ganapathi Bhat
> >> Subject: Re: [v11,3/3] Bluetooth: hci_uart: Support firmware download
> >> for Marvell
> >>
> >> On 2016-5-6 23:31, Amitkumar Karwar wrote:
> >>> From: Ganapathi Bhat <gbhat@marvell.com>
> >>>
> >>> This patch implement firmware download feature for Marvell Bluetooth
> >>> devices. If firmware is already downloaded, it will skip
> downloading.
>
>
> >>> +static struct sk_buff *mrvl_process_fw_data(struct hci_uart *hu,
> >>> + struct sk_buff *skb,
> >>> + u8 *buf, int count)
> >>> +{
> >>> + struct mrvl_data *mrvl = hu->priv;
> >>> + struct fw_data *fw_data = mrvl->fwdata;
> >>> + int i = 0, len;
> >>> +
> >>> + if (!skb) {
> >>> + while (buf[i] != fw_data->expected_ack && i < count)
> >>> + i++;
> >>> + if (i == count)
> >>> + return ERR_PTR(-EILSEQ);
> >>> +
> >>> + skb = bt_skb_alloc(MRVL_FW_HDR_LEN, GFP_KERNEL);
>
> Why you don't test skb here.
>
> >>> + }
> >>> +
> >>> + if (!skb)
> >>> + return ERR_PTR(-ENOMEM);
> >>> +
> >>> + len = count - i;
> >>> + memcpy(skb_put(skb, len), &buf[i], len);
>
> You copy all the remaining data from buf into your skb, but what if buf
> contains more than one packet ? out of skb.
> Don't assume that buf contains a full packet as well as only one packet.
>
>
Thanks for your comments. We have added necessarily checks to address these comments. I will submit updated version shortly.
Regards,
Amitkumar Karwar
^ permalink raw reply [flat|nested] 13+ messages in thread