Re: [PATCH] net: phy: broadcom: add support for BCM54210E

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,