Re: [EXT] Re: [PATCH v3 net 2/2] net: stmmac: dwmac-imx: pause the TXC clock in fixed-link

From: Johannes Zink
Date: Thu Aug 03 2023 - 02:37:47 EST

Hi Shenwei,


Also: does this only apply to i.MX93, or would we have to test and
enable it on e.g. i.MX8MP as well?

Yes, it is required when the EQOS MAC is selected. However, this patch
just enables The feature on i.MX93.

If this behaviour is required on all EQOS, I think the name
imx_dwmac_fix_speed_mx93() is misleading. It should either be
imx_dwmac_fix_speed() if applicable to all imx implementations, or
dwmac_fix_speed() (and moved to a non-gluecode file) if applicable for all
implementations in general.

It has the general fix_speed function there named imx_dwmac_fix_speed.
This one is the special for this mx93 fix.

I think I might have misunderstood your last statement or I failed to express my point. If you need to replace the dwmac_fix_speed() on mx93, because this SoC implementation requires doing so (the usual reason for doing something like this is something like reset quirks because of screwed up IP Core integration), then your approach is imho valid.

But if I got your last comment right, your changes should apply to EQOS MAC in general (but you want to only enable it for mx93 at the moment). In this case this quirk will later be as the fix_mac_speed function for other hardware as well, in which case the name ..._mx93 is misleading, and imho rather a descriptive name should be used (i.e. have the name describe what it does rather than for what hardware it is implemented).

Except if the maintainers have a strong opinion that the ..._mx93 suffix version is exactly how you should proceed...

Best regards


Pengutronix e.K. | Johannes Zink |
Steuerwalder Str. 21 | |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686| Fax: +49-5121-206917-5555 |