On Thu, 2019-08-08 at 21:32 +0200, Heiner Kallweit wrote: > [External] > > On 08.08.2019 17:24, Andrew Lunn wrote: > > On Thu, Aug 08, 2019 at 03:30:13PM +0300, Alexandru Ardelean wrote: > > > The ADIN PHYs can operate with Clause 45, however they are not typical for > > > how phylib considers Clause 45 PHYs. > > > > > > If the `features` field & the `get_features` hook are unspecified, and the > > > device wants to operate via Clause 45, it would also try to read features > > > via the `genphy_c45_pma_read_abilities()`, which will try to read PMA regs > > > that are unsupported. > > > > > > Hooking the `genphy_read_abilities()` function to the `get_features` hook > > > will ensure that this does not happen and the PHY features are read > > > correctly regardless of Clause 22 or Clause 45 operation. > > > > I think we need to stop and think about a PHY which supports both C22 > > and C45. > > > > How does bus enumeration work? Is it discovered twice? I've always > > considered phydev->is_c45 means everything is c45, not that some > > registers can be accessed via c45. But the driver is mixing c22 and > > c45. Does the driver actually require c45? Are some features which are > > only accessibly via C45? What does C45 actually bring us for this > > device? > > Hmm, I can't answer [all] these questions. These PHYs seem to be a bit different from the rest that I looked at in drivers/net/phy with regard to C45 & C22. And C45 seems to be more/closer related to 10G PHYs [from what I can tell]. The ADIN PHYs can operate only in C22. The only thing that is needed [and a bit special] is EEE, which [for C22] requires the translation layer to convert C45 reg addresses to internal C22 equivalent. > genphy_c45_pma_read_abilities() is only called if phydev->is_c45 is set. > And this flag means that the PHY complies with Clause 45 incl. all the > standard devices like PMA. In the case here only some vendor-specific > registers can be accessed via Clause 45 and therefore is_c45 shouldn't > bet set. As a consequence this patch isn't needed. ack, will drop patch > > > Andrew > > > Heiner >
