netdev - Re: [PATCH] net: phy: dp83822: Fix RGMII TX delay configuration

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]

Date: Sat, 3 Feb 2024 14:39:50 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Tim Pambor <tp@...systeme.de>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Dan Murphy <dmurphy@...com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: phy: dp83822: Fix RGMII TX delay configuration

On Sat, Feb 03, 2024 at 02:11:51PM +0100, Tim Pambor wrote:
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index b7cb71817780..b061036f654a 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -398,16 +398,15 @@ static int dp83822_config_init(struct phy_device *phydev)
>  		tx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
>  						      false);
>  		if (tx_int_delay <= 0)
> -			rgmii_delay &= ~DP83822_TX_CLK_SHIFT;
> -		else
>  			rgmii_delay |= DP83822_TX_CLK_SHIFT;
> +		else
> +			rgmii_delay &= ~DP83822_TX_CLK_SHIFT;

Further cleanup is possible here:

                rx_int_delay = phy_get_internal_delay(phydev, dev, NULL, 0,
                                                      true);

                if (rx_int_delay <= 0)
                        rgmii_delay = 0;
                else
                        rgmii_delay = DP83822_RX_CLK_SHIFT;

At this point, rgmii_delay can only contain one of two possible values.
Zero, and bit 12 set.

The the code above modifies this value by either setting bit 11, or
clearing the already guaranteed to be clear bit 11. So, the only thing
that has any effect is setting bit 12, so we can omit the code path that
clears bit 11. Therefore, this can become:

		// TX_CLK_SHIFT disables the delay
                if (tx_int_delay <= 0)
                        rgmii_delay |= DP83822_TX_CLK_SHIFT;

I would also add a comment that RX_CLK_SHIFT enables the delay.

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