Re: [PATCH net-next 11/12] net: dsa: mv88e6xxx: add Energy Detect ops

[Andrew Lunn <andrew@lunn.ch>]

> diff --git a/drivers/net/dsa/mv88e6xxx/phy.c b/drivers/net/dsa/mv88e6xxx/phy.c
> index 436668bd50dc..317ae89cfa68 100644
> --- a/drivers/net/dsa/mv88e6xxx/phy.c
> +++ b/drivers/net/dsa/mv88e6xxx/phy.c
> @@ -246,3 +246,99 @@ int mv88e6xxx_phy_setup(struct mv88e6xxx_chip *chip)
>  {
>  	return mv88e6xxx_phy_ppu_enable(chip);
>  }
> +
> +/* Page 0, Register 16: Copper Specific Control Register 1 */
> +
> +int mv88e6352_phy_energy_detect_read(struct mv88e6xxx_chip *chip, int phy,
> +				     struct ethtool_eee *eee)
> +{
> +	u16 val;
> +	int err;
> +
> +	err = mv88e6xxx_phy_read(chip, phy, MV88E6XXX_PHY_CSCTL1, &val);
> +	if (err)
> +		return err;
> +
> +	val &= MV88E6352_PHY_CSCTL1_ENERGY_DETECT_MASK;
> +
> +	eee->eee_enabled = false;
> +	eee->tx_lpi_enabled = false;
> +
> +	switch (val) {
> +	case MV88E6352_PHY_CSCTL1_ENERGY_DETECT_SENSE_NLP:
> +		eee->tx_lpi_enabled = true;
> +		/* fall through... */
> +	case MV88E6352_PHY_CSCTL1_ENERGY_DETECT_SENSE_RCV:
> +		eee->eee_enabled = true;
> +	}
> +
> +	return 0;
> +}

Hi Vivien

I never liked this. I think it is architecturally wrong for the switch
to be poking around in the PHY. It should ask the PHY driver. This is
especially true for external PHYs which might not be a Marvell PHY.

   Andrew