2023-08-30 18:54:25

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH 2/2] net: phy: Provide Module 4 KSZ9477 errata (DS80000754C)

On Wed, Aug 30, 2023 at 11:21:19AM +0200, Lukasz Majewski wrote:
> + /* 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.
> + *
> + * Although, the KSZ9477 MMD register
> + * (MMD_DEVICE_ID_EEE_ADV.MMD_EEE_ADV) advertise that EEE is
> + * operational one needs to manualy clear them to follow the chip
> + * errata.
> + */
> + linkmode_and(phydev->supported_eee, phydev->supported, zero);

Hi,

I'm wondering whether you had a reason to write the above, rather than
use the simpler:

linkmode_zero(phydev->supported_eee);

Thanks.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


2023-08-30 22:03:03

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH 2/2] net: phy: Provide Module 4 KSZ9477 errata (DS80000754C)

On Wed, Aug 30, 2023 at 12:08:17PM +0100, Russell King (Oracle) wrote:
> On Wed, Aug 30, 2023 at 11:21:19AM +0200, Lukasz Majewski wrote:
> > + /* 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.
> > + *
> > + * Although, the KSZ9477 MMD register
> > + * (MMD_DEVICE_ID_EEE_ADV.MMD_EEE_ADV) advertise that EEE is
> > + * operational one needs to manualy clear them to follow the chip
> > + * errata.
> > + */
> > + linkmode_and(phydev->supported_eee, phydev->supported, zero);
>
> Hi,
>
> I'm wondering whether you had a reason to write the above, rather than
> use the simpler:
>
> linkmode_zero(phydev->supported_eee);

Ah, I wondered what was the proper name for this and was not able to
found it.

Thank you!

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |