lkml.org 
[lkml]   [2023]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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
    >

    \
     
     \ /
      Last update: 2023-05-24 20:16    [W:4.139 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site