diff mbox series

[net-next] net: ethernet: rtsn: Add MDIO read/write support for C45

Message ID 20240903171853.631343-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series [net-next] net: ethernet: rtsn: Add MDIO read/write support for C45 | expand

Commit Message

Niklas Söderlund Sept. 3, 2024, 5:18 p.m. UTC
Add C45 specific read and write implementations to support C45 PHY
access using SIOCGMIIREG and SIOCSMIIREG IOCTLs.

While the core can handle a C45 PHY using only the MDIO bus C22 read()
and write() callbacks there are PHY interactions that are not possible
without them. One use-case is accessing PHY registers using the
SIOCGMIIREG and SIOCSMIIREG IOCTLs. Without these callbacks trying to
access C45 PHY registers using these IOCTLs result in -EOPNOTSUPP.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/net/ethernet/renesas/rtsn.c | 40 +++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Andrew Lunn Sept. 3, 2024, 7:28 p.m. UTC | #1
On Tue, Sep 03, 2024 at 07:18:53PM +0200, Niklas Söderlund wrote:
> Add C45 specific read and write implementations to support C45 PHY
> access using SIOCGMIIREG and SIOCSMIIREG IOCTLs.
> 
> While the core can handle a C45 PHY using only the MDIO bus C22 read()
> and write() callbacks there are PHY interactions that are not possible
> without them. One use-case is accessing PHY registers using the
> SIOCGMIIREG and SIOCSMIIREG IOCTLs. Without these callbacks trying to
> access C45 PHY registers using these IOCTLs result in -EOPNOTSUPP.

This is the wrong solution. You should not be reproducing
functionality which phylib already has.

Please extend the IOCTL code to do what the core does:

int __phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
{
        if (regnum > (u16)~0 || devad > 32)
                return -EINVAL;

        if (phydev->drv && phydev->drv->read_mmd)
                return phydev->drv->read_mmd(phydev, devad, regnum);

        return mmd_phy_read(phydev->mdio.bus, phydev->mdio.addr,
                            phydev->is_c45, devad, regnum);
}

mmd_phy_read() will fall back to indirect access if need be.

Just watch out for mii_data->phy_id and what it implies.

    Andrew

---
pw-bot: cr
Yoshihiro Shimoda Sept. 4, 2024, 1:48 a.m. UTC | #2
Hi Niklas-san,

> From: Niklas Söderlund, Sent: Wednesday, September 4, 2024 2:19 AM
> 
> Add C45 specific read and write implementations to support C45 PHY
> access using SIOCGMIIREG and SIOCSMIIREG IOCTLs.
> 
> While the core can handle a C45 PHY using only the MDIO bus C22 read()
> and write() callbacks there are PHY interactions that are not possible
> without them. One use-case is accessing PHY registers using the
> SIOCGMIIREG and SIOCSMIIREG IOCTLs. Without these callbacks trying to
> access C45 PHY registers using these IOCTLs result in -EOPNOTSUPP.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

And, I tested this on my environment (R-Car V4H White Hawk Single board),
and then it worked correctly. So,

Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda


