linux-kernel - Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for supply and reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a11cf72d-3878-4af1-89c5-c66d55794ea6@kernel.org>
Date: Tue, 22 Oct 2024 09:32:15 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Marco Felsch <m.felsch@...gutronix.de>
Cc: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>,
 Sherry Sun <sherry.sun@....com>, Amitkumar Karwar
 <amitkumar.karwar@....com>, Neeraj Sanjay Kale <neeraj.sanjaykale@....com>,
 "marcel@...tmann.org" <marcel@...tmann.org>,
 "luiz.dentz@...il.com" <luiz.dentz@...il.com>,
 "robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
 <krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
 "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
 "linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 GEO-CHHER-bsp-development <bsp-development.geo@...ca-geosystems.com>
Subject: Re: [PATCH 1/2] dt-bindings: net: bluetooth: nxp: add support for
 supply and reset

On 22/10/2024 09:30, Krzysztof Kozlowski wrote:
> On 22/10/2024 09:12, Marco Felsch wrote:
>> On 24-10-22, Krzysztof Kozlowski wrote:
>>> On 21/10/2024 12:25, Marco Felsch wrote:
>>>> On 24-10-21, Krzysztof Kozlowski wrote:
>>>>> On 21/10/2024 08:41, Marco Felsch wrote:
>>>>>> On 24-10-07, Krzysztof Kozlowski wrote:
>>
>> ...
>>
>>>>>>> Based on earlier message:
>>>>>>>
>>>>>>> "For NXP WIFI/BT chip, WIFI and BT share the one PDn pin, which means
>>>>>>> that both wifi and BT controller will be powered on and off at the same
>>>>>>> time."
>>>>>>>
>>>>>>> but maybe that's not needed. No clue, I don't know the hardware. But be
>>>>>>> carefully what you write in the bindings, because then it will be ABI.
>>>>>>
>>>>>> We noticed the new power-sequencing infrastructure which is part of 6.11
>>>>>> too but I don't think that this patch is wrong. The DT ABI won't break
>>>>>> if we switch to the power-sequencing later on since the "reset-gpios"
>>>>>> are not marked as required. So it is up to the driver to handle it
>>>>>> either via a separate power-sequence driver or via "power-supply" and
>>>>>> "reset-gpios" directly.
>>>>>
>>>>> That's not the point. We expect correct hardware description. If you say
>>>>> now it has "reset-gpios" but later say "actually no, because it has
>>>>> PMU", I respond: no. Describe the hardware, not current Linux.
>>>>
>>>> I know that DT abstracts the HW. That said I don't see the problem with
>>>> this patch. The HW is abstracted just fine:
>>>>
>>>> shared PDn          -> reset-gpios
>>>> shared power-supply -> vcc-supply
>>>>
>>>> Right now the DT ABI for the BT part is incomplete since it assume a
>>>> running WLAN part or some hog-gpios to pull the device out-of-reset
>>>> which is addressed by this patchset.
>>>>
>>>> Making use of the new power-sequencing fw is a Linux detail and I don't
>>>> see why the DT can't be extended later on. We always extend the DT if
>>>> something is missing or if we found a better way to handle devices.
>>>
>>> Sure, although I am not really confident that you understand the
>>> implications - you will not be able to switch to proper power-sequencing
>>> with above bindings, because it will not be just possible without
>>> breaking the ABI or changing hardware description (which you say it is
>>> "fine", so complete/done). I am fine with it, just mind the implications.
>>
>> Sorry can you please share your concerns? I don't get the point yet why
>> we do break the DT ABI if we are going from
> 
> Not necessarily breaking ABI, but changing the description.
>>
>> bt {
>> 	reset-gpios = <&gpio 4 0>;
>> 	vcc-supply = <&supply>;
>> };
>>
>> to
>>
>> bt {
>> 	vcc-supply = <&pmu_supply>;
> 
> ...because you just removed reset-gpios which is a property of this device.
> 
>> };
>>
>> or:
>>
>> bt {
>> 	pmu = <&pmu>;

Ah, and I forgot here: this also might not be correct, because if you
have PMU, then the PMU consumes VCC, not the Bluetooth. Therefore both
of above two solutions might be inaccurate description if you decide to
go with PMU.

>> };
>>
>> Of course the driver need to support all 2/3 cases due to backward
>> compatibility but from DT pov I don't see any breakage since we already
>> need to define the power handling properties (gpio & supply) as
>> optional.
> 
> Either existing binding is complete or not. Not half-done.
> 
>>
>> That beeing said I don't see the need for a PMU driver for this WLAN/BT
>> combi chip which is way simpler than the Qualcomm one from Bartosz. Also
>> there is physically no PMU device which powers the chip unlike the
>> Qualcomm one. I'm not sure if you would accept virtual PMU devices.
> 
> Virtual PMU, of course not. I would like to have complete hardware
> description, not something which matches your current driver model.
> 
> Best regards,
> Krzysztof
> 

Best regards,
Krzysztof