On 31.07.2019 02:12, Tao Ren wrote: > On 7/29/19 11:00 PM, Heiner Kallweit wrote: >> On 30.07.2019 07:05, Tao Ren wrote: >>> On 7/29/19 8:35 PM, Andrew Lunn wrote: >>>> On Mon, Jul 29, 2019 at 05:25:32PM -0700, Tao Ren wrote: >>>>> BCM54616S feature "PHY_GBIT_FEATURES" was removed by commit dcdecdcfe1fc >>>>> ("net: phy: switch drivers to use dynamic feature detection"). As dynamic >>>>> feature detection doesn't work when BCM54616S is working in RGMII-Fiber >>>>> mode (different sets of MII Control/Status registers being used), let's >>>>> set "PHY_GBIT_FEATURES" for BCM54616S explicitly. >>>> >>>> Hi Tao >>>> >>>> What exactly does it get wrong? >>>> >>>> Thanks >>>> Andrew >>> >>> Hi Andrew, >>> >>> BCM54616S is set to RGMII-Fiber (1000Base-X) mode on my platform, and none >>> of the features (1000BaseT/100BaseT/10BaseT) can be detected by >>> genphy_read_abilities(), because the PHY only reports 1000BaseX_Full|Half >>> ability in this mode. >>> >> Are you going to use the PHY in copper or fibre mode? >> In case you use fibre mode, why do you need the copper modes set as >> supported? >> Or does the PHY just start in fibre mode and you want to switch it to copper >> mode? > > Hi Heiner, > > The phy starts in fiber mode and that's the mode I want. > My observation is: phydev->link is always 0 (Link status bit is never set in > MII_BMSR) by using dynamic ability detection on my machine. I checked > phydev->supported and it's set to "AutoNeg | TP | MII | Pause | Asym_Pause" > by dynamic ability detection. Is it normal/expected? Or maybe the fix should > go to different places? Thank you for your help. >
Not sure whether you stated already which kernel version you're using. There's a brand-new extension to auto-detect 1000BaseX: f30e33bcdab9 ("net: phy: Add more 1000BaseX support detection") It's included in the 5.3-rc series. If a feature can be read from a vendor-specific register only, then the preferred way is: Implement callback get_features in the PHY driver, call genphy_read_abilities for the basic features and complement it with reading the vendor-specific register(s). > > Thanks, > > Tao > Heiner