RE: [PATCH] net/phy: Micrel KSZ8061 PHY link failure after cable connect

From: Onnasch, Alexander (EXT)
Date: Tue Jun 19 2018 - 10:23:51 EST


Hi Andrew
thanks for the hint. But actually I cannot confirm - or I don't see it yet.

Without having tested, just from the code, the struct phy_driver instance for PHY_ID_KSZ8061 in micrel.c does not have a .write_mmd function assigned, thus phy_write_mmd should evaluate to its else-clause (see below) and not to mdiobus_write (as in phy_write).

Also the ksz8061_extended_write() function which I have added uses the same principle as already existing HW-specific functions in micrel.c for simular reasons (kszphy_extended_write and ksz9031_extended_write).
They use phy_write all over the place in that file and never phy_write_mmd - for whatever reason they had.
Thus I thought it would be a good idea ...

best regards, Alex


static inline int phy_write(struct phy_device *phydev, u32 regnum, u16 val)
{
return mdiobus_write(phydev->mdio.bus, phydev->mdio.addr, regnum, val);
}



int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
{
int ret;

if (regnum > (u16)~0 || devad > 32)
return -EINVAL;

if (phydev->drv->write_mmd) {
ret = phydev->drv->write_mmd(phydev, devad, regnum, val);
} else if (phydev->is_c45) {
u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0xffff);

ret = mdiobus_write(phydev->mdio.bus, phydev->mdio.addr,
addr, val);
} else {
struct mii_bus *bus = phydev->mdio.bus;
int phy_addr = phydev->mdio.addr;

mutex_lock(&bus->mdio_lock);
mmd_phy_indirect(bus, phy_addr, devad, regnum);

/* Write the data into MMD's selected register */
bus->write(bus, phy_addr, MII_MMD_DATA, val);
mutex_unlock(&bus->mdio_lock);

ret = 0;
}
return ret;
}

-----Original Message-----
From: Andrew Lunn <andrew@xxxxxxx>
Sent: Mittwoch, 6. Juni 2018 14:40
To: Onnasch, Alexander (EXT) <Alexander.Onnasch@xxxxxxxxxxxxx>
Cc: Florian Fainelli <f.fainelli@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] net/phy: Micrel KSZ8061 PHY link failure after cable connect

On Wed, Jun 06, 2018 at 10:03:18AM +0200, Alexander Onnasch wrote:
> With Micrel KSZ8061 PHY, the link may occasionally not come up after
> Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
> sheet 80000688A.pdf describes the problem and possible workarounds in
> detail, see below.
> The patch implements workaround 1, which permanently fixes the issue.
>
> DESCRIPTION
> Link-up may not occur properly when the Ethernet cable is initially
> connected. This issue occurs more commonly when the cable is connected
> slowly, but it may occur any time a cable is connected. This issue
> occurs in the auto-negotiation circuit, and will not occur if
> auto-negotiation is disabled (which requires that the two link
> partners be set to the same speed and duplex).
>
> END USER IMPLICATIONS
> When this issue occurs, link is not established. Subsequent cable
> plug/unplug cycles will not correct the issue.
>
> WORK AROUND
> There are four approaches to work around this issue:
> 1. This issue can be prevented by setting bit 15 in MMD device address 1,
> register 2, prior to connecting the cable or prior to setting the
> Restart Auto-Negotiation bit in register 0h.The MMD registers are
> accessed via the indirect access registers Dh and Eh, or via the Micrel
> EthUtil utility as shown here:
> â If using the EthUtil utility (usually with a Micrel KSZ8061
> Evaluation Board), type the following commands:
> > address 1
> > mmd 1
> > iw 2 b61a
> â Alternatively, write the following registers to write to the
> indirect MMD register:
> Write register Dh, data 0001h
> Write register Eh, data 0002h
> Write register Dh, data 4001h
> Write register Eh, data B61Ah
> 2. The issue can be avoided by disabling auto-negotiation in the KSZ8061,
> either by the strapping option, or by clearing bit 12 in register 0h.
> Care must be taken to ensure that the KSZ8061 and the link partner
> will link with the same speed and duplex. Note that the KSZ8061
> defaults to full-duplex when auto-negotiation is off, but other
> devices may default to half-duplex in the event of failed
> auto-negotiation.
> 3. The issue can be avoided by connecting the cable prior to powering-up
> or resetting the KSZ8061, and leaving it plugged in thereafter.
> 4. If the above measures are not taken and the problem occurs, link can
> be recovered by setting the Restart Auto-Negotiation bit in
> register 0h, or by resetting or power cycling the device. Reset may
> be either hardware reset or software reset (register 0h, bit 15).
>
> PLAN
> This errata will not be corrected in a future revision.
>
> Signed-off-by: Alexander Onnasch <alexander.onnasch@xxxxxxxxxxxxx>
> ---
> drivers/net/phy/micrel.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index
> 6c45ff6..7d80a00 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -339,6 +339,28 @@ static int ksz8041_config_aneg(struct phy_device *phydev)
> return genphy_config_aneg(phydev);
> }
>
> +#define MII_KSZ8061RN_MMD_CTRL_REG 0x0d
> +#define MII_KSZ8061RN_MMD_REGDATA_REG 0x0e
> +
> +static int ksz8061_extended_write(struct phy_device *phydev,
> + u8 mode, u32 dev_addr, u32 regnum, u16 val) {
> + phy_write(phydev, MII_KSZ8061RN_MMD_CTRL_REG, dev_addr);
> + phy_write(phydev, MII_KSZ8061RN_MMD_REGDATA_REG, regnum);
> + phy_write(phydev, MII_KSZ8061RN_MMD_CTRL_REG, (mode << 14) | dev_addr);
> + return phy_write(phydev, MII_KSZ8061RN_MMD_REGDATA_REG, val); }

Hi Alexander

This looks a lot like phy_write_mmd().

Andrew