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

[Jia Wang <wangjia@ultrarisc.com>]

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.

Best regards,
Jia Wang