[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZELc6MjOicjsPGGb@makrotopia.org>
Date: Fri, 21 Apr 2023 19:58:48 +0100
From: Daniel Golle <daniel@...rotopia.org>
To: arinc9.unal@...il.com
Cc: Sean Wang <sean.wang@...iatek.com>,
Landen Chao <Landen.Chao@...iatek.com>,
DENG Qingfang <dqfext@...il.com>, Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
Russell King <linux@...linux.org.uk>,
Arınç ÜNAL <arinc.unal@...nc9.com>,
Richard van Schagen <richard@...terhints.com>,
Richard van Schagen <vschagen@...com>,
Frank Wunderlich <frank-w@...lic-files.de>,
erkin.bozoglu@...ont.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [RFC PATCH net-next 21/22] net: dsa: mt7530: get rid of useless
error returns on phylink code path
On Fri, Apr 21, 2023 at 05:36:47PM +0300, arinc9.unal@...il.com wrote:
> From: Arınç ÜNAL <arinc.unal@...nc9.com>
>
> Remove error returns on the cases where they are already handled with the
> function the mac_port_get_caps member points to.
>
> mt7531_mac_config() is also called from mt7531_cpu_port_config() outside of
> phylink but the port and interface modes are already handled there.
>
> Change the functions and the mac_port_config function pointer to void now
> that there're no error returns anymore.
>
> Remove mt753x_is_mac_port() that used to help the said error returns.
>
> On mt7531_mac_config(), switch to if statements to simplify the code.
>
> Remove internal phy cases from mt753x_phylink_mac_config() as there is no
> configuration to be done for them. There's also no need to check the
> interface mode as that's already handled with the function the
> mac_port_get_caps member points to.
>
> Tested-by: Arınç ÜNAL <arinc.unal@...nc9.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@...nc9.com>
Acked-by: Daniel Golle <daniel@...rotopia.org>
Tested-by: Daniel Golle <daniel@...rotopia.org>
(on BPi-R3 MT7986A+MT7531AE, BPi-R64 MT7622+MT7531BE and MT7988A rfb)
> ---
> drivers/net/dsa/mt7530.c | 81 ++++++++--------------------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 17 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 8ece3d0d820c..3d19e06061cb 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2556,7 +2556,7 @@ static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
> }
> }
>
> -static int
> +static void
> mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> phy_interface_t interface)
> {
> @@ -2567,22 +2567,14 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> } else if (port == 6) {
> mt7530_setup_port6(priv->ds, interface);
> }
> -
> - return 0;
> }
>
> -static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
> - phy_interface_t interface,
> - struct phy_device *phydev)
> +static void mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
> + phy_interface_t interface,
> + struct phy_device *phydev)
> {
> u32 val;
>
> - if (priv->p5_sgmii) {
> - dev_err(priv->dev, "RGMII mode is not available for port %d\n",
> - port);
> - return -EINVAL;
> - }
> -
> val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
> val |= GP_CLK_EN;
> val &= ~GP_MODE_MASK;
> @@ -2610,20 +2602,14 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
> case PHY_INTERFACE_MODE_RGMII_ID:
> break;
> default:
> - return -EINVAL;
> + break;
> }
> }
> - mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
>
> - return 0;
> -}
> -
> -static bool mt753x_is_mac_port(u32 port)
> -{
> - return (port == 5 || port == 6);
> + mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
> }
>
> -static int
> +static void
> mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> phy_interface_t interface)
> {
> @@ -2631,42 +2617,21 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> struct phy_device *phydev;
> struct dsa_port *dp;
>
> - if (!mt753x_is_mac_port(port)) {
> - dev_err(priv->dev, "port %d is not a MAC port\n", port);
> - return -EINVAL;
> - }
> -
> - switch (interface) {
> - case PHY_INTERFACE_MODE_RGMII:
> - case PHY_INTERFACE_MODE_RGMII_ID:
> - case PHY_INTERFACE_MODE_RGMII_RXID:
> - case PHY_INTERFACE_MODE_RGMII_TXID:
> + if (phy_interface_mode_is_rgmii(interface)) {
> dp = dsa_to_port(ds, port);
> phydev = dp->slave->phydev;
> - return mt7531_rgmii_setup(priv, port, interface, phydev);
> - case PHY_INTERFACE_MODE_SGMII:
> - case PHY_INTERFACE_MODE_NA:
> - case PHY_INTERFACE_MODE_1000BASEX:
> - case PHY_INTERFACE_MODE_2500BASEX:
> - /* handled in SGMII PCS driver */
> - return 0;
> - default:
> - return -EINVAL;
> + mt7531_rgmii_setup(priv, port, interface, phydev);
> }
> -
> - return -EINVAL;
> }
>
> -static int
> +static void
> mt753x_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> const struct phylink_link_state *state)
> {
> struct mt7530_priv *priv = ds->priv;
>
> - if (!priv->info->mac_port_config)
> - return 0;
> -
> - return priv->info->mac_port_config(ds, port, mode, state->interface);
> + if (priv->info->mac_port_config)
> + priv->info->mac_port_config(ds, port, mode, state->interface);
> }
>
> static struct phylink_pcs *
> @@ -2695,30 +2660,18 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> u32 mcr_cur, mcr_new;
>
> switch (port) {
> - case 0 ... 4: /* Internal phy */
> - if (state->interface != PHY_INTERFACE_MODE_GMII &&
> - state->interface != PHY_INTERFACE_MODE_INTERNAL)
> - goto unsupported;
> - break;
> case 5: /* Port 5, can be used as a CPU port. */
> if (priv->p5_configured)
> break;
>
> - if (mt753x_mac_config(ds, port, mode, state) < 0)
> - goto unsupported;
> + mt753x_mac_config(ds, port, mode, state);
> break;
> case 6: /* Port 6, can be used as a CPU port. */
> if (priv->p6_configured)
> break;
>
> - if (mt753x_mac_config(ds, port, mode, state) < 0)
> - goto unsupported;
> + mt753x_mac_config(ds, port, mode, state);
> break;
> - default:
> -unsupported:
> - dev_err(ds->dev, "%s: unsupported %s port: %i\n",
> - __func__, phy_modes(state->interface), port);
> - return;
> }
>
> mcr_cur = mt7530_read(priv, MT7530_PMCR_P(port));
> @@ -2811,7 +2764,6 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
> struct mt7530_priv *priv = ds->priv;
> phy_interface_t interface;
> int speed;
> - int ret;
>
> switch (port) {
> case 5:
> @@ -2836,9 +2788,8 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
> else
> speed = SPEED_1000;
>
> - ret = mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
> - if (ret)
> - return ret;
> + mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
> +
> mt7530_write(priv, MT7530_PMCR_P(port),
> PMCR_CPU_PORT_SETTING(priv->id));
> mt753x_phylink_pcs_link_up(&priv->pcs[port].pcs, MLO_AN_FIXED,
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index cad9115de22b..ee2b3d2d6258 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -722,7 +722,7 @@ struct mt753x_info {
> void (*mac_port_validate)(struct dsa_switch *ds, int port,
> phy_interface_t interface,
> unsigned long *supported);
> - int (*mac_port_config)(struct dsa_switch *ds, int port,
> + void (*mac_port_config)(struct dsa_switch *ds, int port,
> unsigned int mode,
> phy_interface_t interface);
> };
> --
> 2.37.2
>
Powered by blists - more mailing lists