[PATCH v3 0/3 net-next] net: phy: disregard "Clause 22 registers present" bit in get_phy_c45_devs_in_pkg

[PATCH v3 0/3 net-next] net: phy: disregard "Clause 22 registers present" bit in get_phy_c45_devs_in_pkg

[Heiner Kallweit]

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 don't define a
MDIO_MMD_XYZ constant, just define a MDIO_DEVS_XYZ constant for
the flag in the device list bitmap.

v2:
- make masking of bit 0 more explicit
- improve commit message

v3:
- replace single patch with a series
- add functionality of patch 1

Heiner Kallweit (3):
  net: phy: remove unneeded masking of PHY register read results
  net: phy: disregard "Clause 22 registers present" bit in
    get_phy_c45_devs_in_pkg
  net: phy: add constant for "Clause 22 registers present" flags in
    device list bitmap

 drivers/net/phy/phy_device.c | 15 +++++++++------
 include/uapi/linux/mdio.h    |  1 +
 2 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.20.1

[PATCH v3 1/3 net-next] net: phy: remove unneeded masking of PHY register read results

[Heiner Kallweit]

PHY registers are only 16 bits wide, therefore, if the read was
successful, there's no need to mask out the higher 16 bits.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9369e1323..82dd72954 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -676,13 +676,13 @@ 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) << 16;
+	*devices_in_package = phy_reg << 16;
 
 	reg_addr = MII_ADDR_C45 | dev_addr << 16 | MDIO_DEVS1;
 	phy_reg = mdiobus_read(bus, addr, reg_addr);
 	if (phy_reg < 0)
 		return -EIO;
-	*devices_in_package |= (phy_reg & 0xffff);
+	*devices_in_package |= phy_reg;
 
 	return 0;
 }
@@ -743,13 +743,13 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
 		phy_reg = mdiobus_read(bus, addr, reg_addr);
 		if (phy_reg < 0)
 			return -EIO;
-		c45_ids->device_ids[i] = (phy_reg & 0xffff) << 16;
+		c45_ids->device_ids[i] = phy_reg << 16;
 
 		reg_addr = MII_ADDR_C45 | i << 16 | MII_PHYSID2;
 		phy_reg = mdiobus_read(bus, addr, reg_addr);
 		if (phy_reg < 0)
 			return -EIO;
-		c45_ids->device_ids[i] |= (phy_reg & 0xffff);
+		c45_ids->device_ids[i] |= phy_reg;
 	}
 	*phy_id = 0;
 	return 0;
@@ -786,14 +786,14 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id,
 		return (phy_reg == -EIO || phy_reg == -ENODEV) ? -ENODEV : -EIO;
 	}
 
-	*phy_id = (phy_reg & 0xffff) << 16;
+	*phy_id = phy_reg << 16;
 
 	/* Grab the bits from PHYIR2, and put them in the lower half */
 	phy_reg = mdiobus_read(bus, addr, MII_PHYSID2);
 	if (phy_reg < 0)
 		return -EIO;
 
-	*phy_id |= (phy_reg & 0xffff);
+	*phy_id |= phy_reg;
 
 	return 0;
 }
-- 
2.20.1

[PATCH v3 2/3 net-next] net: phy: disregard "Clause 22 registers present" bit in get_phy_c45_devs_in_pkg

[Heiner Kallweit]

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.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 82dd72954..31f9e7c49 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -684,6 +684,9 @@ static int get_phy_c45_devs_in_pkg(struct mii_bus *bus, int addr, int dev_addr,
 		return -EIO;
 	*devices_in_package |= phy_reg;
 
+	/* Bit 0 doesn't represent a device, it indicates c22 regs presence */
+	*devices_in_package &= ~BIT(0);
+
 	return 0;
 }
 
-- 
2.20.1

[PATCH v3 3/3 net-next] net: phy: add constant for "Clause 22 registers present" flags in device list bitmap

[Heiner Kallweit]

Bit 0 ("Clause 22 registers present") is a flag and doesn't represent
a device, therefore don't define a MDIO_MMD_XYZ constant, just define
a MDIO_DEVS_XYZ constant for the flag in the device list bitmap.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 include/uapi/linux/mdio.h | 1 +
 1 file changed, 1 insertion(+)

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)
 #define MDIO_DEVS_PMAPMD		MDIO_DEVS_PRESENT(MDIO_MMD_PMAPMD)
 #define MDIO_DEVS_WIS			MDIO_DEVS_PRESENT(MDIO_MMD_WIS)
 #define MDIO_DEVS_PCS			MDIO_DEVS_PRESENT(MDIO_MMD_PCS)
-- 
2.20.1

Re: [PATCH v3 1/3 net-next] net: phy: remove unneeded masking of PHY register read results

[Andrew Lunn]

On Fri, Feb 08, 2019 at 08:20:34PM +0100, Heiner Kallweit wrote:
> PHY registers are only 16 bits wide, therefore, if the read was
> successful, there's no need to mask out the higher 16 bits.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v3 2/3 net-next] net: phy: disregard "Clause 22 registers present" bit in get_phy_c45_devs_in_pkg

