Messages in this thread | | | Date | Wed, 24 May 2023 21:15:12 +0300 | From | Vladimir Oltean <> | Subject | Re: [PATCH net-next 12/30] net: dsa: mt7530: move XTAL check to mt7530_setup() |
| |
On Mon, May 22, 2023 at 03:15:14PM +0300, arinc9.unal@gmail.com wrote: > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > The crystal frequency concerns the switch core. The frequency should be > checked when the switch is being set up so the driver can reject the > unsupported hardware earlier and without requiring port 6 to be used. > > Move it to mt7530_setup(). > > Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com> > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > ---
Do you know why a crystal frequency of 20 MHz is not supported?
> drivers/net/dsa/mt7530.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c > index 049f7be0d790..fa48273269c4 100644 > --- a/drivers/net/dsa/mt7530.c > +++ b/drivers/net/dsa/mt7530.c > @@ -408,13 +408,6 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface) > > xtal = mt7530_read(priv, MT7530_HWTRAP) & HWTRAP_XTAL_MASK; > > - if (xtal == HWTRAP_XTAL_20MHZ) { > - dev_err(priv->dev, > - "%s: MT7530 with a 20MHz XTAL is not supported!\n", > - __func__); > - return -EINVAL; > - } > - > switch (interface) { > case PHY_INTERFACE_MODE_RGMII: > trgint = 0; > @@ -2133,7 +2126,7 @@ mt7530_setup(struct dsa_switch *ds) > struct mt7530_dummy_poll p; > phy_interface_t interface; > struct dsa_port *cpu_dp; > - u32 id, val; > + u32 id, val, xtal; > int ret, i; > > /* The parent node of master netdev which holds the common system > @@ -2203,6 +2196,15 @@ mt7530_setup(struct dsa_switch *ds) > return -ENODEV; > } > > + xtal = mt7530_read(priv, MT7530_HWTRAP) & HWTRAP_XTAL_MASK; > + > + if (xtal == HWTRAP_XTAL_20MHZ) { > + dev_err(priv->dev, > + "%s: MT7530 with a 20MHz XTAL is not supported!\n", > + __func__);
I don't think __func__ brings much value here, it could be dropped in the process of moving the code.
Also, the HWTRAP register is already read once, here (stored in "val"):
INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_HWTRAP); ret = readx_poll_timeout(_mt7530_read, &p, val, val != 0, 20, 1000000);
I wonder if we really need to read it twice.
> + return -EINVAL; > + } > + > /* Reset the switch through internal reset */ > mt7530_write(priv, MT7530_SYS_CTRL, > SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST | > -- > 2.39.2 >
| |