On 08.04.2019 00:13, Andrew Lunn wrote: > On Sun, Apr 07, 2019 at 11:57:13AM +0200, Heiner Kallweit wrote: >> Recently genphy_read_abilities() has been added that dynamically detects >> clause 22 PHY abilities. I *think* this detection should work with all >> supported PHY's, at least for the ones with basic features sets, i.e. >> PHY_BASIC_FEATURES and PHY_GBIT_FEATURES. So let's remove setting these >> features explicitly and rely on phylib feature detection. > > Hi Heiner > Hi Andrew,
> We could make this a two step process, to avoid regressions. For one > cycle compare genphy_read_abilities() against .features and raise a > WARN_ON() if they differ. And keep using the .features value. > in general this is a good idea. I say in general because this would fail with several, if not most GBit PHY's. Reason is that the hardcoded features currently pretend we're supporting 1000BT/Half, whilst several PHY's don't support this mode. 1000BT/Half has been specified but never really been used. If we see it from this angle, the series is actually a fix. The feature detection uses very basic C22 registers/bits, therefore I consider the risk of breaking something to be relatively low. And just in case we have the rc phase to fix support for a broken PHY. Splitting the series and waiting for a Tested-by, as proposed by Richard, may be problematic because most PHY drivers don't have a dedicated maintainer, and we lack the hardware to test. > Then a release late, complete the swap removing .features and the > WARN_ON. > > Andrew > . > Heiner
