Merge branch '2023-09-13-phy-improvements' into next

- YT8511 and BCM54210E PHY support, cleanup and then use
  generic_phy_valid more, and then further clean up
  generic_{setup,shutdown}_phy()

arch/sandbox/dts/test.dts

@@ -433,6 +433,11 @@
 		#phy-cells = <0>;
 	};
 
+	phy_provider3: gen_phy@3 {
+		compatible = "sandbox,phy";
+		#phy-cells = <2>;
+	};
+
 	gen_phy_user: gen_phy_user {
 		compatible = "simple-bus";
 		phys = <&phy_provider0 0>, <&phy_provider0 1>, <&phy_provider1>;
@@ -445,6 +450,12 @@
 		phy-names = "phy1", "phy2";
 	};
 
+	gen_phy_user2: gen_phy_user2 {
+		compatible = "simple-bus";
+		phys = <&phy_provider3 0 0>;
+		phy-names = "phy1";
+	};
+
 	some-bus {
 		#address-cells = <1>;
 		#size-cells = <0>;

drivers/ata/sata_ceva.c

@@ -217,7 +217,7 @@ static int sata_ceva_probe(struct udevice *dev)
 		}
 	}
 
-	if (phy.dev) {
+	if (generic_phy_valid(&phy)) {
 		dev_dbg(dev, "Perform PHY power on\n");
 		ret = generic_phy_power_on(&phy);
 		if (ret) {

drivers/net/phy/Kconfig

@@ -224,7 +224,7 @@ config PHY_MOTORCOMM
 	tristate "Motorcomm PHYs"
 	help
 	  Enables support for Motorcomm network PHYs.
-	  Currently supports the YT8531 Gigabit Ethernet PHYs.
+	  Currently supports the YT8511 and YT8531 Gigabit Ethernet PHYs.
 
 config PHY_MSCC
 	bool "Microsemi Corp Ethernet PHYs support"

drivers/net/phy/broadcom.c

@@ -30,10 +30,87 @@
 #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 int bcm54xx_config_clock_delay(struct phy_device *phydev)
+{
+	int rc, val;
+
+	/* handling PHY's internal RX clock delay */
+	val = bcm54xx_auxctl_read(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC);
+	val |= MIIM_BCM_AUXCNTL_MISC_WREN;
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII ||
+	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+		/* Disable RGMII RXC-RXD skew */
+		val &= ~MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN;
+	}
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+		/* Enable RGMII RXC-RXD skew */
+		val |= MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN;
+	}
+	rc = bcm54xx_auxctl_write(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC, val);
+	if (rc < 0)
+		return rc;
+
+	/* handling PHY's internal TX clock delay */
+	val = bcm_phy_read_shadow(phydev, BCM54810_SHD_CLK_CTL);
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII ||
+	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+		/* Disable internal TX clock delay */
+		val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN;
+	}
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+		/* Enable internal TX clock delay */
+		val |= BCM54810_SHD_CLK_CTL_GTXCLK_EN;
+	}
+	rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
 static void bcm_phy_write_misc(struct phy_device *phydev,
 			       u16 reg, u16 chl, u16 value)
 {
@@ -62,6 +139,18 @@ static int bcm5461_config(struct phy_device *phydev)
 	return 0;
 }
 
