lkml.org 
[lkml]   [2018]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.

\
 
 \ /
  Last update: 2018-10-16 16:03    [W:0.076 / U:1.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site