| Message ID | 20220712131554.2737792-3-michael@walle.cc (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1db8587078502d0bc7a74338867a318deb8753ec |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: mxl-gpy: version fix and improvements | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 85 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
> + 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
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
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
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);
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(-)