Hi Balakrishna,
On 2018-10-18 13:28, 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@xxxxxxxxxxxxxx>
Signed-off-by: Balakrishna Godavarthi
<bgodavar@xxxxxxxxxxxxxx>
---
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.
[Bala]: even we the felt the same to remove ACL packet header before
sending to stack via an diagnostic channel,
here is the case, let us assume that an in future qaulcomm bt
chip want to send diagnosis data as a diagnostic packet.
if we remove ACL header for ACL debug packet, how can the stack
differentiate between debug packet and diagnosis packet.
fair enough. Can we also do the same for the other Qualcomm / Atheros
devices on USB and SMD bus? And what about implementing hdev->set_diag
to enable / disable sending of debug logs. And of course the addition
to btmon to decode the diagnostic channel.
[Bala]:
Does thing change need to be done for other Qualcomm/Atheros
device on USB and smd: For the legacy Qualcomm/Atheros Chips
(Rome/AR30x2/AR30x3), they don’t have this ACL data debug log feature.
hdev->set_diag implementation to enable or disable the debug logs
from controller: will study further and get back to you on this
Qualcomm specific file in Bluez stack to decode the bypassed
debug packets in btmon: soon we will be posting the parsing file.
Regards
Marcel
--
Regards
Balakrishna.
