Re: [PATCH net-next v3 5/9] net: stmmac: mediatek: add RX/TX delay stage divider in platform data

[Louis-Alexis Eyraud]

Hi Maxime,

On Fri, 2026-10-16 at 13:37 +0200, Maxime Chevallier wrote:
> Hi Louis-Alexis,
> 
> On 10/16/26 12:31, Louis-Alexis Eyraud wrote:
> > In preparation of newer SoC support, that have like MT8189 SoC
> > different RX and TX internal delay stage steps than MT8188/MT8195,
> > add
> > the RX and TX delay stage divider values in the variant platform
> > data
> > and replace the hardcoded value uses in mt2712_set_delay and
> > mt8195_set_delay.
> > 
> > Reviewed-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
> > Signed-off-by: Louis-Alexis Eyraud
> > <louisalexis.eyraud@xxxxxxxxxxxxx>
> > ---
> >  .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   | 46
> > ++++++++++++++++++----
> >  1 file changed, 38 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > index 596411d4a5b2..cb5bda53edb9 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > @@ -108,6 +108,10 @@ struct mediatek_dwmac_variant {
> >  
> >  	u32 rx_delay_max;
> >  	u32 tx_delay_max;
> > +	u16 rx_rmii_delay_stage_div;
> > +	u16 tx_rmii_delay_stage_div;
> > +	u16 rx_rgmii_delay_stage_div;
> > +	u16 tx_rgmii_delay_stage_div;
> >  	u8 dma_bit_mask;
> >  };
> >  
> > @@ -146,14 +150,22 @@ static int mt2712_set_delay(struct
> > mediatek_dwmac_plat_data *plat)
> >  	u32 fine_val = 0;
> >  
> >  	if (phy_interface_mode_is_rgmii(plat->phy_mode)) {
> > -		rx_delay_stage_div =
> > MT2712_RGMII_DLY_RXC_STAGE_DIV;
> > -		tx_delay_stage_div =
> > MT2712_RGMII_DLY_TXC_STAGE_DIV;
> > +		rx_delay_stage_div = plat->variant-
> > >rx_rgmii_delay_stage_div;
> > +		tx_delay_stage_div = plat->variant-
> > >tx_rgmii_delay_stage_div;
> >  	} else {
> > -		rx_delay_stage_div =
> > MT2712_RMII_DLY_RXC_STAGE_DIV;
> > -		tx_delay_stage_div =
> > MT2712_RMII_DLY_TXC_STAGE_DIV;
> > +		rx_delay_stage_div = plat->variant-
> > >rx_rmii_delay_stage_div;
> > +		tx_delay_stage_div = plat->variant-
> > >tx_rmii_delay_stage_div;
> 
> Looking closer at this, what's RMII delay ? There's no delays in
> RMII, and
> it's a single clock not a RX and TX clock like RGMII, there's no RMII
> TXC/RXC.
> 
You are right, these structures field and define names are wrong for
RMII/MII cases.
The mediatek,tx-delay-ps and mediatek,rx-delay-ps vendor properties are
used by this driver to fine tune different clocks depending on the phy
mode:
 - RX_CLK/TX_CLK in MII
 - REF_CLK in RMII
 - TXC/RXC in RGMII

I'll fix the fields and define names in next series revision, along the
issues the Sashiko reviews found.

> I think it's the same as on allwinner, the delay handling has been
> mixed-up between
> RMII and RGMII in the glue, you should be able to simplify this even
> more by
> ditching the RMII case.
The register fields that the driver sets in RMII or MII mode to tune
the needed clock(s) are different than the ones set in RGMII.
So I cannot really ditch the case but I'll simplify the delay handling.
For instance I could use a single divider field per mode, since for all
compatibles the value is the same in both RX and TX.

Best regards,
Louis-Alexis
> 
> Maxime