[PATCH net v2 0/2] net: dsa: mt7530: fix two crashes on driver unbind

[PATCH net v2 0/2] net: dsa: mt7530: fix two crashes on driver unbind

[Aleksei Sviridkin]

Unbinding the MT7530 driver from an MT7531 dereferences NULL in
regulator_disable(). On a Netcraze NC-1012 (MT7981B + MT7531, 6.18.44):

  # echo mdio-bus:1f > /sys/bus/mdio_bus/drivers/mt7530-mdio/unbind

oopses there, and the build it was found on sets CONFIG_PANIC_ON_OOPS, so
the board goes down with it. Fix that and the same command gets as far as
mt7530_remove_common(), which disposes interrupt mappings the switch's own
regmap-irq chip still owns; the regmap-irq thread then faults in
handle_nested_irq() later in the same teardown. rmmod reaches both, since
mdio_module_driver() calls .remove on module exit.

Patch 1 is the regulator one. mt7530_probe() requests the core and io
supplies only for ID_MT7530 and mt7530_setup() enables them under the same
test, but mt7530_remove() disables them unconditionally, so on an MT7621 or
an MT7531 both pointers are still NULL from devm_kzalloc(). It reaches the
MDIO front end only.

Patch 2 is the interrupt one, and it reaches further.
mt7530_remove_common() disposes the per-PHY interrupt mappings by hand from
.remove, while the regmap-irq chip that owns the domain is devm-registered
and its parent interrupt is only freed once .remove has returned.
regmap_del_irq_chip() disposes the same mappings itself, in an order that
cannot race, so the driver's call adds nothing but a window. That helper is
called from both front ends, so the defect also covers the MMIO parts -
MT7988, EN7581, AN7583 and EN7528 - which have no regulators and never meet
the first defect at all.

The order is not arbitrary. On an MT7531 the regulator fault happens in the
first thing mt7530_remove() does with the switch, so execution never
reaches the interrupt defect. The second only became visible once the first
was fixed, which is also how both came to be found on one board.

Found and verified there. Without patch 1 the unbind panics in
regulator_disable(); with patch 1 alone the panic moves on to
handle_nested_irq(); with both, three unbind/bind cycles run, two back to
back and a third after a pause. In the two whose dmesg was captured, each
unbind removes the switch from the driver directory and takes lan1 to lan4
with it, each bind brings them back, and lan1 relinks at 1Gbps/full after
both binds, lan4 after the second. uptime rose from 58 to 202 seconds
across the three without resetting and pstore gained no new record. The
third cycle stayed unbound long enough to read the descriptors: no mt7530
line in /proc/interrupts and no irq/79, irq/80 or irq/81 directory, and the
next bind reuses those three numbers - regmap-irq freeing and disposing
what the driver no longer touches. The kernel under test was identified by
the sha256 of its ELF notes section, read from /sys/kernel/notes on the
running board and computed in advance from the flashed image.

What hardware could not answer here. There is no MT7530 or MT7621 part on
this bench, so the ID_MT7530 branch that patch 1 adds was checked by
reading the generated code rather than by running it, and no MMIO part was
available to exercise patch 2 on that front end either. One unrelated WARN
remains across the unbind, from sysfs_remove_link() under
dsa_user_destroy(); it is a separate DSA teardown-ordering defect and is
not addressed here.

v2:
- patch 2 changes shape. Moving dsa_unregister_switch() in front of the
  dispose, as v1 did, only covers the phylib side: the regmap-irq chip is
  devm-registered, so its parent interrupt outlives .remove and the thread
  can still dispatch on a mapping the driver has just disposed. Dropping
  the call is what closes that: regmap_del_irq_chip() walks every hwirq
  below chip->num_irqs and disposes each one that maps, which covers
  whatever the driver created - the driver's own set is the user ports
  below MT7530_NUM_PHYS, hwirq 0 to 2 on the board below - and it does so
  after freeing the parent interrupt and before removing the domain.
