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

[PATCH v2 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 v2 2/2] net: dsa: mt7530: fix active-low reset sequence

[Chen Minqiang]

With GPIO_ACTIVE_LOW configured in DTS, gpiod_set_value(1) asserts reset
(drives the line low), and gpiod_set_value(0) deasserts reset (drives high).

Update the reset sequence so that the driver:
 - asserts reset by driving the GPIO low first
 - waits for the required reset interval
 - deasserts reset by driving it high

This ensures MT7531 receives a correct low-to-high reset pulse.

Compatibility notes:

The previous implementation contained a polarity mismatch: the DTS
described the reset line as active-high, while the driver asserted reset
by driving the GPIO low. The two mistakes matched each other, so the
reset sequence accidentally worked.

This patch fixes both sides: the DTS is corrected to use
GPIO_ACTIVE_LOW, and the driver now asserts reset by driving the line
low (value = 1 for active-low) and then deasserts it by driving it high
(value = 0).

Because the old behaviour relied on a matched pair of bugs, this change
is not compatible with mixed combinations of old DTS and new kernel, or
new DTS and old kernel. Both sides must be updated together.

Upstream DTS and upstream kernels will remain fully compatible after
this patch. Out-of-tree DT blobs must update their reset-gpios flags to
match the correct hardware polarity, or the switch may remain stuck in
reset or fail to reset properly.

There is no practical way to maintain compatibility with the previous
incorrect behaviour without adding non-detectable heuristics, so fixing
the binding and the driver together is the correct approach.

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

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 548b85befbf4..24c9adff191d 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2405,9 +2405,9 @@ mt7530_setup(struct dsa_switch *ds)
 		usleep_range(5000, 5100);
 		reset_control_deassert(priv->rstc);
 	} else {
-		gpiod_set_value_cansleep(priv->reset, 0);
-		usleep_range(5000, 5100);
 		gpiod_set_value_cansleep(priv->reset, 1);
+		usleep_range(5000, 5100);
+		gpiod_set_value_cansleep(priv->reset, 0);
 	}
 
 	/* Waiting for MT7530 got to stable */
@@ -2643,9 +2643,9 @@ mt7531_setup(struct dsa_switch *ds)
 		usleep_range(5000, 5100);
 		reset_control_deassert(priv->rstc);
 	} else {
-		gpiod_set_value_cansleep(priv->reset, 0);
-		usleep_range(5000, 5100);
 		gpiod_set_value_cansleep(priv->reset, 1);
+		usleep_range(5000, 5100);
+		gpiod_set_value_cansleep(priv->reset, 0);
 	}
 
 	/* Waiting for MT7530 got to stable */
-- 
2.17.1

Re: [PATCH v2 2/2] net: dsa: mt7530: fix active-low reset sequence

[Vladimir Oltean]

Hi Chen,

On Thu, 20 Nov 2025 at 23:40, Chen Minqiang <ptpt52@gmail.com> wrote:
>
> With GPIO_ACTIVE_LOW configured in DTS, gpiod_set_value(1) asserts reset
> (drives the line low), and gpiod_set_value(0) deasserts reset (drives high).
>
> Update the reset sequence so that the driver:
>  - asserts reset by driving the GPIO low first
>  - waits for the required reset interval
>  - deasserts reset by driving it high
>
> This ensures MT7531 receives a correct low-to-high reset pulse.
>
> Compatibility notes:
>
> The previous implementation contained a polarity mismatch: the DTS
> described the reset line as active-high, while the driver asserted reset
> by driving the GPIO low. The two mistakes matched each other, so the
> reset sequence accidentally worked.
>
> This patch fixes both sides: the DTS is corrected to use
> GPIO_ACTIVE_LOW, and the driver now asserts reset by driving the line
> low (value = 1 for active-low) and then deasserts it by driving it high
> (value = 0).
>
> Because the old behaviour relied on a matched pair of bugs, this change
> is not compatible with mixed combinations of old DTS and new kernel, or
> new DTS and old kernel. Both sides must be updated together.
>
> Upstream DTS and upstream kernels will remain fully compatible after
> this patch. Out-of-tree DT blobs must update their reset-gpios flags to
> match the correct hardware polarity, or the switch may remain stuck in
> reset or fail to reset properly.
>
> There is no practical way to maintain compatibility with the previous
> incorrect behaviour without adding non-detectable heuristics, so fixing
> the binding and the driver together is the correct approach.
>
> Signed-off-by: Chen Minqiang <ptpt52@gmail.com>
> ---

If the switch reset is active low and that is a known invariant fact about this
hardware IP, why not use gpiod_is_active_low() in the driver to decide whether
to call the old reset sequence of the new one?

Then you can fix device trees without regressing the kernel. You'll still
regress old kernels which boot on updated (fixed) device trees, but for the
latter, you can request a backport of the DSA driver patch to stable kernels.

In networking process terms (be sure to read
Documentation/process/maintainer-netdev.rst)
it means you'll send one patch to the 'net' tree with a Fixes: tag like this:
Fixes: c288575f7810 ("net: dsa: mt7530: Add the support of MT7531 switch")
(when unsure, use "git log" and watch https://lore.kernel.org/netdev/ to see
how others do things), and separate patch sets to the respective SoC maintainers
for updating device trees.

BTW, didn't ./scripts/get_maintainer.pl also suggest my name in the CC
list for this email?

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

[Andrew Lunn]

On Fri, Nov 21, 2025 at 05:38:04AM +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.

Sometimes we need to live with issues like this, because of:

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

So the real question is, does this actually harm anything? Do we see
any real issues?

	Andrew