PROBLEM: smsc95xx loses config on link down/up

PROBLEM: smsc95xx loses config on link down/up

[Sam Lewis]

I'm using a LAN9514 chip in my embedded Linux device and have noticed
that changing Ethernet configuration (with ethtool for example) does
not persist after putting the link up.

I have tested this on kernel versions 4.14.0 and 5.0.0-36. As far as I
can tell the driver hasn't had any related fixes since 5.0.0, so I
don't think the behavior has changed in more recent kernel versions.

To demonstrate, what I mean, if I:

1) Take the link down (with `ip link set eth0 down`)
2) Turn auto-negotiation off (with `ethtool -s eth0 autoneg off`)
3) Take the link up (with `ip link set eth0 up`)

Then auto-negotiation is turned back on after the Ethernet interface
is brought back up. This seems to be true for any of the ethtool
configuration settings, like speed and duplex as well.

This is frustrating for a few reasons:

- I can't set the Ethernet configuration before I put the link up
- I can't use systemd .link files for managing link properties as they
seem to set the properties of the link before it's up

I've hacked through the driver code (without really knowing what I'm
doing, just adding various print statements) and I think this happens
because setting a link up causes the `smsc95xx_reset` function to be
called which seems to clear all configuration through:

1) Doing a PHY reset (with `smsc95xx_write_reg(dev, PM_CTRL, PM_CTL_PHY_RST_)`)
2) Doing (another?) PHY reset (with `smsc95xx_mdio_write(dev->net,
dev->mii.phy_id, MII_BMCR, BMCR_RESET)`)

I tested this by looking at the configuration through calling
`mii_ethtool_gset` before and after those two resets. After the
resets, it appears the configuration is cleared.

I'm using the LAN9514 without an attached EEPROM, so understand that
any settings set will not persist through a power cycle, but it would
still be nice if they persisted through setting the interface down and
then up. This seems to be the behavior on other Ethernet devices that
I've tried (even ones without NV storage), so maybe this is a bug with
the LAN95xx driver implementation?

It's very possible that I'm doing something wrong though, I'm happy to
hear if there's some other way to achieve what I'm trying to do.

If this is a real bug I'd be happy to take a look into trying to fix
it. Would it be acceptable to restore any configuration read from a
`mii_ethtool_gset` after the `smsc95xx_reset` is run?

Re: PROBLEM: smsc95xx loses config on link down/up

[Andrew Lunn]

On Thu, Nov 28, 2019 at 06:19:14PM +1100, Sam Lewis wrote:
> I'm using a LAN9514 chip in my embedded Linux device and have noticed
> that changing Ethernet configuration (with ethtool for example) does
> not persist after putting the link up.

Hi Sam

Did you ever get a reply to this?

> I've hacked through the driver code (without really knowing what I'm
> doing, just adding various print statements) and I think this happens
> because setting a link up causes the `smsc95xx_reset` function to be
> called which seems to clear all configuration through:
> 
> 1) Doing a PHY reset (with `smsc95xx_write_reg(dev, PM_CTRL, PM_CTL_PHY_RST_)`)
> 2) Doing (another?) PHY reset (with `smsc95xx_mdio_write(dev->net,
> dev->mii.phy_id, MII_BMCR, BMCR_RESET)`)

In general, BMCR_RESET does not clear configuration registers such as
auto-neg etc. It generally just gives the PHY a kick to restart itself
using the configuration as set. So i would initially point a finger at
PM_CTL_PHY_RST_.

Is there a full datasheet somewhere?

You might want to think about using PM_CTL_PHY_RST_ once during probe,
and only BMCR_RESET in open.

    Andrew

Re: PROBLEM: smsc95xx loses config on link down/up

[Sam Lewis]

On Tue, 3 Dec 2019 at 00:46, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Thu, Nov 28, 2019 at 06:19:14PM +1100, Sam Lewis wrote:
> > I'm using a LAN9514 chip in my embedded Linux device and have noticed
> > that changing Ethernet configuration (with ethtool for example) does
> > not persist after putting the link up.
>
> Hi Sam
>
> Did you ever get a reply to this?
>

Hi Andrew - I hadn't, but I understand that it's a quiet time of year.
:) Thanks for getting back to me!

