[PATCH v3 1/2] ARM64: dts: mediatek: fix MT7531 reset GPIO polarity on multiple boards

[PATCH v3 1/2] ARM64: dts: mediatek: fix MT7531 reset GPIO polarity on multiple boards

[Chen Minqiang]

The MT7531 reset pin is active-low, but several DTS files configured the
reset-gpios property without GPIO_ACTIVE_LOW. This causes the reset GPIO
to behave as active-high and prevents the switch from being properly reset.

Update all affected DTS files to correctly use GPIO_ACTIVE_LOW so that
the reset polarity matches the hardware design.

Boards fixed:
 - mt7622-bananapi-bpi-r64
 - mt7622-rfb1
 - mt7986a-bananapi-bpi-r3
 - mt7986a-rfb
 - mt7986b-rfb

Note: the previous DTS description used the wrong polarity but the
driver also assumed the opposite polarity, resulting in a matched pair
of bugs that worked together. Updating the DTS requires updating the
driver at the same time; old kernels will not reset the switch correctly
when used with this DTS.

Compatibility
-------------

Correcting the polarity creates intentional incompatibility:

 * New kernel + old DTS:
   The driver now expects active-low, but out-of-tree DTS still marks
   active-high, causing the reset sequence to invert.

 * Old kernel + new DTS:
   The old driver toggles reset assuming active-high, which now
   conflicts with the corrected active-low DTS.

This was unavoidable because the original DTS was factually wrong.
Out-of-tree DTS users must update their DTS together with the kernel.

Signed-off-by: Chen Minqiang <ptpt52@gmail.com>
---
 arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 2 +-
 arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts             | 2 +-
 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts | 2 +-
 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts             | 2 +-
 arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts             | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index 9f100b18a676..6f29ce828fdb 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -155,7 +155,7 @@
 			interrupt-controller;
 			#interrupt-cells = <1>;
 			interrupts-extended = <&pio 53 IRQ_TYPE_LEVEL_HIGH>;
