RE: [EXT] Re: [PATCH v3 net 2/2] net: stmmac: dwmac-imx: pause the TXC clock in fixed-link
From: Shenwei Wang <shenwei.wang@nxp.com>
Date: 2023-08-03 13:11:16
Also in:
imx, linux-amlogic, linux-arm-kernel, lkml
-----Original Message----- From: Johannes Zink <redacted> Sent: Thursday, August 3, 2023 1:36 AM To: Shenwei Wang <shenwei.wang@nxp.com>; Russell King [off-list ref]; David S. Miller [off-list ref]; Eric Dumazet [off-list ref]; Jakub Kicinski [off-list ref]; Paolo Abeni [off-list ref]; Maxime Coquelin [off-list ref]; Shawn Guo [off-list ref]; Sascha Hauer [off-list ref]; Neil Armstrong [off-list ref]; Kevin Hilman [off-list ref]; Vinod Koul [off-list ref]; Chen- Yu Tsai [off-list ref]; Jernej Skrabec [off-list ref]; Samuel Holland [off-list ref] Cc: Giuseppe Cavallaro <redacted>; Alexandre Torgue [off-list ref]; Jose Abreu [off-list ref]; Pengutronix Kernel Team [off-list ref]; Fabio Estevam [off-list ref]; dl-linux-imx [off-list ref]; Jerome Brunet [off-list ref]; Martin Blumenstingl [off-list ref]; Bhupesh Sharma [off-list ref]; Nobuhiro Iwamatsu [off-list ref]; Simon Hormanquoted
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 Johannesquoted
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 |