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

[Shenwei Wang]

> -----Original Message-----
> From: Johannes Zink <j.zink at pengutronix.de>
> Sent: Thursday, August 3, 2023 1:36 AM
> To: Shenwei Wang <shenwei.wang at nxp.com>; Russell King
> <linux at armlinux.org.uk>; David S. Miller <davem at davemloft.net>; Eric
> Dumazet <edumazet at google.com>; Jakub Kicinski <kuba at kernel.org>; Paolo
> Abeni <pabeni at redhat.com>; Maxime Coquelin
> <mcoquelin.stm32 at gmail.com>; Shawn Guo <shawnguo at kernel.org>; Sascha
> Hauer <s.hauer at pengutronix.de>; Neil Armstrong <neil.armstrong at linaro.org>;
> Kevin Hilman <khilman at baylibre.com>; Vinod Koul <vkoul at kernel.org>; Chen-
> Yu Tsai <wens at csie.org>; Jernej Skrabec <jernej.skrabec at gmail.com>; Samuel
> Holland <samuel at sholland.org>
> Cc: Giuseppe Cavallaro <peppe.cavallaro at st.com>; Alexandre Torgue
> <alexandre.torgue at foss.st.com>; Jose Abreu <joabreu at synopsys.com>;
> Pengutronix Kernel Team <kernel at pengutronix.de>; Fabio Estevam
> <festevam at gmail.com>; dl-linux-imx <linux-imx at nxp.com>; Jerome Brunet
> <jbrunet at baylibre.com>; Martin Blumenstingl
> <martin.blumenstingl at googlemail.com>; Bhupesh Sharma
> <bhupesh.sharma at linaro.org>; Nobuhiro Iwamatsu
> <nobuhiro1.iwamatsu at toshiba.co.jp>; Simon Horman
> > 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).
>

The idea of supporting the SJA1105 is general, but the implementation depends on the
specific SoC. This patch provides the implementation for the MX93 SoC. To support the
MX8x SoCs, the implementation would need to be rewritten based on the current state
of the dwmac-imx driver.

I agree the function name is somewhat ugly. Maybe a better name could be:
mx93_dwmac_fix_speed()

The assumption is that the process of pausing the clock can still be considered part of fixing the speed.

Thanks,
Shenwei

> 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.pe/
> ngutronix.de%2F&data=05%7C01%7Cshenwei.wang%40nxp.com%7Cd328d89d
> 14e847270d5a08db93ebff01%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C638266414027048795%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
> %7C&sdata=dkE9Es7kl3uNYx8zJYZt2r6933dSNtYDpQzCEagAV3M%3D&reserved
> =0    |
> 31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
> Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |