| Message ID | 20230120224011.796097-5-michael@walle.cc (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: C45-over-C22 access | expand |
On Fri, Jan 20, 2023 at 11:40:10PM +0100, Michael Walle wrote: > If not explitly asked to be probed as a C45 PHY, on a bus which is > capable of doing both C22 and C45 transfers, C45 PHYs are first tried to > be probed as C22 PHYs. To be able to promote the PHY to be a C45 one, > the driver can call phy_promote_to_c45() in its probe function. > > This was already done in the mxl-gpy driver by the following snippet: > > if (!phydev->has_c45) { > ret = phy_get_c45_ids(phydev); > if (ret < 0) > return ret; > } > > Move that code into the core by creating a new function > phy_promote_to_c45(). If a PHY is promoted, C45-over-C22 access is used, > regardless if the bus supports C45 or not. That is because there might > be C22 PHYs on the bus which gets confused by C45 accesses. It is my understanding that C45 PHYs do not have to respond to C22 accesses. So, wouldn't this lead to some C45 PHYs not being detected?
Am 2023-01-21 00:20, schrieb Russell King (Oracle): > On Fri, Jan 20, 2023 at 11:40:10PM +0100, Michael Walle wrote: >> If not explitly asked to be probed as a C45 PHY, on a bus which is >> capable of doing both C22 and C45 transfers, C45 PHYs are first tried >> to >> be probed as C22 PHYs. To be able to promote the PHY to be a C45 one, >> the driver can call phy_promote_to_c45() in its probe function. >> >> This was already done in the mxl-gpy driver by the following snippet: >> >> if (!phydev->has_c45) { >> ret = phy_get_c45_ids(phydev); >> if (ret < 0) >> return ret; >> } >> >> Move that code into the core by creating a new function >> phy_promote_to_c45(). If a PHY is promoted, C45-over-C22 access is >> used, >> regardless if the bus supports C45 or not. That is because there might >> be C22 PHYs on the bus which gets confused by C45 accesses. > > It is my understanding that C45 PHYs do not have to respond to C22 > accesses. So, wouldn't this lead to some C45 PHYs not being detected? In that case, the PHY already has to be a C45 PHY, correct? Because no access to c22 means, it can't be probed as a c22 phy; therefore, it has the has_c45 set to true. Then phy_promote_to_c45() is a noop and c45-over-c22 wont be set. -michael
diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c index e86aea4381c2..4dc1093dbdc1 100644 --- a/drivers/net/phy/mxl-gpy.c +++ b/drivers/net/phy/mxl-gpy.c @@ -281,11 +281,10 @@ static int gpy_probe(struct phy_device *phydev) int fw_version; int ret; - if (!phydev->has_c45) { - ret = phy_get_c45_ids(phydev); - if (ret < 0) - return ret; - } + /* This might have been probed as a C22 PHY, but this is a C45 PHY */ + ret = phy_promote_to_c45(phydev); + if (ret) + return ret; priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index d5ea034cde98..ae24b62abfac 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1058,18 +1058,33 @@ void phy_device_remove(struct phy_device *phydev) EXPORT_SYMBOL(phy_device_remove); /** - * phy_get_c45_ids - Read 802.3-c45 IDs for phy device. - * @phydev: phy_device structure to read 802.3-c45 IDs + * phy_promote_to_c45 - Promote to a C45 PHY + * @phydev: phy_device structure + * + * If a PHY supports both C22 and C45 and it isn't specifically asked to probe + * as a C45 PHY it might be probed as a C22 PHY. The driver can call this + * function to promote a PHY from C22 to C45. + * + * Can also be called if a PHY is already a C45 one. In that case it does + * nothing. + * + * Promoted PHYs will always use C45-over-C22 accesses to prevent any C45 + * transactions on the bus, which might confuse C22-only PHYs. * * Returns zero on success, %-EIO on bus access error, or %-ENODEV if * the "devices in package" is invalid. */ -int phy_get_c45_ids(struct phy_device *phydev) +int phy_promote_to_c45(struct phy_device *phydev) { + if (phydev->has_c45) + return 0; + + phydev->has_c45 = true; + phydev->c45_over_c22 = true; return get_phy_c45_ids(phydev->mdio.bus, phydev->mdio.addr, &phydev->c45_ids); } -EXPORT_SYMBOL(phy_get_c45_ids); +EXPORT_SYMBOL(phy_promote_to_c45); /** * phy_find_first - finds the first PHY device on the bus diff --git a/include/linux/phy.h b/include/linux/phy.h index 4c02c468a24b..9473e2ed8496 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1588,7 +1588,7 @@ static inline int phy_device_register(struct phy_device *phy) static inline void phy_device_free(struct phy_device *phydev) { } #endif /* CONFIG_PHYLIB */ void phy_device_remove(struct phy_device *phydev); -int phy_get_c45_ids(struct phy_device *phydev); +int phy_promote_to_c45(struct phy_device *phydev); int phy_init_hw(struct phy_device *phydev); int phy_suspend(struct phy_device *phydev); int phy_resume(struct phy_device *phydev);
If not explitly asked to be probed as a C45 PHY, on a bus which is capable of doing both C22 and C45 transfers, C45 PHYs are first tried to be probed as C22 PHYs. To be able to promote the PHY to be a C45 one, the driver can call phy_promote_to_c45() in its probe function. This was already done in the mxl-gpy driver by the following snippet: if (!phydev->has_c45) { ret = phy_get_c45_ids(phydev); if (ret < 0) return ret; } Move that code into the core by creating a new function phy_promote_to_c45(). If a PHY is promoted, C45-over-C22 access is used, regardless if the bus supports C45 or not. That is because there might be C22 PHYs on the bus which gets confused by C45 accesses. Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/net/phy/mxl-gpy.c | 9 ++++----- drivers/net/phy/phy_device.c | 23 +++++++++++++++++++---- include/linux/phy.h | 2 +- 3 files changed, 24 insertions(+), 10 deletions(-)