linux-kernel - Re: [PATCH v2 01/11] Revert "regulator: pca9450: Add sd-vsel GPIO"

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: <b5456480-a68f-4e71-831b-f145453e2646@kontron.de>
Date: Tue, 10 Dec 2024 16:59:06 +0100
From: Frieder Schrempf <frieder.schrempf@...tron.de>
To: Conor Dooley <conor@...nel.org>, Frieder Schrempf <frieder@...s.de>
Cc: linux-arm-kernel@...ts.infradead.org, Marek Vasut <marex@...x.de>,
 Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Liam Girdwood
 <lgirdwood@...il.com>, linux-kernel@...r.kernel.org,
 Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
 Robin Gong <yibin.gong@....com>, Joy Zou <joy.zou@....com>,
 Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH v2 01/11] Revert "regulator: pca9450: Add sd-vsel GPIO"

On 28.11.24 6:37 PM, Conor Dooley wrote:
> On Wed, Nov 27, 2024 at 05:42:17PM +0100, Frieder Schrempf wrote:
>> From: Frieder Schrempf <frieder.schrempf@...tron.de>
>>
>> This reverts commit 27866e3e8a7e93494f8374f48061aa73ee46ceb2.
>>
>> It turned out that this feature was implemented based on
>> the wrong assumption that the SD_VSEL signal needs to be
>> controlled as GPIO in any case.
>>
>> In fact the straight-forward approach is to mux the signal
>> as USDHC_VSELECT and let the USDHC controller do the job.
>>
>> Most users never even used this property and the few who
>> did have been or are getting migrated to the alternative
>> approach.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@...tron.de>
>> ---
>> Changes for v2:
>> * split revert into separate patch
>> ---
>>  .../devicetree/bindings/regulator/nxp,pca9450-regulator.yaml | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> index f8057bba747a5..79fc0baf5fa2f 100644
>> --- a/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> +++ b/Documentation/devicetree/bindings/regulator/nxp,pca9450-regulator.yaml
>> @@ -77,11 +77,6 @@ properties:
>>  
>>      additionalProperties: false
>>  
>> -  sd-vsel-gpios:
>> -    description: GPIO that is used to switch LDO5 between being configured by
>> -      LDO5CTRL_L or LDO5CTRL_H register. Use this if the SD_VSEL signal is
>> -      connected to a host GPIO.
> 
> Your driver side of this, that I wasn't sent and cba downloading an
> mbox of is not backwards compatible. The code has been there for a few
> years, are you sure that there are no out of tree users or other OSes
> that use the property?

Yes, this is not backwards compatible. I introduced the original meaning
for the sd-vsel-gpios property based on some misunderstanding of how the
hardware actually works. Therefore I'm quite sure that except for the
cases where someone copied my erroneous implementation into their
devicetree, nobody has really any reason to actually use this.

In-tree all users have been removed (one fix still included in this
series). Of course we can't be fully sure that there isn't someone out
there having non-standard hardware (SD_VSEL not connected to
USDHC_VSELECT but to GPIO only) and using the old sd-vsel-gpios, but the
probability is very, very low.

IMHO taking the small risk here is better than keeping the misleading
implementation which will likely cause confusion and failures in the
future. But of course that's not up to me to decide.

> 
> tbh, I think all 3 of your dt-binding patches should be squashed rather
> than drip-feeding the conversion. It makes more sense as a single
> change, rather than splitting the rationales across 3 patches.

Ok, if you like this better in one change I can squash these for the
next version.

Thanks!