| Message ID | 20181001135102.17453-1-bgodavar@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v1] Bluetooth: hci_qca: Add support for controller debug logs. | expand |
Hi Balakrishna, On Mon, Oct 01, 2018 at 07:21:02PM +0530, Balakrishna Godavarthi wrote: > 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> > --- > drivers/bluetooth/hci_qca.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index d98ed0442201..a00910aaed54 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 > + > /* 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)) > + return hci_recv_diag(hdev, skb); > + else > + return hci_recv_frame(hdev, skb); nit: you could just do hci_recv_frame() after the if branch, instead of doing it in 'else'. That would make it clearer at first sight that the QCA debug packets are a special case and other packets normal operation. > +} > + > #define QCA_IBS_SLEEP_IND_EVENT \ > .type = HCI_IBS_SLEEP_IND, \ > .hlen = 0, \ > @@ -870,13 +888,20 @@ static int qca_ibs_wake_ack(struct hci_dev *hdev, struct sk_buff *skb) > .lsize = 0, \ > .maxlen = HCI_MAX_IBS_SIZE > > +#define QCA_RECV_ACL_DATA \ > + .type = HCI_ACLDATA_PKT, \ > + .hlen = HCI_ACL_HDR_SIZE, \ > + .loff = 2, \ > + .lsize = 2, \ > + .maxlen = HCI_MAX_FRAME_SIZE Is there any actual advantage of adding this 'macro' instead of just using H4_RECV_ACL, which is exactly the same? > static const struct h4_recv_pkt qca_recv_pkts[] = { > - { H4_RECV_ACL, .recv = hci_recv_frame }, > { H4_RECV_SCO, .recv = hci_recv_frame }, > { H4_RECV_EVENT, .recv = hci_recv_frame }, > { QCA_IBS_WAKE_IND_EVENT, .recv = qca_ibs_wake_ind }, > { QCA_IBS_WAKE_ACK_EVENT, .recv = qca_ibs_wake_ack }, > { QCA_IBS_SLEEP_IND_EVENT, .recv = qca_ibs_sleep_ind }, > + { QCA_RECV_ACL_DATA, .recv = qca_recv_acl_data }, > }; > > static int qca_recv(struct hci_uart *hu, const void *data, int count)
Hi Matthias, On 2018-10-02 01:18, Matthias Kaehlcke wrote: > Hi Balakrishna, > > On Mon, Oct 01, 2018 at 07:21:02PM +0530, Balakrishna Godavarthi wrote: >> 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> >> --- >> drivers/bluetooth/hci_qca.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index d98ed0442201..a00910aaed54 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 >> + >> /* 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)) >> + return hci_recv_diag(hdev, skb); >> + else >> + return hci_recv_frame(hdev, skb); > > nit: you could just do hci_recv_frame() after the if branch, instead > of doing it in 'else'. That would make it clearer at first sight that > the QCA debug packets are a special case and other packets normal > operation. > [Bala]: will update. >> +} >> + >> #define QCA_IBS_SLEEP_IND_EVENT \ >> .type = HCI_IBS_SLEEP_IND, \ >> .hlen = 0, \ >> @@ -870,13 +888,20 @@ static int qca_ibs_wake_ack(struct hci_dev >> *hdev, struct sk_buff *skb) >> .lsize = 0, \ >> .maxlen = HCI_MAX_IBS_SIZE >> >> +#define QCA_RECV_ACL_DATA \ >> + .type = HCI_ACLDATA_PKT, \ >> + .hlen = HCI_ACL_HDR_SIZE, \ >> + .loff = 2, \ >> + .lsize = 2, \ >> + .maxlen = HCI_MAX_FRAME_SIZE > > Is there any actual advantage of adding this 'macro' instead of just > using H4_RECV_ACL, which is exactly the same? > [Bala]: yes we can use same macro, will update. >> static const struct h4_recv_pkt qca_recv_pkts[] = { >> - { H4_RECV_ACL, .recv = hci_recv_frame }, >> { H4_RECV_SCO, .recv = hci_recv_frame }, >> { H4_RECV_EVENT, .recv = hci_recv_frame }, >> { QCA_IBS_WAKE_IND_EVENT, .recv = qca_ibs_wake_ind }, >> { QCA_IBS_WAKE_ACK_EVENT, .recv = qca_ibs_wake_ack }, >> { QCA_IBS_SLEEP_IND_EVENT, .recv = qca_ibs_sleep_ind }, >> + { QCA_RECV_ACL_DATA, .recv = qca_recv_acl_data }, >> }; >> >> static int qca_recv(struct hci_uart *hu, const void *data, int count)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index d98ed0442201..a00910aaed54 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 + /* 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)) + return hci_recv_diag(hdev, skb); + else + return hci_recv_frame(hdev, skb); +} + #define QCA_IBS_SLEEP_IND_EVENT \ .type = HCI_IBS_SLEEP_IND, \ .hlen = 0, \ @@ -870,13 +888,20 @@ static int qca_ibs_wake_ack(struct hci_dev *hdev, struct sk_buff *skb) .lsize = 0, \ .maxlen = HCI_MAX_IBS_SIZE +#define QCA_RECV_ACL_DATA \ + .type = HCI_ACLDATA_PKT, \ + .hlen = HCI_ACL_HDR_SIZE, \ + .loff = 2, \ + .lsize = 2, \ + .maxlen = HCI_MAX_FRAME_SIZE + static const struct h4_recv_pkt qca_recv_pkts[] = { - { H4_RECV_ACL, .recv = hci_recv_frame }, { H4_RECV_SCO, .recv = hci_recv_frame }, { H4_RECV_EVENT, .recv = hci_recv_frame }, { QCA_IBS_WAKE_IND_EVENT, .recv = qca_ibs_wake_ind }, { QCA_IBS_WAKE_ACK_EVENT, .recv = qca_ibs_wake_ack }, { QCA_IBS_SLEEP_IND_EVENT, .recv = qca_ibs_sleep_ind }, + { QCA_RECV_ACL_DATA, .recv = qca_recv_acl_data }, }; static int qca_recv(struct hci_uart *hu, const void *data, int count)