Re: [PATCH net-next v6 2/7] net: phy: Support speed selection for PHY loopback

[Andrew Lunn <andrew@lunn.ch>]

> +int phy_loopback(struct phy_device *phydev, bool enable, int speed)
> +{
> +	bool link_up = false;
> +	int ret = 0;
> +
> +	if (!phydev->drv)
> +		return -EIO;
> +
> +	mutex_lock(&phydev->lock);
> +
> +	if (enable && phydev->loopback_enabled) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (!enable && !phydev->loopback_enabled) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (enable) {
> +		/*
> +		 * Link up is signaled with a defined speed. If speed changes,
> +		 * then first link down and after that link up needs to be
> +		 * signaled.
> +		 */
> +		if (phydev->link && phydev->state == PHY_RUNNING) {
> +			/* link is up and signaled */
> +			if (speed && phydev->speed != speed) {
> +				/* signal link down and up for new speed */
> +				phydev->link = false;
> +				phydev->state = PHY_NOLINK;
> +				phy_link_down(phydev);

If you set the link down here...

> +
> +				link_up = true;
> +			}
> +		} else {
> +			/* link is not signaled */
> +			if (speed) {
> +				/* signal link up for new speed */
> +				link_up = true;
> +			}
> +		}
> +	}
> +
> +	if (phydev->drv->set_loopback)
> +		ret = phydev->drv->set_loopback(phydev, enable, speed);
> +	else
> +		ret = genphy_loopback(phydev, enable, speed);
> +
> +	if (ret)
> +		goto out;

and this fails, you leave the link down. You should make an attempt to
restore the link to the old state before returning the error.

	Andrew