netdev - Re: [PATCH v2 2/2] net: dsa: mt7530: fix active-low 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: <CA+h21hoW3v_hL+woeQRwpxVCQ00ywWhJ0H+DfhaRqpMWB2ZSng@mail.gmail.com>
Date: Thu, 20 Nov 2025 23:57:15 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Chen Minqiang <ptpt52@...il.com>
Cc: 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>, Andrew Lunn <andrew@...n.ch>, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org
Subject: Re: [PATCH v2 2/2] net: dsa: mt7530: fix active-low reset sequence

Hi Chen,

On Thu, 20 Nov 2025 at 23:40, Chen Minqiang <ptpt52@...il.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@...il.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?