Re: [PATCH v3] Marvell phy: add fiber status check and configuration for some phys

[Andrew Lunn <andrew@lunn.ch>]

> >>>> +static int marvell_resume_fiber(struct phy_device *phydev)
> >>>> +{
> >>>> +	int err;
> >>>> +
> >>>> +	/* Resume the fiber mode first */
> >>>> +	err = phy_write(phydev, MII_MARVELL_PHY_PAGE, MII_M1111_FIBER);
> >>>> +	if (err < 0)
> >>>> +		goto error;
> >>>> +
> >>>> +	err = genphy_resume(phydev);
> >>>> +	if (err < 0)
> >>>> +		goto error;
> >>>> +
> >>>> +	/* Then, the copper link */
> >>>> +	err = phy_write(phydev, MII_MARVELL_PHY_PAGE, MII_M1111_COPPER);
> >>>> +	if (err < 0)
> >>>> +		goto error;
> >>>> +
> >>>> +	return genphy_resume(phydev);
> >>>
> >>> Should it be resumed twice? Or just once at the end?  Same question
> >>> for suspend.
> >>
> >> I don't understand your question.
> > 
> > You call genphy_resume(phydev) twice. Once is sufficient.
> 
> Yes, but it's normal because each interface could be suspended or resumed independently.

> genphy_* functions use BMCR register which are identical between
> fiber and copper link. But each link has its own register to change.

Ah! Now i get it. I think you need a comment here. Something like:

    /* With the page set, use the generic resume */

What i was worried about is that there is some reference counting
going on inside these functions. And so suspending the same phydev
multiple times will mess up the reference counts. But no, it just
twiddles a register bit, so that is O.K.

	 Andrew