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

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

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.

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.

Maxime