-			reset-gpios = <&pio 54 0>;
+			reset-gpios = <&pio 54 GPIO_ACTIVE_LOW>;
 
 			ports {
 				#address-cells = <1>;
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
index 8c3e2e2578bc..6600f06ccebf 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
@@ -137,7 +137,7 @@
 		switch@0 {
 			compatible = "mediatek,mt7531";
 			reg = <0>;
-			reset-gpios = <&pio 54 0>;
+			reset-gpios = <&pio 54 GPIO_ACTIVE_LOW>;
 
 			ports {
 				#address-cells = <1>;
diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
index e7654dc9a1c9..8ec2ec78ee46 100644
--- a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dts
@@ -203,7 +203,7 @@
 		interrupt-parent = <&pio>;
 		interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;
 		#interrupt-cells = <1>;
-		reset-gpios = <&pio 5 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&pio 5 GPIO_ACTIVE_LOW>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
index 5d8e3d3f6c20..958ce291336b 100644
--- a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
@@ -87,7 +87,7 @@
 	switch: switch@0 {
 		compatible = "mediatek,mt7531";
 		reg = <31>;
-		reset-gpios = <&pio 5 0>;
+		reset-gpios = <&pio 5 1>;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts b/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
index 58f77d932429..0780b5a36259 100644
--- a/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7986b-rfb.dts
@@ -64,7 +64,7 @@
 		switch@0 {
 			compatible = "mediatek,mt7531";
 			reg = <31>;
-			reset-gpios = <&pio 5 0>;
+			reset-gpios = <&pio 5 1>;
 
 			ports {
 				#address-cells = <1>;
-- 
2.17.1

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

[Chen Minqiang]

The MT7530/MT7531 reset pin is active-low in hardware, but the driver
historically hardcoded a high-active reset sequence by toggling the GPIO
as 0 → 1. This only worked because several DTS files incorrectly marked
the reset GPIO as active-high, making both DTS and driver wrong in the
same way.

This patch changes the driver to respect the GPIO polarity using
gpiod_is_active_low(), and generates the reset sequence as:

    assert   = drive logical active level
    deassert = drive logical inactive level

As a result, both cases now correctly produce the required
high → low → high transition on the actual reset pin.

Compatibility
-------------

This change makes the driver fully backward-compatible with older,
incorrect DTS files that marked the reset line as GPIO_ACTIVE_HIGH:

 * Old DTS marked active-high:
       is_active_low = 0
       driver drives 0 → 1
       actual levels: high → low → high  (correct)

 * New DTS marked active-low:
       is_active_low = 1
       driver drives 1 → 0
       actual levels: high → low → high  (correct)

Therefore, regardless of whether a DTS is old or new, correct or
incorrect, the driver now generates the correct electrical reset pulse.

Going forward, DTS files should use GPIO_ACTIVE_LOW to match the
hardware, but no regressions will occur with older DTS blobs.

Signed-off-by: Chen Minqiang <ptpt52@gmail.com>
---
 drivers/net/dsa/mt7530.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 548b85befbf4..615e9a5709ca 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2405,9 +2405,10 @@ mt7530_setup(struct dsa_switch *ds)
 		usleep_range(5000, 5100);
 		reset_control_deassert(priv->rstc);
 	} else {
-		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);
 		usleep_range(5000, 5100);
-		gpiod_set_value_cansleep(priv->reset, 1);
+		gpiod_set_value_cansleep(priv->reset, !is_active_low);
 	}
 
 	/* Waiting for MT7530 got to stable */
@@ -2643,9 +2644,10 @@ mt7531_setup(struct dsa_switch *ds)
 		usleep_range(5000, 5100);
 		reset_control_deassert(priv->rstc);
 	} else {
-		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);
 		usleep_range(5000, 5100);
-		gpiod_set_value_cansleep(priv->reset, 1);
+		gpiod_set_value_cansleep(priv->reset, !is_active_low);
 	}
 
 	/* Waiting for MT7530 got to stable */
-- 
2.17.1

Re: [PATCH v3 1/2] ARM64: dts: mediatek: fix MT7531 reset GPIO polarity on multiple boards

[Andrew Lunn]

On Sun, Nov 30, 2025 at 07:46:02AM +0800, Chen Minqiang wrote:
> The MT7531 reset pin is active-low, but several DTS files configured the
> reset-gpios property without GPIO_ACTIVE_LOW. This causes the reset GPIO
> to behave as active-high and prevents the switch from being properly reset.
> 
> Update all affected DTS files to correctly use GPIO_ACTIVE_LOW so that
> the reset polarity matches the hardware design.
> 
> Boards fixed:
>  - mt7622-bananapi-bpi-r64
>  - mt7622-rfb1
>  - mt7986a-bananapi-bpi-r3
>  - mt7986a-rfb
>  - mt7986b-rfb
> 
> Note: the previous DTS description used the wrong polarity but the
> driver also assumed the opposite polarity, resulting in a matched pair
> of bugs that worked together. Updating the DTS requires updating the
> driver at the same time; old kernels will not reset the switch correctly
> when used with this DTS.
> 
> Compatibility
> -------------
> 
> Correcting the polarity creates intentional incompatibility:
> 
>  * New kernel + old DTS:
>    The driver now expects active-low, but out-of-tree DTS still marks
>    active-high, causing the reset sequence to invert.
> 
>  * Old kernel + new DTS:
>    The old driver toggles reset assuming active-high, which now
>    conflicts with the corrected active-low DTS.
> 
> This was unavoidable because the original DTS was factually wrong.

So, the real question is, are the regressions you are going to cause
by breaking backwards compatibility worth it? 

What happens if we don't fix this? Does anything break if we leave it
as it is?

I personally think it be better to just document the issue in the
device tree binding.

	Andrew

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

[Andrew Lunn]

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

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

[Vladimir Oltean]

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

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

[Vladimir Oltean]

On Sun, Nov 30, 2025 at 07:46:03AM +0800, Chen Minqiang wrote:
> This change makes the driver fully backward-compatible with older,
> incorrect DTS files that marked the reset line as GPIO_ACTIVE_HIGH

The driver _is_ already backward-compatible with incorrect device trees.
This patch makes it compatible with "correct" device trees.

We need care taken in one more area: when you make updates to the device
tree, *old* versions of the kernel are not compatible with the latest
device tree, which is not OK.

So ideally:
- patch 2/2 should be considered a bug fix and backported to stable kernels
- you wait for some time to pass between when patch 2/2 is merged, and
  when patch 1/2 is merged, so that users who get an updated device tree
  have gotten the kernel compatibility patch through stable channels

Then you need to consider that you break "git bisect" if you keep the
device tree the same (i.e. the latest) and just change kernels. That is
a trade-off that needs to be well justified (cost/benefit).

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

[Andrew Lunn]

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?

So lets work on the commit message some more, but i'm still not
convinced it is worth the effort, unless there is some board today
which really is broken and cannot be fixed unless a change like this
is made.

	Andrew

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

[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?

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

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

[Krzysztof Kozlowski]

On 30/11/2025 00:46, Chen Minqiang wrote:
> The MT7530/MT7531 reset pin is active-low in hardware, but the driver
> historically hardcoded a high-active reset sequence by toggling the GPIO
> as 0 → 1. This only worked because several DTS files incorrectly marked
> the reset GPIO as active-high, making both DTS and driver wrong in the
> same way.
> 
> This patch changes the driver to respect the GPIO polarity using
> gpiod_is_active_low(), and generates the reset sequence as:
> 
>     assert   = drive logical active level
>     deassert = drive logical inactive level
> 
> As a result, both cases now correctly produce the required
> high → low → high transition on the actual reset pin.
> 
> Compatibility
> -------------
> 
> This change makes the driver fully backward-compatible with older,
> incorrect DTS files that marked the reset line as GPIO_ACTIVE_HIGH:
> 

You must describe all cases, so old DTS using inverter, thus described
as active-low. This is perfectly possible setup.

>  * Old DTS marked active-high:
>        is_active_low = 0
>        driver drives 0 → 1
>        actual levels: high → low → high  (correct)
> 
>  * New DTS marked active-low:
>        is_active_low = 1
>        driver drives 1 → 0
>        actual levels: high → low → high  (correct)
> 

And new DTS working with inverter, so described with active-high.


Best regards,
Krzysztof

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

[Frank Wunderlich]

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
>

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

[Daniel Golle]

On Tue, Dec 02, 2025 at 12:52:44PM +0100, Frank Wunderlich wrote:
> 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.

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.

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

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

[Vladimir Oltean]

On Tue, Dec 02, 2025 at 12:20:31PM +0000, Daniel Golle wrote:
> On Tue, Dec 02, 2025 at 12:52:44PM +0100, Frank Wunderlich wrote:
> > 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.
> 
> 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.
> 
> > 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
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.

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

[Daniel Golle]

On Thu, Dec 04, 2025 at 03:16:26PM +0200, Vladimir Oltean wrote:
> On Tue, Dec 02, 2025 at 12:20:31PM +0000, Daniel Golle wrote:
> > On Tue, Dec 02, 2025 at 12:52:44PM +0100, Frank Wunderlich wrote:
> > > 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.
> > 
> > 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.
> > 
> > > 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
> 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.

From OpenWrt's point of view this would be kind of ugly as we would either
have to extend the list of affected boards downstream, or fix the polarity
in some but not all of our downstream DTS files. I'd prefer to rather
have the option to force the "wrong" GPIO polarity for theoretical future
boards with that (very unlikely to ever exist) NOT gate between the SoC
GPIO and switch reset line. That would allow to gradually update boards
to reflect the physical reality and yet the driver would not break if the
GPIO polarity is stated wrongly.

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

[Krzysztof Kozlowski]

On 04/12/2025 14:16, Vladimir Oltean wrote:
> On Tue, Dec 02, 2025 at 12:20:31PM +0000, Daniel Golle wrote:
>> On Tue, Dec 02, 2025 at 12:52:44PM +0100, Frank Wunderlich wrote:
>>> 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.
>>
>> 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.
>>
>>> 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

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

[Andrew Lunn]

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

I've still not seen a good answer to my question, why not just leave
it 'broken', and document the fact.

Does the fact it is inverted in both DT and the driver prevent us from
making some board work?

Why do we need to fix this?

Sometimes it is better to just leave it alone, if it is not hurting
anybody.

	Andrew

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

[Vladimir Oltean]

On Thu, Dec 04, 2025 at 04:22:10PM +0100, Andrew Lunn wrote:
> > 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.
> 
> I've still not seen a good answer to my question, why not just leave
> it 'broken', and document the fact.
> 
> Does the fact it is inverted in both DT and the driver prevent us from
> making some board work?
> 
> Why do we need to fix this?
> 
> Sometimes it is better to just leave it alone, if it is not hurting
> anybody.
> 
> 	Andrew

Frank said that the fact the driver expecting a wrong device tree is
forcing him to keep introducing even more wrong device trees for new
boards.

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

[Vladimir Oltean]

On Thu, Dec 04, 2025 at 01:50:52PM +0000, Daniel Golle wrote:
> On Thu, Dec 04, 2025 at 03:16:26PM +0200, Vladimir Oltean wrote:
> > 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.
> 
> From OpenWrt's point of view this would be kind of ugly as we would either
> have to extend the list of affected boards downstream, or fix the polarity
> in some but not all of our downstream DTS files.

Including the downstream-only boards, how many compatibles are we
talking about? If we patched mainline to cover all, are you confident
that it would be an exhaustive solution?

> I'd prefer to rather have the option to force the "wrong" GPIO
> polarity for theoretical future boards with that (very unlikely to
> ever exist) NOT gate between the SoC GPIO and switch reset line. That
> would allow to gradually update boards to reflect the physical reality
> and yet the driver would not break if the GPIO polarity is stated
> wrongly.

I didn't get that from this thread - how would you prefer having this
option implemented exactly?

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

[Andrew Lunn]

> Frank said that the fact the driver expecting a wrong device tree is
> forcing him to keep introducing even more wrong device trees for new
> boards.

So it sounds like there is no technical reason to fix this, it is just
aesthetics?

	Andrew

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

[Krzysztof Kozlowski]

On 04/12/2025 16:37, Vladimir Oltean wrote:
> On Thu, Dec 04, 2025 at 04:22:10PM +0100, Andrew Lunn wrote:
>>> 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.
>>
>> I've still not seen a good answer to my question, why not just leave
>> it 'broken', and document the fact.
>>
>> Does the fact it is inverted in both DT and the driver prevent us from
>> making some board work?
>>
>> Why do we need to fix this?
>>
>> Sometimes it is better to just leave it alone, if it is not hurting
>> anybody.
>>
>> 	Andrew
> 
> Frank said that the fact the driver expecting a wrong device tree is
> forcing him to keep introducing even more wrong device trees for new
> boards.


Yeah. BTW, you can also refer to one of my commits -
738455858a2d21b769f673892546cf8300c9fd78 - but also note that similar
work later combined with much more useful change of gpio->gpiod API was
rejected by Mark Brown on basis:

"No, the DT is supposed to be an ABI.  The point in having a domain
specific language with a compiler is to allow device trees to be
distributed independently of the kernel."

https://lore.kernel.org/all/9942c3a9-51d1-4161-8871-f6ec696cb4db@sirena.org.uk/

What's interesting, exactly the same commit for the same file, done by
different person (Peng), introducing the same issues without addressing
them, was then merged by Mark:
https://patch.msgid.link/20250324-wcd-gpiod-v2-2-773f67ce3b56@nxp.com
(commit c2d359b4acfbe847d3edcc25d3dc4e594daf9010)

so you know... it's all relative. :)

Best regards,
Krzysztof

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

[Vladimir Oltean]

On Thu, Dec 04, 2025 at 03:49:52PM +0100, Krzysztof Kozlowski wrote:
> On 04/12/2025 14:16, Vladimir Oltean wrote:
> > 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.

I don't want to dismiss this comment, but I don't really understand it.
What do you mean by "line inverter", is it the component inside the GPIO
pin which makes it active low?

I thought that the premise of this patch set is that old device trees
are all (incorrectly) defined as GPIO_ACTIVE_HIGH, but someone familiar
with the matter needs to fact-check this statement.

Anyway, you and Andrew are talking about different things, you haven't
made it clear (or at least it wasn't clear to me) that the inverter you
are talking about isn't his NOT gate (that isn't described in the device
tree at all, as opposed to your inverter which would make the GPIO line
GPIO_ACTIVE_LOW - that's something verifiable).

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

Agree, the communication should be better.

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

[Vladimir Oltean]

On Thu, Dec 04, 2025 at 04:52:53PM +0100, Krzysztof Kozlowski wrote:
> Yeah. BTW, you can also refer to one of my commits -
> 738455858a2d21b769f673892546cf8300c9fd78 - but also note that similar
> work later combined with much more useful change of gpio->gpiod API was
> rejected by Mark Brown on basis:
> 
> "No, the DT is supposed to be an ABI.  The point in having a domain
> specific language with a compiler is to allow device trees to be
> distributed independently of the kernel."
> 
> https://lore.kernel.org/all/9942c3a9-51d1-4161-8871-f6ec696cb4db@sirena.org.uk/
> 
> What's interesting, exactly the same commit for the same file, done by
> different person (Peng), introducing the same issues without addressing
> them, was then merged by Mark:
> https://patch.msgid.link/20250324-wcd-gpiod-v2-2-773f67ce3b56@nxp.com
> (commit c2d359b4acfbe847d3edcc25d3dc4e594daf9010)
> 
> so you know... it's all relative. :)

Yeah, IDK. May we all stand by our own past statements.

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

[Krzysztof Kozlowski]

On 04/12/2025 17:02, Vladimir Oltean wrote:
> On Thu, Dec 04, 2025 at 03:49:52PM +0100, Krzysztof Kozlowski wrote:
>> On 04/12/2025 14:16, Vladimir Oltean wrote:
>>> 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.
> 
> I don't want to dismiss this comment, but I don't really understand it.
> What do you mean by "line inverter", is it the component inside the GPIO
> pin which makes it active low?

I mentioned much earlier "line inverter" and here the "NOT gate"
appeared and both suggest you need some dedicated component.

What I want to say that it is not true. You do not need dedicated
component to have different polarity, because it is as simple as
connecting the wire to ground or to voltage and moving a resistor from
one place to another.

> 
> I thought that the premise of this patch set is that old device trees
> are all (incorrectly) defined as GPIO_ACTIVE_HIGH, but someone familiar
> with the matter needs to fact-check this statement.

Yes, that's most likely true.

> 
> Anyway, you and Andrew are talking about different things, you haven't
> made it clear (or at least it wasn't clear to me) that the inverter you
> are talking about isn't his NOT gate (that isn't described in the device
> tree at all, as opposed to your inverter which would make the GPIO line
> GPIO_ACTIVE_LOW - that's something verifiable).

Both are the same - inverter or NOT gate, same stuff. It is just
connecting wire to pull up, not actual component on the board (although
one could make and buy such component as well...). We never describe
these inverters in the DTS, these are just too trivial circuits, thus
the final GPIO_ACTIVE_XXX should already include whatever is on the wire
between SoC and device.

> 
>> 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.
> 
> Agree, the communication should be better.


Best regards,
Krzysztof

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

[Vladimir Oltean]

On Thu, Dec 04, 2025 at 05:48:07PM +0100, Krzysztof Kozlowski wrote:
> Both are the same - inverter or NOT gate, same stuff. It is just
> connecting wire to pull up, not actual component on the board (although
> one could make and buy such component as well...). We never describe
> these inverters in the DTS, these are just too trivial circuits, thus
> the final GPIO_ACTIVE_XXX should already include whatever is on the wire
> between SoC and device.

Please read what Andrew said:
https://lore.kernel.org/netdev/3fbc4e67-b931-421c-9d83-2214aaa2f6ed@lunn.ch/

  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?
                   ~~~~~~~~~~~~~~

You two are *not* talking about the same thing. I dismissed the
probability of there being a NOT gate in the form of a discrete chip on
the PCB, *exactly because* you can most likely invert the signal in the
GPIO pin itself.

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

[Krzysztof Kozlowski]

On 04/12/2025 18:11, Vladimir Oltean wrote:
> On Thu, Dec 04, 2025 at 05:48:07PM +0100, Krzysztof Kozlowski wrote:
>> Both are the same - inverter or NOT gate, same stuff. It is just
>> connecting wire to pull up, not actual component on the board (although
>> one could make and buy such component as well...). We never describe
>> these inverters in the DTS, these are just too trivial circuits, thus
>> the final GPIO_ACTIVE_XXX should already include whatever is on the wire
>> between SoC and device.
> 
> Please read what Andrew said:
> https://lore.kernel.org/netdev/3fbc4e67-b931-421c-9d83-2214aaa2f6ed@lunn.ch/
> 
>   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?
>                    ~~~~~~~~~~~~~~
> 
> You two are *not* talking about the same thing. I dismissed the


It's the same thing. NOT gate is just pulling some pin down or up.

> probability of there being a NOT gate in the form of a discrete chip on

We do not describe NOT gates as discreet chips. I don't think anyone
actually places something as NOT gate. It's logical NOT gate, but on
circuit it is just pull up/down as I said multiple times. The pull +
resistor is the "NOT gate".

It's so easy and that's why it is potentially so common design.

> the PCB, *exactly because* you can most likely invert the signal in the
> GPIO pin itself.



Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 04/12/2025 18:23, Krzysztof Kozlowski wrote:
> On 04/12/2025 18:11, Vladimir Oltean wrote:
>> On Thu, Dec 04, 2025 at 05:48:07PM +0100, Krzysztof Kozlowski wrote:
>>> Both are the same - inverter or NOT gate, same stuff. It is just
>>> connecting wire to pull up, not actual component on the board (although
>>> one could make and buy such component as well...). We never describe
>>> these inverters in the DTS, these are just too trivial circuits, thus
>>> the final GPIO_ACTIVE_XXX should already include whatever is on the wire
>>> between SoC and device.
>>
>> Please read what Andrew said:
>> https://lore.kernel.org/netdev/3fbc4e67-b931-421c-9d83-2214aaa2f6ed@lunn.ch/
>>
>>   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?
>>                    ~~~~~~~~~~~~~~
>>
>> You two are *not* talking about the same thing. I dismissed the
> 
> 
> It's the same thing. NOT gate is just pulling some pin down or up.

Although transistor would be still needed, so indeed that's still a bit
more than a wire and resistor as I implied.

It looks like:
https://www.electronics-tutorials.ws/wp-content/uploads/2018/05/logic-log47.gif


> 
>> probability of there being a NOT gate in the form of a discrete chip on
> 
> We do not describe NOT gates as discreet chips. I don't think anyone
> actually places something as NOT gate. It's logical NOT gate, but on
> circuit it is just pull up/down as I said multiple times. The pull +
> resistor is the "NOT gate".
> 
> It's so easy and that's why it is potentially so common design.


We still do not describe transistors in DTS. We do not describe NOT
gates. The final flag should include that as I said.


Best regards,
Krzysztof

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

[Russell King (Oracle)]

On Thu, Dec 04, 2025 at 06:23:48PM +0100, Krzysztof Kozlowski wrote:
> On 04/12/2025 18:11, Vladimir Oltean wrote:
> > You two are *not* talking about the same thing. I dismissed the
> 
> It's the same thing. NOT gate is just pulling some pin down or up.
> 
> > probability of there being a NOT gate in the form of a discrete chip on
> 
> We do not describe NOT gates as discreet chips. I don't think anyone
> actually places something as NOT gate. It's logical NOT gate, but on
> circuit it is just pull up/down as I said multiple times. The pull +
> resistor is the "NOT gate".

You can get SOT-23 packages that are NOT gates. E.g.
https://www.ti.com/lit/ds/symlink/sn74ahc1g04.pdf?ts=1764851465989

This is a real NOT gate - it's absolute max ratings state that it
can source/sink up to 25mA through its output. So no external
resistor required.

What you describe sounds to me more like a normal transistor though.

Irrespective of this, the exact nature of a device that inverts the
level of a reset signal in the path between a GPIO pin and a device
is irrelevant. What matters is whether there is or is not such a
device.

So, can we stop splitting hairs about what a NOT gate is in this
discussion? It's irrelevant.

> It's so easy and that's why it is potentially so common design.

Do you have any real evidence of this when used with a GPIO pin for
a reset input?

It would make sense if a single GPIO pin is used for resetting
several devices, some of them with an active-high reset input and
others with active-low.

What matters for a GPIO pin used to source a reset signal is "what
is the active level at the GPIO pin for the reset to be asserted to
the connected device(s)."

If we have a device that requires an active-low reset input, but there
is some form of inversion in the path to that input from a GPIO, then
the GPIO _should_ be marked active-high. If the same active-low reset
input is connected directly to a GPIO, the GPIO _should_ be marked as
active-low. Thus, to assert reset, writing '1' through
gpiod_set_value() _should_ assert the reset input on the target device
in both cases.

This is why gpiolib supports software inversion - so software engineers
can think in terms of positive non-inverted logic when programming
GPIOs.

Sadly, we keep having people mark active-low signals as "active high"
in DT, and then have to write '0' to assert the signal. These people
basically don't understand electronics and/or our GPIO model.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

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

[Vladimir Oltean]

On Thu, Dec 04, 2025 at 05:45:30PM +0000, Russell King (Oracle) wrote:
> It would make sense if a single GPIO pin is used for resetting
> several devices, some of them with an active-high reset input and
> others with active-low.
> 
> What matters for a GPIO pin used to source a reset signal is "what
> is the active level at the GPIO pin for the reset to be asserted to
> the connected device(s)."
> 
> If we have a device that requires an active-low reset input, but there
> is some form of inversion in the path to that input from a GPIO, then
> the GPIO _should_ be marked active-high. If the same active-low reset
> input is connected directly to a GPIO, the GPIO _should_ be marked as
> active-low. Thus, to assert reset, writing '1' through
> gpiod_set_value() _should_ assert the reset input on the target device
> in both cases.
> 
> This is why gpiolib supports software inversion - so software engineers
> can think in terms of positive non-inverted logic when programming
> GPIOs.

Thanks for this message, to me it brought new info which I didn't
consider before, which is that if there are *future* boards where some
sort of inversion on the reset signal (likely due to it being shared as
you say), they should be described as GPIO_ACTIVE_HIGH, but this patch
locks us out of that possibility.

If I may bring one more data point into the discussion: the switch
binding specifically requests not to describe shared reset lines
(because unrelated drivers might reset the switch after it probed):

  reset-gpios:
    description: |
      GPIO to reset the switch. Use this if mediatek,mcm is not used.
      This property is optional because some boards share the reset line with
      other components which makes it impossible to probe the switch if the
      reset line is used.

so I guess that the probability for a reset signal inversion, even if
it exists, to cause problems in the GPIO_ACTIVE_HIGH reinterpretation,
is still low, albeit for a completely different reason than the one I
initially claimed (which is bogus and Krzysztof was right to challenge it).

> Sadly, we keep having people mark active-low signals as "active high"
> in DT, and then have to write '0' to assert the signal. These people
> basically don't understand electronics and/or our GPIO model.

This is the case we seem to be in.

On a scale of safety, having quirks for the affected device trees still
ranks a bit higher, though.

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

[Russell King (Oracle)]

On Thu, Dec 04, 2025 at 06:32:23PM +0100, Krzysztof Kozlowski wrote:
> On 04/12/2025 18:23, Krzysztof Kozlowski wrote:
> > On 04/12/2025 18:11, Vladimir Oltean wrote:
> >> On Thu, Dec 04, 2025 at 05:48:07PM +0100, Krzysztof Kozlowski wrote:
> >>> Both are the same - inverter or NOT gate, same stuff. It is just
> >>> connecting wire to pull up, not actual component on the board (although
> >>> one could make and buy such component as well...). We never describe
> >>> these inverters in the DTS, these are just too trivial circuits, thus
> >>> the final GPIO_ACTIVE_XXX should already include whatever is on the wire
> >>> between SoC and device.
> >>
> >> Please read what Andrew said:
> >> https://lore.kernel.org/netdev/3fbc4e67-b931-421c-9d83-2214aaa2f6ed@lunn.ch/
> >>
> >>   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?
> >>                    ~~~~~~~~~~~~~~
> >>
> >> You two are *not* talking about the same thing. I dismissed the
> > 
> > 
> > It's the same thing. NOT gate is just pulling some pin down or up.
> 
> Although transistor would be still needed, so indeed that's still a bit
> more than a wire and resistor as I implied.
> 
> It looks like:
> https://www.electronics-tutorials.ws/wp-content/uploads/2018/05/logic-log47.gif

A bit more than even that... I do hope folk don't use exactly that,
that's the kind of thing that would be used in "learning about
electronics" projects! That's a recipe to drive a transistor into
full saturation, which makes it comparitively very slow to turn off.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!