Re: [PATCH v7 2/2] phy: Add driver for EyeQ5 Ethernet PHY wrapper

[Vladimir Oltean <olteanv@gmail.com>]

On Wed, Feb 25, 2026 at 05:54:41PM +0100, Théo Lebrun wrote:
> +static int eq5_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> +{
> +	struct eq5_phy_inst *inst = phy_get_drvdata(phy);
> +
> +	if (eq5_phy_validate(phy, mode, submode, NULL))
> +		return -EOPNOTSUPP;

Propagate the phy_validate() return code, don't generate your own.
-EINVAL should be preferable to -EOPNOTSUPP, so that callers can
distinguish between "phy_set_mode() not implemented" and "phy_set_mode()
failed".

(yeah, phy_set_mode() was made optional a while ago, IMO incorrectly,
but that's another story)

> +
> +	if (submode == inst->phy_interface)
> +		return 0;

I think this simple comparison fails to serve its intended purpose
(avoid PHY reset when not changing modes) for RGMII modes, of which
there exist 4 variants.

Maybe:
	if ((phy_interface_mode_is_rgmii(submode) &&
	     phy_interface_mode_is_rgmii(inst->phy_interface)) ||
	    submode == inst->phy_interface)
		return 0;

Does the EyeQ5 platform support internal RGMII delays? If yes, which
layer enables them? The Generic PHY?

> +
> +	inst->phy_interface = submode;
> +
> +	if (phy->power_count) {
> +		eq5_phy_init(phy);
> +		return eq5_phy_power_on(phy);
> +	}
> +
> +	return 0;
> +}

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy