On 11/02/2019 18:28, Florian Fainelli wrote:
On 2/10/19 3:45 AM, Rodolfo Giometti wrote:
On 09/02/2019 20:34, Andrew Lunn wrote:
So we I see two possible solutions:

1) having both ds->slave_mii_bus and ds->ops->phy_read already
defined is an
error, then it must be signaled to the calling code, or

I don't think we can do that. mv88e6xxx optionally instantiates the
MDIO busses, depending on what is in device tree. If there is no mdio
property, we need the DSA core to create an MDIO bus.

OK, but using the following check to know if DSA did such allocation is
not correct because DSA drivers can allocate it by their own:

static void dsa_switch_teardown(struct dsa_switch *ds)
{
         if (ds->slave_mii_bus && ds->ops->phy_read)
                 mdiobus_unregister(ds->slave_mii_bus);

Maybe can we add a flag to register ds->slave_mii_bus allocation by DSA?

If drivers allocate the slave_mii_bus, or use it as a pointer to their
bus, then they should not be providing a ds->ops->phy_read() callback
since we assume they would have mii_bus::read and mii_bus::write set to
their driver internal version.

I see, so having ds->slave_mii_bus and ds->ops->phy_read both not NULL into dsa_switch_setup() is a potential bug, I suppose... If so why not adding a BUG_ON() call to signal it instead of doing nothing? :-o

Ciao,

Rodolfo

--
GNU/Linux Solutions                  e-mail: giome...@enneenne.com
Linux Device Driver                          giome...@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti

Reply via email to