netdev - Re: [PATCH net v2 2/2] net: phy: marvell10g: hwmon support for 2110

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200421132230.GB937199@lunn.ch>
Date:   Tue, 21 Apr 2020 15:22:30 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Baruch Siach <baruch@...s.co.il>
Cc:     Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>
Subject: Re: [PATCH net v2 2/2] net: phy: marvell10g: hwmon support for 2110

On Tue, Apr 21, 2020 at 01:56:15PM +0300, Baruch Siach wrote:
> Read the temperature sensor register from the correct location for the
> 88E2110 PHY. There is no enable/disable bit, so leave
> mv3310_hwmon_config() for 88X3310 only.
> 
> Signed-off-by: Baruch Siach <baruch@...s.co.il>
> ---
> v2: Fix indentation (Andrew Lunn)
> ---
>  drivers/net/phy/marvell10g.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 69530a84450f..b6115537eb66 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -66,6 +66,8 @@ enum {
>  	MV_PCS_CSSR1_SPD2_2500	= 0x0004,
>  	MV_PCS_CSSR1_SPD2_10000	= 0x0000,
>  
> +	MV_PCS_TEMP		= 0x8042,
> +
>  	/* These registers appear at 0x800X and 0xa00X - the 0xa00X control
>  	 * registers appear to set themselves to the 0x800X when AN is
>  	 * restarted, but status registers appear readable from either.
> @@ -104,6 +106,14 @@ static umode_t mv3310_hwmon_is_visible(const void *data,
>  	return 0;
>  }
>  
> +static int mv3310_hwmon_read_temp_reg(struct phy_device *phydev)
> +{
> +	if (phydev->drv->phy_id == MARVELL_PHY_ID_88X3310)
> +		return phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
> +	else /* MARVELL_PHY_ID_88E2110 */
> +		return phy_read_mmd(phydev, MDIO_MMD_PCS, MV_PCS_TEMP);
> +}
> +

Hi Baruch

Given the discussion about splitting the driver up a bit, could you
add

mv2110_hwmon_read_temp_reg()

And maybe a wrapper

mvxx10_hwmon_read_temp_reg() which does the call into the correct
version? Not that i particularly like that prefix.

Do we have enough differences yet it is worth adding a structure of
function pointers for family member? But i would save that for
net-next, where as you are aiming for net with these patches.

	 Andrew