2022-07-12 13:17:36

by Michael Walle

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

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


2022-07-12 13:17:55

by Michael Walle

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

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 <[email protected]>
---
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

2022-07-12 13:31:28

by Michael Walle

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

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

Signed-off-by: Michael Walle <[email protected]>
---
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

2022-07-12 13:32:49

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next 2/4] net: phy: mxl-gpy: cache PHY firmware 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");

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

Andrew

2022-07-12 13:33:58

by Andrew Lunn

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

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 <[email protected]>

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

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2022-07-12 13:34:59

by Andrew Lunn

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

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 <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2022-07-12 13:51:33

by Michael Walle

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

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 <[email protected]>
---
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

2022-07-12 14:01:35

by Michael Walle

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

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

Signed-off-by: Michael Walle <[email protected]>
---
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

2022-07-12 14:06:40

by Michael Walle

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

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

2022-07-12 14:07:31

by Andrew Lunn

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

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 <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2022-07-13 13:43:04

by patchwork-bot+netdevbpf

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

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <[email protected]>:

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