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

[Jia Wang]

On 2026-09-07 09:28 +0200, Maxime Chevallier wrote:
> Hi,
> 
> On 9/7/26 07:48, Jia Wang wrote:
> > 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.
> Let's go with 1 for now I'd say. UtraRISC will be the only one that's basically
> an empty glue calling core code (maybe you have some plat->flags to passs ?).
> 
> If we manage to simplify even further (e.g. tx clock handling for exmaple), we could
> also deal with dwmac-sophgo the same way. We could then consider something like
> dwmac-simple.c , keeping dwmac-generic.c for the raw dwmac IPs.
> 

Thanks, I will go with approach 1 in the next revision. DP1000 does not
currently require any additional plat->flags.

Best regards,
Jia Wang