[PATCH 0/3] Fix SSR issues caused by BT_EN being pulled up by hardware

[PATCH 0/3] Fix SSR issues caused by BT_EN being pulled up by hardware

[Shuai Zhang]

This patch series addresses issues encountered during SSR when
the BT_EN pin is pulled up by hardware. The main issues fixed are:

1. Timeout when sending reset command.
2. IBS state of host and controller not being synchronized.
3. Multiple triggers of SSR generating only one coredump file.

Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>

Shuai Zhang (3):
  driver: bluetooth: hci_qca: fix ssr fail when BT_EN is pulled up by hw
  driver: bluetooth: hci_qca: fix host IBS state after SSR
  driver: bluetooth: hci_qca: Multiple triggers of SSR only generate one
    coredump file

 drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

-- 
2.34.1

[PATCH 1/3] driver: bluetooth: hci_qca: fix ssr fail when BT_EN is pulled up by hw

[Shuai Zhang]

the QCA_SSR_TRIGGERED and QCA_IBS_DISABLED bits cannot be cleared.
This leads to reset command timeout.

Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
---
 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,
+	 * 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.
+	 */
+	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);
 }
 
-- 
2.34.1

[PATCH 2/3] driver: bluetooth: hci_qca: fix host IBS state after SSR

[Shuai Zhang]

After SSR, host will not download the firmware, causing
controller to remain in the IBS_WAKE state. Host needs
to synchronize with the controller to maintain proper operation.

Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
---
 drivers/bluetooth/hci_qca.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 791f8d472..a17d3f7ae 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1658,10 +1658,14 @@ static void qca_hw_error(struct hci_dev *hdev, u8 code)
 	 * 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.
+	 *
+	 * Host will not download the firmware after SSR, controller to remain
+	 * in the IBS_WAKE state, and the host needs to synchronize with it
 	 */
 	if (!test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
 		clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
 		clear_bit(QCA_IBS_DISABLED, &qca->flags);
+		qca->tx_ibs_state = HCI_IBS_TX_AWAKE;
 		msleep(50);
 	}
 
-- 
2.34.1

[PATCH 3/3] driver: bluetooth: hci_qca: Multiple triggers of SSR only generate one coredump file

[Shuai Zhang]

Multiple triggers of SSR only first generate coredump file,
duo to memcoredump_flag no clear.

add clear coredump flag when ssr completed.

Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
---
 drivers/bluetooth/hci_qca.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index a17d3f7ae..e836b2c29 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1661,11 +1661,14 @@ static void qca_hw_error(struct hci_dev *hdev, u8 code)
 	 *
 	 * Host will not download the firmware after SSR, controller to remain
 	 * in the IBS_WAKE state, and the host needs to synchronize with it
+	 *
+	 * clear memcoredump_flag to ensure next submission of coredump date.
 	 */
 	if (!test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
 		clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
 		clear_bit(QCA_IBS_DISABLED, &qca->flags);
 		qca->tx_ibs_state = HCI_IBS_TX_AWAKE;
+		qca->memdump_state = QCA_MEMDUMP_IDLE;
 		msleep(50);
 	}
 
-- 
2.34.1

Re: [PATCH 1/3] driver: bluetooth: hci_qca: fix ssr fail when BT_EN is pulled up by hw

[Konrad Dybcio]

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.

> 
> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
> ---
>  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?

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..

> +	 * 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"?

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);
>  }
>  

Re: [PATCH 2/3] driver: bluetooth: hci_qca: fix host IBS state after SSR

[Konrad Dybcio]

