Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-4353-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: <bdf55ed0-f9d9-4e84-90d8-6a788568a371@163.com>
Date: Tue, 7 May 2024 22:34:06 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: path to landing patch to fix warm boot issue for qca6390
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 Wren Turkal <wt@penguintechs.org>, linux-bluetooth@vger.kernel.org,
 Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Zijun Hu <quic_zijuhu@quicinc.com>,
 Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
References: <3475f0dd-6a0f-4319-9590-379eee33f504@penguintechs.org>
 <08241854-2837-4e8b-ba12-79bcbc970c1b@linaro.org>
Content-Language: en-US
From: Lk Sii <lk_sii@163.com>
In-Reply-To: <08241854-2837-4e8b-ba12-79bcbc970c1b@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 2024/5/7 14:27, Krzysztof Kozlowski wrote:
> On 06/05/2024 21:21, Wren Turkal wrote:
>> Krzysztof,
>>
>> I am reaching out to you as you had the most important objections to the 
>> change to fix qca6390 for the warm boot/module reload bug that I am 
>> experiencing.
>>
>> For context, the problem is that the hci_uart module will send specific 
>> vendor specfic commands during shutdown of the hardware under most 
>> situations. These VSCs put the bluetooth device into a non-working state 
>> on my Dell XPS 13 9310 with qca6390 bluetooth hardware.
>>
>> Zijun's proposed fix is to not send these commands when it's not 
>> appropriate for the hardware. The vendor commands should be avoided when 
>> the hardware does not have persistent configuration or when the device 
>> is in setup state (indicating that is has never been setup and should 
>> not be sent the VSCs on the shutdown path). This is what Zijun's patch 
>> implements.
>>
>> In addition, Zijun's change removes the influence of both
>> the QCA_BT_OFF qca flag and and HCI_RUNNING hdev flag. Zijun asserts 
>> that those flags should not influence the sending of the VSCs in the 
>> shutdown path. If I understand KK's objections properly, this is where 
>> his objection is stemming from. KK, is this correct?
> 
> Yes, because this basically reverts my fix for shutdown path and
> re-opens the race condition.
> 
>>
>> Zijun's proposed fix can be seen here: 
>> https://patchwork.kernel.org/project/bluetooth/patch/1713932807-19619-3-git-send-email-quic_zijuhu@quicinc.com/
>>
>> I'm wondering if we can resolve this impasse by splitting the change 
>> into two changes, as follows:
>>
>> 1. Change that removes the influence of the QCA_BT_OFF and HCI_RUNNING 
>> flags in the shutdown path.
> 
> There was no explanation why this was removed, so it does not matter to
> me whether this should be separate path or not.
> 
> Zijun, even though I asked four times, did not provide information on
> which kernel this was prepared and tested, and on which hardware.
> 
> I assume Zijun did not understand original issue, thus assumes changing
> the code to HCI_SETUP fixes it.
> 
> 
>> 2. Add the quirk from Zijun's patch that fixes my hardward configuration.
>>
>> I'm hoping that better clearer descriptions for #1 can help get that 
>> landed since the logic current appears to be at odds with how the 
>> hardware works.
>>
>> Also, I am happy to split the patches into the two patches, or (maybe 
>> more ideally) just modify the commit message to better indicate the 
>> reason the change. I just need guidance from maintainers so that 
>> whatever work I do leads to something acceptable for y'all.
>>
>> So, please help me get this done. I am just a user with broken hardware 
>> and a fondness for Linux. I would love to help do what's needed to get 
>> this fix landed.
> 
it seems there are a new below discussion thread for the single patch
https://patchwork.kernel.org/project/bluetooth/patch/1714658761-15326-1-git-send-email-quic_zijuhu@quicinc.com/
you maybe go there to discuss.
thanks
> Best regards,
> Krzysztof
> 
> 
>