On 06/14/2016 11:12 PM, Andrew Lunn wrote:
On Tue, Jun 14, 2016 at 10:49:20PM +0300, 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 <[email protected]>
---
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()...

Hi Sergei

The mdio layer, when it allocates the mdiobus structure, will
initialise all the phy interrupts to polling.

And this whole array would get overwritten with zeros by memcpy() iff that one was correct (it really overwrites only index 0).

   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'.

0 is not a valid interrupt.

   Or at least Linus says so... :-)

So it should probably loop over the array
and copy any which are not 0 into pdata->mii_bus->irq[].

I don't see where that array is explicitly initialized in the first place. So it seems actively harmful...

    Andrew

MBR, Sergei

Reply via email to