Hi Suraj, > >> The function "hci_recv_fragment()" was designed to avoid messy Bluetooth Rx packet reassembly on the HCI transport driver. > >> It does work well for HCI USB transport driver but it becomes a bit redundant when used on HCI UART transport driver. > >> > >> This is basically due to the fact that the function require the caller to provide the HCI Packet type as input parameter. > >> > >> This is pretty straight forward for a BT USB transport driver as both ACL data and HCI events are received through different callbacks. > >> Which means you will have 2 calls of hci_recv_fragment(). One with HCI_EVENT_PKT as packet type and other with HCI_ACLDATA_PKT, with packet type hard coded. > >> > >> As far as HCI UART transport driver is concerned, it does not have this luxury. Both event and data are received through the same callback. > >> So, if the driver has to provide the packet type as input to hci_recv_fragment(), it will have to parse the HCI Rx data to get it in the first place. > >> > >> This means driver will have to do everything hci_recv_fragment() does minus the memcpy, implementing the same messy code. > >> > >> I know that we should be able to work around it by checking whether which reassembly buffer is not null and so on. But this is just a hack not a solution. > > > > I remember why I added the packet type to the function. The reason is > > that events and ACL packets arrive on different USB endpoints and in > > theory they can arrive at exactly the same time or intermix with each > > other. So that needs to be protected. > > > > So I think we need something like hci_recv_packet_fragment and > > hci_recv_stream_fragment. > > > > This would result at least in that we can consolidate all this code > > duplication in the Bluetooth core. > > Yes, that would be great. Do you want me to wait for this implementation > before resubmitting the patch? I actually expect you to work on such a patch that add this to the core first. Then you can submit your patch. > >> The second reason is, hci_recv_fragment() implements a certain policy on the driver i.e > >> > >> " Whenever HCI Rx data is reassembled it directly has to be sent to host, without the driver interfering". > >> > >> This robs the driver a chance have a look at the HCI event and do some housekeeping (it is entirely up to the driver what he wants to do then). > >> > >> This is the one reason why someone would write a custom HCI transport driver protocol. > >> Other ways he could have used the standard H4 implementation if he just wanted to transfer packets to and from Host. > > > > This is actually fully on purpose. A driver should not interfere with > > HCI event or commands for that matter at all. It should be just a pure > > transport. > > > > If we need to hook something into vendor command/event handling then we > > should find a different way in doing this. The major job of a HCI driver > > is to be a pure, simple and stupid transport driver. > > It will be great if we can have some mechanism to let the driver keep > track of specific commands/Events. > Until then, there is no other option. Please come up with a proposal for this. It might be useful for other drivers as well. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