>  drivers/net/ethernet/renesas/rtsn.c | 40 +++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/net/ethernet/renesas/rtsn.c b/drivers/net/ethernet/renesas/rtsn.c
> index 0e6cea42f007..8d6ffaa13e5b 100644
> --- a/drivers/net/ethernet/renesas/rtsn.c
> +++ b/drivers/net/ethernet/renesas/rtsn.c
> @@ -771,6 +771,32 @@ static int rtsn_mii_access(struct mii_bus *bus, bool read, int phyad,
>  	return ret;
>  }
> 
> +static int rtsn_mii_access_indirect(struct mii_bus *bus, bool read, int phyad,
> +				    int devnum, int regnum, u16 data)
> +{
> +	int ret;
> +
> +	ret = rtsn_mii_access(bus, false, phyad, MII_MMD_CTRL, devnum);
> +	if (ret)
> +		return ret;
> +
> +	ret = rtsn_mii_access(bus, false, phyad, MII_MMD_DATA, regnum);
> +	if (ret)
> +		return ret;
> +
> +	ret = rtsn_mii_access(bus, false, phyad, MII_MMD_CTRL,
> +			      devnum | MII_MMD_CTRL_NOINCR);
> +	if (ret)
> +		return ret;
> +
> +	if (read)
> +		ret = rtsn_mii_access(bus, true, phyad, MII_MMD_DATA, 0);
> +	else
> +		ret = rtsn_mii_access(bus, false, phyad, MII_MMD_DATA, data);
> +
> +	return ret;
> +}
> +
>  static int rtsn_mii_read(struct mii_bus *bus, int addr, int regnum)
>  {
>  	return rtsn_mii_access(bus, true, addr, regnum, 0);
> @@ -781,6 +807,18 @@ static int rtsn_mii_write(struct mii_bus *bus, int addr, int regnum, u16 val)
>  	return rtsn_mii_access(bus, false, addr, regnum, val);
>  }
> 
> +static int rtsn_mii_read_c45(struct mii_bus *bus, int addr, int devnum,
> +			     int regnum)
> +{
> +	return rtsn_mii_access_indirect(bus, true, addr, devnum, regnum, 0);
> +}
> +
> +static int rtsn_mii_write_c45(struct mii_bus *bus, int addr, int devnum,
> +			      int regnum, u16 val)
> +{
> +	return rtsn_mii_access_indirect(bus, false, addr, devnum, regnum, val);
> +}
> +
>  static int rtsn_mdio_alloc(struct rtsn_private *priv)
>  {
>  	struct platform_device *pdev = priv->pdev;
> @@ -818,6 +856,8 @@ static int rtsn_mdio_alloc(struct rtsn_private *priv)
>  	mii->priv = priv;
>  	mii->read = rtsn_mii_read;
>  	mii->write = rtsn_mii_write;
> +	mii->read_c45 = rtsn_mii_read_c45;
> +	mii->write_c45 = rtsn_mii_write_c45;
>  	mii->parent = dev;
> 
>  	ret = of_mdiobus_register(mii, mdio_node);
> --
> 2.46.0
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/renesas/rtsn.c b/drivers/net/ethernet/renesas/rtsn.c
index 0e6cea42f007..8d6ffaa13e5b 100644
--- a/drivers/net/ethernet/renesas/rtsn.c
+++ b/drivers/net/ethernet/renesas/rtsn.c
@@ -771,6 +771,32 @@  static int rtsn_mii_access(struct mii_bus *bus, bool read, int phyad,
 	return ret;
 }
 
+static int rtsn_mii_access_indirect(struct mii_bus *bus, bool read, int phyad,
+				    int devnum, int regnum, u16 data)
+{
+	int ret;
+
+	ret = rtsn_mii_access(bus, false, phyad, MII_MMD_CTRL, devnum);
+	if (ret)
+		return ret;
+
+	ret = rtsn_mii_access(bus, false, phyad, MII_MMD_DATA, regnum);
+	if (ret)
+		return ret;
+
+	ret = rtsn_mii_access(bus, false, phyad, MII_MMD_CTRL,
+			      devnum | MII_MMD_CTRL_NOINCR);
+	if (ret)
+		return ret;
+
+	if (read)
+		ret = rtsn_mii_access(bus, true, phyad, MII_MMD_DATA, 0);
+	else
+		ret = rtsn_mii_access(bus, false, phyad, MII_MMD_DATA, data);
+
+	return ret;
+}
+
 static int rtsn_mii_read(struct mii_bus *bus, int addr, int regnum)
 {
 	return rtsn_mii_access(bus, true, addr, regnum, 0);
@@ -781,6 +807,18 @@  static int rtsn_mii_write(struct mii_bus *bus, int addr, int regnum, u16 val)
 	return rtsn_mii_access(bus, false, addr, regnum, val);
 }
 
+static int rtsn_mii_read_c45(struct mii_bus *bus, int addr, int devnum,
+			     int regnum)
+{
+	return rtsn_mii_access_indirect(bus, true, addr, devnum, regnum, 0);
+}
+
+static int rtsn_mii_write_c45(struct mii_bus *bus, int addr, int devnum,
+			      int regnum, u16 val)
+{
+	return rtsn_mii_access_indirect(bus, false, addr, devnum, regnum, val);
+}
+
 static int rtsn_mdio_alloc(struct rtsn_private *priv)
 {
 	struct platform_device *pdev = priv->pdev;
@@ -818,6 +856,8 @@  static int rtsn_mdio_alloc(struct rtsn_private *priv)
 	mii->priv = priv;
 	mii->read = rtsn_mii_read;
 	mii->write = rtsn_mii_write;
+	mii->read_c45 = rtsn_mii_read_c45;
+	mii->write_c45 = rtsn_mii_write_c45;
 	mii->parent = dev;
 
 	ret = of_mdiobus_register(mii, mdio_node);