Received-SPF: pass (google.com: domain of linux-kernel+bounces-51117-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sat, 3 Feb 2024 14:39:50 +0000
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Tim Pambor <tp@osasysteme.de>
Cc: Andrew Lunn <andrew@lunn.ch>, Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Dan Murphy <dmurphy@ti.com>, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: phy: dp83822: Fix RGMII TX delay configuration
Message-ID: <Zb5QNkcDvM6CtALf@shell.armlinux.org.uk>
References: <20240203131152.61958-1-tp@osasysteme.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240203131152.61958-1-tp@osasysteme.de>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

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!