On 4/19/2024 12:48 AM, Krzysztof Kozlowski wrote:
> On 18/04/2024 16:06, Zijun Hu wrote:
>> From: Zijun Hu <zijuhu@xxxxxxxxxxxxxxxx>
>>
>> Commit 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed
>> serdev") will cause regression issue for QCA_QCA6390 if BT reset pin is
>> not configured by DT or ACPI, the regression issue is that BT can't be
>> enabled after disable then warm reboot, fixed by correcting conditions
>> for sending the VSC reset controller within qca_serdev_shutdown().
>
> I have trouble understanding what is the bug. Can you rephrase it?
>
Think about below sequences:
cold reboot(power off then power on) -> Enable BT -> Disable BT -> Warm reboot -> Enable BT again ...
BT is failed to be enabled after warm reboot.
>>
>> Fixes: 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed serdev")
>> Reported-by: Wren Turkal <wt@xxxxxxxxxxxxxxxx>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218726
>> Signed-off-by: Zijun Hu <zijuhu@xxxxxxxxxxxxxxxx>
>> Tested-by: Wren Turkal <wt@xxxxxxxxxxxxxxxx>
>> ---
>> drivers/bluetooth/hci_qca.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 160175a23a49..2a47a60ecc25 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -2437,15 +2437,21 @@ static void qca_serdev_shutdown(struct device *dev)
>> struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev);
>> struct hci_uart *hu = &qcadev->serdev_hu;
>> struct hci_dev *hdev = hu->hdev;
>> - struct qca_data *qca = hu->priv;
>> const u8 ibs_wake_cmd[] = { 0xFD };
>> const u8 edl_reset_soc_cmd[] = { 0x01, 0x00, 0xFC, 0x01, 0x05 };
>>
>> if (qcadev->btsoc_type == QCA_QCA6390) {
>> - if (test_bit(QCA_BT_OFF, &qca->flags) ||
>> - !test_bit(HCI_RUNNING, &hdev->flags))
>> + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
>> + BT_INFO("QCA do not need to send EDL_RESET_REQ");
>> return;
>> + }
>> +
>> + if (hci_dev_test_flag(hdev, HCI_SETUP)) {
>
> Commit msg does not help me at all to understand why you are changing
> the test bits.
it is test bits not changing bits.
>
>> + BT_INFO("QCA do not send EDL_RESET_REQ");
>> + return;
>> + }
>>
>> + BT_INFO("QCA start to send EDL_RESET_REQ");
>
> Why debugging info is part of the fix?
>
they are reserved intentionally to print critical info.
> Best regards,
> Krzysztof
>
