[PATCH net-next 0/4] net: phy: mxl-gpy: version fix and improvements

[PATCH net-next 0/4] net: phy: mxl-gpy: version fix and improvements

[Michael Walle]

Fix the version reporting which was introduced earlier. The version will
not change during runtime, so cache it and save a PHY read on every auto
negotiation. Also print the version in a human readable form.

Michael Walle (4):
  net: phy: mxl-gpy: fix version reporting
  net: phy: mxl-gpy: cache PHY firmware version
  net: phy: mxl-gpy: rename the FW type field name
  net: phy: mxl-gpy: print firmware in human readable form

 drivers/net/phy/mxl-gpy.c | 55 +++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

-- 
2.30.2

[PATCH net-next 1/4] net: phy: mxl-gpy: fix version reporting

[Michael Walle]

The commit 09ce6b20103b ("net: phy: mxl-gpy: add temperature sensor")
will overwrite the return value and the reported version will be wrong.
Fix it.

Fixes: 09ce6b20103b ("net: phy: mxl-gpy: add temperature sensor")
Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/mxl-gpy.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 5b99acf44337..9728ef93fc0b 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -198,6 +198,7 @@ static int gpy_config_init(struct phy_device *phydev)
 
 static int gpy_probe(struct phy_device *phydev)
 {
+	int fw_version;
 	int ret;
 
 	if (!phydev->is_c45) {
@@ -207,16 +208,16 @@ static int gpy_probe(struct phy_device *phydev)
 	}
 
 	/* Show GPY PHY FW version in dmesg */
-	ret = phy_read(phydev, PHY_FWV);
-	if (ret < 0)
-		return ret;
+	fw_version = phy_read(phydev, PHY_FWV);
+	if (fw_version < 0)
+		return fw_version;
 
 	ret = gpy_hwmon_register(phydev);
 	if (ret)
 		return ret;
 
-	phydev_info(phydev, "Firmware Version: 0x%04X (%s)\n", ret,
-		    (ret & PHY_FWV_REL_MASK) ? "release" : "test");
+	phydev_info(phydev, "Firmware Version: 0x%04X (%s)\n", fw_version,
+		    (fw_version & PHY_FWV_REL_MASK) ? "release" : "test");
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH net-next 1/4] net: phy: mxl-gpy: fix version reporting

[Andrew Lunn]

On Tue, Jul 12, 2022 at 03:15:51PM +0200, Michael Walle wrote:
> The commit 09ce6b20103b ("net: phy: mxl-gpy: add temperature sensor")
> will overwrite the return value and the reported version will be wrong.
> Fix it.
> 
> Fixes: 09ce6b20103b ("net: phy: mxl-gpy: add temperature sensor")
> Signed-off-by: Michael Walle <michael@walle.cc>

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

    Andrew

[PATCH net-next 2/4] net: phy: mxl-gpy: cache PHY firmware version

[Michael Walle]

Cache the firmware version during probe. There is no need to read the
firmware version on every autonegotiation.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/mxl-gpy.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 9728ef93fc0b..b6303089d425 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -77,6 +77,11 @@
 #define VPSPEC2_WOL_AD45	0x0E0A
 #define WOL_EN			BIT(0)
 
