Hi Andrew,
Andrew Lunn <and...@lunn.ch> writes:
> On Fri, May 27, 2016 at 10:55:45AM -0400, Vivien Didelot wrote:
>> Hi Andrew,
>>
>> Andrew Lunn <and...@lunn.ch> writes:
>>
>> > - 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
