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: <66d33097-37ed-4e89-a356-285eda743a5c@kernel.org>
Date: Tue, 22 Oct 2024 09:30:56 +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: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>;
> };
> 
> 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