| Message ID | 20230805033847.743458-1-marex@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | net: phy: broadcom: add support for BCM54210E | expand |
On 5.08.2023 05:38, Marek Vasut wrote: > It's Broadcom PHY simply described as single-port > RGMII 10/100/1000BASE-T PHY. It requires disabling > delay skew and GTXCLK bits. > > Ported from Linux kernel commit > 0fc9ae1076697 ("net: phy: broadcom: add support for BCM54210E") > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Joe Hershberger <joe.hershberger@ni.com> > Cc: Marek Vasut <marex@denx.de> > Cc: Michal Simek <michal.simek@amd.com> > Cc: Ramon Fried <rfried.dev@gmail.com> > Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk> Including original author may be a good idea too :) This patch isn't really correct, see below. > --- > drivers/net/phy/broadcom.c | 66 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 65 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c > index 36c70da181a..0967363b3bc 100644 > --- a/drivers/net/phy/broadcom.c > +++ b/drivers/net/phy/broadcom.c > @@ -30,10 +30,47 @@ > #define MIIM_BCM54XX_EXP_SEL_ER 0x0f00 /* Expansion register select */ > > #define MIIM_BCM_AUXCNTL_SHDWSEL_MISC 0x0007 > -#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800 > +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_WIRESPEED_EN 0x0010 > +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_EN 0x0080 > +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN 0x0100 > +#define MIIM_BCM_AUXCNTL_MISC_FORCE_AMDIX 0x0200 > +#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800 > +#define MIIM_BCM_AUXCNTL_MISC_WREN 0x8000 > > #define MIIM_BCM_CHANNEL_WIDTH 0x2000 > > +#define BCM54810_SHD_CLK_CTL 0x3 > +#define BCM54810_SHD_CLK_CTL_GTXCLK_EN BIT(9) > + > +static int bcm54xx_auxctl_read(struct phy_device *phydev, u16 regnum) > +{ > + /* The register must be written to both the Shadow Register Select and > + * the Shadow Read Register Selector > + */ > + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL, > + MIIM_BCM54xx_AUXCNTL_ENCODE(regnum)); > + return phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL); > +} > + > +static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val) > +{ > + return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL, regnum | val); > +} > + > +static int bcm_phy_read_shadow(struct phy_device *phydev, u16 shadow) > +{ > + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD, > + MIIM_BCM54XX_SHD_VAL(shadow)); > + return MIIM_BCM54XX_SHD_DATA(phy_read(phydev, MDIO_DEVAD_NONE, > + MIIM_BCM54XX_SHD)); > +} > + > +static int bcm_phy_write_shadow(struct phy_device *phydev, u16 shadow, u16 val) > +{ > + return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD, > + MIIM_BCM54XX_SHD_WR_ENCODE(shadow, val)); > +} > + > static void bcm_phy_write_misc(struct phy_device *phydev, > u16 reg, u16 chl, u16 value) > { > @@ -62,6 +99,23 @@ static int bcm5461_config(struct phy_device *phydev) > return 0; > } > > +/* Broadcom BCM54210E */ > +static int bcm54210e_config(struct phy_device *phydev) > +{ > + int val; > + > + val = bcm54xx_auxctl_read(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC); > + val &= ~MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN; > + val |= MIIM_BCM_AUXCNTL_MISC_WREN; > + bcm54xx_auxctl_write(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC, val); > + > + val = bcm_phy_read_shadow(phydev, BCM54810_SHD_CLK_CTL); > + val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN; > + bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val); > + > + return bcm5461_config(phydev); > +} Above function hardcodes setup for a specific PHY mode. It should be done depending on PHY mode specified in DT. See kernel commit fea7fda7f50a ("net: phy: broadcom: Fix RGMII delays configuration for BCM54210E") > + > static int bcm54xx_parse_status(struct phy_device *phydev) > { > unsigned int mii_reg; > @@ -311,6 +365,16 @@ static int bcm5482_startup(struct phy_device *phydev) > return bcm54xx_parse_status(phydev); > } > > +U_BOOT_PHY_DRIVER(bcm54210e) = { > + .name = "Broadcom BCM54210E", > + .uid = 0x600d84a0, > + .mask = 0xfffffff0, > + .features = PHY_GBIT_FEATURES, > + .config = &bcm54210e_config, > + .startup = &bcm54xx_startup, > + .shutdown = &genphy_shutdown, > +}; > + > U_BOOT_PHY_DRIVER(bcm5461s) = { > .name = "Broadcom BCM5461S", > .uid = 0x2060c0,
On 8/5/23 11:34, Rafał Miłecki wrote: > On 5.08.2023 05:38, Marek Vasut wrote: >> It's Broadcom PHY simply described as single-port >> RGMII 10/100/1000BASE-T PHY. It requires disabling >> delay skew and GTXCLK bits. >> >> Ported from Linux kernel commit >> 0fc9ae1076697 ("net: phy: broadcom: add support for BCM54210E") >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> --- >> Cc: Joe Hershberger <joe.hershberger@ni.com> >> Cc: Marek Vasut <marex@denx.de> >> Cc: Michal Simek <michal.simek@amd.com> >> Cc: Ramon Fried <rfried.dev@gmail.com> >> Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > > Including original author may be a good idea too :) Done in V2 . TBH I'm still unsure whether or not CC the kernel people on U-Boot patch ports. > This patch isn't really correct, see below. > > >> --- >> drivers/net/phy/broadcom.c | 66 +++++++++++++++++++++++++++++++++++++- >> 1 file changed, 65 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c >> index 36c70da181a..0967363b3bc 100644 >> --- a/drivers/net/phy/broadcom.c >> +++ b/drivers/net/phy/broadcom.c >> @@ -30,10 +30,47 @@ >> #define MIIM_BCM54XX_EXP_SEL_ER 0x0f00 /* Expansion >> register select */ >> #define MIIM_BCM_AUXCNTL_SHDWSEL_MISC 0x0007 >> -#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800 >> +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_WIRESPEED_EN 0x0010 >> +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_EN 0x0080 >> +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN 0x0100 >> +#define MIIM_BCM_AUXCNTL_MISC_FORCE_AMDIX 0x0200 >> +#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800 >> +#define MIIM_BCM_AUXCNTL_MISC_WREN 0x8000 >> #define MIIM_BCM_CHANNEL_WIDTH 0x2000 >> +#define BCM54810_SHD_CLK_CTL 0x3 >> +#define BCM54810_SHD_CLK_CTL_GTXCLK_EN BIT(9) >> + >> +static int bcm54xx_auxctl_read(struct phy_device *phydev, u16 regnum) >> +{ >> + /* The register must be written to both the Shadow Register >> Select and >> + * the Shadow Read Register Selector >> + */ >> + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL, >> + MIIM_BCM54xx_AUXCNTL_ENCODE(regnum)); >> + return phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL); >> +} >> + >> +static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 >> regnum, u16 val) >> +{ >> + return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL, >> regnum | val); >> +} >> + >> +static int bcm_phy_read_shadow(struct phy_device *phydev, u16 shadow) >> +{ >> + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD, >> + MIIM_BCM54XX_SHD_VAL(shadow)); >> + return MIIM_BCM54XX_SHD_DATA(phy_read(phydev, MDIO_DEVAD_NONE, >> + MIIM_BCM54XX_SHD)); >> +} >> + >> +static int bcm_phy_write_shadow(struct phy_device *phydev, u16 >> shadow, u16 val) >> +{ >> + return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD, >> + MIIM_BCM54XX_SHD_WR_ENCODE(shadow, val)); >> +} >> + >> static void bcm_phy_write_misc(struct phy_device *phydev, >> u16 reg, u16 chl, u16 value) >> { >> @@ -62,6 +99,23 @@ static int bcm5461_config(struct phy_device *phydev) >> return 0; >> } >> +/* Broadcom BCM54210E */ >> +static int bcm54210e_config(struct phy_device *phydev) >> +{ >> + int val; >> + >> + val = bcm54xx_auxctl_read(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC); >> + val &= ~MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN; >> + val |= MIIM_BCM_AUXCNTL_MISC_WREN; >> + bcm54xx_auxctl_write(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC, val); >> + >> + val = bcm_phy_read_shadow(phydev, BCM54810_SHD_CLK_CTL); >> + val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN; >> + bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val); >> + >> + return bcm5461_config(phydev); >> +} > > Above function hardcodes setup for a specific PHY mode. It should be > done depending on PHY mode specified in DT. See kernel commit > fea7fda7f50a ("net: phy: broadcom: Fix RGMII delays configuration for > BCM54210E") Updated in v2, thanks !
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 36c70da181a..0967363b3bc 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -30,10 +30,47 @@ #define MIIM_BCM54XX_EXP_SEL_ER 0x0f00 /* Expansion register select */ #define MIIM_BCM_AUXCNTL_SHDWSEL_MISC 0x0007 -#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800 +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_WIRESPEED_EN 0x0010 +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_EN 0x0080 +#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN 0x0100 +#define MIIM_BCM_AUXCNTL_MISC_FORCE_AMDIX 0x0200 +#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800 +#define MIIM_BCM_AUXCNTL_MISC_WREN 0x8000 #define MIIM_BCM_CHANNEL_WIDTH 0x2000 +#define BCM54810_SHD_CLK_CTL 0x3 +#define BCM54810_SHD_CLK_CTL_GTXCLK_EN BIT(9) + +static int bcm54xx_auxctl_read(struct phy_device *phydev, u16 regnum) +{ + /* The register must be written to both the Shadow Register Select and + * the Shadow Read Register Selector + */ + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL, + MIIM_BCM54xx_AUXCNTL_ENCODE(regnum)); + return phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL); +} + +static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val) +{ + return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL, regnum | val); +} + +static int bcm_phy_read_shadow(struct phy_device *phydev, u16 shadow) +{ + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD, + MIIM_BCM54XX_SHD_VAL(shadow)); + return MIIM_BCM54XX_SHD_DATA(phy_read(phydev, MDIO_DEVAD_NONE, + MIIM_BCM54XX_SHD)); +} + +static int bcm_phy_write_shadow(struct phy_device *phydev, u16 shadow, u16 val) +{ + return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD, + MIIM_BCM54XX_SHD_WR_ENCODE(shadow, val)); +} + static void bcm_phy_write_misc(struct phy_device *phydev, u16 reg, u16 chl, u16 value) { @@ -62,6 +99,23 @@ static int bcm5461_config(struct phy_device *phydev) return 0; } +/* Broadcom BCM54210E */ +static int bcm54210e_config(struct phy_device *phydev) +{ + int val; + + val = bcm54xx_auxctl_read(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC); + val &= ~MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN; + val |= MIIM_BCM_AUXCNTL_MISC_WREN; + bcm54xx_auxctl_write(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC, val); + + val = bcm_phy_read_shadow(phydev, BCM54810_SHD_CLK_CTL); + val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN; + bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val); + + return bcm5461_config(phydev); +} + static int bcm54xx_parse_status(struct phy_device *phydev) { unsigned int mii_reg; @@ -311,6 +365,16 @@ static int bcm5482_startup(struct phy_device *phydev) return bcm54xx_parse_status(phydev); } +U_BOOT_PHY_DRIVER(bcm54210e) = { + .name = "Broadcom BCM54210E", + .uid = 0x600d84a0, + .mask = 0xfffffff0, + .features = PHY_GBIT_FEATURES, + .config = &bcm54210e_config, + .startup = &bcm54xx_startup, + .shutdown = &genphy_shutdown, +}; + U_BOOT_PHY_DRIVER(bcm5461s) = { .name = "Broadcom BCM5461S", .uid = 0x2060c0,
It's Broadcom PHY simply described as single-port RGMII 10/100/1000BASE-T PHY. It requires disabling delay skew and GTXCLK bits. Ported from Linux kernel commit 0fc9ae1076697 ("net: phy: broadcom: add support for BCM54210E") Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Joe Hershberger <joe.hershberger@ni.com> Cc: Marek Vasut <marex@denx.de> Cc: Michal Simek <michal.simek@amd.com> Cc: Ramon Fried <rfried.dev@gmail.com> Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk> --- drivers/net/phy/broadcom.c | 66 +++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-)