Re: [PATCH] net: smsc911x: If PHY doesn't have an interrupt then POLL

[Jeremy Linton <jeremy.linton@arm.com>]

On 06/14/2016 02:49 PM, Sergei Shtylyov wrote:
> On 06/14/2016 10:27 PM, Sergei Shtylyov wrote:
>
>>>> If the interrupt configuration isn't set and we are using the
>>>> internal phy, then we need to poll the phy to reliably detect
>>>> phy state changes.
>>>>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> ---
>>>>  drivers/net/ethernet/smsc/smsc911x.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/smsc/smsc911x.c
>>>> b/drivers/net/ethernet/smsc/smsc911x.c
>>>> index 8af2556..369dc7d 100644
>>>> --- a/drivers/net/ethernet/smsc/smsc911x.c
>>>> +++ b/drivers/net/ethernet/smsc/smsc911x.c
>>>> @@ -1020,6 +1020,9 @@ static int smsc911x_mii_probe(struct
>>>> net_device *dev)
>>>>          return -ENODEV;
>>>>      }
>>>>
>>>> +    if ((!phydev->irq) && (!pdata->using_extphy))
>>>
>>>    Inner parens aren't needed at all.
>>
>>    Hm, 'phydev->irq' shouldn't be 0 in the first place. It seems to me we
>> should correctly initialize 'pdata->phy_irq[]' in smsc911x_mii_init()...
>
>     And looking at that array, I doubt it's really useful for
> anything... And the memcpy() there seems buggy as well -- it copies just
> 4 bytes of this array to 'pdata->mii_bus->irq'. I do care about this
> driver, so might be a good idea to clean it up a bit...

	The use of phy_connect_direct() in the driver probe is incorrect, and 
keeps the driver from being unloaded.

	Also, some portion of smsc's can deliver mii state change interrupts 
via the smsc interrupt, but that code is no longer in this driver.

	I suspect a portion of the problem, besides all the strange hardware 
configurations this driver supports are the emulated hardware like QEMU 
that also uses it.