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.