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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

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.

> 
> 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