public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v3] net: ethernet: mtk_eth_soc: avoid writing to ESW registers on MT7628
@ 2026-01-22 18:58 Joris Vaisvila
  2026-01-25 21:36 ` Jakub Kicinski
  0 siblings, 1 reply; 3+ messages in thread
From: Joris Vaisvila @ 2026-01-22 18:58 UTC (permalink / raw)
  To: netdev
  Cc: nbd, sean.wang, lorenzo, andrew+netdev, davem, edumazet, kuba,
	pabeni, Joris Vaisvila

The MT7628 has a fixed-link PHY and does not expose MAC control
registers. Writes to these registers only corrupt the ESW VLAN
configuration.

This patch registers empty phylink_mac_ops for MT7628, as removing the
invalid register accesses leaves nothing to do on this SoC.

Fixes: 296c9120752b ("net: ethernet: mediatek: Add MT7628/88 SoC support")
Signed-off-by: Joris Vaisvila <joey@tinyisr.com>
---
v3: 
 - Register separate mac_ops for MT7628 instead of using early return
   based on SoC for all phylink mac operations
 - Update commit message
v2: https://lore.kernel.org/netdev/20260106052845.1945352-1-joey@tinyisr.com/
 - Add missing fixes tag
v1: https://lore.kernel.org/netdev/20251230091151.129176-1-joey@tinyisr.com/

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 37 ++++++++++++++++++---
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 35fef28ee2f9..ede8efdc3413 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -562,9 +562,7 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
 	int val, ge_mode, err = 0;
 	u32 i;
 
-	/* MT76x8 has no hardware settings between for the MAC */
-	if (!MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628) &&
-	    mac->interface != state->interface) {
+	if (mac->interface != state->interface) {
 		/* Setup soc pin functions */
 		switch (state->interface) {
 		case PHY_INTERFACE_MODE_TRGMII:
@@ -956,6 +954,30 @@ static const struct phylink_mac_ops mtk_phylink_ops = {
 	.mac_enable_tx_lpi = mtk_mac_enable_tx_lpi,
 };
 
+static void rt5350_mac_config(struct phylink_config *config, unsigned int mode,
+				const struct phylink_link_state *state)
+{
+}
+
+static void rt5350_mac_link_down(struct phylink_config *config, unsigned int mode,
+				phy_interface_t interface)
+{
+}
+
+static void rt5350_mac_link_up(struct phylink_config *config,
+			    struct phy_device *phy,
+			    unsigned int mode, phy_interface_t interface,
+			    int speed, int duplex, bool tx_pause, bool rx_pause)
+{
+}
+
+/* MT76x8 (rt5350-eth) does not expose any MAC control registers */
+static const struct phylink_mac_ops rt5350_phylink_ops = {
+	.mac_config = rt5350_mac_config,
+	.mac_link_down = rt5350_mac_link_down,
+	.mac_link_up = rt5350_mac_link_up,
+};
+
 static void mtk_mdio_config(struct mtk_eth *eth)
 {
 	u32 val;
@@ -4779,6 +4801,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	int id, err;
 	int txqs = 1;
 	u32 val;
+	const struct phylink_mac_ops *mac_ops = &mtk_phylink_ops;
 
 	if (!_id) {
 		dev_err(eth->dev, "missing mac id\n");
@@ -4906,9 +4929,15 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 			  mac->phylink_config.supported_interfaces);
 	}
 
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
+		mac_ops = &rt5350_phylink_ops;
+
 	phylink = phylink_create(&mac->phylink_config,
 				 of_fwnode_handle(mac->of_node),
-				 phy_mode, &mtk_phylink_ops);
+				 phy_mode,
+				 mac_ops);
+
 	if (IS_ERR(phylink)) {
 		err = PTR_ERR(phylink);
 		goto free_netdev;
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v3] net: ethernet: mtk_eth_soc: avoid writing to ESW registers on MT7628
  2026-01-22 18:58 [PATCH net-next v3] net: ethernet: mtk_eth_soc: avoid writing to ESW registers on MT7628 Joris Vaisvila
@ 2026-01-25 21:36 ` Jakub Kicinski
  2026-01-29  6:22   ` Joris Vaišvila
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2026-01-25 21:36 UTC (permalink / raw)
  To: Joris Vaisvila
  Cc: netdev, nbd, sean.wang, lorenzo, andrew+netdev, davem, edumazet,
	pabeni

On Thu, 22 Jan 2026 20:58:08 +0200 Joris Vaisvila wrote:
> The MT7628 has a fixed-link PHY and does not expose MAC control
> registers. Writes to these registers only corrupt the ESW VLAN
> configuration.
> 
> This patch registers empty phylink_mac_ops for MT7628, as removing the
> invalid register accesses leaves nothing to do on this SoC.
> 
> Fixes: 296c9120752b ("net: ethernet: mediatek: Add MT7628/88 SoC support")

If we're going with net-next please drop the Fixes tag.
We don't need a backport. You can quote the commit if you prefer eg:

This code was introduced by commit 296c9120752b ("net: ethernet:
mediatek: Add MT7628/88 SoC support")

> +static void rt5350_mac_config(struct phylink_config *config, unsigned int mode,
> +				const struct phylink_link_state *state)
> +{
> +}
> +
> +static void rt5350_mac_link_down(struct phylink_config *config, unsigned int mode,
> +				phy_interface_t interface)
> +{
> +}
> +
> +static void rt5350_mac_link_up(struct phylink_config *config,
> +			    struct phy_device *phy,
> +			    unsigned int mode, phy_interface_t interface,
> +			    int speed, int duplex, bool tx_pause, bool rx_pause)
> +{
> +}

Is there any other driver that implements fixed link with phylink this
way? I know regrettably little about phylink. I'd think that mac up/down
usually would still do _something_.

> @@ -4779,6 +4801,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
>  	int id, err;
>  	int txqs = 1;
>  	u32 val;
> +	const struct phylink_mac_ops *mac_ops = &mtk_phylink_ops;

nit: try to keep local variables ordered longest line to shortest

>  	if (!_id) {
>  		dev_err(eth->dev, "missing mac id\n");
> @@ -4906,9 +4929,15 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
>  			  mac->phylink_config.supported_interfaces);
>  	}
>  
> +

nit: double new line

> +	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
> +		mac_ops = &rt5350_phylink_ops;
> +
>  	phylink = phylink_create(&mac->phylink_config,
>  				 of_fwnode_handle(mac->of_node),
> -				 phy_mode, &mtk_phylink_ops);
> +				 phy_mode,
> +				 mac_ops);
> +

nit: please don't insert empty lines between function call and its
error check

>  	if (IS_ERR(phylink)) {
>  		err = PTR_ERR(phylink);
>  		goto free_netdev;
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v3] net: ethernet: mtk_eth_soc: avoid writing to ESW registers on MT7628
  2026-01-25 21:36 ` Jakub Kicinski
