Message ID | 20210213002825.2557444-1-robert.hancock@calian.com |
---|---|
Headers | show |
Series | Broadcom PHY driver updates | expand |
On 2/12/2021 5:45 PM, Robert Hancock wrote: > On Fri, 2021-02-12 at 17:26 -0800, Florian Fainelli wrote: >> >> On 2/12/2021 4:28 PM, 'Robert Hancock' via BCM-KERNEL-FEEDBACK-LIST,PDL >> wrote: >>> The default configuration for the BCM54616S PHY may not match the desired >>> mode when using 1000BaseX or SGMII interface modes, such as when it is on >>> an SFP module. Add code to explicitly set the correct mode using >>> programming sequences provided by Bel-Fuse: >>> >>> https://urldefense.com/v3/__https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-05-series.pdf__;!!IOGos0k!20FhZqRHEiz2-qFJ7J8XC4xX2qG-ajZ17Ma1W-VwDgwdQZeIhHEpWKlNldWW8DyFaQo$ >>> https://urldefense.com/v3/__https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-06-series.pdf__;!!IOGos0k!20FhZqRHEiz2-qFJ7J8XC4xX2qG-ajZ17Ma1W-VwDgwdQZeIhHEpWKlNldWW58K3fY4$ >>> >>> Signed-off-by: Robert Hancock <robert.hancock@calian.com> >>> --- >>> drivers/net/phy/broadcom.c | 83 ++++++++++++++++++++++++++++++++------ >>> include/linux/brcmphy.h | 4 ++ >>> 2 files changed, 75 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c >>> index 0472b3470c59..78542580f2b2 100644 >>> --- a/drivers/net/phy/broadcom.c >>> +++ b/drivers/net/phy/broadcom.c >>> @@ -64,6 +64,63 @@ static int bcm54612e_config_init(struct phy_device >>> *phydev) >>> return 0; >>> } >>> >>> +static int bcm54616s_config_init(struct phy_device *phydev) >>> +{ >>> + int rc, val; >>> + >>> + if (phydev->interface == PHY_INTERFACE_MODE_SGMII || >>> + phydev->interface == PHY_INTERFACE_MODE_1000BASEX) { >> >> Can you reverse the condition so as to save a level of identation? > > Can do. > >> >>> + /* Ensure proper interface mode is selected. */ >>> + /* Disable RGMII mode */ >>> + val = bcm54xx_auxctl_read(phydev, >>> MII_BCM54XX_AUXCTL_SHDWSEL_MISC); >>> + if (val < 0) >>> + return val; >>> + val &= ~MII_BCM54XX_AUXCTL_SHDWSEL_MISC_RGMII_EN; >>> + rc = bcm54xx_auxctl_write(phydev, >>> MII_BCM54XX_AUXCTL_SHDWSEL_MISC, >>> + val); >>> + if (rc < 0) >>> + return rc; >> >> I don't think this write is making it through since you are not setting >> MII_BCM54XX_AUXCTL_MISC_WREN in val, I know this is an annoying detail, >> and we could probably fold that to be within bcm54xx_auxctl_write() >> directly, similarly to what bcm_phy_write_shadow() does. > > Ah, indeed. I assume that is specific to the MII_BCM54XX_AUXCTL_SHDWSEL_MISC > register? I suppose bcm54xx_auxctl_write could add that automatically for > writes to that register. Not sure if that is too much magic for that function > or not.. Upon closer look it's a bit more subtle than that, as we need to apply the write enable bit only when targeting the "misc" shadow register, a million ways to die in the west :)