Hi Balakrishna, >>>>>>> This patch will prevent error messages splashing on console. >>>>>>> [ 78.426697] Bluetooth: hci_core.c:hci_acldata_packet() hci0: ACL >>>>>>> packet for unknown connection handle 3804 >>>>>>> [ 78.436682] Bluetooth: hci_core.c:hci_acldata_packet() hci0: ACL >>>>>>> packet for unknown connection handle 3804 >>>>>>> [ 78.446639] Bluetooth: hci_core.c:hci_acldata_packet() hci0: ACL >>>>>>> packet for unknown connection handle 3804 >>>>>>> [ 78.456596] Bluetooth: hci_core.c:hci_acldata_packet() hci0: ACL >>>>>>> packet for unknown connection handle 3804 >>>>>>> QCA wcn3990 will send the debug logs in the form of ACL packets. >>>>>>> While decoding packet in qca_recv(), marking the received debug log >>>>>>> packet as diagnostic packet. >>>>>>> Signed-off-by: Harish Bandi <[email protected]> >>>>>>> Signed-off-by: Balakrishna Godavarthi <[email protected]> >>>>>>> --- >>>>>>> v2: >>>>>>> * Addressed reviewer comments. >>>>>>> v1: >>>>>>> * initial patch >>>>>>> --- >>>>>>> drivers/bluetooth/hci_qca.c | 20 +++++++++++++++++++- >>>>>>> 1 file changed, 19 insertions(+), 1 deletion(-) >>>>>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >>>>>>> index d98ed0442201..63ac3c6b4149 100644 >>>>>>> --- a/drivers/bluetooth/hci_qca.c >>>>>>> +++ b/drivers/bluetooth/hci_qca.c >>>>>>> @@ -63,6 +63,10 @@ >>>>>>> /* susclk rate */ >>>>>>> #define SUSCLK_RATE_32KHZ 32768 >>>>>>> +/* Controller debug log header */ >>>>>>> +#define QCA_DEBUG_HDR_MSB 0xDC >>>>>>> +#define QCA_DEBUG_HDR_LSB 0x2E >>>>>>> + >>>>>> since this is actually the ACL handle, why not call it QCA_DEBUG_HANDLE. >>>>> [Bala]: will update. >>>>>>> /* HCI_IBS transmit side sleep protocol states */ >>>>>>> enum tx_ibs_states { >>>>>>> HCI_IBS_TX_ASLEEP, >>>>>>> @@ -849,6 +853,20 @@ static int qca_ibs_wake_ack(struct hci_dev *hdev, >>>>>>> struct sk_buff *skb) >>>>>>> return 0; >>>>>>> } >>>>>>> +static int qca_recv_acl_data(struct hci_dev *hdev, struct sk_buff *skb) >>>>>>> +{ >>>>>>> + /* We receive debug logs from chip as an ACL packets. >>>>>>> + * Instead of sending the data to ACL to decode the >>>>>>> + * received data, we are pushing them to the above layers >>>>>>> + * as a diagnostic packet. >>>>>>> + */ >>>>>>> + if ((skb->len > 2) && (skb->data[0] == QCA_DEBUG_HDR_MSB) && >>>>>>> + (skb->data[1] == QCA_DEBUG_HDR_LSB)) >>>>>> Skip the individual () since they are not needed. Also the skb->len >>>>>> check is not needed since the H4_RECV_ACL already ensures the proper >>>>>> length of the header. >>>>>> And just use get_unaligned_le16(skb->data) here (or be16 if that is >>>>>> the byte order). >>>>> [Bala] : will update condition with _le16() >>>>>>> + return hci_recv_diag(hdev, skb); >>>>>> Is there any reason to keep the 4 octets hci_acl_hdr with this SKB? Or >>>>>> would it be better to be stripped off. Mainly asking are they any >>>>>> other magic handles that we might want to feed through the diag >>>>>> channel? >>>>> [Bala]: yes we need header in the stack, to differentiate between actual >>>>> diagnostic packet and debug packet. >>>> please explain that a bit more. There is just one debug handle or do >>>> you have more special handles? >>> [Bala]: Qualcomm bluetooth chip sends the debug data of the chip with ACL >>> header along with fixed handler i.e. 0x2edc.. >>> this handler is dedicated for the debug logs sent by qcomm bt chip >>> and it is not used for any other connection handlers. >>> so here we are checking this handler and sending to stack as an >>> a diagnostic packet.. so during the log capture we will read this >>> diagnostic packet and handler,to decode the debug packet. >>> that is the reason why we don't remove handler during condition >>> check, in future, if qualcomm chip sends diagnostic packet, >>> then this will create an problem to upper layers to decide whether it >>> is an diagnostic packet or the debug packet. >>> this is how we do it. >>> if( diagnostic packet && handler == 0x2edc) >>> treat them as an ddebug packet >>> else >>> treat as diagnostic packet. >> let me interpret this answer. Besides debug handle 0x2edc, there are >> other handles that are not connection handles. And in future updates >> you want to send these handles also via hci_recv_diag channel. > > No, 0x2edc is an dedicated handle for debug logs, i don't really think they > we will have an new handle coming up. > this is only the handle we will send via hci_recv_diag channel to the stack. > > what i mean in my previous answer was,we don't to remove debug connection > handle from the skb, because in the stack > we look for a diagnostic packet and debug connection handler to decode the > message.
if 0x2edc is the only magic connection handle, then lets strip the SKB of the ACL packet header and send only the payload to via hci_recv_diag. All the information is present and you would be just sending the ACL packet header for no reason. The diagnostic packets get a separate channel on the monitor interface. Regards Marcel