[Andrew Lunn]

On Fri, Feb 08, 2019 at 08:21:24PM +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.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v3 3/3 net-next] net: phy: add constant for "Clause 22 registers present" flags in device list bitmap

[Andrew Lunn]

On Fri, Feb 08, 2019 at 08:22:17PM +0100, Heiner Kallweit wrote:
> Bit 0 ("Clause 22 registers present") is a flag and doesn't represent
> a device, therefore don't define a MDIO_MMD_XYZ constant, just define
> a MDIO_DEVS_XYZ constant for the flag in the device list bitmap.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next] net: phy: remove unneeded masking of PHY register read results

[Heiner Kallweit]

PHY registers are only 16 bits wide, therefore, if the read was
successful, there's no need to mask out the higher 16 bits.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index d4fc1fd8a..31f9e7c49 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -676,13 +676,13 @@ 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) << 16;
+	*devices_in_package = phy_reg << 16;
 
 	reg_addr = MII_ADDR_C45 | dev_addr << 16 | MDIO_DEVS1;
 	phy_reg = mdiobus_read(bus, addr, reg_addr);
 	if (phy_reg < 0)
 		return -EIO;
-	*devices_in_package |= (phy_reg & 0xffff);
+	*devices_in_package |= phy_reg;
 
 	/* Bit 0 doesn't represent a device, it indicates c22 regs presence */
 	*devices_in_package &= ~BIT(0);
@@ -746,13 +746,13 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
 		phy_reg = mdiobus_read(bus, addr, reg_addr);
 		if (phy_reg < 0)
 			return -EIO;
-		c45_ids->device_ids[i] = (phy_reg & 0xffff) << 16;
+		c45_ids->device_ids[i] = phy_reg << 16;
 
 		reg_addr = MII_ADDR_C45 | i << 16 | MII_PHYSID2;
 		phy_reg = mdiobus_read(bus, addr, reg_addr);
 		if (phy_reg < 0)
 			return -EIO;
-		c45_ids->device_ids[i] |= (phy_reg & 0xffff);
+		c45_ids->device_ids[i] |= phy_reg;
 	}
 	*phy_id = 0;
 	return 0;
@@ -789,14 +789,14 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id,
 		return (phy_reg == -EIO || phy_reg == -ENODEV) ? -ENODEV : -EIO;
 	}
 
-	*phy_id = (phy_reg & 0xffff) << 16;
+	*phy_id = phy_reg << 16;
 
 	/* Grab the bits from PHYIR2, and put them in the lower half */
 	phy_reg = mdiobus_read(bus, addr, MII_PHYSID2);
 	if (phy_reg < 0)
 		return -EIO;
 
-	*phy_id |= (phy_reg & 0xffff);
+	*phy_id |= phy_reg;
 
 	return 0;
 }
-- 
2.20.1

Re: [PATCH net-next] net: phy: remove unneeded masking of PHY register read results

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 9 Feb 2019 09:46:53 +0100

> PHY registers are only 16 bits wide, therefore, if the read was
> successful, there's no need to mask out the higher 16 bits.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Applied, thanks Heiner.

Re: [PATCH v3 0/3 net-next] net: phy: disregard "Clause 22 registers present" bit in get_phy_c45_devs_in_pkg

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 8 Feb 2019 20:19:17 +0100

> 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 don't define a
> MDIO_MMD_XYZ constant, just define a MDIO_DEVS_XYZ constant for
> the flag in the device list bitmap.
 ...

This doesn't apply cleanly to net-next, please respin.

Thanks.

Re: [PATCH v3 0/3 net-next] net: phy: disregard "Clause 22 registers present" bit in get_phy_c45_devs_in_pkg

[Heiner Kallweit]

On 11.02.2019 22:19, David Miller wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> Date: Fri, 8 Feb 2019 20:19:17 +0100
> 
>> 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 don't define a
>> MDIO_MMD_XYZ constant, just define a MDIO_DEVS_XYZ constant for
>> the flag in the device list bitmap.
>  ...
> 
> This doesn't apply cleanly to net-next, please respin.
> 
You applied v2 already, and with a follow-up patch I "upgraded"
it to v3. So you can disregard this series.

> Thanks.
> 

Re: [PATCH v3 0/3 net-next] net: phy: disregard "Clause 22 registers present" bit in get_phy_c45_devs_in_pkg

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Mon, 11 Feb 2019 22:34:34 +0100

> On 11.02.2019 22:19, David Miller wrote:
>> From: Heiner Kallweit <hkallweit1@gmail.com>
>> Date: Fri, 8 Feb 2019 20:19:17 +0100
>> 
>>> 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 don't define a
>>> MDIO_MMD_XYZ constant, just define a MDIO_DEVS_XYZ constant for
>>> the flag in the device list bitmap.
>>  ...
>> 
>> This doesn't apply cleanly to net-next, please respin.
>> 
> You applied v2 already, and with a follow-up patch I "upgraded"
> it to v3. So you can disregard this series.

Oh that's right, thanks.