| Message ID | 20231126235141.17996-3-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2,1/4] net: phy: extend PHY package API to support multiple global address | expand |
On Mon, Nov 27, 2023 at 12:51:40AM +0100, Christian Marangi wrote: > Restructure phy_write_mmd and phy_read_mmd to implement generic helper > for direct mdiobus access for mmd and use these helper for phydev user. > > This is needed in preparation of PHY package API that requires generic > access to the mdiobus and are deatched from phydev struct but instead > access them based on PHY package base_addr and offsets. Why is this all going into the header file? Andrew
On Tue, Nov 28, 2023 at 01:46:10AM +0100, Andrew Lunn wrote: > On Mon, Nov 27, 2023 at 12:51:40AM +0100, Christian Marangi wrote: > > Restructure phy_write_mmd and phy_read_mmd to implement generic helper > > for direct mdiobus access for mmd and use these helper for phydev user. > > > > This is needed in preparation of PHY package API that requires generic > > access to the mdiobus and are deatched from phydev struct but instead > > access them based on PHY package base_addr and offsets. > > Why is this all going into the header file? > Was following the pattern done by phy_package_read/write. Considering those API are not single function call... I wonder if those should be moved in phy_core.c instead of static inline them in the header. What do you think?
On Tue, Nov 28, 2023 at 01:11:00PM +0100, Christian Marangi wrote: > On Tue, Nov 28, 2023 at 01:46:10AM +0100, Andrew Lunn wrote: > > On Mon, Nov 27, 2023 at 12:51:40AM +0100, Christian Marangi wrote: > > > Restructure phy_write_mmd and phy_read_mmd to implement generic helper > > > for direct mdiobus access for mmd and use these helper for phydev user. > > > > > > This is needed in preparation of PHY package API that requires generic > > > access to the mdiobus and are deatched from phydev struct but instead > > > access them based on PHY package base_addr and offsets. > > > > Why is this all going into the header file? > > > > Was following the pattern done by phy_package_read/write. > > Considering those API are not single function call... I wonder if those > should be moved in phy_core.c instead of static inline them in the > header. phy_package_{read,write} are simple affairs - one test and a call to a function. That makes them fairly small. The proposed new functions aren't small, which means that we get a load of code each time they're used. Therefore, it's better that they're out of line.
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c index b4f80847eefd..65d312889f3b 100644 --- a/drivers/net/phy/phy-core.c +++ b/drivers/net/phy/phy-core.c @@ -537,26 +537,14 @@ int phy_speed_down_core(struct phy_device *phydev) */ int __phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum) { - int val; - if (regnum > (u16)~0 || devad > 32) return -EINVAL; - if (phydev->drv && phydev->drv->read_mmd) { - val = phydev->drv->read_mmd(phydev, devad, regnum); - } else if (phydev->is_c45) { - val = __mdiobus_c45_read(phydev->mdio.bus, phydev->mdio.addr, - devad, regnum); - } else { - struct mii_bus *bus = phydev->mdio.bus; - int phy_addr = phydev->mdio.addr; - - mmd_phy_indirect(bus, phy_addr, devad, regnum); + if (phydev->drv && phydev->drv->read_mmd) + return phydev->drv->read_mmd(phydev, devad, regnum); - /* Read the content of the MMD's selected register */ - val = __mdiobus_read(bus, phy_addr, MII_MMD_DATA); - } - return val; + return mmd_phy_read(phydev->mdio.bus, phydev->mdio.addr, + phydev->is_c45, devad, regnum); } EXPORT_SYMBOL(__phy_read_mmd); @@ -593,28 +581,14 @@ EXPORT_SYMBOL(phy_read_mmd); */ int __phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val) { - int ret; - if (regnum > (u16)~0 || devad > 32) return -EINVAL; - if (phydev->drv && phydev->drv->write_mmd) { - ret = phydev->drv->write_mmd(phydev, devad, regnum, val); - } else if (phydev->is_c45) { - ret = __mdiobus_c45_write(phydev->mdio.bus, phydev->mdio.addr, - devad, regnum, val); - } else { - struct mii_bus *bus = phydev->mdio.bus; - int phy_addr = phydev->mdio.addr; - - mmd_phy_indirect(bus, phy_addr, devad, regnum); - - /* Write the data into MMD's selected register */ - __mdiobus_write(bus, phy_addr, MII_MMD_DATA, val); + if (phydev->drv && phydev->drv->write_mmd) + return phydev->drv->write_mmd(phydev, devad, regnum, val); - ret = 0; - } - return ret; + return mmd_phy_write(phydev->mdio.bus, phydev->mdio.addr, + phydev->is_c45, devad, regnum, val); } EXPORT_SYMBOL(__phy_write_mmd); diff --git a/include/linux/phy.h b/include/linux/phy.h index a63d45880b1a..96f6f34be051 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1330,6 +1330,28 @@ static inline void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad devad | MII_MMD_CTRL_NOINCR); } +static inline int mmd_phy_read(struct mii_bus *bus, int phy_addr, bool is_c45, + int devad, u32 regnum) +{ + if (is_c45) + return __mdiobus_c45_read(bus, phy_addr, devad, regnum); + + mmd_phy_indirect(bus, phy_addr, devad, regnum); + /* Read the content of the MMD's selected register */ + return __mdiobus_read(bus, phy_addr, MII_MMD_DATA); +} + +static inline int mmd_phy_write(struct mii_bus *bus, int phy_addr, bool is_c45, + int devad, u32 regnum, u16 val) +{ + if (is_c45) + return __mdiobus_c45_write(bus, phy_addr, devad, regnum, val); + + mmd_phy_indirect(bus, phy_addr, devad, regnum); + /* Write the data into MMD's selected register */ + return __mdiobus_write(bus, phy_addr, MII_MMD_DATA, val); +} + /* * phy_read_mmd - Convenience function for reading a register * from an MMD on a given PHY.
Restructure phy_write_mmd and phy_read_mmd to implement generic helper for direct mdiobus access for mmd and use these helper for phydev user. This is needed in preparation of PHY package API that requires generic access to the mdiobus and are deatched from phydev struct but instead access them based on PHY package base_addr and offsets. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- Changes v2: - Introduce this patch drivers/net/phy/phy-core.c | 42 ++++++++------------------------------ include/linux/phy.h | 22 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 34 deletions(-)