Use DELAY_ENABLE macro for RK3328, RK3566/RK3568 and RK3588

[PATCH 3/3] net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3588

Posted by Jonas Karlman 11 months, 1 week ago

Support for Rockchip RK3588 GMAC was added without use of the
DELAY_ENABLE macro to assist with enable/disable use of MAC rx/tx delay.

Change to use a variant of the DELAY_ENABLE macro to help disable MAC
delay when RGMII_ID/RXID/TXID is used.

Fixes: 2f2b60a0ec28 ("net: ethernet: stmmac: dwmac-rk: Add gmac support for rk3588")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 37eb86e4e325..79db81d68afd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -91,6 +91,10 @@ struct rk_priv_data {
 	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE : soc##_GMAC_TXCLK_DLY_DISABLE) | \
 	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE))
 
+#define DELAY_ENABLE_BY_ID(soc, tx, rx, id) \
+	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE(id) : soc##_GMAC_TXCLK_DLY_DISABLE(id)) | \
+	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE(id) : soc##_GMAC_RXCLK_DLY_DISABLE(id)))
+
 #define PX30_GRF_GMAC_CON1		0x0904
 
 /* PX30_GRF_GMAC_CON1 */
@@ -1322,8 +1326,7 @@ static void rk3588_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3588_GMAC_CLK_RGMII_MODE(id));
 
 	regmap_write(bsp_priv->grf, RK3588_GRF_GMAC_CON7,
-		     RK3588_GMAC_RXCLK_DLY_ENABLE(id) |
-		     RK3588_GMAC_TXCLK_DLY_ENABLE(id));
+		     DELAY_ENABLE_BY_ID(RK3588, tx_delay, rx_delay, id));
 
 	regmap_write(bsp_priv->grf, offset_con,
 		     RK3588_GMAC_CLK_RX_DL_CFG(rx_delay) |
-- 
2.48.1

Re: [PATCH 3/3] net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3588

Posted by Dragan Simic 11 months, 1 week ago

Hello Jonas,

On 2025-03-06 21:38, Jonas Karlman wrote:
> Support for Rockchip RK3588 GMAC was added without use of the
> DELAY_ENABLE macro to assist with enable/disable use of MAC rx/tx 
> delay.
> 
> Change to use a variant of the DELAY_ENABLE macro to help disable MAC
> delay when RGMII_ID/RXID/TXID is used.
> 
> Fixes: 2f2b60a0ec28 ("net: ethernet: stmmac: dwmac-rk: Add gmac
> support for rk3588")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 37eb86e4e325..79db81d68afd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -91,6 +91,10 @@ struct rk_priv_data {
>  	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE : soc##_GMAC_TXCLK_DLY_DISABLE) 
> | \
>  	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE))
> 
> +#define DELAY_ENABLE_BY_ID(soc, tx, rx, id) \
> +	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE(id) : 
> soc##_GMAC_TXCLK_DLY_DISABLE(id)) | \
> +	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE(id) : 
> soc##_GMAC_RXCLK_DLY_DISABLE(id)))
> +
>  #define PX30_GRF_GMAC_CON1		0x0904
> 
>  /* PX30_GRF_GMAC_CON1 */
> @@ -1322,8 +1326,7 @@ static void rk3588_set_to_rgmii(struct
> rk_priv_data *bsp_priv,
>  		     RK3588_GMAC_CLK_RGMII_MODE(id));
> 
>  	regmap_write(bsp_priv->grf, RK3588_GRF_GMAC_CON7,
> -		     RK3588_GMAC_RXCLK_DLY_ENABLE(id) |
> -		     RK3588_GMAC_TXCLK_DLY_ENABLE(id));
> +		     DELAY_ENABLE_BY_ID(RK3588, tx_delay, rx_delay, id));
> 
>  	regmap_write(bsp_priv->grf, offset_con,
>  		     RK3588_GMAC_CLK_RX_DL_CFG(rx_delay) |

Thanks for this patch...  It's looking good to me, and good job
spotting this issue!  Please, free to include:

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

Re: [PATCH 3/3] net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3588

Posted by Sebastian Reichel 11 months, 1 week ago

Hi,

On Thu, Mar 06, 2025 at 10:33:23PM +0100, Dragan Simic wrote:
> Hello Jonas,
> 
> On 2025-03-06 21:38, Jonas Karlman wrote:
> > Support for Rockchip RK3588 GMAC was added without use of the
> > DELAY_ENABLE macro to assist with enable/disable use of MAC rx/tx delay.
> > 
> > Change to use a variant of the DELAY_ENABLE macro to help disable MAC
> > delay when RGMII_ID/RXID/TXID is used.
> > 
> > Fixes: 2f2b60a0ec28 ("net: ethernet: stmmac: dwmac-rk: Add gmac
> > support for rk3588")
> > Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > index 37eb86e4e325..79db81d68afd 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > @@ -91,6 +91,10 @@ struct rk_priv_data {
> >  	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE : soc##_GMAC_TXCLK_DLY_DISABLE) |
> > \
> >  	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE))
> > 
> > +#define DELAY_ENABLE_BY_ID(soc, tx, rx, id) \
> > +	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE(id) :
> > soc##_GMAC_TXCLK_DLY_DISABLE(id)) | \
> > +	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE(id) :
> > soc##_GMAC_RXCLK_DLY_DISABLE(id)))
> > +
> >  #define PX30_GRF_GMAC_CON1		0x0904
> > 
> >  /* PX30_GRF_GMAC_CON1 */
> > @@ -1322,8 +1326,7 @@ static void rk3588_set_to_rgmii(struct
> > rk_priv_data *bsp_priv,
> >  		     RK3588_GMAC_CLK_RGMII_MODE(id));
> > 
> >  	regmap_write(bsp_priv->grf, RK3588_GRF_GMAC_CON7,
> > -		     RK3588_GMAC_RXCLK_DLY_ENABLE(id) |
> > -		     RK3588_GMAC_TXCLK_DLY_ENABLE(id));
> > +		     DELAY_ENABLE_BY_ID(RK3588, tx_delay, rx_delay, id));
> > 
> >  	regmap_write(bsp_priv->grf, offset_con,
> >  		     RK3588_GMAC_CLK_RX_DL_CFG(rx_delay) |
> 
> Thanks for this patch...  It's looking good to me, and good job
> spotting this issue!  Please, free to include:
> 
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian