Thread (21 messages) 21 messages, 7 authors, 2023-08-03

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 Horman
quoted
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
quoted
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    |
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help