@ 2026-01-29  6:22   ` Joris Vaišvila
  0 siblings, 0 replies; 3+ messages in thread
From: Joris Vaišvila @ 2026-01-29  6:22 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, nbd, sean.wang, lorenzo, andrew+netdev, davem, edumazet,
	pabeni

> > +static void rt5350_mac_config(struct phylink_config *config, unsigned int mode,
> > +				const struct phylink_link_state *state)
> > +{
> > +}
> > +
> > +static void rt5350_mac_link_down(struct phylink_config *config, unsigned int mode,
> > +				phy_interface_t interface)
> > +{
> > +}
> > +
> > +static void rt5350_mac_link_up(struct phylink_config *config,
> > +			    struct phy_device *phy,
> > +			    unsigned int mode, phy_interface_t interface,
> > +			    int speed, int duplex, bool tx_pause, bool rx_pause)
> > +{
> > +}
> 
> Is there any other driver that implements fixed link with phylink this
> way? I know regrettably little about phylink. I'd think that mac up/down
> usually would still do _something_.
> 

`net/dsa/port.c` stubs are the only other place with no-op phylink mac
ops.

On MT7628, the existing `mtk_gdm_mac_link_up()` does not actually
commit any configuration to the MAC hardware. The only potentially
observable change is `mac->speed = speed`, however leaving that as
`UNKNOWN_SPEED` appears more accurate for this SoC, though I will
recheck for side-effects of that.

`mtk_mac_link_down()` also does not touch any valid MAC registers. Both
of these functions are already effectively no-ops in terms of MAC
configuration on this SoC. They only clobber unrelated ESW registers.

While the ESW block seems to provide a way to control the CPU port speed
and link state, this is a separate IP and I don't think it's appropriate
to have the MAC driver program it.

While I'm not very familiar with phylink either, this is no different
from how the original driver handles link up/down on this SoC.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-01-29  6:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-22 18:58 [PATCH net-next v3] net: ethernet: mtk_eth_soc: avoid writing to ESW registers on MT7628 Joris Vaisvila
2026-01-25 21:36 ` Jakub Kicinski
2026-01-29  6:22   ` Joris Vaišvila

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox