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

Aleksei Sviridkin <[email protected]>
Newsgroups org.kernel.vger.netdev,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-mediatek,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
> 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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.