Messages in this thread | | | Date | Mon, 15 Jan 2024 23:37:20 +0200 | From | Vladimir Oltean <> | Subject | Re: [RFC PATCH net-next 6/8] net: dsa: mt7530: simplify mt7530_setup_port6() and change to void |
| |
On Sat, Jan 13, 2024 at 01:25:27PM +0300, Arınç ÜNAL wrote: > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 3ce4e0bb04dd..3a02308763ca 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -414,72 +414,56 @@ mt753x_preferred_default_local_cpu_port(struct dsa_switch *ds) > } > > /* Setup port 6 interface mode and TRGMII TX circuit */ > -static int > +static void > mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface) > { > struct mt7530_priv *priv = ds->priv; > - u32 ncpo1, ssc_delta, trgint, xtal; > + u32 ncpo1, ssc_delta, xtal; > > mt7530_clear(priv, MT7530_MHWTRAP, MHWTRAP_P6_DIS); > > + if (interface == PHY_INTERFACE_MODE_RGMII) > + return;
It would be good to add a comment here which states that the port comes out of reset with values good for RGMII.
Also, there's a built-in assumption in this patch, that dynamically switching between RGMII and TRGMII is not possible. This is because phylink mac_config() is not necesarily called only once immediately after reset, but after each major_config().
The fact that the driver sets both PHY_INTERFACE_MODE_RGMII and PHY_INTERFACE_MODE_TRGMII at once in config->supported_interfaces does not disprove that dynamic reconfiguration is possible. Normally, interfaces for which it doesn't make sense to dynamically reconfigure (they are wired to fixed pinout) have a single bit set in supported_interfaces. Is this switching something that makes any sense at all, given that port 6 is internal? It's not something that phylink knows to do today, but if this is theoretically possible and remotely useful, someone might end up wanting, in the future, to revert this patch.
| |