+/* Broadcom BCM54210E */
+static int bcm54210e_config(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = bcm54xx_config_clock_delay(phydev);
+	if (ret < 0)
+		return ret;
+
+	return bcm5461_config(phydev);
+}
+
 static int bcm54xx_parse_status(struct phy_device *phydev)
 {
 	unsigned int mii_reg;
@@ -311,6 +400,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,

drivers/net/phy/motorcomm.c

@@ -11,6 +11,7 @@
 #include <phy.h>
 #include <linux/bitfield.h>
 
+#define PHY_ID_YT8511				0x0000010a
 #define PHY_ID_YT8531				0x4f51e91b
 #define PHY_ID_MASK				GENMASK(31, 0)
 
@@ -26,6 +27,31 @@
 #define YTPHY_DTS_OUTPUT_CLK_25M		25000000
 #define YTPHY_DTS_OUTPUT_CLK_125M		125000000
 
+#define YT8511_EXT_CLK_GATE	0x0c
+#define YT8511_EXT_DELAY_DRIVE	0x0d
+#define YT8511_EXT_SLEEP_CTRL	0x27
+
+/* 2b00 25m from pll
+ * 2b01 25m from xtl *default*
+ * 2b10 62.m from pll
+ * 2b11 125m from pll
+ */
+#define YT8511_CLK_125M		(BIT(2) | BIT(1))
+#define YT8511_PLLON_SLP	BIT(14)
+
+/* RX Delay enabled = 1.8ns 1000T, 8ns 10/100T */
+#define YT8511_DELAY_RX		BIT(0)
+
+/* TX Gig-E Delay is bits 7:4, default 0x5
+ * TX Fast-E Delay is bits 15:12, default 0xf
+ * Delay = 150ps * N - 250ps
+ * On = 2000ps, off = 50ps
+ */
+#define YT8511_DELAY_GE_TX_EN	(0xf << 4)
+#define YT8511_DELAY_GE_TX_DIS	(0x2 << 4)
+#define YT8511_DELAY_FE_TX_EN	(0xf << 12)
+#define YT8511_DELAY_FE_TX_DIS	(0x2 << 12)
+
 #define YT8531_SCR_SYNCE_ENABLE		BIT(6)
 /* 1b0 output 25m clock   *default*
  * 1b1 output 125m clock
@@ -347,6 +373,58 @@ static void ytphy_dt_parse(struct phy_device *phydev)
 		priv->flag |= TX_CLK_1000_INVERTED;
 }
 
+static int yt8511_config(struct phy_device *phydev)
+{
+	u32 ge, fe;
+	int ret;
+
+	ret = genphy_config_aneg(phydev);
+	if (ret < 0)
+		return ret;
+
+	switch (phydev->interface) {
+	case PHY_INTERFACE_MODE_RGMII:
+		ge = YT8511_DELAY_GE_TX_DIS;
+		fe = YT8511_DELAY_FE_TX_DIS;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+		ge = YT8511_DELAY_RX | YT8511_DELAY_GE_TX_DIS;
+		fe = YT8511_DELAY_FE_TX_DIS;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		ge = YT8511_DELAY_GE_TX_EN;
+		fe = YT8511_DELAY_FE_TX_EN;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_ID:
+		ge = YT8511_DELAY_RX | YT8511_DELAY_GE_TX_EN;
+		fe = YT8511_DELAY_FE_TX_EN;
+		break;
+	default: /* do not support other modes */
+		return -EOPNOTSUPP;
+	}
+
+	ret = ytphy_modify_ext(phydev, YT8511_EXT_CLK_GATE,
+			       (YT8511_DELAY_RX | YT8511_DELAY_GE_TX_EN), ge);
+	if (ret < 0)
+		return ret;
+	/* set clock mode to 125m */
+	ret = ytphy_modify_ext(phydev, YT8511_EXT_CLK_GATE,
+			       YT8511_CLK_125M, YT8511_CLK_125M);
+	if (ret < 0)
+		return ret;
+	ret = ytphy_modify_ext(phydev, YT8511_EXT_DELAY_DRIVE,
+			       YT8511_DELAY_FE_TX_EN, fe);
+	if (ret < 0)
+		return ret;
+	/* sleep control, disable PLL in sleep for now */
+	ret = ytphy_modify_ext(phydev, YT8511_EXT_SLEEP_CTRL, YT8511_PLLON_SLP,
+			       0);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int yt8531_config(struct phy_device *phydev)
 {
 	struct ytphy_plat_priv	*priv = phydev->priv;
@@ -425,6 +503,16 @@ static int yt8531_probe(struct phy_device *phydev)
 	return 0;
 }
 
