linux-kernel - Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate correct reset sequence

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 for Android: free password hash cracker in your pocket

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

Message-ID: <00f308a1-a4b1-4f20-8d8e-459ddf4c39b1@gmx.de>
Date: Tue, 2 Dec 2025 12:52:44 +0100
From: Frank Wunderlich <frankwu@....de>
To: Krzysztof Kozlowski <krzk@...nel.org>, Andrew Lunn <andrew@...n.ch>,
 Vladimir Oltean <olteanv@...il.com>
Cc: Chen Minqiang <ptpt52@...il.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 "Chester A. Unal" <chester.a.unal@...nc9.com>,
 Daniel Golle <daniel@...rotopia.org>, DENG Qingfang <dqfext@...il.com>,
 Sean Wang <sean.wang@...iatek.com>, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH v3 2/2] net: dsa: mt7530: Use GPIO polarity to generate
 correct reset sequence

Hi,

Am 01.12.25 um 08:48 schrieb Krzysztof Kozlowski:
> On 30/11/2025 21:17, Andrew Lunn wrote:
>> On Sun, Nov 30, 2025 at 10:07:31AM +0200, Vladimir Oltean wrote:
>>> On Sun, Nov 30, 2025 at 02:11:05AM +0100, Andrew Lunn wrote:
>>>>> -		gpiod_set_value_cansleep(priv->reset, 0);
>>>>> +		int is_active_low = !!gpiod_is_active_low(priv->reset);
>>>>> +		gpiod_set_value_cansleep(priv->reset, is_active_low);
>>>> I think you did not correctly understand what Russell said. You pass
>>>> the logical value to gpiod_set_value(). If the GPIO has been marked as
>>>> active LOW, the GPIO core will invert the logical values to the raw
>>>> value. You should not be using gpiod_is_active_low().
>>>>
>>>> But as i said to the previous patch, i would just leave everything as
>>>> it is, except document the issue.
>>>>
>>>> 	Andrew
>>>>
>>> It was my suggestion to do it like this (but I don't understand why I'm
>>> again not in CC).
>>>
>>> We _know_ that the reset pin of the switch should be active low. So by
>>> using gpiod_is_active_low(), we can determine whether the device tree is
>>> wrong or not, and we can work with a wrong device tree too (just invert
>>> the logical values).
>> Assuming there is not a NOT gate placed between the GPIO and the reset
>> pin, because the board designer decided to do that for some reason?
jumping in because i prepare mt7987 / BPI-R4Lite dts for upstreaming 
when driver-changes are in.
With current driver i need to define the reset-gpio for mt7531 again 
wrong to get it
working. So to have future dts correct, imho this (or similar) change to 
driver is needed.

Of course we cannot simply say that current value is wrong and just 
invert it because of
possible "external" inversion of reset signal between SoC and switch.
I have to look on schematics for the boards i have (BPI-R64, BPI-R3, 
BPI-R2Pro) if there is such circuit.
Maybe the mt7988 (mt7530-mmio) based boards also affected?
But not changing driver imho results in future broken devicetrees or can 
this
avoided somehow?
Nethertheless the driverchange needs to be done before dts change.

regards Frank
> Yeah, I cannot imagine how this could possibly support old and new DTS
> without breaking some users, unless people over-simplified and discarded
> some cases. But then this should clearly mark these broken cases instead
> of falsely claim that impossible task of rewriting the flag is done
> correctly.
>
> BTW, the code clearly does not handle such cases, so "we can determine
> whether the device tree is wrong or not" statement is obviously not true.
>
> Best regards,
> Krzysztof
>