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

[Marek Vasut]

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 <ma...@denx.de>
---
Cc: Joe Hershberger <joe.hershber...@ni.com>
Cc: Marek Vasut <ma...@denx.de>
Cc: Michal Simek <michal.si...@amd.com>
Cc: Ramon Fried <rfried....@gmail.com>
Cc: Rasmus Villemoes <rasmus.villem...@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 !