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

On 04/12/2025 14:16, Vladimir Oltean wrote:

On Tue, Dec 02, 2025 at 12:20:31PM +0000, Daniel Golle wrote:

quoted

On Tue, Dec 02, 2025 at 12:52:44PM +0100, Frank Wunderlich wrote:

quoted

Hi,

Am 01.12.25 um 08:48 schrieb Krzysztof Kozlowski:

quoted

On 30/11/2025 21:17, Andrew Lunn wrote:

quoted

On Sun, Nov 30, 2025 at 10:07:31AM +0200, Vladimir Oltean wrote:

quoted

On Sun, Nov 30, 2025 at 02:11:05AM +0100, Andrew Lunn wrote:

quoted

quoted

-		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.

I'm also not aware of any board which doesn't directly connect the
reset of the MT7530 to a GPIO pin of the SoC. For MediaTek's designs
there is often even a specific pin desginated for this purpose and
most vendors do follow this. If they deviate at all, then it's just
that a different pin is used for the switch reset, but I've never
seen any logic between the SoC's GPIO pin and the switch reset.

quoted

Maybe the mt7988 (mt7530-mmio) based boards also affected?

There is no GPIO reset for switches which are integrated in the SoC,
so this only matters for external MT7530 and MT7531 ICs for which an
actual GPIO line connected to the SoC is used to reset the switch.

I get the feeling that we're complicating a simple solution because of a
theoretical "what if" scenario. The "NOT" gate is somewhat contrived

You downplay this case and suggest (if I get it right) that NOT gate is
something unusual.

 I mentioned "line inverter" but it's not about NOT gate. There is no
need for NOT gate at all, like some magical component which no one puts
to the board. The only thing needed is just to pull the GPIO up or down,
that's it. It's completely normal design thus it CAN happen.

Of course "can" does not mean it actually does, because certain
configurations like powerdown-fail-safe are more likely and I am not an
electric circuit designer to tell which one is better, but that
downplaying does not help here.

Just to clarify: I expect clear communication that some users will be
broken with as good as you can provide analysis of the impact (which
users). I only object the clame here "no one can ever pull down a GPIO
line thus I handled all possible cases and made it backward compatible".

And that claim to quote was:
"Therefore, regardless of whether a DTS is old or new, correct or
incorrect, the driver now generates the correct electrical reset pulse."

which is 100% false and I am surprised how one could claim that.

given the fact that most GPIOs can already be active high or low, but OK.

If this is blocking progress for new device trees, can we just construct,
using of_machine_is_compatible(), a list of all boards where the device
tree defines incorrect reset polarity that shouldn't be trusted by the
driver when driving the reset GPIO? If we do this, we can also leave
those existing device trees alone.

Works for me. You can also print fat warning for every unusual case.
Nothing like that was here in the original patch...


Best regards,
Krzysztof