Messages in this thread | | | Date | Wed, 19 Jul 2023 09:21:57 +0200 | From | Michael Walle <> | Subject | Re: [PATCH net-next v3 07/11] net: phy: introduce phy_mdiobus_read_mmd() |
| |
Am 2023-07-19 01:54, schrieb Andrew Lunn: >> +static int __phy_mdiobus_read_mmd(struct mii_bus *bus, int phy_addr, >> + enum phy_access_mode access_mode, >> + int devad, u32 regnum) >> +{ >> + switch (access_mode) { >> + case PHY_ACCESS_C45: >> + return __mdiobus_c45_read(bus, phy_addr, devad, regnum); >> + case PHY_ACCESS_C22: >> + /* ignore return value for legacy reasons */ >> + mmd_phy_indirect(bus, phy_addr, devad, regnum, false); >> + >> + /* Read the content of the MMD's selected register */ >> + return __mdiobus_read(bus, phy_addr, MII_MMD_DATA); >> + default: >> + return -EOPNOTSUPP; >> + } > > So this is reading a C45 register space register, otherwise it would > not be called _mmd and have a devad. So access_mode should really be > transfer mode. Until now, only transfer mode C45 can be used to access > C45 register space. The point of this patchset is to add a new > C45_OVER_C22 transfer mode.
So you suggest to rename access_mode to transfer_mode, right?
> And C22 would should give -EINVAL, since you cannot use plain C22 bus > transactions to access C45 register space.
We had indirect mmd access before with c22 PHYs before, we could theoretically fold that into c45-over-c22, but the old indirect mmd access wasn't checking for error codes and according to Russell we cannot change that. Honestly, I'd just duplicate the code and leave the old non-error checking code in __phy_read_mmd() while __phy_mdiobus_read_mmd() will do error checking and return -EINVAL with PHY_ACCESS_C22. I had that in one of the first versions but you suggested to not copy the code :), then this ugly check_rc thing came. Or __phy_mdiobus_(read,write)_mmd() will have a check_rc parameter.
-michael
| |