On 08.02.2019 15:02, Andrew Lunn wrote: > On Fri, Feb 08, 2019 at 08:12:47AM +0100, Heiner Kallweit wrote: >> Bit 0 in register 1.5 doesn't represent a device but is a flag that >> Clause 22 registers are present. Therefore disregard this bit when >> populating the device list. If code needs this information it >> should read register 1.5 directly instead of accessing the device >> list. Because this bit doesn't represent a device I didn't add a >> MDIO_MMD_C22PRESENT constant or similar. >> >> Signed-off-by: Heiner Kallweit <[email protected]> >> --- >> drivers/net/phy/phy_device.c | 3 ++- >> include/uapi/linux/mdio.h | 1 + >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c >> index 9369e1323..c2316a117 100644 >> --- a/drivers/net/phy/phy_device.c >> +++ b/drivers/net/phy/phy_device.c >> @@ -682,7 +682,8 @@ static int get_phy_c45_devs_in_pkg(struct mii_bus *bus, >> int addr, int dev_addr, >> phy_reg = mdiobus_read(bus, addr, reg_addr); >> if (phy_reg < 0) >> return -EIO; >> - *devices_in_package |= (phy_reg & 0xffff); >> + /* Bit 0 doesn't represent a device, it indicates c22 regs presence */ >> + *devices_in_package |= (phy_reg & 0xfffe); > > Hi Heiner > > Just for readability, can we use BIT(0) in there somehow? > You think 0xfffe together with the comment is still not clear enough? But sure, I can make it more explicit.
>> >> return 0; >> } >> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h >> index 2e6e309f0..0e012b168 100644 >> --- a/include/uapi/linux/mdio.h >> +++ b/include/uapi/linux/mdio.h >> @@ -115,6 +115,7 @@ >> >> /* Device present registers. */ >> #define MDIO_DEVS_PRESENT(devad) (1 << (devad)) >> +#define MDIO_DEVS_C22PRESENT MDIO_DEVS_PRESENT(0) > > Err. The commit message says you did not add this... > Maybe I'm not clear enough in the commit message. Typically we have two constants for a device: MDIO_MMD_XXX (for the device) MDIO_DEVS_XXX (for the bit of the device in the device list bitmap) For the C22PRESENT flag I don't define the first one (because it's not a device) but the second one (because it uses a bit in the device list bitmap). > Andrew > Heiner
