Re: [PATCH] Bluetooth: hci_qca: Set SSR triggered flags when SSR command is sent out

[Marcel Holtmann <marcel@holtmann.org>]

Hi Balakrishna,

>>> This change sets SSR triggered flags when QCA SSR command is sent to
>>> SoC. After the SSR command sent, driver discards the incoming data from
>>> the upper layers. This way will ensure to read full dumps from the
>>> BT SoC without any flow control issues due to excess of data receiving
>>> from the HOST in audio usecases.
>>> Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
>>> ---
>>> drivers/bluetooth/hci_qca.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>>> index 53deea2..5cbed6a 100644
>>> --- a/drivers/bluetooth/hci_qca.c
>>> +++ b/drivers/bluetooth/hci_qca.c
>>> @@ -69,6 +69,8 @@
>>> #define QCA_LAST_SEQUENCE_NUM		0xFFFF
>>> #define QCA_CRASHBYTE_PACKET_LEN	1096
>>> #define QCA_MEMDUMP_BYTE		0xFB
>>> +#define QCA_SSR_OPCODE			0xFC0C
>>> +#define QCA_SSR_PKT_LEN		5
>>> enum qca_flags {
>>> 	QCA_IBS_DISABLED,
>>> @@ -871,6 +873,14 @@ static int qca_enqueue(struct hci_uart *hu, struct sk_buff *skb)
>>> 	/* Prepend skb with frame type */
>>> 	memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
>>> +	if (hci_skb_pkt_type(skb) == HCI_COMMAND_PKT &&
>>> +	    skb->len == QCA_SSR_PKT_LEN &&
>>> +	    hci_skb_opcode(skb) == QCA_SSR_OPCODE) {
>>> +		bt_dev_info(hu->hdev, "Triggering ssr");
>>> +		set_bit(QCA_SSR_TRIGGERED, &qca->flags);
>>> +		set_bit(QCA_MEMDUMP_COLLECTION, &qca->flags);
>>> +	}
>>> +
>> can we please stop hacking around by parsing opcodes in an enqueue
>> function. Sounds like someone is injecting raw HCI vendor commands and
>> then having a driver react to it.
> [Bala]: yes this opcode is injected via hcitool to test BT SoC dump procedure or
> to collect the dumps to debug the issue during issue cases. When audio usecases are running,
> HOST sends ACL packets to SoC, in meantime if this command is sent to SoC using hcitool
> to collect dumps at particular point,  With out this check HOST is pumping continues data to
> SoC and SoC RFR line goes high, sometimes SoC become unresponsive and driver starts logging
> command timeout error. Instead here, once a cmd with this opcode is sent, timer is started
> to ensure that SSR is in progress. If no response from SoC for 8 seconds. Driver will be restarted.

so why would I add a kernel work-around for this?

Design a proper interface for this and don’t rely on injecting HCI commands via HCI raw channel.

Regards

Marcel