Messages in this thread | | | Date | Tue, 16 Oct 2018 19:32:32 +0530 | From | Balakrishna Godavarthi <> | Subject | Re: [PATCH v2] Bluetooth: hci_qca: Add support for controller debug logs. |
| |
Hi Marcel,
On 2018-10-03 22:26, Marcel Holtmann wrote: > 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 <c-hbandi@codeaurora.org> >> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org> >> --- >> 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.
>> + >> + return hci_recv_frame(hdev, skb); >> +} >> + > > Regards > > Marcel
-- Regards Balakrishna.
| |