Re: Possible bug into DSA2 code.

[Rodolfo Giometti <giometti@enneenne.com>]

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: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti