linux-kernel - Re: [PATCH net-next v2] net: phy: dp83867: Add speed optimization feature

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: <f977a302-16fc-de68-e84b-d41a0eca4c12@gmail.com>
Date:   Tue, 4 Feb 2020 21:08:07 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Dan Murphy <dmurphy@...com>, andrew@...n.ch, f.fainelli@...il.com
Cc:     linux@...linux.org.uk, davem@...emloft.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2] net: phy: dp83867: Add speed optimization
 feature

On 04.02.2020 19:13, Dan Murphy wrote:
> Set the speed optimization bit on the DP83867 PHY.
> This feature can also be strapped on the 64 pin PHY devices
> but the 48 pin devices do not have the strap pin available to enable
> this feature in the hardware.  PHY team suggests to have this bit set.
> 
> With this bit set the PHY will auto negotiate and report the link
> parameters in the PHYSTS register.  This register provides a single
> location within the register set for quick access to commonly accessed
> information.
> 
> In this case when auto negotiation is on the PHY core reads the bits
> that have been configured or if auto negotiation is off the PHY core
> reads the BMCR register and sets the phydev parameters accordingly.
> 
> This Giga bit PHY can throttle the speed to 100Mbps or 10Mbps to accomodate a
> 4-wire cable.  If this should occur the PHYSTS register contains the
> current negotiated speed and duplex mode.
> 
> In overriding the genphy_read_status the dp83867_read_status will do a
> genphy_read_status to setup the LP and pause bits.  And then the PHYSTS
> register is read and the phydev speed and duplex mode settings are
> updated.
> 
> Signed-off-by: Dan Murphy <dmurphy@...com>

net-next is closed currently. See here for details:
https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
Reopening will be announced on the netdev mailing list, you can also
check net-next status here: http://vger.kernel.org/~davem/net-next.html

> ---
> v2 - Updated read status to call genphy_read_status first, added link_change
> callback to notify of speed change and use phy_set_bits - https://lore.kernel.org/patchwork/patch/1188348/ 
> 
>  drivers/net/phy/dp83867.c | 55 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index 967f57ed0b65..6f86ca1ebb51 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -21,6 +21,7 @@
>  #define DP83867_DEVADDR		0x1f
>  
>  #define MII_DP83867_PHYCTRL	0x10
> +#define MII_DP83867_PHYSTS	0x11
>  #define MII_DP83867_MICR	0x12
>  #define MII_DP83867_ISR		0x13
>  #define DP83867_CFG2		0x14
> @@ -118,6 +119,15 @@
>  #define DP83867_IO_MUX_CFG_CLK_O_SEL_MASK	(0x1f << 8)
>  #define DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT	8
>  
> +/* PHY STS bits */
> +#define DP83867_PHYSTS_1000			BIT(15)
> +#define DP83867_PHYSTS_100			BIT(14)
> +#define DP83867_PHYSTS_DUPLEX			BIT(13)
> +#define DP83867_PHYSTS_LINK			BIT(10)
> +
> +/* CFG2 bits */
> +#define DP83867_SPEED_OPTIMIZED_EN		(BIT(8) | BIT(9))
> +
>  /* CFG3 bits */
>  #define DP83867_CFG3_INT_OE			BIT(7)
>  #define DP83867_CFG3_ROBUST_AUTO_MDIX		BIT(9)
> @@ -287,6 +297,43 @@ static int dp83867_config_intr(struct phy_device *phydev)
>  	return phy_write(phydev, MII_DP83867_MICR, micr_status);
>  }
>  
> +static void dp83867_link_change_notify(struct phy_device *phydev)
> +{
> +	if (phydev->state != PHY_RUNNING)
> +		return;
> +
> +	if (phydev->speed == SPEED_100 || phydev->speed == SPEED_10)
> +		phydev_warn(phydev, "Downshift detected connection is %iMbps\n",
> +			    phydev->speed);
> +}
> +
> +static int dp83867_read_status(struct phy_device *phydev)
> +{
> +	int status = phy_read(phydev, MII_DP83867_PHYSTS);
> +	int ret;
> +
> +	ret = genphy_read_status(phydev);
> +	if (ret)
> +		return ret;
> +
> +	if (status < 0)
> +		return status;
> +
> +	if (status & DP83867_PHYSTS_DUPLEX)
> +		phydev->duplex = DUPLEX_FULL;
> +	else
> +		phydev->duplex = DUPLEX_HALF;
> +
> +	if (status & DP83867_PHYSTS_1000)
> +		phydev->speed = SPEED_1000;
> +	else if (status & DP83867_PHYSTS_100)
> +		phydev->speed = SPEED_100;
> +	else
> +		phydev->speed = SPEED_10;
> +
> +	return 0;
> +}
> +
>  static int dp83867_config_port_mirroring(struct phy_device *phydev)
>  {
>  	struct dp83867_private *dp83867 =
> @@ -467,6 +514,11 @@ static int dp83867_config_init(struct phy_device *phydev)
>  	int ret, val, bs;
>  	u16 delay;
>  
> +	/* Force speed optimization for the PHY even if it strapped */
> +	ret = phy_set_bits(phydev, DP83867_CFG2, DP83867_SPEED_OPTIMIZED_EN);
> +	if (ret)
> +		return ret;
> +
>  	ret = dp83867_verify_rgmii_cfg(phydev);
>  	if (ret)
>  		return ret;
> @@ -655,6 +707,9 @@ static struct phy_driver dp83867_driver[] = {
>  		.config_init	= dp83867_config_init,
>  		.soft_reset	= dp83867_phy_reset,
>  
> +		.read_status	= dp83867_read_status,
> +		.link_change_notify = dp83867_link_change_notify,
> +
>  		.get_wol	= dp83867_get_wol,
>  		.set_wol	= dp83867_set_wol,
>  
>