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

[Johannes Zink]

Hi Shenwei,

[snip]
> > > > 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
Johannes

> Thanks,
> Shenwei
> [snip]


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