| Message ID | 20230405-net-next-topic-net-phy-reset-v1-5-7e5329f08002@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Rework PHY reset handling | expand |
On Wed, Apr 05, 2023 at 11:26:56AM +0200, Marco Felsch wrote: > Some phy's don't report the correct phy-id, e.g. the TJA1102 dual-port > report 0 for the 2nd port. To fix this a driver needs to supply the > phyid instead and tell the phy framework to not try to readout the > phyid. The latter case is done via the new 'phy_id_broken' flag which > tells the phy framework to skip phyid readout for the corresponding phy. In general, we try to avoid work around for broken hardware in the core. Please try to solve this within nxp-tja11xx.c. Andrew
On 4/5/2023 5:27 AM, Andrew Lunn wrote: > On Wed, Apr 05, 2023 at 11:26:56AM +0200, Marco Felsch wrote: >> Some phy's don't report the correct phy-id, e.g. the TJA1102 dual-port >> report 0 for the 2nd port. To fix this a driver needs to supply the >> phyid instead and tell the phy framework to not try to readout the >> phyid. The latter case is done via the new 'phy_id_broken' flag which >> tells the phy framework to skip phyid readout for the corresponding phy. > > In general, we try to avoid work around for broken hardware in the > core. Please try to solve this within nxp-tja11xx.c. Agreed, and one way to solve working around broken PHY identification registers is to provide them through the compatible string via "ethernet-phyAAAA.BBBB". This forces the PHY library not to read from those registers yet instantiate the PHY device and force it to bind to a certain phy_driver.
On 23-04-05, Florian Fainelli wrote: > On 4/5/2023 5:27 AM, Andrew Lunn wrote: > > On Wed, Apr 05, 2023 at 11:26:56AM +0200, Marco Felsch wrote: > > > Some phy's don't report the correct phy-id, e.g. the TJA1102 dual-port > > > report 0 for the 2nd port. To fix this a driver needs to supply the > > > phyid instead and tell the phy framework to not try to readout the > > > phyid. The latter case is done via the new 'phy_id_broken' flag which > > > tells the phy framework to skip phyid readout for the corresponding phy. > > > > In general, we try to avoid work around for broken hardware in the > > core. Please try to solve this within nxp-tja11xx.c. > > Agreed, and one way to solve working around broken PHY identification > registers is to provide them through the compatible string via > "ethernet-phyAAAA.BBBB". This forces the PHY library not to read from those > registers yet instantiate the PHY device and force it to bind to a certain > phy_driver. The nxp-tja11xx.c is a bit special in case of two-port devices since the 2nd port registers a 2nd phy device which is correct but don't have a dedicated compatible and so on. My 2nd idea here was to check if phy_id is !0 and in this case just use it. I went this way to make it a bit more explicit. Regards, Marco > -- > Florian >
> The nxp-tja11xx.c is a bit special in case of two-port devices since the > 2nd port registers a 2nd phy device which is correct but don't have a > dedicated compatible and so on. My 2nd idea here was to check if phy_id > is !0 and in this case just use it. I went this way to make it a bit > more explicit. What is actually wrong with the current solution? It is nicely hidden away in the driver, where workarounds for broken hardware should be. If you have found device 1 of 2, does that not suggest its resets are already in a good state and nothing needs to be done for the second PHY? So just register the second PHY with the code from within the driver. Andrew
diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c index b5e03d66b7f5..2a4c0f6d74eb 100644 --- a/drivers/net/phy/nxp-tja11xx.c +++ b/drivers/net/phy/nxp-tja11xx.c @@ -560,6 +560,7 @@ static void tja1102_p1_register(struct work_struct *work) .mii_bus = bus, /* Real PHY ID of Port 1 is 0 */ .phy_id = PHY_ID_TJA1102, + .phy_id_broken = true, }; struct phy_device *phy; diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index bb465a324eef..7e4b3b3caba9 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -969,6 +969,9 @@ static int phy_device_detect(struct phy_device_config *config) int addr = config->phy_addr; int r; + if (config->phy_id_broken) + return 0; + if (is_c45) r = get_phy_c45_ids(bus, addr, c45_ids); else diff --git a/include/linux/phy.h b/include/linux/phy.h index 498f4dc7669d..0f0cb72a08ab 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -764,6 +764,8 @@ static inline struct phy_device *to_phy_device(const struct device *dev) * @phy_id: UID for this device found during discovery * @c45_ids: 802.3-c45 Device Identifiers if is_c45. * @is_c45: If true the PHY uses the 802.3 clause 45 protocol + * @phy_id_broken: Skip the phy_id detection instead use the supplied phy_id or + * c45_ids. * * The struct contain possible configuration parameters for a PHY device which * are used to setup the struct phy_device. @@ -775,6 +777,7 @@ struct phy_device_config { u32 phy_id; struct phy_c45_device_ids c45_ids; bool is_c45; + bool phy_id_broken; }; /**
Some phy's don't report the correct phy-id, e.g. the TJA1102 dual-port report 0 for the 2nd port. To fix this a driver needs to supply the phyid instead and tell the phy framework to not try to readout the phyid. The latter case is done via the new 'phy_id_broken' flag which tells the phy framework to skip phyid readout for the corresponding phy. Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- drivers/net/phy/nxp-tja11xx.c | 1 + drivers/net/phy/phy_device.c | 3 +++ include/linux/phy.h | 3 +++ 3 files changed, 7 insertions(+)