Re: [PATCH net-next] net: phy: micrel: Add support for lan8842

[Oleksij Rempel <o.rempel@pengutronix.de>]

Hi Horatiu,

On Mon, Jul 21, 2025 at 09:14:05AM +0200, Horatiu Vultur wrote:

> +static int lan8842_config_init(struct phy_device *phydev)
> +{
> +	int val;
> +	int ret;
> +
> +	/* Reset the PHY */
> +	val = lanphy_read_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET);

It would be good to use defines for MMD pages.

> +	if (val < 0)
> +		return val;
> +	val |= LAN8814_QSGMII_SOFT_RESET_BIT;
> +	lanphy_write_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET, val);

Please, do not ignore return values.

> +
> +	/* Disable ANEG with QSGMII PCS Host side
> +	 * It has the same address as lan8814
> +	 */
> +	val = lanphy_read_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG);
> +	if (val < 0)
> +		return val;
> +	val &= ~LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA;
> +	ret = lanphy_write_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG,
> +				    val);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Disable also the SGMII_AUTO_ANEG_ENA, this will determine what is the
> +	 * PHY autoneg with the other end and then will update the host side
> +	 */
> +	lanphy_write_page_reg(phydev, 4, LAN8842_SGMII_AUTO_ANEG_ENA, 0);
> +
> +	/* To allow the PHY to control the LEDs the GPIOs of the PHY should have
> +	 * a function mode and not the GPIO. Apparently by default the value is
> +	 * GPIO and not function even though the datasheet it says that it is
> +	 * function. Therefore set this value.
> +	 */
> +	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_EN2, 0);
> +
> +	/* Enable the Fast link failure, at the top level, at the bottom level
> +	 * it would be set/cleared inside lan8842_config_intr
> +	 */
> +	val = lanphy_read_page_reg(phydev, 0, LAN8842_FLF);
> +	if (val < 0)
> +		return val;
> +	val |= LAN8842_FLF_ENA | LAN8842_FLF_ENA_LINK_DOWN;

If I see it correctly, FLF support will make link fail after ~1ms, while
IEEE 802.3 recommends 750ms. Since a link recovery of a PHY with autoneg
support usually takes multiple seconds, I see the benefit for FLF
support only mostly for SyncE environment at same time it seems to be
a disadvantage for other environments.

I would prefer to have IEEE 802.3 recommended link behavior by default
and have separate Netlink configuration interface for FLF.

Best 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 |