diff mbox series

[net] net: ethernet: mtk_eth_soc: drop clocks unused by Ethernet driver

Message ID 38559102c729a811dc8a85f6c7cee07228cffd3e.1721739769.git.daniel@makrotopia.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] net: ethernet: mtk_eth_soc: drop clocks unused by Ethernet driver | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 273 this patch: 273
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 281 this patch: 281
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 291 this patch: 291
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 31 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-07-23--15-00 (tests: 688)

Commit Message

Daniel Golle July 23, 2024, 1:04 p.m. UTC
Clocks for SerDes and PHY are going to be handled by standalone drivers
for each of those hardware components. Drop them from the Ethernet driver.

Fixes: 445eb6448ed3 ("net: ethernet: mtk_eth_soc: add basic support for MT7988 SoC")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
The dt-bindings part has been taken care of already in commit
cc349b0771dc dt-bindings: net: mediatek: remove wrongly added clocks and SerDes

 drivers/net/ethernet/mediatek/mtk_eth_soc.h | 14 --------------
 1 file changed, 14 deletions(-)

Comments

AngeloGioacchino Del Regno July 23, 2024, 3:07 p.m. UTC | #1
Il 23/07/24 15:04, Daniel Golle ha scritto:
> Clocks for SerDes and PHY are going to be handled by standalone drivers
> for each of those hardware components. Drop them from the Ethernet driver.
> 
> Fixes: 445eb6448ed3 ("net: ethernet: mtk_eth_soc: add basic support for MT7988 SoC")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Andrew Lunn July 23, 2024, 6:35 p.m. UTC | #2
On Tue, Jul 23, 2024 at 02:04:02PM +0100, Daniel Golle wrote:
> Clocks for SerDes and PHY are going to be handled by standalone drivers
> for each of those hardware components. Drop them from the Ethernet driver.

Please could you explain in more details how this does not break
backwards compatibility. Should there also be a depends on, to ensure
the new driver is loaded? Will old DT blobs still work?

Thanks
	Andrew
Daniel Golle July 23, 2024, 9:47 p.m. UTC | #3
Hi Andrew,

On Tue, Jul 23, 2024 at 08:35:16PM +0200, Andrew Lunn wrote:
> On Tue, Jul 23, 2024 at 02:04:02PM +0100, Daniel Golle wrote:
> > Clocks for SerDes and PHY are going to be handled by standalone drivers
> > for each of those hardware components. Drop them from the Ethernet driver.
> 
> Please could you explain in more details how this does not break
> backwards compatibility. Should there also be a depends on, to ensure
> the new driver is loaded? Will old DT blobs still work?

At this stage the Ethernet driver only supports the first MAC which
is hard-wired to the built-in DSA switch.
The clocks which are being removed for this patch are responsible for
the for the SerDes PCS and PHYs used for the 2nd and 3rd MAC which
are anyway not yet supported.

Those clocks are basically a left-over from the implementation found in
MediaTek's SDK which does all that inside the Ethernet driver and using
lots of syscon regmaps to access the various parts of the SoC.

This has been deemed unsuitable for inclusion in upstream Linux[1] and I
was asked to implement standalone PHY, CLK and PCS drivers instead,
which is obviously more clean and also results in the device tree being
more understandable.

By now, a CLK driver and a PHY driver (PHY as in drivers/phy, not
drivers/net/phy) has landed in upstream Linux([2], [3]), I'm currently
finalizing the PCS drivers which are going to be in charge of handling
the clocks which are now going to be removed from the Ethernet driver.

tl;dr: The clocks were added by mistake and features of the SoC using
them are up to now unsupported by vanilla Linux.

[1]: https://patchwork.kernel.org/comment/25517462/