> > I've hacked through the driver code (without really knowing what I'm
> > doing, just adding various print statements) and I think this happens
> > because setting a link up causes the `smsc95xx_reset` function to be
> > called which seems to clear all configuration through:
> >
> > 1) Doing a PHY reset (with `smsc95xx_write_reg(dev, PM_CTRL, PM_CTL_PHY_RST_)`)
> > 2) Doing (another?) PHY reset (with `smsc95xx_mdio_write(dev->net,
> > dev->mii.phy_id, MII_BMCR, BMCR_RESET)`)
>
> In general, BMCR_RESET does not clear configuration registers such as
> auto-neg etc. It generally just gives the PHY a kick to restart itself
> using the configuration as set. So i would initially point a finger at
> PM_CTL_PHY_RST_.
>
> Is there a full datasheet somewhere?
>
> You might want to think about using PM_CTL_PHY_RST_ once during probe,
> and only BMCR_RESET in open.
>

Thanks, doing the PM_CTL_PHY_RST_ reset only during probing does sound
a good (& necessary) fix but it unfortunately doesn't look like it'll
get it all the way there.

I managed to track down the datasheet for the PHY, available here:
http://ww1.microchip.com/downloads/en//softwarelibrary/man-lan95xx-dat/lan9514_lan9514i%20databook%20rev.%201.2%20(03-01-12).pdf

Basically it looks as though doing a BMCR_RESET does, in fact, reset
every PHY R/W register bit except for those marked as "Not Affected by
Software Reset" (NASR). This means it will reset, to the default
value:

- Autonegotiation
- Speed
- Duplex
- Auto MDIX
- Energy Detect Power-Down
- Auto Negotiation Advertisement
- PHY Identification (although I don't know why you'd change this?)
- Power down
- Loopback

I tested this by checking the value of the BMCR register before and
after doing a BMCR_RESET and it did reset the BMCR register to its
default values.

The PHY does provide a NASR "Special Modes Register" which effectively
allows you to set 'default' values for the duplex, speed and autoneg
that are applied after a BMCR_RESET. See page 205 of the datasheet for
more details. Setting this to 0x0061 allowed me to set the PHY to be
full duplex, 100M and no autoneg on after a BMCR_RESET, for example.

However, given that the Special Modes register only allows saving a
subset of settings it perhaps isn't the best solution? (Auto MDIX is
something else I'd like to set differently to the default value in my
application and I imagine the other settings probably shouldn't be
lost either). The only other fix that I can think of is to save all
the PHY R/W registers before doing a BMCR_RESET and then restoring
them after the reset. Would this be an acceptable solution?

I do have to ask though - is it strictly necessary to do the
BMCR_RESET (& the PHY initialisation) every time on link up/open? What
is the reasoning behind doing it? Excuse my ignorance on this if it's
a dumb question! If it was only done on probing, it would make this
easier (well, easier for me at least :)).

Interested to know what you think - I'm still keen on seeing if I can
make a generic patch for this. I think I know enough now that I can
hack together a patch that'll work for my particular application, but
it would be cool to upstream something that will benefit everyone.

Sam

Re: PROBLEM: smsc95xx loses config on link down/up

[Andrew Lunn]

> Basically it looks as though doing a BMCR_RESET does, in fact, reset
> every PHY R/W register bit except for those marked as "Not Affected by
> Software Reset" (NASR). This means it will reset, to the default
> value:
> 
> - Autonegotiation
> - Speed
> - Duplex
> - Auto MDIX
> - Energy Detect Power-Down
> - Auto Negotiation Advertisement
> - PHY Identification (although I don't know why you'd change this?)
> - Power down
> - Loopback
> 
> I tested this by checking the value of the BMCR register before and
> after doing a BMCR_RESET and it did reset the BMCR register to its
> default values.

O.K, not what we want.

So there are two different paths here you can follow:

1) Moving the reset out of open and into bind.
2) Re-write the driver to make use of the core phylib support.

1) is probably the quick and simple solution, but watch out for
suspend/resume.

2) is more work, but brings the driver into line with other MAC
drivers. phylib would then program the PHY, maybe via PHY driver. It
would do this during open, using state information. So it should
correctly handle state change while the interface is down, or
suspended etc.

The USB-ethernet drivers lan78xx and ax88172a both use phylib.  They
can give you ideas how this should work.

drivers/net/phy/smsc.c might be a good basis for a PHY driver, but it
looks like you will need to extend it for MDIX, etc.

      Andrew