Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3739-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <8751981b-3063-4228-bbec-f36bd544e0fb@quicinc.com>
Date: Fri, 19 Apr 2024 04:34:37 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 2/2] Bluetooth: qca: Fix BT enable failure for
 QCA_QCA6390 after disable then warm reboot
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        <luiz.dentz@gmail.com>, <luiz.von.dentz@intel.com>,
        <marcel@holtmann.org>
CC: <linux-bluetooth@vger.kernel.org>, <wt@penguintechs.org>,
        <bartosz.golaszewski@linaro.org>
References: <1713449192-25926-1-git-send-email-quic_zijuhu@quicinc.com>
 <1713449192-25926-3-git-send-email-quic_zijuhu@quicinc.com>
 <7cd6f395-1f1b-4a73-840f-d70fff5da220@linaro.org>
Content-Language: en-US
From: quic_zijuhu <quic_zijuhu@quicinc.com>
In-Reply-To: <7cd6f395-1f1b-4a73-840f-d70fff5da220@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 4/19/2024 12:48 AM, Krzysztof Kozlowski wrote:
> On 18/04/2024 16:06, Zijun Hu wrote:
>> From: Zijun Hu <zijuhu@qti.qualcomm.com>
>>
>> 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@penguintechs.org>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218726
>> Signed-off-by: Zijun Hu <zijuhu@qti.qualcomm.com>
>> Tested-by: Wren Turkal <wt@penguintechs.org>
>> ---
>>  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
>