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

On 2026-09-07 09:28 +0200, Maxime Chevallier wrote:

Hi,

On 9/7/26 07:48, Jia Wang wrote:

quoted

On 2026-09-04 14:01 +0800, Jia Wang wrote:

quoted

On 2026-09-03 14:30 +0200, Maxime Chevallier wrote:

quoted

On 9/3/26 14:10, Andrew Lunn wrote:

quoted

quoted

quoted

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.

quoted

quoted

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