Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3804-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <1cbe19a8-37e2-4933-b1c1-9e6c30d359cc@quicinc.com>
Date: Sun, 21 Apr 2024 05:42:30 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 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>, <bartosz.golaszewski@linaro.org>,
        <wt@penguintechs.org>
References: <1713449192-25926-1-git-send-email-quic_zijuhu@quicinc.com>
 <1713564212-21725-1-git-send-email-quic_zijuhu@quicinc.com>
 <1713564212-21725-3-git-send-email-quic_zijuhu@quicinc.com>
 <63772c3f-eab7-4be0-8b1c-0cebea361eae@linaro.org>
Content-Language: en-US
From: quic_zijuhu <quic_zijuhu@quicinc.com>
In-Reply-To: <63772c3f-eab7-4be0-8b1c-0cebea361eae@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 4/20/2024 7:07 PM, Krzysztof Kozlowski wrote:
> On 20/04/2024 00:03, Zijun Hu wrote:
>> From: Zijun Hu <zijuhu@qti.qualcomm.com>
>>
>> Commit 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed
>> serdev") will cause below regression issue:
>>
>> BT can't be enabled after below steps:
>> cold boot -> enable BT -> disable BT -> warm reboot -> BT enable failure
>> if BT reset pin is not configured within DT|ACPI for QCA_QCA6390.
> 
> Please mention if QCA6390 requires reset pin, according to datasheet or
> some hardware guideline.
> 
will update commit message.
>>
>> Fixed by sending the VSC to reset controller within qca_serdev_shutdown()
>> once BT was ever enabled.
>>
>> 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 | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index 0934e74112a6..3f5173f1180b 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -2438,13 +2438,12 @@ 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) ||
>> +		    hci_dev_test_flag(hdev, HCI_SETUP))
>>  			return;
> 
> I am sorry, but why do we need to perform shutdown procedure now if
> device is off?
> 
this is shutdown of serdev and not hdev's shutdown.
> I raised questions about this and I still don't understand. Not much got
> better comparing to previous version. Actually, I have no clue what
> changed. Where is the changelog?
> 
my reply at below link will help you
https://lore.kernel.org/all/fe1a0e3b-3408-4a33-90e9-d4ffcfc7a99b@quicinc.com/
> 
> Best regards,
> Krzysztof
>