Hi Amitkumar,
> Thanks for your review.
> We will address these comments in updated version.
>
>>> +
>>> +/* 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_DNLD_FW, &mrvl->flags)) {
>>> + 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;
>>> + }
>>
>> This part actually worries me a bit. Yes, we can do it this way, but in
>> general it sounds a bit more like we need a generic approach in
>> hci_ldisc.c to handle pre-HCI firmware loading and/or setup.
>>
>> In the btusb.c driver we added ->setup_on_usb callback. And for it
>> sounds like we need something similar here. So that hci_ldisc.c can
>> handle most of the basic TX/RX.
>
> Even if we added "->setup_on_usb" in hci_ldisc.c, we will need protocol
> specific changes in receive path during firmware download.
> With this patch, those changes are smoothly handled in hci_mrvl.c file.
> [hci_uart_tty_receive] -> [proto->recv] -> [mrvl_recv] -> [normal Rx path/FW
> download Rx handling]
we might need to find a better callback name, but yes, we want to make this
generic in hci_ldisc.c. I do not have a perfect solution or quick answer, but I
think this needs a bit more generic framework.
Regards
Marcel
