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.
@@ -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);
@@ -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.