Re: [PATCH net-next v8 09/11] net: phy: ncn26000: Support for loopback

[Andrew Lunn]

> +/**
> + * genphy_loopback_fixed_speed - Helper to modify the PHY loopback mode
> + * without affecting any other settings.
> + * @phydev: The phy_device struct
> + * @enable: Flag to enable or disable the PHY level loopback.
> + * @speed: Speed setting. Not expected to be set. Error if it is set.

Naming is hard, and i'm not too sure about this one.

fixed_speed could actually mean, fix the PHY to use the given speed.
It is in fact doing the opposite, the speed is not fixed at all, it is
left to whatever the device supports. In this case, it just supports a
single speed, but that is unusual for PHYs.

Maybe call it genphy_loopback_no_speed()?

This helper is also not what most devices actually want to use, since
it only works if the device does not support autoneg. The "without
affecting any other settings" is not the best way to describe why you
would want to use this, not genphy_loopback. Please expand the
documentation a bit to make it clearer why you would want to use this
and not genphy_loopback.


	Andrew