[PATCH net-next v1 1/2] net: phy: c45: add genphy_c45_pma_read_ext_abilities() function

[PATCH net-next v1 1/2] net: phy: c45: add genphy_c45_pma_read_ext_abilities() function

[Oleksij Rempel]

Move part of the genphy_c45_pma_read_abilities() code to a separate
function.

Some PHYs do not implement PMA/PMD status 2 register (Register 1.8) but
do implement PMA/PMD extended ability register (Register 1.11). To make
use of it, we need to be able to access this part of code separately.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy-c45.c | 129 ++++++++++++++++++++++----------------
 include/linux/phy.h       |   1 +
 2 files changed, 75 insertions(+), 55 deletions(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 8e6fd4962c48..747d14bf152c 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -919,6 +919,79 @@ int genphy_c45_pma_baset1_read_abilities(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_pma_baset1_read_abilities);
 
+/**
+ * genphy_c45_pma_read_ext_abilities - read supported link modes from PMA
+ * @phydev: target phy_device struct
+ *
+ * Read the supported link modes from the PMA/PMD extended ability register
+ * (Register 1.11).
+ */
+int genphy_c45_pma_read_ext_abilities(struct phy_device *phydev)
+{
+	int val;
+
+	val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE);
+	if (val < 0)
+		return val;
+
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_10GBLRM);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_10GBT);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_10GBKX4);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_10GBKR);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_1000BT);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_1000BKX);
+
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_100BTX);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_100BTX);
+
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_10BT);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+			 phydev->supported,
+			 val & MDIO_PMA_EXTABLE_10BT);
+
+	if (val & MDIO_PMA_EXTABLE_NBT) {
+		val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
+				   MDIO_PMA_NG_EXTABLE);
+		if (val < 0)
+			return val;
+
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+				 phydev->supported,
+				 val & MDIO_PMA_NG_EXTABLE_2_5GBT);
+
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+				 phydev->supported,
+				 val & MDIO_PMA_NG_EXTABLE_5GBT);
+	}
+
+	if (val & MDIO_PMA_EXTABLE_BT1) {
+		val = genphy_c45_pma_baset1_read_abilities(phydev);
+		if (val < 0)
+			return val;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_pma_read_ext_abilities);
+
 /**
  * genphy_c45_pma_read_abilities - read supported link modes from PMA
  * @phydev: target phy_device struct
@@ -962,63 +1035,9 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev)
 			 val & MDIO_PMA_STAT2_10GBER);
 
 	if (val & MDIO_PMA_STAT2_EXTABLE) {
-		val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_EXTABLE);
+		val = genphy_c45_pma_read_ext_abilities(phydev);
 		if (val < 0)
 			return val;
-
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_10GBLRM);
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_10GBT);
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_10GBKX4);
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_10GBKR);
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_1000BT);
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_1000BKX);
-
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_100BTX);
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_100BTX);
-
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_10BT);
-		linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
-				 phydev->supported,
-				 val & MDIO_PMA_EXTABLE_10BT);
-
-		if (val & MDIO_PMA_EXTABLE_NBT) {
-			val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD,
-					   MDIO_PMA_NG_EXTABLE);
-			if (val < 0)
-				return val;
-
-			linkmode_mod_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
-					 phydev->supported,
-					 val & MDIO_PMA_NG_EXTABLE_2_5GBT);
-
-			linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
-					 phydev->supported,
-					 val & MDIO_PMA_NG_EXTABLE_5GBT);
-		}
-
-		if (val & MDIO_PMA_EXTABLE_BT1) {
-			val = genphy_c45_pma_baset1_read_abilities(phydev);
-			if (val < 0)
-				return val;
-		}
 	}
 
 	/* This is optional functionality. If not supported, we may get an error
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 6e7ebcc50b85..dbb5e13e3e1b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1866,6 +1866,7 @@ int genphy_c45_an_config_aneg(struct phy_device *phydev);
 int genphy_c45_an_disable_aneg(struct phy_device *phydev);
 int genphy_c45_read_mdix(struct phy_device *phydev);
 int genphy_c45_pma_read_abilities(struct phy_device *phydev);
+int genphy_c45_pma_read_ext_abilities(struct phy_device *phydev);
 int genphy_c45_pma_baset1_read_abilities(struct phy_device *phydev);
 int genphy_c45_read_eee_abilities(struct phy_device *phydev);
 int genphy_c45_pma_baset1_read_master_slave(struct phy_device *phydev);
-- 
2.39.2

[PATCH net-next v1 2/2] net: phy: Add support for the DP83TG720S Ethernet PHY

[Oleksij Rempel]

The DP83TG720S-Q1 device is an IEEE 802.3bp and Open Alliance compliant
automotive Ethernet physical layer transceiver.

This driver was tested with i.MX8MP EQOS (stmmac) on the MAC side and
TI same PHY on other side.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/Kconfig     |  13 +++
 drivers/net/phy/Makefile    |   1 +
 drivers/net/phy/dp83tg720.c | 190 ++++++++++++++++++++++++++++++++++++
 3 files changed, 204 insertions(+)
 create mode 100644 drivers/net/phy/dp83tg720.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 25cfc5ded1da..bab10c796f24 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -372,6 +372,19 @@ config DP83TC811_PHY
 	help
 	  Supports the DP83TC811 PHY.
 
+config DP83TG720_PHY
+	tristate "Texas Instruments DP83TG720 Ethernet 1000Base-T1 PHY"
+	help
+	  The DP83TG720S-Q1 is an automotive Ethernet physical layer
+	  transceiver compliant with IEEE 802.3bp and Open Alliance
+	  standards. It supports key functions necessary for
+	  transmitting and receiving data over both unshielded and
+	  shielded single twisted-pair cables. This device offers
+	  flexible xMII interface options, including support for both
+	  RGMII and SGMII MAC interfaces. It's suitable for applications
+	  requiring high-speed data transmission in automotive
+	  networking environments.
+
 config DP83848_PHY
 	tristate "Texas Instruments DP83848 PHY"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index f65e85c91fc1..defaef190962 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_DP83848_PHY)	+= dp83848.o
 obj-$(CONFIG_DP83867_PHY)	+= dp83867.o
 obj-$(CONFIG_DP83869_PHY)	+= dp83869.o
 obj-$(CONFIG_DP83TC811_PHY)	+= dp83tc811.o
+obj-$(CONFIG_DP83TG720_PHY)	+= dp83tg720.o
 obj-$(CONFIG_DP83TD510_PHY)	+= dp83td510.o
 obj-$(CONFIG_FIXED_PHY)		+= fixed_phy.o
 obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
new file mode 100644
index 000000000000..133952e88c6c
--- /dev/null
+++ b/drivers/net/phy/dp83tg720.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Driver for the Texas Instruments DP83TG720 PHY
+ * Copyright (c) 2023 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+ */
+#include <linux/bitfield.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/phy.h>
+
+#define DP83TG720S_PHY_ID			0x2000a284
+
+/* MDIO_MMD_VEND2 registers */
+#define DP83TG720S_MII_REG_10			0x10
+#define DP83TG720S_STS_MII_INT			BIT(7)
+#define DP83TG720S_LINK_STATUS			BIT(0)
+
+#define DP83TG720S_PHY_RESET			0x1f
+#define DP83TG720S_HW_RESET			BIT(15)
+
+#define DP83TG720S_RGMII_DELAY_CTRL		0x602
+/* In RGMII mode, Enable or disable the internal delay for RXD */
+#define DP83TG720S_RGMII_RX_CLK_SEL		BIT(1)
+/* In RGMII mode, Enable or disable the internal delay for TXD */
+#define DP83TG720S_RGMII_TX_CLK_SEL		BIT(0)
+
+#define DP83TG720S_SQI_REG_1			0x871
+#define DP83TG720S_SQI_OUT_WORST		GENMASK(7, 5)
+#define DP83TG720S_SQI_OUT			GENMASK(3, 1)
+
+#define DP83TG720_SQI_MAX			7
+
+static int dp83tg720_read_status(struct phy_device *phydev)
+{
+	u16 phy_sts;
+	int ret;
+
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	/* Most of Clause 45 registers are not present, so we can't use
+	 * genphy_c45_read_status() here.
+	 */
+	phy_sts = phy_read(phydev, DP83TG720S_MII_REG_10);
+	phydev->link = !!(phy_sts & DP83TG720S_LINK_STATUS);
+	if (!phydev->link) {
+		/* According to the "DP83TC81x, DP83TG72x Software
+		 * Implementation Guide", the PHY needs to be reset after a
+		 * link loss or if no link is created after at least 100ms.
+		 *
+		 * Currently we are polling with the PHY_STATE_TIME (1000ms)
+		 * interval, which is still enough for not automotive use cases.
+		 */
+		ret = phy_init_hw(phydev);
+		if (ret)
+			return ret;
+
+		/* After HW reset we need to restore master/slave configuration.
+		 */
+		if (phydev->drv->config_aneg) {
+			ret = phydev->drv->config_aneg(phydev);
+			if (ret)
+				return ret;
+		}
+
+		phydev->speed = SPEED_UNKNOWN;
+		phydev->duplex = DUPLEX_UNKNOWN;
+	} else {
+		/* PMA/PMD control 1 register (Register 1.0) is present, but it
+		 * doesn't contain the link speed information.
+		 * So genphy_c45_read_pma() can't be used here.
+		 */
+		ret = genphy_c45_pma_baset1_read_master_slave(phydev);
+		if (ret)
+			return ret;
+
+		phydev->duplex = DUPLEX_FULL;
+		phydev->speed = SPEED_1000;
+	}
+
+	return 0;
+}
+
+static int dp83tg720_config_aneg(struct phy_device *phydev)
+{
+	/* Autoneg is not supported and this PHY supports only one speed.
+	 * We need to care only about master/slave configuration if it was
+	 * changed by user.
+	 */
+	return genphy_c45_pma_baset1_setup_master_slave(phydev);
+}
+
+static int dp83tg720_get_sqi(struct phy_device *phydev)
+{
+	int ret;
+
+	if (!phydev->link)
+		return 0;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_SQI_REG_1);
+	if (ret < 0)
+		return ret;
+
+	return FIELD_GET(DP83TG720S_SQI_OUT, ret);
+}
+
+static int dp83tg720_get_sqi_max(struct phy_device *phydev)
+{
+	return DP83TG720_SQI_MAX;
+}
+
+static int dp83tg720_config_rgmii_delay(struct phy_device *phydev)
+{
+	u16 rgmii_delay_mask;
+	u16 rgmii_delay = 0;
+
+	switch (phydev->interface) {
+	case PHY_INTERFACE_MODE_RGMII:
+		rgmii_delay = 0;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_ID:
+		rgmii_delay = DP83TG720S_RGMII_RX_CLK_SEL |
+				DP83TG720S_RGMII_TX_CLK_SEL;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+		rgmii_delay = DP83TG720S_RGMII_RX_CLK_SEL;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		rgmii_delay = DP83TG720S_RGMII_TX_CLK_SEL;
+		break;
+	default:
+		return 0;
+	}
+
+	rgmii_delay_mask = DP83TG720S_RGMII_RX_CLK_SEL |
+		DP83TG720S_RGMII_TX_CLK_SEL;
+
+	return phy_modify_mmd(phydev, MDIO_MMD_VEND2,
+			      DP83TG720S_RGMII_DELAY_CTRL, rgmii_delay_mask,
+			      rgmii_delay);
+}
+
+static int dp83tg720_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Software Restart is not enough to recover from a link failure.
+	 * Using Hardware Reset instead.
+	 */
+	ret = phy_write(phydev, DP83TG720S_PHY_RESET, DP83TG720S_HW_RESET);
+	if (ret)
+		return ret;
+
+	/* Wait until MDC can be used again.
+	 * The wait value of one 1ms is documented in "DP83TG720S-Q1 1000BASE-T1
+	 * Automotive Ethernet PHY with SGMII and RGMII" datasheet.
+	 */
+	usleep_range(1000, 2000);
+
+	if (phy_interface_is_rgmii(phydev))
+		return dp83tg720_config_rgmii_delay(phydev);
+
+	return 0;
+}
+
+static struct phy_driver dp83tg720_driver[] = {
+{
+	PHY_ID_MATCH_MODEL(DP83TG720S_PHY_ID),
+	.name		= "TI DP83TG720S",
+
+	.config_aneg	= dp83tg720_config_aneg,
+	.read_status	= dp83tg720_read_status,
+	.get_features	= genphy_c45_pma_read_ext_abilities,
+	.config_init	= dp83tg720_config_init,
+	.get_sqi	= dp83tg720_get_sqi,
+	.get_sqi_max	= dp83tg720_get_sqi_max,
+
+	.suspend	= genphy_suspend,
+	.resume		= genphy_resume,
+} };
+module_phy_driver(dp83tg720_driver);
+
+static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
+	{ PHY_ID_MATCH_MODEL(DP83TG720S_PHY_ID) },
+	{ }
+};
+MODULE_DEVICE_TABLE(mdio, dp83tg720_tbl);
+
+MODULE_DESCRIPTION("Texas Instruments DP83TG720S PHY driver");
+MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
+MODULE_LICENSE("GPL");
-- 
2.39.2

