Re: [PATCH net-next 2/4] net: phy: marvell10g: Fix power-up when strapped to start powered down

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Thu, Dec 14, 2023 at 09:14:40PM +0100, Tobias Waldekranz wrote:
> On devices which are hardware strapped to start powered down (PDSTATE
> == 1), make sure that we clear the power-down bit on all units
> affected by this setting.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  drivers/net/phy/marvell10g.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 83233b30d7b0..1c1333d867fb 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -344,11 +344,22 @@ static int mv3310_power_down(struct phy_device *phydev)
>  
>  static int mv3310_power_up(struct phy_device *phydev)
>  {
> +	static const u16 resets[][2] = {
> +		{ MDIO_MMD_PCS,    MV_PCS_BASE_R    + MDIO_CTRL1 },
> +		{ MDIO_MMD_PCS,    MV_PCS_1000BASEX + MDIO_CTRL1 },

This is not necessary. The documentation states that the power down
bit found at each of these is the same physical bit appearing in two
different locations. So only one is necessary.

> +		{ MDIO_MMD_PCS,    MV_PCS_BASE_T    + MDIO_CTRL1 },
> +		{ MDIO_MMD_PMAPMD, MDIO_CTRL1 },
> +		{ MDIO_MMD_VEND2,  MV_V2_PORT_CTRL },
> +	};
>  	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
> -	int ret;
> +	int i, ret;
>  
> -	ret = phy_clear_bits_mmd(phydev, MDIO_MMD_VEND2, MV_V2_PORT_CTRL,
> -				 MV_V2_PORT_CTRL_PWRDOWN);
> +	for (i = 0; i < ARRAY_SIZE(resets); i++) {
> +		ret = phy_clear_bits_mmd(phydev, resets[i][0], resets[i][1],
> +					 MV_V2_PORT_CTRL_PWRDOWN);

While MV_V2_PORT_CTRL_PWRDOWN may correspond with the correct bit for
the MDIO CTRL1 register, we have MDIO_CTRL1_LPOWER which describes
this bit. Probably the simplest solution would be to leave the
existing phy_clear_bits_mmd(), remove the vendor 2 entry from the
table, and run through that table first.

Lastly, how does this impact a device which has firmware, and the
firmware manages the power-down state (the manual states that unused
blocks will be powered down - I assume by the firmware.) If this
causes blocks which had been powered down by the firmware because
they're not being used to then be powered up, that is a regression.
Please check that this is not the case.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!