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://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-05-series.pdf
> https://www.belfuse.com/resources/datasheets/powersolutions/ds-bps-sfp-1gbt-06-series.pdf
>
> Signed-off-by: Robert Hancock <robert.hanc...@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?
> + /* 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.
The reset of the sequence and changes looks fine to me.
--
Florian
