| Message ID | 20240903171853.631343-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: ethernet: rtsn: Add MDIO read/write support for C45 | expand |
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
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 --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);
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(+)