[2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4b4719437d85f0173d344f2c76fa1a5b7f7d184b

[3]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ac4aa9dbc702329c447d968325b055af84ae1b59
Andrew Lunn July 23, 2024, 9:54 p.m. UTC | #4
On Tue, Jul 23, 2024 at 10:47:00PM +0100, Daniel Golle wrote:
> Hi Andrew,
> 
> On Tue, Jul 23, 2024 at 08:35:16PM +0200, Andrew Lunn wrote:
> > On Tue, Jul 23, 2024 at 02:04:02PM +0100, Daniel Golle wrote:
> > > Clocks for SerDes and PHY are going to be handled by standalone drivers
> > > for each of those hardware components. Drop them from the Ethernet driver.
> > 
> > Please could you explain in more details how this does not break
> > backwards compatibility. Should there also be a depends on, to ensure
> > the new driver is loaded? Will old DT blobs still work?
> 
> At this stage the Ethernet driver only supports the first MAC which
> is hard-wired to the built-in DSA switch.

> The clocks which are being removed for this patch are responsible for
> the for the SerDes PCS and PHYs used for the 2nd and 3rd MAC which
> are anyway not yet supported.

O.K. This last part, not yet supported, would of been good to have in
the commit message. It then makes it clear backwards compatibility is
not an issue, it never worked in the first place.

Thanks
	Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index eb1708b43aa3..0d5225f1d3ee 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -724,12 +724,8 @@  enum mtk_clks_map {
 	MTK_CLK_ETHWARP_WOCPU2,
 	MTK_CLK_ETHWARP_WOCPU1,
 	MTK_CLK_ETHWARP_WOCPU0,
-	MTK_CLK_TOP_USXGMII_SBUS_0_SEL,
-	MTK_CLK_TOP_USXGMII_SBUS_1_SEL,
 	MTK_CLK_TOP_SGM_0_SEL,
 	MTK_CLK_TOP_SGM_1_SEL,
-	MTK_CLK_TOP_XFI_PHY_0_XTAL_SEL,
-	MTK_CLK_TOP_XFI_PHY_1_XTAL_SEL,
 	MTK_CLK_TOP_ETH_GMII_SEL,
 	MTK_CLK_TOP_ETH_REFCK_50M_SEL,
 	MTK_CLK_TOP_ETH_SYS_200M_SEL,
@@ -800,19 +796,9 @@  enum mtk_clks_map {
 				 BIT_ULL(MTK_CLK_GP3) | BIT_ULL(MTK_CLK_XGP1) | \
 				 BIT_ULL(MTK_CLK_XGP2) | BIT_ULL(MTK_CLK_XGP3) | \
 				 BIT_ULL(MTK_CLK_CRYPTO) | \
-				 BIT_ULL(MTK_CLK_SGMII_TX_250M) | \
-				 BIT_ULL(MTK_CLK_SGMII_RX_250M) | \
-				 BIT_ULL(MTK_CLK_SGMII2_TX_250M) | \
-				 BIT_ULL(MTK_CLK_SGMII2_RX_250M) | \
 				 BIT_ULL(MTK_CLK_ETHWARP_WOCPU2) | \
 				 BIT_ULL(MTK_CLK_ETHWARP_WOCPU1) | \
 				 BIT_ULL(MTK_CLK_ETHWARP_WOCPU0) | \
-				 BIT_ULL(MTK_CLK_TOP_USXGMII_SBUS_0_SEL) | \
-				 BIT_ULL(MTK_CLK_TOP_USXGMII_SBUS_1_SEL) | \
-				 BIT_ULL(MTK_CLK_TOP_SGM_0_SEL) | \
-				 BIT_ULL(MTK_CLK_TOP_SGM_1_SEL) | \
-				 BIT_ULL(MTK_CLK_TOP_XFI_PHY_0_XTAL_SEL) | \
-				 BIT_ULL(MTK_CLK_TOP_XFI_PHY_1_XTAL_SEL) | \
 				 BIT_ULL(MTK_CLK_TOP_ETH_GMII_SEL) | \
 				 BIT_ULL(MTK_CLK_TOP_ETH_REFCK_50M_SEL) | \
 				 BIT_ULL(MTK_CLK_TOP_ETH_SYS_200M_SEL) | \