On 4/25/2024 6:28 PM, quic_zijuhu wrote:
> On 4/25/2024 5:14 PM, Krzysztof Kozlowski wrote:
>> On 25/04/2024 10:30, Bartosz Golaszewski wrote:
>>> On Thu, 25 Apr 2024 at 08:30, Wren Turkal <wt@xxxxxxxxxxxxxxxx> wrote:
>>>>
>>>> On 4/24/24 11:05 PM, Krzysztof Kozlowski wrote:
>>>>> On 25/04/2024 00:01, Wren Turkal wrote:
>>>>>>>>
>>>>>>>> 3) only care about the case property enable-gpios is not configured,
>>>>>>>> the original BT driver design logic indeed matches with binging spec's
>>>>>>>> requirements before bartosz's wrong change
>>>>>>>
>>>>>>> What? There is no such case according to bindings. I told you already
>>>>>>> two times: Either change bindings or this is not valid.
>>>>>>
>>>>>> @krzysztof, I'm curious. There is no entry in the binding specifically
>>>>>> for qca6390. Should there be?
>>>>>
>>>>> qca6390 is documented in the bindings, but you are right that it lacks
>>>>> if:then: entry narrowing/choosing specific supplies and pins. It should
>>>>> have one, indeed.
>>>>
>>>> Would creating an entry for the qca6390 hardware fix the issue you are
>>>> worried about?
>>>>
>>>> Again, sorry for all the, what I assume are, basic questions. I am so
>>>> far out of my depth here. I am just trying to figure out how to move
>>>> forward. I really do appreciate your guidance here.
>>>>
>>>
>>> Wren, Krzysztof: I cannot stop you from doing this but I'm afraid this
>>> will complicate the power sequencing work unnecessarily. The QCA6390
>>> PMU bindings I'm proposing[1] are consumers of the BT enable GPIOs. In
>>> my series I also create an entry for QCA6390 Bluetooth[2] but without
>>> enable-gpios and with the inputs from the PMU, not host. Please
>>> consider that if you decide to go with this.
>>
>> I don't have a near plan to describe QCA6390 supplies and pins, so don't
>> worry. I also don't think Qualcomm BT understand what are bindings, so I
>> don't expect patches from them.
>>
>
> For property enable-gpios of QCA6390, it is optional so not marked as
> required by the dts spec, dts spec is right, i would like to share
> history about QCA6390.
History information/links can be given like the change log under --- for
the initial patch set.
It will benefit the reviewers know the histories.
>
> 1) it was firstly brought-up by Rocky Liao who is the same team as me.
> e5d6468fe9d8 Bluetooth: hci_qca: Add support for Qualcomm Bluetooth SoC
> QCA6390
>
> 2) for its first product at that time, there are no H/W pin for
> enable-gpios, so has issue that BT enable failure after warm reboot. so
> i submitted below commit to solve it.
> Commit: 7e7bbddd029b ("Bluetooth: hci_qca: Fix qca6390 enable failure
> after warm reboot")
>
> 3) then Krzysztof Kozlowski submitted below commit to solve
> use-after-free issue but also introduced the regression issue
> Commit: 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed
> serdev")
Fixes tag can be given in the commit message for a specific commit id.
>
> 4) the issue is reported by Wren recently by below link, and i was
> notified to notice this issue and co-work with you to solve it.
> https://lore.kernel.org/linux-bluetooth/ea20bb9b-6b60-47fc-ae42-5eed918ad7b4@xxxxxxxxxxx/T/#m73d6a71d2f454bb03588c66f3ef7912274d37c6f
> https://bugzilla.kernel.org/show_bug.cgi?id=218726
It is suggested to add the Link into the commit message itself.
>
> 5) i known the root cause when i saw the issue description and have
> below formal fix [v7,2/2] and it is verified several times and can
> solve the issue reported. and the first debugging change included the
> fix logic.
> https://patchwork.kernel.org/project/bluetooth/list/?series=847290
>
> 6) it does not matter if my fix is not expected, please wait for other
> good fix.
>
> 7) i really don't want to discuss any more since i really talk two much
> as shown by below link, i will disappear for a short of time.
> https://lore.kernel.org/linux-bluetooth/1713771497-5733-1-git-send-email-quic_zijuhu@xxxxxxxxxxx/#t
>
>> Best regards,
>> Krzysztof
>>
>
--
Thx and BRs,
Aiqun(Maria) Yu