- patch 2 points Fixes: at commit 254f6b272e3b ("dsa: mt7530: Utilize
  REGMAP_IRQ for interrupt handling") instead of commit ba751e28d442 ("net:
  dsa: mt7530: add interrupt support"). Before the regmap-irq conversion
  the driver owned the domain and removed it by hand, where
  irq_domain_remove() disposes nothing, so the call was required there; it
  became redundant when regmap-irq took the domain over.
- patch 1: same diff; the trailers are reordered and the message now names
  devm_kzalloc() as where the NULL comes from.
- v1: https://lore.kernel.org/netdev/20260914202421.2737079-1-f@lex.la/

Aleksei Sviridkin (2):
  net: dsa: mt7530: fix NULL dereference on unbind of MT7531 and MT7621
  net: dsa: mt7530: leave the MDIO IRQ mappings to regmap-irq

 drivers/net/dsa/mt7530-mdio.c | 18 ++++++++++--------
 drivers/net/dsa/mt7530.c      |  3 ---
 2 files changed, 10 insertions(+), 11 deletions(-)

-- 
2.53.0

[PATCH net v2 1/2] net: dsa: mt7530: fix NULL dereference on unbind of MT7531 and MT7621

[Aleksei Sviridkin]

The core and io supplies are only requested for ID_MT7530: both the
devm_regulator_get() in probe and the regulator_enable() in
mt7530_setup() are guarded by the switch id, but mt7530_remove()
disables them unconditionally. On an MT7621 or an MT7531 both pointers
are still NULL from devm_kzalloc(), so rmmod or a sysfs unbind calls
regulator_disable() on NULL.

Fixes: ddda1ac116c8 ("net: dsa: mt7530: support the 7530 switch on the Mediatek MT7621 SoC")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Found by accident on a Netcraze NC-1012 (MT7981B + MT7531, 6.18.44) while
looking for a way to tear a DSA port down at runtime:

  # echo mdio-bus:1f > /sys/bus/mdio_bus/drivers/mt7530-mdio/unbind
  Unable to handle kernel access to user memory outside uaccess routines
    at virtual address 0000000000000078
  pc : regulator_disable+0x14/0x48
  lr : mt7530_remove+0x1c/0x80
  ...
  x0 : 0000000000000000
  Call trace:
   regulator_disable+0x14/0x48 (P)
   mt7530_remove+0x1c/0x80
   mdio_remove+0x20/0x40
   device_remove+0x68/0x80
   device_release_driver_internal+0x1cc/0x220
   device_driver_detach+0x14/0x20
   unbind_store+0xac/0xb0
   ...
  Kernel panic - not syncing: Oops: Fatal exception

The oops itself is a process-context oops that kills the writing task. It
became a panic and a reboot because OpenWrt's generic kernel config sets
CONFIG_PANIC_ON_OOPS=y and this target does not override it, not because of
anything local to this bench. With CONFIG_REGULATOR=n the stub
regulator_disable() returns 0 and nothing is dereferenced at all -
NET_DSA_MT7530 neither selects nor depends on REGULATOR - so the severity
is config-dependent, and the commit message states the mechanism rather
than an outcome.

x0 is the regulator pointer and regulator_disable() reads regulator->rdev
straight away, so the NULL comes from the field never being assigned rather
than from an error pointer: with CONFIG_REGULATOR=y devm_regulator_get()
hands back a valid pointer or an ERR_PTR, and on anything but ID_MT7530 it
is never called at all.

The same shape applies to MT7621, which mt7530_of_match also binds. The
MMIO driver is unaffected: it makes no regulator calls at all, though it
does still carry the include.

The id test is used rather than a NULL check because the driver already
says "these supplies belong to ID_MT7530" that way in the other two places
it matters: the devm_regulator_get() pair in mt7530_probe() and the
regulator_set_voltage()/regulator_enable() pair in mt7530_setup(). A NULL
check would be a third spelling of the same condition.

Tested on the board above. Without the patch the unbind panics as shown;
both pointers come out of devm_kzalloc() and are never assigned on an
MT7531, so the fault is structural rather than timing-dependent. With this
patch plus patch 2, the same unbind runs to completion: mdio-bus:1f leaves
/sys/bus/mdio_bus/drivers/mt7530-mdio/, lan1-lan4 disappear, the kernel
prints "DSA: tree 0 torn down", and uptime does not reset. The kernel under
test was identified by the sha256 of its ELF notes section, read from
/sys/kernel/notes on the running board and computed in advance from the
image that was flashed.

dmesg is not silent across that unbind. It gains one WARN - a single cut
here/WARNING/end trace block - from sysfs_remove_link() under
dsa_user_destroy() reaching an already-removed netdev directory: "kernfs:
can not remove 'phydev', no directory". That is a DSA teardown-ordering
defect rather than a regulator one, and in this run it fired on the first
unbind after boot. dmesg is where it shows: pstore gained no new record
across the run, but pstore records only oopses and panics and could not
have caught a WARN.

That run needs patch 2 on top, a different defect in the same teardown:
mt7530_remove_common() disposes the per-PHY interrupt mappings from .remove
while the regmap-irq chip that owns the domain is still live, and the board
dies in handle_nested_irq() later in the same teardown. With this patch
alone, the panic moves from regulator_disable+0x14 to that second defect,
and mt7530_remove is reached at +0x24/0x90 rather than +0x1c/0x80 - which
is the point: the NULL dereference is gone and execution now gets past it.

The disable stays where it is, ahead of mt7530_remove_common() and so ahead
of the register writes dsa_unregister_switch() makes on the way down. On a
real MT7530 that is teardown talking to a switch whose rails are already
off, which is a separate and pre-existing ordering question, not addressed
here; moving it would change when the supplies drop, which is more than a
NULL-pointer fix should do.

Not tested: the ID_MT7530 branch, which must still disable both rails.
There is no MT7530 or MT7621 hardware here, so the disassembly stands in
for it - before the change mt7530_remove() falls from the priv NULL check
straight into ldr x0, [x19, #40] / bl regulator_disable; after it, ldr w0,
[x19, #72] (priv->id) / cbz w0 gates both calls, and ID_MT7530 is 0. Built
with W=1, no warnings; checkpatch --strict clean.

 drivers/net/dsa/mt7530-mdio.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
index 784dd58a7158..de42f70afcfa 100644
--- a/drivers/net/dsa/mt7530-mdio.c
+++ b/drivers/net/dsa/mt7530-mdio.c
@@ -227,15 +227,17 @@ mt7530_remove(struct mdio_device *mdiodev)
 	if (!priv)
 		return;
 
-	ret = regulator_disable(priv->core_pwr);
-	if (ret < 0)
-		dev_err(priv->dev,
-			"Failed to disable core power: %d\n", ret);
+	if (priv->id == ID_MT7530) {
+		ret = regulator_disable(priv->core_pwr);
+		if (ret < 0)
+			dev_err(priv->dev,
+				"Failed to disable core power: %d\n", ret);
 
-	ret = regulator_disable(priv->io_pwr);
-	if (ret < 0)
-		dev_err(priv->dev, "Failed to disable io pwr: %d\n",
-			ret);
+		ret = regulator_disable(priv->io_pwr);
+		if (ret < 0)
+			dev_err(priv->dev, "Failed to disable io pwr: %d\n",
+				ret);
+	}
 
 	mt7530_remove_common(priv);
 
-- 
2.53.0

[PATCH net v2 2/2] net: dsa: mt7530: leave the MDIO IRQ mappings to regmap-irq

[Aleksei Sviridkin]

mt7530_remove_common() disposes the per-PHY interrupt mappings from
.remove, but the regmap-irq chip that owns the domain is devm-registered,
so its parent interrupt is only freed once .remove has returned. The
switch's own regmap-irq thread can therefore still dispatch on a mapping
that is already gone: irq_find_mapping() returns 0, irq_to_desc() returns
NULL and handle_nested_irq() locks desc->lock without checking it. The
attached PHYs have not given those interrupts back yet either, which the
kernel warns about a moment before the fault.

regmap_del_irq_chip() disposes the same mappings itself, after freeing the
parent interrupt and before removing the domain, so there is nothing left
for the driver to do here. Until it runs the descriptors stay alive, and a
late dispatch on one of them is harmless: dsa_unregister_switch() has freed
the PHY handlers by then, so handle_nested_irq() finds no action and
returns.

Fixes: 254f6b272e3b ("dsa: mt7530: Utilize REGMAP_IRQ for interrupt handling")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Found on a Netcraze NC-1012 (MT7981B + MT7531, 6.18.44) directly behind the
regulator fix in patch 1: with that one applied the unbind stops faulting
in mt7530_remove() and reaches the teardown, where the kernel says what is
wrong in words before it dies.

  # echo mdio-bus:1f > /sys/bus/mdio_bus/drivers/mt7530-mdio/unbind
  remove_proc_entry: removing non-empty directory 'irq/81', leaking at least
    'mt7530-0:02'
  WARNING: CPU: 0 PID: 4629 at remove_proc_entry+0x1d0/0x1f0
  ...
  Call trace:
   remove_proc_entry+0x1d0/0x1f0 (P)
   unregister_irq_proc+0xd0/0x104
   free_desc+0x38/0xa0
   irq_free_descs+0x64/0x98
   irq_dispose_mapping+0x70/0x14c
   mt7530_free_mdio_irq+0x5c/0x60
   mt7530_remove_common+0x1c/0x30
   mt7530_remove+0x24/0x90
   mdio_remove+0x20/0x40
   device_remove+0x68/0x80
   device_release_driver_internal+0x1cc/0x220
   device_driver_detach+0x14/0x20
   unbind_store+0xac/0xb0
   ...

  Unable to handle kernel read from unreadable memory at virtual address
    00000000000000ac
  pc : handle_nested_irq+0x28/0x168
  ...
  Call trace:
   handle_nested_irq+0x28/0x168 (P)
   regmap_irq_thread+0x19c/0x2e8
   irq_thread_fn+0x28/0x88
   irq_thread+0x18c/0x28c
   kthread+0xe4/0x1ac
   ret_from_fork+0x10/0x20
  Kernel panic - not syncing: Oops: Fatal exception

The WARN comes from unregister_irq_proc() under irq_free_descs(), fired for
a mapping a PHY still holds. The captured record shows one, for
mt7530-0:02, and already carries the W taint bit, so at least one earlier
WARN fell outside the ramoops window. Later in the same teardown, and in
the same ramoops record, the switch's own regmap-irq thread - PID 627, Comm
irq/53-mt7530 - dispatches for a mapping that is already gone:
irq_find_mapping() returns 0, irq_to_desc() returns NULL and
handle_nested_irq() takes desc->lock on it, which is the read at virtual
address 0xac in the trace.

The mappings regmap-irq disposes are a superset of the driver's.
mt7530_setup_mdio_irq() maps hwirq p for each user port p below
MT7530_NUM_PHYS - at most 0 to 4, and 0 to 2 on the board below, since the
loop tests ds->phys_mii_mask. regmap_del_irq_chip() walks hwirq 0 to
chip->num_irqs and skips only entries whose mask is zero; mt7530_irqs[] is
written with designated initialisers up to [31], so num_irqs is 32 with 12
zero-mask holes, none of them below 5 - hwirq 0 to 4 carry masks 0x1 to
0x10. A devicetree that gives the PHYs their own interrupts lands on the
same hwirqs, since regmap_domain_ops uses irq_domain_xlate_onetwocell;
today the driver disposes those too without ever having created them, and
after this patch the remove path no longer does. mt7530_free_mdio_irq()
does nothing but dispose - it neither removes the domain nor clears
bus->irq[] - so the call is the whole of what goes away.

The devres order is the right way round as well: mt7530_setup_irq()
registers the chip before mt7530_setup_mdio() registers the bus, so the bus
is released first and the chip after, and regmap_del_irq_chip() frees the
parent interrupt before it disposes anything.

Fixes names the regmap-irq conversion rather than the 2021 commit that put
this call in .remove. Before 254f6b272e3b the driver created the domain
with irq_domain_add_linear() and tore it down in mt7530_free_irq_common(),
where irq_domain_remove() disposes nothing, so mt7530_free_mdio_irq() was
required there. The conversion handed both the parent interrupt and the
domain to regmap-irq and left the call behind.

The two remaining callers are error paths in mt7530_setup_mdio() and
mt753x_setup(), reached before probe completes, and only one of them can
run in a given probe: a failing mt7530_setup_mdio() returns from
mt753x_setup() before the second is reached. An early dispose there costs
nothing anyway, because regmap_del_irq_chip() looks each hwirq up again and
only disposes the ones that still map. Dropping those calls is a cleanup,
not a fix, so they stay.

Tested on the board above with both patches applied, on a kernel identified
by the sha256 of its ELF notes section - read from /sys/kernel/notes on the
running board and computed in advance from the flashed image. Three
unbind/bind cycles. In the two whose dmesg was captured, each unbind
dropped mdio-bus:1f from the driver directory and took lan1 to lan4 with
it, each bind brought them back, lan1 relinked at 1Gbps/full after both and
lan4 after the second; the third logged interrupt descriptors instead, as
below. uptime rose from 58 to 202 seconds across the three cycles without
resetting and pstore gained no record. At the end of the two logged cycles
dmesg carried no handle_nested_irq, no Oops and no remove_proc_entry line,
against 82 lines mentioning mt7530 in that same dmesg, so those zeros are
absences and not a broken grep; the third cycle re-read the first two
counters, still zero, against 91. One unrelated WARN remains, on the first
unbind only: sysfs_remove_link() under dsa_user_destroy(), a separate DSA
teardown-ordering defect.

The third cycle was left unbound for a moment to look at the descriptors.
/proc/interrupts then had no mt7530 line at all - the parent 53 gone along
with the per-PHY 79, 80 and 81 - and /proc/irq had lost those three
directories; the next bind came back on the same three numbers. That is
regmap_del_irq_chip() doing both the free and the dispose once the driver
stopped doing half of it by hand. Had it not, the directories would have
stayed behind and the rebind would have taken the next free virqs.

What this board cannot show is the race itself. The window is narrow, and
reordering the two calls instead of removing one ran just as clean here.
The panic quoted above is what the unfixed path does, captured on the same
board and the same base with only patch 1 applied. Both kernels also
carried a local debug msleep() in phy_remove(), left over from unrelated
work in the same tree. It only widens the window this patch closes:
phy_remove() runs after .remove has returned and before
regmap_del_irq_chip() frees the parent interrupt, which is exactly the span
an early dispose leaves open. The clean runs, the WARN and the descriptor
readings do not depend on it; the panic quoted above was captured with it
in place.

Not tested: any MMIO part - there is no MT7988, EN7581, AN7583 or EN7528
hardware here. The object file was read instead: mt7530_remove_common() now
compiles to a single call to dsa_unregister_switch(), and
mt7530_free_mdio_irq() keeps its two remaining callers. Built with W=1, no
warnings; checkpatch --strict clean.

 drivers/net/dsa/mt7530.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 3e61eb3c2b1e..96832852c65a 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -3593,9 +3593,6 @@ EXPORT_SYMBOL_GPL(mt7530_probe_common);
 void
 mt7530_remove_common(struct mt7530_priv *priv)
 {
-	if (priv->irq_domain)
-		mt7530_free_mdio_irq(priv);
-
 	dsa_unregister_switch(priv->ds);
 
 	mutex_destroy(&priv->reg_mutex);
-- 
2.53.0

Re: [PATCH net v2 1/2] net: dsa: mt7530: fix NULL dereference on unbind of MT7531 and MT7621

[Andrew Lunn]

On Fri, Sep 18, 2026 at 04:50:19AM +0300, Aleksei Sviridkin wrote:
> The core and io supplies are only requested for ID_MT7530: both the
> devm_regulator_get() in probe and the regulator_enable() in
> mt7530_setup() are guarded by the switch id, but mt7530_remove()
> disables them unconditionally. On an MT7621 or an MT7531 both pointers
> are still NULL from devm_kzalloc(), so rmmod or a sysfs unbind calls
> regulator_disable() on NULL.

Did you consider using devm_regulator_get_enable()?

	Andrew

Re: [PATCH net v2 1/2] net: dsa: mt7530: fix NULL dereference on unbind of MT7531 and MT7621

[Aleksei Sviridkin]

> Did you consider using devm_regulator_get_enable()?

No. Looking at it now, it is the better shape, but not for this patch.

Two things stop it from being a swap. The driver sets the voltages before
it enables, 1.0 V on core and 3.3 V on io, and get_enable returns an int,
so no handle is left for regulator_set_voltage(). On the BPI-R2, which is
also the example in the binding, both supplies are MT6323 regulators with
a range, vpa 0.5 to 3.65 V and vemc3v3 3.0 to 3.3 V, so those calls are
not decoration. The other is placement: the get is in probe and the enable
is in mt7530_setup(), which DSA calls much later and which resets the chip
on the next lines, so get_enable would bring the rails up at a different
point.

It is still worth doing, for a reason this patch deliberately leaves
alone. mt7530_remove() disables the supplies before
mt7530_remove_common() calls dsa_unregister_switch(), which still writes
switch registers over MDIO, so on a board where those rails really drop
the teardown is talking to a switch that is already off. A devres disable
runs after .remove returns and would fix that too.

So I would keep this one as the NULL check with a Fixes tag, and send the
conversion to net-next, where the voltage handling and the power-up
ordering can be argued on their own.

Re: [PATCH net v2 1/2] net: dsa: mt7530: fix NULL dereference on unbind of MT7531 and MT7621

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (2) · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The regulator enable reference is taken in the DSA `.setup`
  callback (mt7530_setup(), drivers/net/dsa/mt7530.c:2493-2509) but…

Pre-existing issues:
- [Medium] On the ID_MT7530 path the patch keeps
  regulator_disable(core_pwr)/regulator_disable(io_pwr) ahead of…
- [Medium] In mt7530_setup() (drivers/net/dsa/mt7530.c:2494 and :2502)
  the return values of `regulator_set_voltage(priv->core_pwr, 1000000,…
- [Low] Binding/driver disagreement about which compatibles the `core-
  supply`/`io-supply` properties apply to.

--

net: dsa: mt7530: fix NULL dereference on unbind of MT7531 and MT7621

The core and io supplies are only requested for ID_MT7530, but
mt7530_remove() disabled them unconditionally, so unbinding an MT7531 or
MT7621 called regulator_disable() on a NULL pointer.  The patch gates the
two regulator_disable() calls on priv->id == ID_MT7530.

> diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
> index 784dd58a71589..de42f70afcfa1 100644
> --- a/drivers/net/dsa/mt7530-mdio.c
> +++ b/drivers/net/dsa/mt7530-mdio.c
> @@ -227,15 +227,17 @@ mt7530_remove(struct mdio_device *mdiodev)
>  	if (!priv)
>  		return;
>  
> -	ret = regulator_disable(priv->core_pwr);
> -	if (ret < 0)
> -		dev_err(priv->dev,
> -			"Failed to disable core power: %d\n", ret);
> -
> -	ret = regulator_disable(priv->io_pwr);
> -	if (ret < 0)
> -		dev_err(priv->dev, "Failed to disable io pwr: %d\n",
> -			ret);
> +	if (priv->id == ID_MT7530) {

[Severity: Low]
This isn't a bug introduced by this patch, but the id test here is now the
third place that encodes "these supplies belong to ID_MT7530", while the
binding does not say that.

In Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml both
core-supply and io-supply are defined as generic top-level properties and
are only listed under required: for the mediatek,mt7530 branch.  The
mediatek,mt7531 branch disallows other properties explicitly:

      properties:
        gpio-controller: false
        mediatek,mcm: false

but leaves the two supplies permitted, and the mediatek,mt7621 branch only
adds mediatek,mcm to required:.

So a schema-valid DT with core-supply on compatible = "mediatek,mt7531" has
that phandle silently ignored, since mt7530_probe() never calls
devm_regulator_get() for any other id:

	if (priv->id == ID_MT7530) {
		priv->core_pwr = devm_regulator_get(&mdiodev->dev, "core");
		...
	}

Should the binding set core-supply: false / io-supply: false on the
mt7531 and mt7621 branches so the two sides agree?

> +		ret = regulator_disable(priv->core_pwr);
> +		if (ret < 0)
> +			dev_err(priv->dev,
> +				"Failed to disable core power: %d\n", ret);
> +
> +		ret = regulator_disable(priv->io_pwr);
> +		if (ret < 0)
> +			dev_err(priv->dev, "Failed to disable io pwr: %d\n",
> +				ret);
> +	}

[Severity: Medium]
Is priv->id the right condition for the disable side?  It records that the
supplies were requested in mt7530_probe(), not that they were ever enabled.
The enable happens in a different callback, mt7530_setup() in mt7530.c,
reached through ds->ops->setup.

Can this produce an unbalanced disable on an ID_MT7530 board?
mt7530_probe() -> dsa_register_switch() -> dsa_tree_setup() can return
success without ever running ds->ops->setup:

net/dsa/dsa.c:dsa_tree_setup() {
	...
	complete = dsa_tree_setup_routing_table(dst);
	if (!complete)
		return 0;
	...
}

A later unbind then takes this new ID_MT7530 branch with
regulator->enable_count still zero:

drivers/regulator/core.c:_regulator_disable() {
	if (WARN(regulator->enable_count == 0,
		 "unbalanced disables for %s\n", rdev_get_name(rdev)))
		return -EIO;
	...
}

which also means the dev_err() above prints -EIO.  A tree teardown and
re-setup cycle, e.g. a peer switch in the same tree unbinding and binding
again, runs mt7530_setup() and therefore regulator_enable() more than once
against this single disable.

Does the opposite direction leak the enable reference?  Every error return
in mt7530_setup() after the core rail is up:

	if (priv->id == ID_MT7530) {
		regulator_set_voltage(priv->core_pwr, 1000000, 1000000);
		ret = regulator_enable(priv->core_pwr);
		...
		regulator_set_voltage(priv->io_pwr, 3300000, 3300000);
		ret = regulator_enable(priv->io_pwr);
		if (ret < 0) {
			dev_err(priv->dev, "Failed to enable io pwr: %d\n",
				ret);
			return ret;
		}
	}

	/* Waiting for MT7530 got to stable */
	INIT_MT7530_DUMMY_POLL(&p, priv, MT753X_TRAP);
	ret = readx_poll_timeout(_mt7530_read, &p, val, val != 0,
				 20, 1000000);
	if (ret < 0) {
		dev_err(priv->dev, "reset timeout\n");
		return ret;

propagates out through mt753x_setup() and dsa_register_switch() to
mt7530_probe(), so .remove is never called.  The devm release path only
puts the regulator:

drivers/regulator/core.c:_regulator_put() {
	...
	/* Docs say you must disable before calling regulator_put() */
	WARN_ON(regulator->enable_count);
	...
}

so the 1.0 V core and 3.3 V I/O rails stay enabled for the lifetime of the
system, with a WARN.  The MT7530_CREV id mismatch and the 20 MHz XTAL
rejection reach the same point.

Would pairing the enable with the existing symmetric hook,
mt753x_teardown() in mt7530.c, or tracking an explicit "enabled" flag and
unwinding the mt7530_setup() error paths, cover the NULL case this patch
targets as well as these two?

>  
>  	mt7530_remove_common(priv);
>  

[Severity: Medium]
This is a pre-existing ordering issue rather than something this patch
introduces, and the cover text says as much, but it may be worth recording
here since the ID_MT7530 path keeps it.

Everything mt7530_remove_common() still does runs against a die whose rails
have just been switched off:

drivers/net/dsa/mt7530.c:mt7530_remove_common() {
	dsa_unregister_switch(priv->ds);

	mutex_destroy(&priv->reg_mutex);
}

dsa_unregister_switch() reaches mt753x_teardown(), the per-port
mt7530_port_disable() PCR read-modify-writes such as

	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_MATRIX_MASK, PCR_MATRIX_CLR);

the mac_link_down() PMCR writes and the internal-PHY suspend writes that go
through the switch's own MDIO bus.  Are those writes simply lost at that
point, leaving the driver's bookkeeping describing a state the hardware
never reached?

Can the MIB poller also read an unpowered die?  mt753x_teardown() is the
only place the self-rearming stats work is cancelled:

drivers/net/dsa/mt7530.c:mt753x_teardown() {
	struct mt7530_priv *priv = ds->priv;

	if (priv->bus)
		cancel_delayed_work_sync(&priv->stats_work);
}

and it runs from inside dsa_unregister_switch(), i.e. after the disables
above.  mt7530_stats_poll() re-arms itself unconditionally:

		mt7530_stats_refresh(priv);
		schedule_delayed_work(&priv->stats_work,
				      MT7530_STATS_POLL_INTERVAL);

so it can be doing MDIO reads while mt7530_remove() drops the rails,
folding all-ones reads into the cached per-port counters.

[Severity: Medium]
This is also pre-existing and untouched by the patch, but while in this
area: mt7530_setup() in drivers/net/dsa/mt7530.c discards the return value
of both regulator_set_voltage() calls:

	if (priv->id == ID_MT7530) {
		regulator_set_voltage(priv->core_pwr, 1000000, 1000000);
		ret = regulator_enable(priv->core_pwr);
		...
		regulator_set_voltage(priv->io_pwr, 3300000, 3300000);
		ret = regulator_enable(priv->io_pwr);

If machine constraints do not permit the requested range, or
REGULATOR_CHANGE_VOLTAGE is not allowed, or a second consumer holds a
shared rail at another voltage, the rail is still enabled and the driver
goes on to reset the die, poll MT753X_TRAP and read MT7530_CREV as if the
1.0 V core / 3.3 V I/O operating points had been applied.  Should these two
calls be checked so the failure surfaces as an error instead of as
intermittent register access failures?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918015020.2518315-1-f%40lex.la

Re: [PATCH net v2 0/2] net: dsa: mt7530: fix two crashes on driver unbind

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 18 Sep 2026 04:50:18 +0300 you wrote:
> Unbinding the MT7530 driver from an MT7531 dereferences NULL in
> regulator_disable(). On a Netcraze NC-1012 (MT7981B + MT7531, 6.18.44):
> 
>   # echo mdio-bus:1f > /sys/bus/mdio_bus/drivers/mt7530-mdio/unbind
> 
> oopses there, and the build it was found on sets CONFIG_PANIC_ON_OOPS, so
> the board goes down with it. Fix that and the same command gets as far as
> mt7530_remove_common(), which disposes interrupt mappings the switch's own
> regmap-irq chip still owns; the regmap-irq thread then faults in
> handle_nested_irq() later in the same teardown. rmmod reaches both, since
> mdio_module_driver() calls .remove on module exit.
> 
> [...]

Here is the summary with links:
  - [net,v2,1/2] net: dsa: mt7530: fix NULL dereference on unbind of MT7531 and MT7621
    https://git.kernel.org/netdev/net/c/c2cdef41e0b4
  - [net,v2,2/2] net: dsa: mt7530: leave the MDIO IRQ mappings to regmap-irq
    https://git.kernel.org/netdev/net/c/0d80ba0a204c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html