+struct gpy_priv {
+	u8 fw_type;
+	u8 fw_minor;
+};
+
 static const struct {
 	int type;
 	int minor;
@@ -198,6 +203,8 @@ static int gpy_config_init(struct phy_device *phydev)
 
 static int gpy_probe(struct phy_device *phydev)
 {
+	struct device *dev = &phydev->mdio.dev;
+	struct gpy_priv *priv;
 	int fw_version;
 	int ret;
 
@@ -207,15 +214,22 @@ static int gpy_probe(struct phy_device *phydev)
 			return ret;
 	}
 
-	/* Show GPY PHY FW version in dmesg */
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	phydev->priv = priv;
+
 	fw_version = phy_read(phydev, PHY_FWV);
 	if (fw_version < 0)
 		return fw_version;
+	priv->fw_type = FIELD_GET(PHY_FWV_TYPE_MASK, fw_version);
+	priv->fw_minor = FIELD_GET(PHY_FWV_MINOR_MASK, fw_version);
 
 	ret = gpy_hwmon_register(phydev);
 	if (ret)
 		return ret;
 
+	/* Show GPY PHY FW version in dmesg */
 	phydev_info(phydev, "Firmware Version: 0x%04X (%s)\n", fw_version,
 		    (fw_version & PHY_FWV_REL_MASK) ? "release" : "test");
 
@@ -224,20 +238,13 @@ static int gpy_probe(struct phy_device *phydev)
 
 static bool gpy_sgmii_need_reaneg(struct phy_device *phydev)
 {
-	int fw_ver, fw_type, fw_minor;
+	struct gpy_priv *priv = phydev->priv;
 	size_t i;
 
-	fw_ver = phy_read(phydev, PHY_FWV);
-	if (fw_ver < 0)
-		return true;
-
-	fw_type = FIELD_GET(PHY_FWV_TYPE_MASK, fw_ver);
-	fw_minor = FIELD_GET(PHY_FWV_MINOR_MASK, fw_ver);
-
 	for (i = 0; i < ARRAY_SIZE(ver_need_sgmii_reaneg); i++) {
-		if (fw_type != ver_need_sgmii_reaneg[i].type)
+		if (priv->fw_type != ver_need_sgmii_reaneg[i].type)
 			continue;
-		if (fw_minor < ver_need_sgmii_reaneg[i].minor)
+		if (priv->fw_minor < ver_need_sgmii_reaneg[i].minor)
 			return true;
 		break;
 	}
@@ -605,18 +612,12 @@ static int gpy_loopback(struct phy_device *phydev, bool enable)
 
 static int gpy115_loopback(struct phy_device *phydev, bool enable)
 {
-	int ret;
-	int fw_minor;
+	struct gpy_priv *priv = phydev->priv;
 
 	if (enable)
 		return gpy_loopback(phydev, enable);
 
-	ret = phy_read(phydev, PHY_FWV);
-	if (ret < 0)
-		return ret;
-
-	fw_minor = FIELD_GET(PHY_FWV_MINOR_MASK, ret);
-	if (fw_minor > 0x0076)
+	if (priv->fw_minor > 0x76)
 		return gpy_loopback(phydev, 0);
 
 	return genphy_soft_reset(phydev);
-- 
2.30.2

Re: [PATCH net-next 2/4] net: phy: mxl-gpy: cache PHY firmware version

[Andrew Lunn]

> +	priv->fw_type = FIELD_GET(PHY_FWV_TYPE_MASK, fw_version);
> +	priv->fw_minor = FIELD_GET(PHY_FWV_MINOR_MASK, fw_version);
>  
>  	ret = gpy_hwmon_register(phydev);
>  	if (ret)
>  		return ret;
>  
> +	/* Show GPY PHY FW version in dmesg */
>  	phydev_info(phydev, "Firmware Version: 0x%04X (%s)\n", fw_version,
>  		    (fw_version & PHY_FWV_REL_MASK) ? "release" : "test");

Maybe use fw_type and fw_minor. It makes the patch a bit bigger, but
makes the code more consistent.

      Andrew

Re: [PATCH net-next 2/4] net: phy: mxl-gpy: cache PHY firmware version

[Michael Walle]

Am 2022-07-12 15:24, schrieb Andrew Lunn:
>> +	priv->fw_type = FIELD_GET(PHY_FWV_TYPE_MASK, fw_version);
>> +	priv->fw_minor = FIELD_GET(PHY_FWV_MINOR_MASK, fw_version);
>> 
>>  	ret = gpy_hwmon_register(phydev);
>>  	if (ret)
>>  		return ret;
>> 
>> +	/* Show GPY PHY FW version in dmesg */
>>  	phydev_info(phydev, "Firmware Version: 0x%04X (%s)\n", fw_version,
>>  		    (fw_version & PHY_FWV_REL_MASK) ? "release" : "test");
> 
> Maybe use fw_type and fw_minor. It makes the patch a bit bigger, but
> makes the code more consistent.

See next patches ;) And fw_{type,minor} doesn't contain the REL_MASK.
I chose not to cached that as it just used during this reporting.

-michael

Re: [PATCH net-next 2/4] net: phy: mxl-gpy: cache PHY firmware version

[Andrew Lunn]

On Tue, Jul 12, 2022 at 03:15:52PM +0200, Michael Walle wrote:
> Cache the firmware version during probe. There is no need to read the
> firmware version on every autonegotiation.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>

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

    Andrew

[PATCH net-next 3/4] net: phy: mxl-gpy: rename the FW type field name

[Michael Walle]

Align the firmware field name with the reference manual where it is
called "major".

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/mxl-gpy.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index b6303089d425..ac62b01c61ed 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -56,7 +56,7 @@
 				 PHY_IMASK_ANC)
 
 #define PHY_FWV_REL_MASK	BIT(15)
-#define PHY_FWV_TYPE_MASK	GENMASK(11, 8)
+#define PHY_FWV_MAJOR_MASK	GENMASK(11, 8)
 #define PHY_FWV_MINOR_MASK	GENMASK(7, 0)
 
 /* SGMII */
@@ -78,12 +78,12 @@
 #define WOL_EN			BIT(0)
 
 struct gpy_priv {
-	u8 fw_type;
+	u8 fw_major;
 	u8 fw_minor;
 };
 
 static const struct {
-	int type;
+	int major;
 	int minor;
 } ver_need_sgmii_reaneg[] = {
 	{7, 0x6D},
@@ -222,7 +222,7 @@ static int gpy_probe(struct phy_device *phydev)
 	fw_version = phy_read(phydev, PHY_FWV);
 	if (fw_version < 0)
 		return fw_version;
-	priv->fw_type = FIELD_GET(PHY_FWV_TYPE_MASK, fw_version);
+	priv->fw_major = FIELD_GET(PHY_FWV_MAJOR_MASK, fw_version);
 	priv->fw_minor = FIELD_GET(PHY_FWV_MINOR_MASK, fw_version);
 
 	ret = gpy_hwmon_register(phydev);
@@ -242,7 +242,7 @@ static bool gpy_sgmii_need_reaneg(struct phy_device *phydev)
 	size_t i;
 
 	for (i = 0; i < ARRAY_SIZE(ver_need_sgmii_reaneg); i++) {
-		if (priv->fw_type != ver_need_sgmii_reaneg[i].type)
+		if (priv->fw_major != ver_need_sgmii_reaneg[i].major)
 			continue;
 		if (priv->fw_minor < ver_need_sgmii_reaneg[i].minor)
 			return true;
-- 
2.30.2

Re: [PATCH net-next 3/4] net: phy: mxl-gpy: rename the FW type field name

[Andrew Lunn]

On Tue, Jul 12, 2022 at 03:15:53PM +0200, Michael Walle wrote:
> Align the firmware field name with the reference manual where it is
> called "major".
> 
> Signed-off-by: Michael Walle <michael@walle.cc>

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

    Andrew

[PATCH net-next 4/4] net: phy: mxl-gpy: print firmware in human readable form

[Michael Walle]

Now having a major and a minor number, also print the firmware in
human readable form "major.minor". Still keep the 4-digit hexadecimal
representation because that form is used in the firmware changelog
documents. Also, drop the "release" string assuming that most common
case, but make it clearer that the user is running a test version.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/net/phy/mxl-gpy.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index ac62b01c61ed..24bae27eedef 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -230,8 +230,9 @@ static int gpy_probe(struct phy_device *phydev)
 		return ret;
 
 	/* Show GPY PHY FW version in dmesg */
-	phydev_info(phydev, "Firmware Version: 0x%04X (%s)\n", fw_version,
-		    (fw_version & PHY_FWV_REL_MASK) ? "release" : "test");
+	phydev_info(phydev, "Firmware Version: %d.%d (0x%04X%s)\n",
+		    priv->fw_major, priv->fw_minor, fw_version,
+		    fw_version & PHY_FWV_REL_MASK ? "" : " test version");
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH net-next 4/4] net: phy: mxl-gpy: print firmware in human readable form

[Andrew Lunn]

On Tue, Jul 12, 2022 at 03:15:54PM +0200, Michael Walle wrote:
> Now having a major and a minor number, also print the firmware in
> human readable form "major.minor". Still keep the 4-digit hexadecimal
> representation because that form is used in the firmware changelog
> documents. Also, drop the "release" string assuming that most common
> case, but make it clearer that the user is running a test version.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>

O.K, this answers my earlier question :-)

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

    Andrew

Re: [PATCH net-next 0/4] net: phy: mxl-gpy: version fix and improvements

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 12 Jul 2022 15:15:50 +0200 you wrote:
> Fix the version reporting which was introduced earlier. The version will
> not change during runtime, so cache it and save a PHY read on every auto
> negotiation. Also print the version in a human readable form.
> 
> Michael Walle (4):
>   net: phy: mxl-gpy: fix version reporting
>   net: phy: mxl-gpy: cache PHY firmware version
>   net: phy: mxl-gpy: rename the FW type field name
>   net: phy: mxl-gpy: print firmware in human readable form
> 
> [...]

Here is the summary with links:
  - [net-next,1/4] net: phy: mxl-gpy: fix version reporting
    https://git.kernel.org/netdev/net-next/c/fc3dd0367e61
  - [net-next,2/4] net: phy: mxl-gpy: cache PHY firmware version
    https://git.kernel.org/netdev/net-next/c/1db858707850
  - [net-next,3/4] net: phy: mxl-gpy: rename the FW type field name
    https://git.kernel.org/netdev/net-next/c/1e9aa7baf096
  - [net-next,4/4] net: phy: mxl-gpy: print firmware in human readable form
    https://git.kernel.org/netdev/net-next/c/d523f2eb1dad

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html