From: Daniel Golle <daniel@makrotopia.org>
To: arinc.unal@arinc9.com
Cc: DENG Qingfang <dqfext@gmail.com>,
Sean Wang <sean.wang@mediatek.com>, Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Matthias Brugger <matthias.bgg@gmail.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com>,
Bartel Eerdekens <bartel.eerdekens@constell8.be>,
mithat.guner@xeront.com, erkin.bozoglu@xeront.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org
Subject: Re: [PATCH net-next 1/2] net: dsa: mt7530-mdio: read PHY address of switch from device tree
Date: Sun, 14 Apr 2024 04:10:16 +0100 [thread overview]
Message-ID: <ZhtJGEdjeS3LfMWF@makrotopia.org> (raw)
In-Reply-To: <20240414-b4-for-netnext-mt7530-phy-addr-from-dt-and-simplify-core-ops-v1-1-b08936df2770@arinc9.com>
On Sun, Apr 14, 2024 at 01:08:19AM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.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@arinc9.com>
See minor nit inline below.
Reviewed-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Daniel Golle <daniel@makrotopia.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
>
>
next prev parent reply other threads:[~2024-04-14 3:10 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-13 22:08 [PATCH net-next 0/2] Read PHY address of switch from device tree on MT7530 DSA subdriver Arınç ÜNAL via B4 Relay
2024-04-13 22:08 ` [PATCH net-next 1/2] net: dsa: mt7530-mdio: read PHY address of switch from device tree Arınç ÜNAL via B4 Relay
2024-04-14 3:10 ` Daniel Golle [this message]
2024-04-14 5:59 ` Arınç ÜNAL
2024-04-14 8:06 ` Christophe JAILLET
2024-04-13 22:08 ` [PATCH net-next 2/2] net: dsa: mt7530: simplify core operations Arınç ÜNAL via B4 Relay
2024-04-14 3:22 ` Daniel Golle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZhtJGEdjeS3LfMWF@makrotopia.org \
--to=daniel@makrotopia.org \
--cc=andrew@lunn.ch \
--cc=angelogioacchino.delregno@collabora.com \
--cc=arinc.unal@arinc9.com \
--cc=bartel.eerdekens@constell8.be \
--cc=davem@davemloft.net \
--cc=dqfext@gmail.com \
--cc=edumazet@google.com \
--cc=erkin.bozoglu@xeront.com \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=mithat.guner@xeront.com \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=sean.wang@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).