Thread (37 messages) flat view 37 messages, 4 authors, 2016-05-28

Re: [RFC PATCH 12/16] dsa: Make mdio bus optional

From: Vivien Didelot <hidden>
Date: 2016-05-27 16:38:50

Hi Andrew,

Andrew Lunn [off-list ref] writes:
On Fri, May 27, 2016 at 10:55:45AM -0400, Vivien Didelot wrote:
quoted
Hi Andrew,

Andrew Lunn [off-list ref] writes:
quoted
-	mdiobus_unregister(ds->slave_mii_bus);
+	if (ds->slave_mii_bus && ds->drv->phy_read)
+		mdiobus_unregister(ds->slave_mii_bus);
So if a driver registered the slave MII bus itself, it may have
unregistered it itself as well, so checking ds->slave_mii_bus is OK
(assuming the driver correctly zero'ed it).

But is it necessary to check ds->drv->phy_read?
It makes the code symmetrical to the register code, which makes the
same check. My experience is that keeping the code symmetrical results
in less surprises late on.

One such surprise could be a driver that does not zero
ds->slave_mii_bus. In fact, mv88e6xxx does not zero it. So we would
get a double unregister happening. We also don't want the core
unregistering it, since we have other cleanup work to do, we have an
of_node_put() to call.
OK good for me, as long as it is intuitive for the driver
implementation.

Thanks,

        Vivien
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help