Subject: Re: [PATCH 2/2] net: dsa: microchip: Provide Module 4 KSZ9477 errata (DS80000754C)

From: Lukasz Majewski <[email-masked]>
Date: Fri, 25 Aug 2023 09:42:46 +0200

On Thu, 24 Aug 2023 08:54:37 -0700
Florian Fainelli <[email-masked]> wrote:

> On 8/24/2023 8:48 AM, Lukasz Majewski wrote:
> > The KSZ9477 errata points out the link up/down problem when EEE is
> > enabled in the device to which the KSZ9477 tries to auto negotiate.
> > 
> > The suggested workaround is to clear advertisement EEE registers
> > (accessed as per port MMD one).
> > 
> > Signed-off-by: Lukasz Majewski <[email-masked]>
> > ---
> >   drivers/net/dsa/microchip/ksz9477.c | 40
> > ++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1
> > deletion(-)
> > 
> > diff --git a/drivers/net/dsa/microchip/ksz9477.c
> > b/drivers/net/dsa/microchip/ksz9477.c index
> > cb6aa7c668a8..563f497ba656 100644 ---
> > a/drivers/net/dsa/microchip/ksz9477.c +++
> > b/drivers/net/dsa/microchip/ksz9477.c @@ -1128,6 +1128,44 @@ int
> > ksz9477_enable_stp_addr(struct ksz_device *dev) return 0;
> >   }
> >   
> > +static int ksz9477_errata(struct dsa_switch *ds)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u16 val;
> > +	int p;
> > +
> > +	/* KSZ9477 Errata DS80000754C
> > +	 *
> > +	 * Module 4: Energy Efficient Ethernet (EEE) feature
> > select must be
> > +	 * manually disabled
> > +	 *   The EEE feature is enabled by default, but it is not
> > fully
> > +	 *   operational. It must be manually disabled through
> > register
> > +	 *   controls. If not disabled, the PHY ports can
> > auto-negotiate
> > +	 *   to enable EEE, and this feature can cause link drops
> > when linked
> > +	 *   to another device supporting EEE.
> > +	 *
> > +	 *   Only PHY ports (dsa user) [0-4] need to have the EEE
> > advertisement
> > +	 *   bits cleared.
> > +	 */
> > +
> > +	for (p = 0; p < ds->num_ports; p++) {
> > +		if (!dsa_is_user_port(ds, p))
> > +			continue;
> > +
> > +		ksz9477_port_mmd_read(dev, p,
> > MMD_DEVICE_ID_EEE_ADV,
> > +				      MMD_EEE_ADV, &val, 1);
> > +
> > +		pr_err("%s: PORT: %d val: 0x%x pc: %d\n",
> > __func__, p, val,
> > +		       ds->num_ports);  
> 
> Left over debugging?
> 

Yes, correct - I will fix it.

> > +
> > +		val &= ~(EEE_ADV_100MBIT | EEE_ADV_1GBIT);
> > +		ksz9477_port_mmd_write(dev, p,
> > MMD_DEVICE_ID_EEE_ADV,
> > +				       MMD_EEE_ADV, &val, 1);
> > +	}
> > +
> > +	return 0;  
> 
> You don't propagate any error, so make this return void?

I will fix this too.

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email-masked]