Re: [PATCH net-next v1 2/2] net: phy: Add support for the DP83TG720S Ethernet PHY

[Andrew Lunn]

On Fri, Dec 08, 2023 at 04:11:59PM +0100, Oleksij Rempel wrote:
> The DP83TG720S-Q1 device is an IEEE 802.3bp and Open Alliance compliant
> automotive Ethernet physical layer transceiver.
> 
> This driver was tested with i.MX8MP EQOS (stmmac) on the MAC side and
> TI same PHY on other side.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/Kconfig     |  13 +++
>  drivers/net/phy/Makefile    |   1 +
>  drivers/net/phy/dp83tg720.c | 190 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 204 insertions(+)
>  create mode 100644 drivers/net/phy/dp83tg720.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 25cfc5ded1da..bab10c796f24 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -372,6 +372,19 @@ config DP83TC811_PHY
>  	help
>  	  Supports the DP83TC811 PHY.
>  
> +config DP83TG720_PHY
> +	tristate "Texas Instruments DP83TG720 Ethernet 1000Base-T1 PHY"
> +	help
> +	  The DP83TG720S-Q1 is an automotive Ethernet physical layer
> +	  transceiver compliant with IEEE 802.3bp and Open Alliance
> +	  standards. It supports key functions necessary for
> +	  transmitting and receiving data over both unshielded and
> +	  shielded single twisted-pair cables. This device offers
> +	  flexible xMII interface options, including support for both
> +	  RGMII and SGMII MAC interfaces. It's suitable for applications
> +	  requiring high-speed data transmission in automotive
> +	  networking environments.
> +
>  config DP83848_PHY
>  	tristate "Texas Instruments DP83848 PHY"
>  	help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index f65e85c91fc1..defaef190962 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_DP83848_PHY)	+= dp83848.o
>  obj-$(CONFIG_DP83867_PHY)	+= dp83867.o
>  obj-$(CONFIG_DP83869_PHY)	+= dp83869.o
>  obj-$(CONFIG_DP83TC811_PHY)	+= dp83tc811.o
> +obj-$(CONFIG_DP83TG720_PHY)	+= dp83tg720.o
>  obj-$(CONFIG_DP83TD510_PHY)	+= dp83td510.o

Maybe it should come after CONFIG_DP83TD510_PHY in a strict sort ? I
also wounder about the Kconfig, which should be sorted on the tristate
string.

>  obj-$(CONFIG_FIXED_PHY)		+= fixed_phy.o
>  obj-$(CONFIG_ICPLUS_PHY)	+= icplus.o
> +		/* After HW reset we need to restore master/slave configuration.
> +		 */
> +		if (phydev->drv->config_aneg) {

This test is a bit strange. You know it exists, its this driver and
the function is there. Why not call it directly?

> +			ret = phydev->drv->config_aneg(phydev);
> +			if (ret)
> +				return ret;
> +		}


    Andrew

---
pw-bot: cr

Re: [PATCH net-next v1 1/2] net: phy: c45: add genphy_c45_pma_read_ext_abilities() function

[Andrew Lunn]

On Fri, Dec 08, 2023 at 04:11:58PM +0100, Oleksij Rempel wrote:
> Move part of the genphy_c45_pma_read_abilities() code to a separate
> function.
> 
> Some PHYs do not implement PMA/PMD status 2 register (Register 1.8) but
> do implement PMA/PMD extended ability register (Register 1.11). To make
> use of it, we need to be able to access this part of code separately.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew