Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on arndale platform

[Ben Hutchings]

On Tue, 2014-11-04 at 20:09 +0000, Charles Keepax wrote:
> On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> > Hello Riku,
> > 
> > >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> > this case on arndale). This is now a regression of functionality from
> > 3.17.
> > >
> > >I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.
> > 
> > I don't fully agree here; 
> > I would like to point out that this commit is a revert itself. Fixing
> > the armdale will then cause breakage in other implementations, such as
> > ours. Blankly reverting breaks other peoples' implementations.
> > 
> > The PHY reset is the thing that breaks ethtool support, so any fix that
> > appeases all would have to take existing PHY state into account. 
[...]
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev)
>  {
>         struct asix_data *data = (struct asix_data *)&dev->data;
>         int ret, embd_phy;
> +       int reg;
>         u16 rx_ctl;
> 
>         ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
>         msleep(150);
> 
>         asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
> -       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> -                       ADVERTISE_ALL | ADVERTISE_CSMA);
> +       reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
> +       if (!reg)
> +               reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> +       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
[...]

Why is there no sleep after setting the RESET bit?  Doesn't that make
the following register writes unreliable?

Ben.

-- 
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
                                                         - Carolyn Scheppner

Attachment: signature.asc
Description: This is a digitally signed message part