Hi Konrad
On 7/19/2025 7:32 AM, Shuai Zhang wrote:
> Hi Konrad
>
> On 7/15/2025 5:11 PM, Konrad Dybcio wrote:
>> On 7/15/25 7:16 AM, Shuai Zhang wrote:
>>> the QCA_SSR_TRIGGERED and QCA_IBS_DISABLED bits cannot be cleared.
>>> This leads to reset command timeout.
>>
>> This is a description of what goes wrong in terms of the code of
>> this driver, and it doesn't explain why you gate the code addition
>> with HCI_QUIRK_NON_PERSISTENT_SETUP, please share more details about
>> what you're doing, and more importantly, why.
>>
>
> The problem encountered is that when the host actively triggers ssr
> and collects the coredump data, the bt will send a reset command to
> the controller. However, due to the aforementioned flag not being set,
> the reset command times out.
>
> I'm not clear whether you want to ask about the function of
> HCI_QUIRK_NON_PERSISTENT_SETUP or why the changes are placed
> under if(!HCI_QUIRK_NON_PERSISTENT_SETUP).
>
> Regarding the purpose of HCI_QUIRK_NON_PERSISTENT_SETUP,
> you can refer to this commit. 740011cfe94859df8d05f5400d589a8693b095e7
>
> As for why it's placed in if(!HCI_QUIRK_NON_PERSISTENT_SETUP),
> since HCI_QUIRK_NON_PERSISTENT_SETUP is related to BT_EN, it can be
> used to determine if BT_EN exists in the DTS.
>
>>>
>>> Signed-off-by: Shuai Zhang <quic_shuaz@xxxxxxxxxxx>
>>> ---
>>> drivers/bluetooth/hci_qca.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>>> index 4e56782b0..791f8d472 100644
>>> --- a/drivers/bluetooth/hci_qca.c
>>> +++ b/drivers/bluetooth/hci_qca.c
>>> @@ -1653,6 +1653,18 @@ static void qca_hw_error(struct hci_dev *hdev, u8 code)
>>> skb_queue_purge(&qca->rx_memdump_q);
>>> }
>>>
>>> + /* If the SoC always enables the bt_en pin via hardware and the driver
>>> + * cannot control the bt_en pin of the SoC chip, then during SSR,
>>
>> What is the "SoC" here? Bluetooth chip? MSM?
>
> yes, Bluetooth chip on qcs9075-evk platform
>
>>
>> What does "enabling the pin via hardware" refer to? Do we ever expect
>> that a proper platform description skips the bt_en pin?
>>
>> Also:
>>
>> /*
>> * If the..
>>
>
> Sorry, I’m not quite sure I follow—could you clarify what you meant?
> Here is my understanding.
>
> Enabling pins through hardware refers to "the pin is pulled up by hardware".
> qcs9075-evk platform use the m.2 connective card, the bt_en always pull up.
>
>
>>> + * the QCA_SSR_TRIGGERED and QCA_IBS_DISABLED bits cannot be cleared.
>>> + * This leads to a reset command timeout failure.
>>> + * Also, add msleep delay to wait for controller to complete SSR.
>>
>> Googling "bluetooth SSR" yields nothing, so it's fair for me to ask
>> you to explain that acronym.. it's used a number of times across the
>> driver, so perhaps a comment somewhere at the top in a separate commit
>> would be good as well. I'm guessing "subsystem reset"?
>
> Just to clarify, SSR is short for Subsystem Restart
>
>>
>> Konrad
>>
>>> + */
>>> + if (!test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
>>> + clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
>>> + clear_bit(QCA_IBS_DISABLED, &qca->flags);
>>> + msleep(50);
>>> + }
>>> +
>>> clear_bit(QCA_HW_ERROR_EVENT, &qca->flags);
>>> }
>>>
>
> Shuai
>
Please let me know if there are any updates. Thank you.
BR,
Shuai