On 7/15/25 7:16 AM, Shuai Zhang wrote:
> After SSR, host will not download the firmware, causing
> controller to remain in the IBS_WAKE state. Host needs
> to synchronize with the controller to maintain proper operation.
> 
> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
> ---
>  drivers/bluetooth/hci_qca.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 791f8d472..a17d3f7ae 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1658,10 +1658,14 @@ static void qca_hw_error(struct hci_dev *hdev, u8 code)
>  	 * 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.
> +	 *
> +	 * Host will not download the firmware after SSR, controller to remain
> +	 * in the IBS_WAKE state, and the host needs to synchronize with it
>  	 */
>  	if (!test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
>  		clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
>  		clear_bit(QCA_IBS_DISABLED, &qca->flags);
> +		qca->tx_ibs_state = HCI_IBS_TX_AWAKE;
>  		msleep(50);

This touches upon the code introduced in the previous patch.

Any reason they should be separate?

Konrad

Re: [PATCH 3/3] driver: bluetooth: hci_qca: Multiple triggers of SSR only generate one coredump file

[Konrad Dybcio]

On 7/15/25 7:16 AM, Shuai Zhang wrote:
> Multiple triggers of SSR only first generate coredump file,
> duo to memcoredump_flag no clear.
> 
> add clear coredump flag when ssr completed.
> 
> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
> ---
>  drivers/bluetooth/hci_qca.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index a17d3f7ae..e836b2c29 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1661,11 +1661,14 @@ static void qca_hw_error(struct hci_dev *hdev, u8 code)
>  	 *
>  	 * Host will not download the firmware after SSR, controller to remain
>  	 * in the IBS_WAKE state, and the host needs to synchronize with it
> +	 *
> +	 * clear memcoredump_flag to ensure next submission of coredump date.
>  	 */
>  	if (!test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
>  		clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
>  		clear_bit(QCA_IBS_DISABLED, &qca->flags);
>  		qca->tx_ibs_state = HCI_IBS_TX_AWAKE;
> +		qca->memdump_state = QCA_MEMDUMP_IDLE;
>  		msleep(50);

Same comment as patch 2

Konrad

Re: [PATCH 1/3] driver: bluetooth: hci_qca: fix ssr fail when BT_EN is pulled up by hw

[Shuai Zhang]

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@quicinc.com>
>> ---
>>  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

Re: [PATCH 2/3] driver: bluetooth: hci_qca: fix host IBS state after SSR

[Shuai Zhang]

On 7/15/2025 5:12 PM, Konrad Dybcio wrote:
> On 7/15/25 7:16 AM, Shuai Zhang wrote:
>> After SSR, host will not download the firmware, causing
>> controller to remain in the IBS_WAKE state. Host needs
>> to synchronize with the controller to maintain proper operation.
>>
>> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
>> ---
>>  drivers/bluetooth/hci_qca.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 791f8d472..a17d3f7ae 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1658,10 +1658,14 @@ static void qca_hw_error(struct hci_dev *hdev, u8 code)
>>  	 * 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.
>> +	 *
>> +	 * Host will not download the firmware after SSR, controller to remain
>> +	 * in the IBS_WAKE state, and the host needs to synchronize with it
>>  	 */
>>  	if (!test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
>>  		clear_bit(QCA_SSR_TRIGGERED, &qca->flags);
>>  		clear_bit(QCA_IBS_DISABLED, &qca->flags);
>> +		qca->tx_ibs_state = HCI_IBS_TX_AWAKE;
>>  		msleep(50);
> 
> This touches upon the code introduced in the previous patch.
> 
> Any reason they should be separate?
> 

Since this is a different issue, I separated it. The same reason applies to patch 3/3.

> Konrad

Shuai

Re: [PATCH 1/3] driver: bluetooth: hci_qca: fix ssr fail when BT_EN is pulled up by hw

[Konrad Dybcio]

On 7/19/25 1: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.

What I'm saying is that you should put this information in the
commit message

> 
>>>
>>> Signed-off-by: Shuai Zhang <quic_shuaz@quicinc.com>
>>> ---
>>>  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.

The comment style.

> 
> 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.

This is not conclusive either. Does the firmware of the bluetooth chip
configure the pin, or is the reset pin connected to an always-on PCIe
supply or similar?

> 
> 
>>> +	 * the QCA_SSR_TRIGGERED and QCA_IBS_DISABLED bits cannot be cleared.

At a glance, they're only cleared in qca_setup(), regardless of their state

>>> +	 * 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

Please write it down somewhere

Konrad

Re: [PATCH 1/3] driver: bluetooth: hci_qca: fix ssr fail when BT_EN is pulled up by hw

[Shuai Zhang]

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@quicinc.com>
>>> ---
>>>  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

Re: [PATCH 1/3] driver: bluetooth: hci_qca: fix ssr fail when BT_EN is pulled up by hw

[Konrad Dybcio]

On 8/12/25 10:03 AM, Shuai Zhang wrote:
> 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@quicinc.com>
>>>> ---
>>>>  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.

You're expected to address the review comments in a subsequent patchset
revision, in this case please put the answers to the questions I asked
in the commit message, or in the comments, so that someone else can
make sense of the change

Konrad