+U_BOOT_PHY_DRIVER(motorcomm8511) = {
+	.name          = "YT8511 Gigabit Ethernet",
+	.uid           = PHY_ID_YT8511,
+	.mask          = PHY_ID_MASK,
+	.features      = PHY_GBIT_FEATURES,
+	.config        = &yt8511_config,
+	.startup       = &genphy_startup,
+	.shutdown      = &genphy_shutdown,
+};
+
 U_BOOT_PHY_DRIVER(motorcomm8531) = {
 	.name          = "YT8531 Gigabit Ethernet",
 	.uid           = PHY_ID_YT8531,

drivers/net/zynq_gem.c

@@ -890,7 +890,8 @@ static int zynq_gem_probe(struct udevice *dev)
 	if (ret)
 		goto err3;
 
-	if (priv->interface == PHY_INTERFACE_MODE_SGMII && phy.dev) {
+	if (priv->interface == PHY_INTERFACE_MODE_SGMII &&
+	    generic_phy_valid(&phy)) {
 		if (IS_ENABLED(CONFIG_DM_ETH_PHY)) {
 			if (device_is_compatible(dev, "cdns,zynqmp-gem") ||
 			    device_is_compatible(dev, "xlnx,zynqmp-gem")) {

drivers/phy/phy-uclass.c

@@ -195,6 +195,7 @@ int generic_phy_get_by_index_nodev(ofnode node, int index, struct phy *phy)
 	return 0;
 
 err:
+	phy->dev = NULL;
 	return ret;
 }
 
@@ -211,6 +212,9 @@ int generic_phy_get_by_name(struct udevice *dev, const char *phy_name,
 
 	debug("%s(dev=%p, name=%s, phy=%p)\n", __func__, dev, phy_name, phy);
 
+	assert(phy);
+	phy->dev = NULL;
+
 	index = dev_read_stringlist_search(dev, "phy-names", phy_name);
 	if (index < 0) {
 		debug("dev_read_stringlist_search() failed: %d\n", index);
@@ -506,44 +510,35 @@ int generic_phy_power_off_bulk(struct phy_bulk *bulk)
 
 int generic_setup_phy(struct udevice *dev, struct phy *phy, int index)
 {
-	int ret = 0;
+	int ret;
-
-	if (!phy)
-		return 0;
 
 	ret = generic_phy_get_by_index(dev, index, phy);
-	if (ret) {
+	if (ret)
-		if (ret != -ENOENT)
+		return ret == -ENOENT ? 0 : ret;
-			return ret;
-	} else {
-		ret = generic_phy_init(phy);
-		if (ret)
-			return ret;
 
-		ret = generic_phy_power_on(phy);
+	ret = generic_phy_init(phy);
-		if (ret)
+	if (ret)
-			ret = generic_phy_exit(phy);
+		return ret;
-	}
+
+	ret = generic_phy_power_on(phy);
+	if (ret)
+		generic_phy_exit(phy);
 
 	return ret;
 }
 
 int generic_shutdown_phy(struct phy *phy)
 {
-	int ret = 0;
+	int ret;
 
-	if (!phy)
+	if (!generic_phy_valid(phy))
 		return 0;
 
-	if (generic_phy_valid(phy)) {
+	ret = generic_phy_power_off(phy);
-		ret = generic_phy_power_off(phy);
+	if (ret)
-		if (ret)
+		return ret;
-			return ret;
 
-		ret = generic_phy_exit(phy);
+	return generic_phy_exit(phy);
-	}
-
-	return ret;
 }
 
 UCLASS_DRIVER(phy) = {

drivers/usb/dwc3/dwc3-generic.c

@@ -541,8 +541,6 @@ int dwc3_glue_probe(struct udevice *dev)
 	} else if (ret != -ENOENT && ret != -ENODATA) {
 		debug("could not get phy (err %d)\n", ret);
 		return ret;
-	} else {
-		phy.dev = NULL;
 	}
 
 	glue->regs = dev_read_addr_size_index(dev, 0, &glue->size);
@@ -555,7 +553,7 @@ int dwc3_glue_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	if (phy.dev) {
+	if (generic_phy_valid(&phy)) {
 		ret = generic_phy_power_on(&phy);
 		if (ret)
 			return ret;

drivers/video/rockchip/dw_mipi_dsi_rockchip.c

@@ -377,7 +377,7 @@ static int dsi_phy_init(void *priv_data)
 	struct dw_rockchip_dsi_priv *dsi = dev_get_priv(dev);
 	int ret, i, vco;
 
-	if (dsi->phy.dev) {
+	if (generic_phy_valid(&dsi->phy)) {
 		ret = generic_phy_configure(&dsi->phy, &dsi->phy_opts);
 		if (ret) {
 			dev_err(dsi->dsi_host,
@@ -559,7 +559,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, struct display_timing *timings,
 	}
 
 	/* for external phy only the mipi_dphy_config is necessary */
-	if (dsi->phy.dev) {
+	if (generic_phy_valid(&dsi->phy)) {
 		phy_mipi_dphy_get_default_config(timings->pixelclock.typ  * 10 / 8,
 						 bpp, lanes,
 						 &dsi->phy_opts);
@@ -859,7 +859,7 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev)
 	}
 
 	/* Get a ref clock only if not using an external phy. */
-	if (priv->phy.dev) {
+	if (generic_phy_valid(&priv->phy)) {
 		dev_dbg(dev, "setting priv->ref to NULL\n");
 		priv->ref = NULL;
 

test/dm/phy.c

@@ -29,7 +29,9 @@ static int dm_test_phy_base(struct unit_test_state *uts)
 	 * Get the same phy port in 2 different ways and compare.
 	 */
 	ut_assertok(generic_phy_get_by_name(parent, "phy1", &phy1_method1));
+	ut_assert(generic_phy_valid(&phy1_method1));
 	ut_assertok(generic_phy_get_by_index(parent, 0, &phy1_method2));
+	ut_assert(generic_phy_valid(&phy1_method2));
 	ut_asserteq(phy1_method1.id, phy1_method2.id);
 
 	/*
@@ -47,9 +49,23 @@ static int dm_test_phy_base(struct unit_test_state *uts)
 	ut_assert(phy2.dev != phy3.dev);
 
 	/* Try to get a non-existing phy */
-	ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 4, &dev));
+	ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 5, &dev));
 	ut_asserteq(-ENODATA, generic_phy_get_by_name(parent,
 					"phy_not_existing", &phy1_method1));
+	ut_assert(!generic_phy_valid(&phy1_method1));
+	ut_asserteq(-ENOENT, generic_phy_get_by_index(parent, 3,
+						      &phy1_method2));
+	ut_assert(!generic_phy_valid(&phy1_method2));
+
+	/* Try to get a phy where of_xlate fail */
+	ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS,
+					      "gen_phy_user2", &parent));
+	ut_asserteq(-EINVAL, generic_phy_get_by_name(parent, "phy1",
+						     &phy1_method1));
+	ut_assert(!generic_phy_valid(&phy1_method1));
+	ut_asserteq(-EINVAL, generic_phy_get_by_index(parent, 0,
+						      &phy1_method2));
+	ut_assert(!generic_phy_valid(&phy1_method2));
 
 	return 0;
 }
@@ -218,3 +234,32 @@ static int dm_test_phy_multi_exit(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_phy_multi_exit, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+static int dm_test_phy_setup(struct unit_test_state *uts)
+{
+	struct phy phy;
+	struct udevice *parent;
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS,
+					      "gen_phy_user", &parent));
+
+	/* normal */
+	ut_assertok(generic_setup_phy(parent, &phy, 0));
+	ut_assertok(generic_shutdown_phy(&phy));
+
+	/* power_off fail with -EIO */
+	ut_assertok(generic_setup_phy(parent, &phy, 1));
+	ut_asserteq(-EIO, generic_shutdown_phy(&phy));
+
+	/* power_on fail with -EIO */
+	ut_asserteq(-EIO, generic_setup_phy(parent, &phy, 2));
+	ut_assertok(generic_shutdown_phy(&phy));
+
+	/* generic_phy_get_by_index fail with -ENOENT */
+	ut_asserteq(-ENOENT, generic_phy_get_by_index(parent, 3, &phy));
+	ut_assertok(generic_setup_phy(parent, &phy, 3));
+	ut_assertok(generic_shutdown_phy(&phy));
+
+	return 0;
+}
+DM_TEST(dm_test_phy_setup, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);