Re: [PATCH net-next 3/3] net: stmmac: Add UltraRISC DP1000 GMAC support

[Jia Wang]

On 2026-09-04 14:01 +0800, Jia Wang wrote:
> On 2026-09-03 14:30 +0200, Maxime Chevallier wrote:
> > 
> > 
> > On 9/3/26 14:10, Andrew Lunn wrote:
> > >>> This looks to be 90% identical to dwmac-sophgo.c.
> > >>>
> > >>> Do you expect any further changes to this driver? Is everything
> > >>> supported? Are there more clocks? A GPIO for WoL?
> > >>>
> > >>> I'm just wondering if the common code should be pulled out, or even
> > >>> dwmac-sophgo.c made generic so it can handle all plain boring devices
> > >>> which have RGMII delays and not a lot else.
> > >>>
> > >>
> > >> The probe boilerplate is indeed largely similar because both drivers
> > >> use the common stmmac platform helpers. However, their platform-specific
> > >> requirements differ: Sophgo requires additional TX clock handling and
> > >> other configuration, while DP1000 requires fixed TX and RX delay
> > >> handling. ACPI support under development is also intended to use the
> > >> UltraRISC glue driver.
> > > 
> > > ACPI is going to be fun, since nobody does ACPI networking.
> > > 
> > > But how big is the change to the glue driver? I expect the change will
> > > be minor, a call to devm_stmmac_probe_config_acpi(), in addition to
> > > devm_stmmac_probe_config_dt(). Anything else?
> > 
> > I agree that if ACPI support is added, let's try to make it part of the
> > core.
> > 
> 
> The ACPI support is still under development, but the current prototype
> adds ACPI device matching and ACPI-specific platform-data setup. Some
> common stmmac changes are also needed for ACPI-described platform
> resources and MDIO.
> 
> > > 
> > > One problem the stmmac driver has is cut/paste glue driver
> > > development. The same code is repeated again and again. Russell King
> > > spent a lot of time and effort trying to reduce the duplication. So we
> > > should not be adding more duplicated code unless it is actually
> > > required.
> > 
> > It seems to me that all the RGMII handling could be made much more generic,
> > as this is commong pattern in glue drivers :
> > 
> > dwmac-sophgo has the "has_internal_rx_delay" field, and dwmac-eic7700 has
> > both has_internal_[r|t]x_delay.
> > 
> > With this third one, it makes sense to start sharing that logic across
> > glues.
> > 
> 
> I agree. DP1000 is another user of this pattern, with fixed internal
> delays on both the TX and RX paths. I will look at moving this handling
> into the common stmmac platform code, leaving each glue driver to
> describe its internal delay capabilities.
> 

After looking further into the implementation, I see two possible
approaches:

1. Keep a minimal UltraRISC glue driver that only describes the fixed TX
and RX delay capabilities, while the common stmmac code handles the PHY
mode adjustment.

2. Store these capabilities as compatible-specific match data in
dwmac-generic and remove the UltraRISC glue driver entirely.

I currently prefer the first approach. The fixed internal delays are
properties of the DP1000 integration rather than the generic Synopsys IP,
so keeping them in the glue driver preserves that distinction. The delay
handling and validation would still be shared by the common stmmac code,
leaving the glue driver responsible only for declaring the hardware
capabilities. This also avoids turning dwmac-generic into a table of
SoC-specific capabilities.

Would this approach be acceptable, or would you prefer using dwmac-generic
for DP1000?

Best regards,
Jia Wang