[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZhtJGEdjeS3LfMWF@makrotopia.org>
Date: Sun, 14 Apr 2024 04:10:16 +0100
From: Daniel Golle <daniel@...rotopia.org>
To: arinc.unal@...nc9.com
Cc: DENG Qingfang <dqfext@...il.com>, Sean Wang <sean.wang@...iatek.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>,
Bartel Eerdekens <bartel.eerdekens@...stell8.be>,
mithat.guner@...ont.com, 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: [PATCH net-next 1/2] net: dsa: mt7530-mdio: read PHY address of
switch from device tree
On Sun, Apr 14, 2024 at 01:08:19AM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@...nc9.com>
>
> Read the PHY address the switch listens on from the reg property of the
> switch node on the device tree. This change brings support for MT7530
> switches on boards with such bootstrapping configuration where the switch
> listens on a different PHY address than the hardcoded PHY address on the
> driver, 31.
>
> As described on the "MT7621 Programming Guide v0.4" document, the MT7530
> switch and its PHYs can be configured to listen on the range of 7-12,
> 15-20, 23-28, and 31 and 0-4 PHY addresses.
>
> There are operations where the switch PHY registers are used. For the PHY
> address of the control PHY, transform the MT753X_CTRL_PHY_ADDR constant
> into a macro and use it. The PHY address for the control PHY is 0 when the
> switch listens on 31. In any other case, it is one greater than the PHY
> address the switch listens on.
>
> Signed-off-by: Arınç ÜNAL <arinc.unal@...nc9.com>
See minor nit inline below.
Reviewed-by: Daniel Golle <daniel@...rotopia.org>
Tested-by: Daniel Golle <daniel@...rotopia.org>
(only MT7530 and MT7531 on address 0x1f has been tested, I don't have
any board configuring the switch to any other base-address)
> ---
> drivers/net/dsa/mt7530-mdio.c | 28 ++++++++++++++--------------
> drivers/net/dsa/mt7530.c | 35 ++++++++++++++++++++++-------------
> drivers/net/dsa/mt7530.h | 4 +++-
> 3 files changed, 39 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
> index fa3ee85a99c1..599b28c8a340 100644
> --- a/drivers/net/dsa/mt7530-mdio.c
> +++ b/drivers/net/dsa/mt7530-mdio.c
> @@ -18,7 +18,8 @@
> static int
> mt7530_regmap_write(void *context, unsigned int reg, unsigned int val)
> {
> - struct mii_bus *bus = context;
> + struct mt7530_priv *priv = context;
> + struct mii_bus *bus = priv->bus;
> u16 page, r, lo, hi;
> int ret;
>
> @@ -27,36 +28,35 @@ mt7530_regmap_write(void *context, unsigned int reg, unsigned int val)
> lo = val & 0xffff;
> hi = val >> 16;
>
> - /* MT7530 uses 31 as the pseudo port */
> - ret = bus->write(bus, 0x1f, 0x1f, page);
> + ret = bus->write(bus, priv->phy_addr, 0x1f, page);
> if (ret < 0)
> return ret;
>
> - ret = bus->write(bus, 0x1f, r, lo);
> + ret = bus->write(bus, priv->phy_addr, r, lo);
> if (ret < 0)
> return ret;
>
> - ret = bus->write(bus, 0x1f, 0x10, hi);
> + ret = bus->write(bus, priv->phy_addr, 0x10, hi);
> return ret;
> }
>
> static int
> mt7530_regmap_read(void *context, unsigned int reg, unsigned int *val)
> {
> - struct mii_bus *bus = context;
> + struct mt7530_priv *priv = context;
> + struct mii_bus *bus = priv->bus;
> u16 page, r, lo, hi;
> int ret;
>
> page = (reg >> 6) & 0x3ff;
> r = (reg >> 2) & 0xf;
>
> - /* MT7530 uses 31 as the pseudo port */
> - ret = bus->write(bus, 0x1f, 0x1f, page);
> + ret = bus->write(bus, priv->phy_addr, 0x1f, page);
> if (ret < 0)
> return ret;
>
> - lo = bus->read(bus, 0x1f, r);
> - hi = bus->read(bus, 0x1f, 0x10);
> + lo = bus->read(bus, priv->phy_addr, r);
> + hi = bus->read(bus, priv->phy_addr, 0x10);
>
> *val = (hi << 16) | (lo & 0xffff);
>
> @@ -107,8 +107,7 @@ mt7531_create_sgmii(struct mt7530_priv *priv)
> mt7531_pcs_config[i]->unlock = mt7530_mdio_regmap_unlock;
> mt7531_pcs_config[i]->lock_arg = &priv->bus->mdio_lock;
>
> - regmap = devm_regmap_init(priv->dev,
> - &mt7530_regmap_bus, priv->bus,
> + regmap = devm_regmap_init(priv->dev, &mt7530_regmap_bus, priv,
> mt7531_pcs_config[i]);
> if (IS_ERR(regmap)) {
> ret = PTR_ERR(regmap);
> @@ -153,6 +152,7 @@ mt7530_probe(struct mdio_device *mdiodev)
>
> priv->bus = mdiodev->bus;
> priv->dev = &mdiodev->dev;
> + priv->phy_addr = mdiodev->addr;
>
> ret = mt7530_probe_common(priv);
> if (ret)
> @@ -203,8 +203,8 @@ mt7530_probe(struct mdio_device *mdiodev)
> regmap_config->reg_stride = 4;
> regmap_config->max_register = MT7530_CREV;
> regmap_config->disable_locking = true;
> - priv->regmap = devm_regmap_init(priv->dev, &mt7530_regmap_bus,
> - priv->bus, regmap_config);
> + priv->regmap = devm_regmap_init(priv->dev, &mt7530_regmap_bus, priv,
> + regmap_config);
> if (IS_ERR(priv->regmap))
> return PTR_ERR(priv->regmap);
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index c0d0bce0b594..fefa6dd151fa 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -86,22 +86,26 @@ core_read_mmd_indirect(struct mt7530_priv *priv, int prtad, int devad)
> int value, ret;
>
> /* Write the desired MMD Devad */
> - ret = bus->write(bus, 0, MII_MMD_CTRL, devad);
> + ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MII_MMD_CTRL, devad);
> if (ret < 0)
> goto err;
>
> /* Write the desired MMD register address */
> - ret = bus->write(bus, 0, MII_MMD_DATA, prtad);
> + ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MII_MMD_DATA, prtad);
> if (ret < 0)
> goto err;
>
> /* Select the Function : DATA with no post increment */
> - ret = bus->write(bus, 0, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
> + ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MII_MMD_CTRL, devad | MII_MMD_CTRL_NOINCR);
> if (ret < 0)
> goto err;
>
> /* Read the content of the MMD's selected register */
> - value = bus->read(bus, 0, MII_MMD_DATA);
> + value = bus->read(bus, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MII_MMD_DATA);
>
> return value;
> err:
> @@ -118,22 +122,26 @@ core_write_mmd_indirect(struct mt7530_priv *priv, int prtad,
> int ret;
>
> /* Write the desired MMD Devad */
> - ret = bus->write(bus, 0, MII_MMD_CTRL, devad);
> + ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MII_MMD_CTRL, devad);
> if (ret < 0)
> goto err;
>
> /* Write the desired MMD register address */
> - ret = bus->write(bus, 0, MII_MMD_DATA, prtad);
> + ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MII_MMD_DATA, prtad);
> if (ret < 0)
> goto err;
>
> /* Select the Function : DATA with no post increment */
> - ret = bus->write(bus, 0, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
> + ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MII_MMD_CTRL, devad | MII_MMD_CTRL_NOINCR);
> if (ret < 0)
> goto err;
>
> /* Write the data into MMD's selected register */
> - ret = bus->write(bus, 0, MII_MMD_DATA, data);
> + ret = bus->write(bus, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MII_MMD_DATA, data);
> err:
> if (ret < 0)
> dev_err(&bus->dev,
> @@ -2671,16 +2679,17 @@ mt7531_setup(struct dsa_switch *ds)
> * phy_[read,write]_mmd_indirect is called, we provide our own
> * mt7531_ind_mmd_phy_[read,write] to complete this function.
> */
> - val = mt7531_ind_c45_phy_read(priv, MT753X_CTRL_PHY_ADDR,
> + val = mt7531_ind_c45_phy_read(priv,
> + MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> MDIO_MMD_VEND2, CORE_PLL_GROUP4);
> val |= MT7531_RG_SYSPLL_DMY2 | MT7531_PHY_PLL_BYPASS_MODE;
> val &= ~MT7531_PHY_PLL_OFF;
> - mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR, MDIO_MMD_VEND2,
> - CORE_PLL_GROUP4, val);
> + mt7531_ind_c45_phy_write(priv, MT753X_CTRL_PHY_ADDR(priv->phy_addr),
> + MDIO_MMD_VEND2, CORE_PLL_GROUP4, val);
>
> /* Disable EEE advertisement on the switch PHYs. */
> - for (i = MT753X_CTRL_PHY_ADDR;
> - i < MT753X_CTRL_PHY_ADDR + MT7530_NUM_PHYS; i++) {
> + for (i = MT753X_CTRL_PHY_ADDR(priv->phy_addr);
> + i < MT753X_CTRL_PHY_ADDR(priv->phy_addr) + MT7530_NUM_PHYS; i++) {
> mt7531_ind_c45_phy_write(priv, i, MDIO_MMD_AN, MDIO_AN_EEE_ADV,
> 0);
> }
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 585db03c0548..76adc9d21bcf 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -625,7 +625,7 @@ enum mt7531_clk_skew {
> #define MT7531_PHY_PLL_OFF BIT(5)
> #define MT7531_PHY_PLL_BYPASS_MODE BIT(4)
>
> -#define MT753X_CTRL_PHY_ADDR 0
> +#define MT753X_CTRL_PHY_ADDR(phy_addr) (phy_addr == 0x1f ? 0 : phy_addr + 1)
#define MT753X_CTRL_PHY_ADDR(phy_addr) (phy_addr + 1 & 0x1f)
It's shorter, and works without conditional operator which is expensive.
>
> #define CORE_PLL_GROUP5 0x404
> #define RG_LCDDS_PCW_NCPO1(x) ((x) & 0xffff)
> @@ -774,6 +774,7 @@ struct mt753x_info {
> * @irq_enable: IRQ enable bits, synced to SYS_INT_EN
> * @create_sgmii: Pointer to function creating SGMII PCS instance(s)
> * @active_cpu_ports: Holding the active CPU ports
> + * @phy_addr: Holding the PHY address the switch listens on
> */
> struct mt7530_priv {
> struct device *dev;
> @@ -800,6 +801,7 @@ struct mt7530_priv {
> u32 irq_enable;
> int (*create_sgmii)(struct mt7530_priv *priv);
> u8 active_cpu_ports;
> + int phy_addr;
> };
>
> struct mt7530_hw_vlan_entry {
>
> --
> 2.40.1
>
>
Powered by blists - more mailing lists