netdev - Re: [PATCH net-next 3/4] net: phy: marvell10g: use genphy_c45_an_config

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2d9ed6ca-60a0-b0da-8e86-f2fd3ef64343@gmail.com>
Date:   Sat, 16 Feb 2019 18:47:18 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Heiner Kallweit <hkallweit1@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        David Miller <davem@...emloft.net>,
        Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 3/4] net: phy: marvell10g: use
 genphy_c45_an_config_an



On 2/16/2019 11:52 AM, Heiner Kallweit wrote:
> From: Andrew Lunn <andrew@...n.ch>
> Use new function genphy_c45_config_aneg() in mv3310_config_aneg().
> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> [hkallweit1@...il.com: patch splitted]
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
>  drivers/net/phy/marvell10g.c | 22 +---------------------
>  1 file changed, 1 insertion(+), 21 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 4a6ae63ab..03fa50087 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -274,13 +274,7 @@ static int mv3310_config_aneg(struct phy_device *phydev)
>  	if (phydev->autoneg == AUTONEG_DISABLE)
>  		return genphy_c45_pma_setup_forced(phydev);
>  
> -	linkmode_and(phydev->advertising, phydev->advertising,
> -		     phydev->supported);
> -
> -	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE,
> -			     ADVERTISE_ALL | ADVERTISE_100BASE4 |
> -			     ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
> -			     linkmode_adv_to_mii_adv_t(phydev->advertising));
> +	ret = genphy_c45_an_config_an(phydev);
>  	if (ret < 0)
>  		return ret;
>  	if (ret > 0)
> @@ -294,20 +288,6 @@ static int mv3310_config_aneg(struct phy_device *phydev)
>  	if (ret > 0)
>  		changed = true;
>  
> -	/* 10G control register */
> -	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
> -			      phydev->advertising))
> -		reg = MDIO_AN_10GBT_CTRL_ADV10G;
> -	else
> -		reg = 0;

This is not strictly equivalent though because marvell10g only checks
for the 10000baseT_Full bit set whereas genphy_c45_an_config_an() calls
 linkmode_adv_to_mii_10gbt_adv_t() which you recently updated to also
check for 2.5G and 5G. This sounds about the right decision, but I
wonder if Russell did this for a reason (like not able to test 2500baseT
and 5000baseT?)

> -
> -	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
> -				     MDIO_AN_10GBT_CTRL_ADV10G, reg);
> -	if (ret < 0)
> -		return ret;
> -	if (ret > 0)
> -		changed = true;
> -
>  	if (changed)
>  		ret = genphy_c45_restart_aneg(phydev);
>  
> 

-- 
Florian