[PATCH 0/5] Extending features on DP83TG720 driver

[PATCH 0/5] Extending features on DP83TG720 driver

[Alvaro (Al-vuh-roe) Reyes]

The DP83TG720S-Q1 device is an IEEE 802.3bp
and Open Alliance compliant automotive Ethernet
physical layer transceiver. It provides all physical layer
functions needed to transmit and receive data over
unshielded/shielded single twisted-pair cables.

Changes include changing Macros Names, adding SGMII support, extending
support to DP83TG721 PHY, and adding the Open Alliance initialization script.

Each of these changes are a separate patch and are meant to
be applied in order.

The following test cases were validated:
PHY detection
PHY link up/down
Speed/Duplex mode
PHY register access
Ping test
Throughput test
SQI test
TDR test
Forced master-slave

Changes in v2:
1. split all changes made into different patches
2. fixed typos in original patch descriptions

Alvaro (Al-vuh-roe) Reyes (5):
  net: phy: dp83tg720: Changed Macro names
  net: phy: dp83tg720: Added SGMII Support
  net: phy: dp83tg720: Extending support to DP83TG721 PHY
  net: phy: dp83tg720: Added OA script
  net: phy: dp83tg720: fixed Linux coding standards issues

 drivers/net/phy/dp83tg720.c | 582 +++++++++++++++++++++++++++++++-----
 1 file changed, 504 insertions(+), 78 deletions(-)

-- 
2.17.1

[PATCH 1/5] net: phy: dp83tg720: Changed Macro names

[Alvaro (Al-vuh-roe) Reyes]

Previous macro referes to DP83TG720S, where this driver works for both
DP83TG720R & DP83TG720S. Macro changed to DP83TG720 to be more generic.

Data sheets:
https://www.ti.com/lit/ds/symlink/dp83tg720s-q1.pdf
https://www.ti.com/lit/ds/symlink/dp83tg720r-q1.pdf

Signed-off-by: Alvaro (Al-vuh-roe) Reyes <a-reyes1@ti.com>
---
 drivers/net/phy/dp83tg720.c | 116 ++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 58 deletions(-)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index 0ef4d7dba065..7e81800cfc5b 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -10,62 +10,62 @@
 
 #include "open_alliance_helpers.h"
 
-#define DP83TG720S_PHY_ID			0x2000a284
+#define DP83TG720_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 DP83TG720_MII_REG_10			0x10
+#define DP83TG720_STS_MII_INT			BIT(7)
+#define DP83TG720_LINK_STATUS			BIT(0)
 
 /* TDR Configuration Register (0x1E) */
-#define DP83TG720S_TDR_CFG			0x1e
+#define DP83TG720_TDR_CFG			0x1e
 /* 1b = TDR start, 0b = No TDR */
-#define DP83TG720S_TDR_START			BIT(15)
+#define DP83TG720_TDR_START			BIT(15)
 /* 1b = TDR auto on link down, 0b = Manual TDR start */
-#define DP83TG720S_CFG_TDR_AUTO_RUN		BIT(14)
+#define DP83TG720_CFG_TDR_AUTO_RUN		BIT(14)
 /* 1b = TDR done, 0b = TDR in progress */
-#define DP83TG720S_TDR_DONE			BIT(1)
+#define DP83TG720_TDR_DONE			BIT(1)
 /* 1b = TDR fail, 0b = TDR success */
-#define DP83TG720S_TDR_FAIL			BIT(0)
+#define DP83TG720_TDR_FAIL			BIT(0)
 
-#define DP83TG720S_PHY_RESET			0x1f
-#define DP83TG720S_HW_RESET			BIT(15)
+#define DP83TG720_PHY_RESET			0x1f
+#define DP83TG720_HW_RESET			BIT(15)
 
-#define DP83TG720S_LPS_CFG3			0x18c
+#define DP83TG720_LPS_CFG3			0x18c
 /* Power modes are documented as bit fields but used as values */
 /* Power Mode 0 is Normal mode */
-#define DP83TG720S_LPS_CFG3_PWR_MODE_0		BIT(0)
+#define DP83TG720_LPS_CFG3_PWR_MODE_0		BIT(0)
 
 /* Open Aliance 1000BaseT1 compatible HDD.TDR Fault Status Register */
-#define DP83TG720S_TDR_FAULT_STATUS		0x30f
+#define DP83TG720_TDR_FAULT_STATUS		0x30f
 
 /* Register 0x0301: TDR Configuration 2 */
-#define DP83TG720S_TDR_CFG2			0x301
+#define DP83TG720_TDR_CFG2			0x301
 
 /* Register 0x0303: TDR Configuration 3 */
-#define DP83TG720S_TDR_CFG3			0x303
+#define DP83TG720_TDR_CFG3			0x303
 
 /* Register 0x0304: TDR Configuration 4 */
-#define DP83TG720S_TDR_CFG4			0x304
+#define DP83TG720_TDR_CFG4			0x304
 
 /* Register 0x0405: Unknown Register */
-#define DP83TG720S_UNKNOWN_0405			0x405
+#define DP83TG720_UNKNOWN_0405			0x405
 
 /* Register 0x0576: TDR Master Link Down Control */
-#define DP83TG720S_TDR_MASTER_LINK_DOWN		0x576
+#define DP83TG720_TDR_MASTER_LINK_DOWN		0x576
 
-#define DP83TG720S_RGMII_DELAY_CTRL		0x602
+#define DP83TG720_RGMII_DELAY_CTRL		0x602
 /* In RGMII mode, Enable or disable the internal delay for RXD */
-#define DP83TG720S_RGMII_RX_CLK_SEL		BIT(1)
+#define DP83TG720_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 DP83TG720_RGMII_TX_CLK_SEL		BIT(0)
 
 /* Register 0x083F: Unknown Register */
-#define DP83TG720S_UNKNOWN_083F			0x83f
+#define DP83TG720_UNKNOWN_083F			0x83f
 
-#define DP83TG720S_SQI_REG_1			0x871
-#define DP83TG720S_SQI_OUT_WORST		GENMASK(7, 5)
-#define DP83TG720S_SQI_OUT			GENMASK(3, 1)
+#define DP83TG720_SQI_REG_1			0x871
+#define DP83TG720_SQI_OUT_WORST		GENMASK(7, 5)
+#define DP83TG720_SQI_OUT			GENMASK(3, 1)
 
 #define DP83TG720_SQI_MAX			7
 
@@ -82,7 +82,7 @@ static int dp83tg720_cable_test_start(struct phy_device *phydev)
 	int ret;
 
 	/* Initialize the PHY to run the TDR test as described in the
-	 * "DP83TG720S-Q1: Configuring for Open Alliance Specification
+	 * "DP83TG720-Q1: Configuring for Open Alliance Specification
 	 * Compliance (Rev. B)" application note.
 	 * Most of the registers are not documented. Some of register names
 	 * are guessed by comparing the register offsets with the DP83TD510E.
@@ -90,38 +90,38 @@ static int dp83tg720_cable_test_start(struct phy_device *phydev)
 
 	/* Force master link down */
 	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
-			       DP83TG720S_TDR_MASTER_LINK_DOWN, 0x0400);
+			       DP83TG720_TDR_MASTER_LINK_DOWN, 0x0400);
 	if (ret)
 		return ret;
 
-	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG2,
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_TDR_CFG2,
 			    0xa008);
 	if (ret)
 		return ret;
 
-	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG3,
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_TDR_CFG3,
 			    0x0928);
 	if (ret)
 		return ret;
 
-	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG4,
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_TDR_CFG4,
 			    0x0004);
 	if (ret)
 		return ret;
 
-	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_UNKNOWN_0405,
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_UNKNOWN_0405,
 			    0x6400);
 	if (ret)
 		return ret;
 
-	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_UNKNOWN_083F,
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_UNKNOWN_083F,
 			    0x3003);
 	if (ret)
 		return ret;
 
 	/* Start the TDR */
-	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG,
-			       DP83TG720S_TDR_START);
+	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_TDR_CFG,
+			       DP83TG720_TDR_START);
 	if (ret)
 		return ret;
 
@@ -146,21 +146,21 @@ static int dp83tg720_cable_test_get_status(struct phy_device *phydev,
 	*finished = false;
 
 	/* Read the TDR status */
-	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG);
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_TDR_CFG);
 	if (ret < 0)
 		return ret;
 
 	/* Check if the TDR test is done */
-	if (!(ret & DP83TG720S_TDR_DONE))
+	if (!(ret & DP83TG720_TDR_DONE))
 		return 0;
 
 	/* Check for TDR test failure */
-	if (!(ret & DP83TG720S_TDR_FAIL)) {
+	if (!(ret & DP83TG720_TDR_FAIL)) {
 		int location;
 
 		/* Read fault status */
 		ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
-				   DP83TG720S_TDR_FAULT_STATUS);
+				   DP83TG720_TDR_FAULT_STATUS);
 		if (ret < 0)
 			return ret;
 
@@ -214,8 +214,8 @@ static int dp83tg720_read_status(struct phy_device *phydev)
 	/* 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);
+	phy_sts = phy_read(phydev, DP83TG720_MII_REG_10);
+	phydev->link = !!(phy_sts & DP83TG720_LINK_STATUS);
 	if (!phydev->link) {
 		/* According to the "DP83TC81x, DP83TG72x Software
 		 * Implementation Guide", the PHY needs to be reset after a
@@ -261,11 +261,11 @@ static int dp83tg720_get_sqi(struct phy_device *phydev)
 	if (!phydev->link)
 		return 0;
 
-	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_SQI_REG_1);
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_SQI_REG_1);
 	if (ret < 0)
 		return ret;
 
-	return FIELD_GET(DP83TG720S_SQI_OUT, ret);
+	return FIELD_GET(DP83TG720_SQI_OUT, ret);
 }
 
 static int dp83tg720_get_sqi_max(struct phy_device *phydev)
@@ -283,24 +283,24 @@ static int dp83tg720_config_rgmii_delay(struct phy_device *phydev)
 		rgmii_delay = 0;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_ID:
-		rgmii_delay = DP83TG720S_RGMII_RX_CLK_SEL |
-				DP83TG720S_RGMII_TX_CLK_SEL;
+		rgmii_delay = DP83TG720_RGMII_RX_CLK_SEL |
+				DP83TG720_RGMII_TX_CLK_SEL;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_RXID:
-		rgmii_delay = DP83TG720S_RGMII_RX_CLK_SEL;
+		rgmii_delay = DP83TG720_RGMII_RX_CLK_SEL;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		rgmii_delay = DP83TG720S_RGMII_TX_CLK_SEL;
+		rgmii_delay = DP83TG720_RGMII_TX_CLK_SEL;
 		break;
 	default:
 		return 0;
 	}
 
-	rgmii_delay_mask = DP83TG720S_RGMII_RX_CLK_SEL |
-		DP83TG720S_RGMII_TX_CLK_SEL;
+	rgmii_delay_mask = DP83TG720_RGMII_RX_CLK_SEL |
+		DP83TG720_RGMII_TX_CLK_SEL;
 
 	return phy_modify_mmd(phydev, MDIO_MMD_VEND2,
-			      DP83TG720S_RGMII_DELAY_CTRL, rgmii_delay_mask,
+			      DP83TG720_RGMII_DELAY_CTRL, rgmii_delay_mask,
 			      rgmii_delay);
 }
 
@@ -311,12 +311,12 @@ static int dp83tg720_config_init(struct phy_device *phydev)
 	/* 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);
+	ret = phy_write(phydev, DP83TG720_PHY_RESET, DP83TG720_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
+	 * The wait value of one 1ms is documented in "DP83TG720-Q1 1000BASE-T1
 	 * Automotive Ethernet PHY with SGMII and RGMII" datasheet.
 	 */
 	usleep_range(1000, 2000);
@@ -330,8 +330,8 @@ static int dp83tg720_config_init(struct phy_device *phydev)
 	/* In case the PHY is bootstrapped in managed mode, we need to
 	 * wake it.
 	 */
-	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_LPS_CFG3,
-			    DP83TG720S_LPS_CFG3_PWR_MODE_0);
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720_LPS_CFG3,
+			    DP83TG720_LPS_CFG3_PWR_MODE_0);
 	if (ret)
 		return ret;
 
@@ -343,8 +343,8 @@ static int dp83tg720_config_init(struct phy_device *phydev)
 
 static struct phy_driver dp83tg720_driver[] = {
 {
-	PHY_ID_MATCH_MODEL(DP83TG720S_PHY_ID),
-	.name		= "TI DP83TG720S",
+	PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID),
+	.name		= "TI DP83TG720",
 
 	.flags          = PHY_POLL_CABLE_TEST,
 	.config_aneg	= dp83tg720_config_aneg,
@@ -362,11 +362,11 @@ static struct phy_driver dp83tg720_driver[] = {
 module_phy_driver(dp83tg720_driver);
 
 static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
-	{ PHY_ID_MATCH_MODEL(DP83TG720S_PHY_ID) },
+	{ PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID) },
 	{ }
 };
 MODULE_DEVICE_TABLE(mdio, dp83tg720_tbl);
 
-MODULE_DESCRIPTION("Texas Instruments DP83TG720S PHY driver");
+MODULE_DESCRIPTION("Texas Instruments DP83TG720 PHY driver");
 MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
 MODULE_LICENSE("GPL");
-- 
2.17.1

[PATCH 2/5] net: phy: dp83tg720: Added SGMII Support

[Alvaro (Al-vuh-roe) Reyes]

Adding SGMII Support to driver by checking if SGMII is enabled and
writing to the SGMII registers to ensure PHY is configured correctly.


Signed-off-by: Alvaro (Al-vuh-roe) Reyes <a-reyes1@ti.com>
---
 drivers/net/phy/dp83tg720.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index 7e81800cfc5b..a6f90293aa61 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -12,6 +12,9 @@
 
 #define DP83TG720_PHY_ID			0x2000a284
 
+#define MMD1F							0x1f
+#define MMD1							0x1
+
 /* MDIO_MMD_VEND2 registers */
 #define DP83TG720_MII_REG_10			0x10
 #define DP83TG720_STS_MII_INT			BIT(7)
@@ -69,6 +72,13 @@
 
 #define DP83TG720_SQI_MAX			7
 
+/* SGMII CTRL Registers/bits */
+#define DP83TG720_SGMII_CTRL			0x0608
+#define SGMII_CONFIG_VAL				0x027B
+#define DP83TG720_SGMII_AUTO_NEG_EN		BIT(0)
+#define DP83TG720_SGMII_EN				BIT(9)
+
+
 /**
  * dp83tg720_cable_test_start - Start the cable test for the DP83TG720 PHY.
  * @phydev: Pointer to the phy_device structure.
@@ -306,7 +316,7 @@ static int dp83tg720_config_rgmii_delay(struct phy_device *phydev)
 
 static int dp83tg720_config_init(struct phy_device *phydev)
 {
-	int ret;
+	int value, ret;
 
 	/* Software Restart is not enough to recover from a link failure.
 	 * Using Hardware Reset instead.
@@ -327,6 +337,19 @@ static int dp83tg720_config_init(struct phy_device *phydev)
 			return ret;
 	}
 
+	value = phy_read_mmd(phydev, MMD1F, DP83TG720_SGMII_CTRL);
+	if (value < 0)
+		return value;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
+		value |= DP83TG720_SGMII_EN;
+	else
+		value &= ~DP83TG720_SGMII_EN;
+
+	ret = phy_write_mmd(phydev, MMD1F, DP83TG720_SGMII_CTRL, value);
+	if (ret < 0)
+		return ret;
+
 	/* In case the PHY is bootstrapped in managed mode, we need to
 	 * wake it.
 	 */
-- 
2.17.1

[PATCH 3/5] net: phy: dp83tg720: Extending support to DP83TG721 PHY

[Alvaro (Al-vuh-roe) Reyes]

The DP83TG721 is the next revision of the DP83TG720 and will share the
same driver. Added PHY_ID and probe funtion to check which version is
being loaded. 

Signed-off-by: Alvaro (Al-vuh-roe) Reyes <a-reyes1@ti.com>
---
 drivers/net/phy/dp83tg720.c | 183 ++++++++++++++++++++++++++++--------
 1 file changed, 146 insertions(+), 37 deletions(-)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index a6f90293aa61..b70802818f3c 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -10,8 +10,8 @@
 
 #include "open_alliance_helpers.h"
 
-#define DP83TG720_PHY_ID			0x2000a284
-
+#define DP83TG720_CS_1_1_PHY_ID				0x2000a284
+#define DP83TG721_CS_1_0_PHY_ID			0x2000a290
 #define MMD1F							0x1f
 #define MMD1							0x1
 
@@ -21,41 +21,41 @@
 #define DP83TG720_LINK_STATUS			BIT(0)
 
 /* TDR Configuration Register (0x1E) */
-#define DP83TG720_TDR_CFG			0x1e
+#define DP83TG720_TDR_CFG				0x1e
 /* 1b = TDR start, 0b = No TDR */
-#define DP83TG720_TDR_START			BIT(15)
+#define DP83TG720_TDR_START				BIT(15)
 /* 1b = TDR auto on link down, 0b = Manual TDR start */
 #define DP83TG720_CFG_TDR_AUTO_RUN		BIT(14)
 /* 1b = TDR done, 0b = TDR in progress */
-#define DP83TG720_TDR_DONE			BIT(1)
+#define DP83TG720_TDR_DONE				BIT(1)
 /* 1b = TDR fail, 0b = TDR success */
-#define DP83TG720_TDR_FAIL			BIT(0)
+#define DP83TG720_TDR_FAIL				BIT(0)
 
-#define DP83TG720_PHY_RESET			0x1f
-#define DP83TG720_HW_RESET			BIT(15)
+#define DP83TG720_PHY_RESET				0x1f
+#define DP83TG720_HW_RESET				BIT(15)
 
-#define DP83TG720_LPS_CFG3			0x18c
+#define DP83TG720_LPS_CFG3				0x18c
 /* Power modes are documented as bit fields but used as values */
 /* Power Mode 0 is Normal mode */
-#define DP83TG720_LPS_CFG3_PWR_MODE_0		BIT(0)
+#define DP83TG720_LPS_CFG3_PWR_MODE_0	BIT(0)
 
 /* Open Aliance 1000BaseT1 compatible HDD.TDR Fault Status Register */
 #define DP83TG720_TDR_FAULT_STATUS		0x30f
 
 /* Register 0x0301: TDR Configuration 2 */
-#define DP83TG720_TDR_CFG2			0x301
+#define DP83TG720_TDR_CFG2				0x301
 
 /* Register 0x0303: TDR Configuration 3 */
-#define DP83TG720_TDR_CFG3			0x303
+#define DP83TG720_TDR_CFG3				0x303
 
 /* Register 0x0304: TDR Configuration 4 */
-#define DP83TG720_TDR_CFG4			0x304
+#define DP83TG720_TDR_CFG4				0x304
 
 /* Register 0x0405: Unknown Register */
 #define DP83TG720_UNKNOWN_0405			0x405
 
 /* Register 0x0576: TDR Master Link Down Control */
-#define DP83TG720_TDR_MASTER_LINK_DOWN		0x576
+#define DP83TG720_TDR_MASTER_LINK_DOWN	0x576
 
 #define DP83TG720_RGMII_DELAY_CTRL		0x602
 /* In RGMII mode, Enable or disable the internal delay for RXD */
@@ -66,11 +66,11 @@
 /* Register 0x083F: Unknown Register */
 #define DP83TG720_UNKNOWN_083F			0x83f
 
-#define DP83TG720_SQI_REG_1			0x871
-#define DP83TG720_SQI_OUT_WORST		GENMASK(7, 5)
-#define DP83TG720_SQI_OUT			GENMASK(3, 1)
+#define DP83TG720_SQI_REG_1				0x871
+#define DP83TG720_SQI_OUT_WORST			GENMASK(7, 5)
+#define DP83TG720_SQI_OUT				GENMASK(3, 1)
 
-#define DP83TG720_SQI_MAX			7
+#define DP83TG720_SQI_MAX				7
 
 /* SGMII CTRL Registers/bits */
 #define DP83TG720_SGMII_CTRL			0x0608
@@ -78,6 +78,54 @@
 #define DP83TG720_SGMII_AUTO_NEG_EN		BIT(0)
 #define DP83TG720_SGMII_EN				BIT(9)
 
+/* Strap Register/bits */
+#define DP83TG720_STRAP					0x045d
+#define DP83TG720_MASTER_MODE			BIT(5)
+#define DP83TG720_RGMII_IS_EN			BIT(12)
+#define DP83TG720_SGMII_IS_EN			BIT(13)
+#define DP83TG720_RX_SHIFT_EN			BIT(14)
+#define DP83TG720_TX_SHIFT_EN			BIT(15)
+
+enum DP83TG720_chip_type {
+	DP83TG720_CS1_1,
+	DP83TG721_CS1,
+};
+
+struct DP83TG720_private {
+	int chip;
+	bool is_master;
+	bool is_rgmii;
+	bool is_sgmii;
+	bool rx_shift;
+	bool tx_shift;
+};
+
+static int dp83tg720_read_straps(struct phy_device *phydev)
+{
+	struct DP83TG720_private *DP83TG720 = phydev->priv;
+	int strap;
+
+	strap = phy_read_mmd(phydev, MMD1F, DP83TG720_STRAP);
+	if (strap < 0)
+		return strap;
+
+	if (strap & DP83TG720_MASTER_MODE)
+		DP83TG720->is_master = true;
+
+	if (strap & DP83TG720_RGMII_IS_EN)
+		DP83TG720->is_rgmii = true;
+
+	if (strap & DP83TG720_SGMII_IS_EN)
+		DP83TG720->is_sgmii = true;
+
+	if (strap & DP83TG720_RX_SHIFT_EN)
+		DP83TG720->rx_shift = true;
+
+	if (strap & DP83TG720_TX_SHIFT_EN)
+		DP83TG720->tx_shift = true;
+
+	return 0;
+};
 
 /**
  * dp83tg720_cable_test_start - Start the cable test for the DP83TG720 PHY.
@@ -364,32 +412,93 @@ static int dp83tg720_config_init(struct phy_device *phydev)
 	return genphy_c45_pma_baset1_read_master_slave(phydev);
 }
 
-static struct phy_driver dp83tg720_driver[] = {
+static int dp83tg720_probe(struct phy_device *phydev)
 {
-	PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID),
-	.name		= "TI DP83TG720",
-
-	.flags          = PHY_POLL_CABLE_TEST,
-	.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,
-	.cable_test_start = dp83tg720_cable_test_start,
-	.cable_test_get_status = dp83tg720_cable_test_get_status,
-
-	.suspend	= genphy_suspend,
-	.resume		= genphy_resume,
-} };
+	struct DP83TG720_private *DP83TG720;
+	int ret;
+
+	DP83TG720 = devm_kzalloc(&phydev->mdio.dev, sizeof(*DP83TG720),
+			       GFP_KERNEL);
+	if (!DP83TG720)
+		return -ENOMEM;
+
+	phydev->priv = DP83TG720;
+
+	ret = dp83tg720_read_straps(phydev);
+	if (ret)
+		return ret;
+
+	switch (phydev->phy_id) {
+	case DP83TG720_CS_1_1_PHY_ID:
+		DP83TG720->chip = DP83TG720_CS1_1;
+		break;
+	case DP83TG721_CS_1_0_PHY_ID:
+		DP83TG720->chip = DP83TG721_CS1;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	return dp83tg720_config_init(phydev);
+}
+
+#define DP83TG720_PHY_DRIVER(_id, _name)                                \
+{                                                                       \
+    PHY_ID_MATCH_EXACT(_id),                                            \
+    .name                   = (_name),                                  \
+    .probe                  = dp83tg720_probe,                          \
+    .flags                  = PHY_POLL_CABLE_TEST,                      \
+    .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,                    \
+    .cable_test_start       = dp83tg720_cable_test_start,               \
+    .cable_test_get_status  = dp83tg720_cable_test_get_status,          \
+    .suspend                = genphy_suspend,                           \
+    .resume                 = genphy_resume,                            \
+}
+
+static struct phy_driver dp83tg720_driver[] = {
+    DP83TG720_PHY_DRIVER(DP83TG720_CS_1_1_PHY_ID, "TI DP83TG720CS1.1"),
+	DP83TG720_PHY_DRIVER(DP83TG721_CS_1_0_PHY_ID, "TI DP83TG721CS1.0"),
+};
 module_phy_driver(dp83tg720_driver);
 
 static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
-	{ PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID) },
-	{ }
+    { PHY_ID_MATCH_EXACT(DP83TG720_CS_1_1_PHY_ID) },
+	{ PHY_ID_MATCH_EXACT(DP83TG721_CS_1_0_PHY_ID) },
+	{ },
 };
 MODULE_DEVICE_TABLE(mdio, dp83tg720_tbl);
 
+// static struct phy_driver dp83tg720_driver[] = {
+// {
+// 	PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID),
+// 	.name		= "TI DP83TG720",
+
+// 	.flags          = PHY_POLL_CABLE_TEST,
+// 	.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,
+// 	.cable_test_start = dp83tg720_cable_test_start,
+// 	.cable_test_get_status = dp83tg720_cable_test_get_status,
+
+// 	.suspend	= genphy_suspend,
+// 	.resume		= genphy_resume,
+// } };
+// module_phy_driver(dp83tg720_driver);
+
+// static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
+// 	{ PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID) },
+// 	{ }
+// };
+// MODULE_DEVICE_TABLE(mdio, dp83tg720_tbl);
+
 MODULE_DESCRIPTION("Texas Instruments DP83TG720 PHY driver");
 MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
 MODULE_LICENSE("GPL");
-- 
2.17.1

[PATCH 4/5] net: phy: dp83tg720: Added OA script

[Alvaro (Al-vuh-roe) Reyes]

For the DP83TG720 & DP83TG721 to function properly, both need an
initialization script to be run at boot up. The init script and a chip_init
function have been added to handle this condition. 

Signed-off-by: Alvaro (Al-vuh-roe) Reyes <a-reyes1@ti.com>
---
 drivers/net/phy/dp83tg720.c | 355 ++++++++++++++++++++++++++++++++----
 1 file changed, 324 insertions(+), 31 deletions(-)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index b70802818f3c..4df6713c51e3 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -31,8 +31,9 @@
 /* 1b = TDR fail, 0b = TDR success */
 #define DP83TG720_TDR_FAIL				BIT(0)
 
-#define DP83TG720_PHY_RESET				0x1f
-#define DP83TG720_HW_RESET				BIT(15)
+#define DP83TG720_PHY_RESET_CTRL		0x1f
+#define DP83TG720_HW_RESET			    BIT(15)
+#define DP83TG720_SW_RESET              BIT(14)
 
 #define DP83TG720_LPS_CFG3				0x18c
 /* Power modes are documented as bit fields but used as values */
@@ -100,6 +101,221 @@ struct DP83TG720_private {
 	bool tx_shift;
 };
 
+struct DP83TG720_init_reg {
+	int MMD;
+	int reg;
+	int val;
+};
+
+/*Refer to SNLA371 for more information*/
+static const struct DP83TG720_init_reg DP83TG720_cs1_1_master_init[] = {
+	{0x1F, 0x001F, 0X8000},
+	{0x1F, 0x0573, 0x0101},
+	{0x1, 0x0834, 0xC001},
+	{0x1F, 0x0405, 0x5800},
+	{0x1F, 0x08AD, 0x3C51},
+	{0x1F, 0x0894, 0x5DF7},
+	{0x1F, 0x08A0, 0x09E7},
+	{0x1F, 0x08C0, 0x4000},
+	{0x1F, 0x0814, 0x4800},
+	{0x1F, 0x080D, 0x2EBF},
+	{0x1F, 0x08C1, 0x0B00},
+	{0x1F, 0x087D, 0x0001},
+	{0x1F, 0x082E, 0x0000},
+	{0x1F, 0x0837, 0x00F4},
+	{0x1F, 0x08BE, 0x0200},
+	{0x1F, 0x08C5, 0x4000},
+	{0x1F, 0x08C7, 0x2000},
+	{0x1F, 0x08B3, 0x005A},
+	{0x1F, 0x08B4, 0x005A},
+	{0x1F, 0x08B0, 0x0202},
+	{0x1F, 0x08B5, 0x00EA},
+	{0x1F, 0x08BA, 0x2828},
+	{0x1F, 0x08BB, 0x6828},
+	{0x1F, 0x08BC, 0x0028},
+	{0x1F, 0x08BF, 0x0000},
+	{0x1F, 0x08B1, 0x0014},
+	{0x1F, 0x08B2, 0x0008},
+	{0x1F, 0x08EC, 0x0000},
+	{0x1F, 0x08C8, 0x0003},
+	{0x1F, 0x08BE, 0x0201},
+	{0x1F, 0x018C, 0x0001},
+	{0x1F, 0x001F, 0x4000},
+	{0x1F, 0x0573, 0x0001},
+	{0x1F, 0x056A, 0x5F41},
+};
+
+/*Refer to SNLA371 for more information*/
+static const struct DP83TG720_init_reg DP83TG720_cs1_1_slave_init[] = {
+	{0x1F, 0x001F, 0x8000},
+	{0x1F, 0x0573, 0x0101},
+	{0x1, 0x0834, 0x8001},
+	{0x1F, 0x0894, 0x5DF7},
+	{0x1F, 0x056a, 0x5F40},
+	{0x1F, 0x0405, 0x5800},
+	{0x1F, 0x08AD, 0x3C51},
+	{0x1F, 0x0894, 0x5DF7},
+	{0x1F, 0x08A0, 0x09E7},
+	{0x1F, 0x08C0, 0x4000},
+	{0x1F, 0x0814, 0x4800},
+	{0x1F, 0x080D, 0x2EBF},
+	{0x1F, 0x08C1, 0x0B00},
+	{0x1F, 0x087d, 0x0001},
+	{0x1F, 0x082E, 0x0000},
+	{0x1F, 0x0837, 0x00f4},
+	{0x1F, 0x08BE, 0x0200},
+	{0x1F, 0x08C5, 0x4000},
+	{0x1F, 0x08C7, 0x2000},
+	{0x1F, 0x08B3, 0x005A},
+	{0x1F, 0x08B4, 0x005A},
+	{0x1F, 0x08B0, 0x0202},
+	{0x1F, 0x08B5, 0x00EA},
+	{0x1F, 0x08BA, 0x2828},
+	{0x1F, 0x08BB, 0x6828},
+	{0x1F, 0x08BC, 0x0028},
+	{0x1F, 0x08BF, 0x0000},
+	{0x1F, 0x08B1, 0x0014},
+	{0x1F, 0x08B2, 0x0008},
+	{0x1F, 0x08EC, 0x0000},
+	{0x1F, 0x08C8, 0x0003},
+	{0x1F, 0x08BE, 0x0201},
+	{0x1F, 0x056A, 0x5F40},
+	{0x1F, 0x018C, 0x0001},
+	{0x1F, 0x001F, 0x4000},
+	{0x1F, 0x0573, 0x0001},
+	{0x1F, 0x056A, 0X5F41},
+};
+
+/*Refer to SNLA371 for more information*/
+static const struct DP83TG720_init_reg DP83TG721_cs1_master_init[] = {
+	{0x1F, 0x001F, 0x8000},
+	{0x1F, 0x0573, 0x0801},
+	{0x1, 0x0834, 0xC001},
+	{0x1F, 0x0405, 0x6C00},
+	{0x1F, 0x08AD, 0x3C51},
+	{0x1F, 0x0894, 0x5DF7},
+	{0x1F, 0x08A0, 0x09E7},
+	{0x1F, 0x08C0, 0x4000},
+	{0x1F, 0x0814, 0x4800},
+	{0x1F, 0x080D, 0x2EBF},
+	{0x1F, 0x08C1, 0x0B00},
+	{0x1F, 0x087D, 0x0001},
+	{0x1F, 0x082E, 0x0000},
+	{0x1F, 0x0837, 0x00F8},
+	{0x1F, 0x08BE, 0x0200},
+	{0x1F, 0x08C5, 0x4000},
+	{0x1F, 0x08C7, 0x2000},
+	{0x1F, 0x08B3, 0x005A},
+	{0x1F, 0x08B4, 0x005A},
+	{0x1F, 0x08B0, 0x0202},
+	{0x1F, 0x08B5, 0x00EA},
+	{0x1F, 0x08BA, 0x2828},
+	{0x1F, 0x08BB, 0x6828},
+	{0x1F, 0x08BC, 0x0028},
+	{0x1F, 0x08BF, 0x0000},
+	{0x1F, 0x08B1, 0x0014},
+	{0x1F, 0x08B2, 0x0008},
+	{0x1F, 0x08EC, 0x0000},
+	{0x1F, 0x08FC, 0x0091},
+	{0x1F, 0x08BE, 0x0201},
+	{0x1F, 0x0335, 0x0010},
+	{0x1F, 0x0336, 0x0009},
+	{0x1F, 0x0337, 0x0208},
+	{0x1F, 0x0338, 0x0208},
+	{0x1F, 0x0339, 0x02CB},
+	{0x1F, 0x033A, 0x0208},
+	{0x1F, 0x033B, 0x0109},
+	{0x1F, 0x0418, 0x0380},
+	{0x1F, 0x0420, 0xFF10},
+	{0x1F, 0x0421, 0x4033},
+	{0x1F, 0x0422, 0x0800},
+	{0x1F, 0x0423, 0x0002},
+	{0x1F, 0x0484, 0x0003},
+	{0x1F, 0x055D, 0x0008},
+	{0x1F, 0x042B, 0x0018},
+	{0x1F, 0x087C, 0x0080},
+	{0x1F, 0x08C1, 0x0900},
+	{0x1F, 0x08fc, 0x4091},
+	{0x1F, 0x0881, 0x5146},
+	{0x1F, 0x08be, 0x02a1},
+	{0x1F, 0x0867, 0x9999},
+	{0x1F, 0x0869, 0x9666},
+	{0x1F, 0x086a, 0x0009},
+	{0x1F, 0x0822, 0x11e1},
+	{0x1F, 0x08f9, 0x1f11},
+	{0x1F, 0x08a3, 0x24e8},
+	{0x1F, 0x018C, 0x0001},
+	{0x1F, 0x001F, 0x4000},
+	{0x1F, 0x0573, 0x0001},
+	{0x1F, 0x056A, 0x5F41},
+};
+
+/*Refer to SNLA371 for more information*/
+static const struct DP83TG720_init_reg DP83TG721_cs1_slave_init[] = {
+	{0x1F, 0x001F, 0x8000},
+	{0x1F, 0x0573, 0x0801},
+	{0x1, 0x0834, 0x8001},
+	{0x1F, 0x0405, 0X6C00},
+	{0x1F, 0x08AD, 0x3C51},
+	{0x1F, 0x0894, 0x5DF7},
+	{0x1F, 0x08A0, 0x09E7},
+	{0x1F, 0x08C0, 0x4000},
+	{0x1F, 0x0814, 0x4800},
+	{0x1F, 0x080D, 0x2EBF},
+	{0x1F, 0x08C1, 0x0B00},
+	{0x1F, 0x087D, 0x0001},
+	{0x1F, 0x082E, 0x0000},
+	{0x1F, 0x0837, 0x00F8},
+	{0x1F, 0x08BE, 0x0200},
+	{0x1F, 0x08C5, 0x4000},
+	{0x1F, 0x08C7, 0x2000},
+	{0x1F, 0x08B3, 0x005A},
+	{0x1F, 0x08B4, 0x005A},
+	{0x1F, 0x08B0, 0x0202},
+	{0x1F, 0x08B5, 0x00EA},
+	{0x1F, 0x08BA, 0x2828},
+	{0x1F, 0x08BB, 0x6828},
+	{0x1F, 0x08BC, 0x0028},
+	{0x1F, 0x08BF, 0x0000},
+	{0x1F, 0x08B1, 0x0014},
+	{0x1F, 0x08B2, 0x0008},
+	{0x1F, 0x08EC, 0x0000},
+	{0x1F, 0x08FC, 0x0091},
+	{0x1F, 0x08BE, 0x0201},
+	{0x1F, 0x0456, 0x0160},
+	{0x1F, 0x0335, 0x0010},
+	{0x1F, 0x0336, 0x0009},
+	{0x1F, 0x0337, 0x0208},
+	{0x1F, 0x0338, 0x0208},
+	{0x1F, 0x0339, 0x02CB},
+	{0x1F, 0x033A, 0x0208},
+	{0x1F, 0x033B, 0x0109},
+	{0x1F, 0x0418, 0x0380},
+	{0x1F, 0x0420, 0xFF10},
+	{0x1F, 0x0421, 0x4033},
+	{0x1F, 0x0422, 0x0800},
+	{0x1F, 0x0423, 0x0002},
+	{0x1F, 0x0484, 0x0003},
+	{0x1F, 0x055D, 0x0008},
+	{0x1F, 0x042B, 0x0018},
+	{0x1F, 0x082D, 0x120F},
+	{0x1F, 0x0888, 0x0438},
+	{0x1F, 0x0824, 0x09E0},
+	{0x1F, 0x0883, 0x5146},
+	{0x1F, 0x08BE, 0x02A1},
+	{0x1F, 0x0822, 0x11E1},
+	{0x1F, 0x056A, 0x5F40},
+	{0x1F, 0x08C1, 0x0900},
+	{0x1F, 0x08FC, 0x4091},
+	{0x1F, 0x08F9, 0x1F11},
+	{0x1F, 0x084F, 0x290C},
+	{0x1F, 0x0850, 0x3D33},
+	{0x1F, 0x018C, 0x0001},
+	{0x1F, 0x001F, 0x4000},
+	{0x1F, 0x0573, 0x0001},
+	{0x1F, 0x056A, 0x5F41},
+};
+
 static int dp83tg720_read_straps(struct phy_device *phydev)
 {
 	struct DP83TG720_private *DP83TG720 = phydev->priv;
@@ -127,6 +343,55 @@ static int dp83tg720_read_straps(struct phy_device *phydev)
 	return 0;
 };
 
+static int dp83tg720_reset(struct phy_device *phydev, bool hw_reset)
+{
+	int ret;
+
+	if (hw_reset)
+		ret = phy_write_mmd(phydev, MMD1F, DP83TG720_PHY_RESET_CTRL,
+				DP83TG720_HW_RESET);
+	else
+		ret = phy_write_mmd(phydev, MMD1F, DP83TG720_PHY_RESET_CTRL,
+				DP83TG720_SW_RESET);
+	if (ret)
+		return ret;
+
+	mdelay(100);
+
+	return 0;
+}
+
+static int dp83tg720_phy_reset(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = dp83tg720_reset(phydev, false);
+	if (ret)
+		return ret;
+
+	ret = dp83tg720_read_straps(phydev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int DP83TG720_write_seq(struct phy_device *phydev,
+			     const struct DP83TG720_init_reg *init_data, int size)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < size; i++) {
+			ret = phy_write_mmd(phydev, init_data[i].MMD, init_data[i].reg,
+				init_data[i].val);
+			if (ret)
+					return ret;
+	}
+
+	return 0;
+}
+
 /**
  * dp83tg720_cable_test_start - Start the cable test for the DP83TG720 PHY.
  * @phydev: Pointer to the phy_device structure.
@@ -362,6 +627,61 @@ static int dp83tg720_config_rgmii_delay(struct phy_device *phydev)
 			      rgmii_delay);
 }
 
+static int dp83tg720_chip_init(struct phy_device *phydev)
+{
+	struct DP83TG720_private *DP83TG720 = phydev->priv;
+	int ret;
+
+	ret = dp83tg720_reset(phydev, true);
+	if (ret)
+		return ret;
+	
+	phydev->autoneg = AUTONEG_DISABLE;
+    phydev->speed = SPEED_1000;
+	phydev->duplex = DUPLEX_FULL;
+    linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, phydev->supported);
+
+	switch (DP83TG720->chip) {
+	case DP83TG720_CS1_1:
+		if (DP83TG720->is_master)
+			ret = DP83TG720_write_seq(phydev, DP83TG720_cs1_1_master_init,
+						ARRAY_SIZE(DP83TG720_cs1_1_master_init));
+		else
+			ret = DP83TG720_write_seq(phydev, DP83TG720_cs1_1_slave_init,
+						ARRAY_SIZE(DP83TG720_cs1_1_slave_init));
+
+		ret = dp83tg720_reset(phydev, false);
+
+		return 1;
+	case DP83TG721_CS1:
+		if (DP83TG720->is_master)
+			ret = DP83TG720_write_seq(phydev, DP83TG721_cs1_master_init,
+						ARRAY_SIZE(DP83TG721_cs1_master_init));
+		else
+			ret = DP83TG720_write_seq(phydev, DP83TG721_cs1_slave_init,
+						ARRAY_SIZE(DP83TG721_cs1_slave_init));
+
+		ret = dp83tg720_reset(phydev, false);
+
+		return 1;
+	default:
+		return -EINVAL;
+	};
+
+	if (ret)
+		return ret;
+
+	/* Enable the PHY */
+	ret = phy_write_mmd(phydev, MMD1F, DP83TG720_LPS_CFG3, DP83TG720_LPS_CFG3_PWR_MODE_0);
+	if (ret)
+		return ret;
+
+	mdelay(10);
+
+	/* Do a software reset to restart the PHY with the updated values */
+	return dp83tg720_reset(phydev, false);
+}
+
 static int dp83tg720_config_init(struct phy_device *phydev)
 {
 	int value, ret;
@@ -369,9 +689,7 @@ static int dp83tg720_config_init(struct phy_device *phydev)
 	/* Software Restart is not enough to recover from a link failure.
 	 * Using Hardware Reset instead.
 	 */
-	ret = phy_write(phydev, DP83TG720_PHY_RESET, DP83TG720_HW_RESET);
-	if (ret)
-		return ret;
+	ret = dp83tg720_chip_init(phydev);
 
 	/* Wait until MDC can be used again.
 	 * The wait value of one 1ms is documented in "DP83TG720-Q1 1000BASE-T1
@@ -447,6 +765,7 @@ static int dp83tg720_probe(struct phy_device *phydev)
     PHY_ID_MATCH_EXACT(_id),                                            \
     .name                   = (_name),                                  \
     .probe                  = dp83tg720_probe,                          \
+	.soft_reset				= dp83tg720_phy_reset,						\
     .flags                  = PHY_POLL_CABLE_TEST,                      \
     .config_aneg            = dp83tg720_config_aneg,                    \
     .read_status            = dp83tg720_read_status,                    \
@@ -473,32 +792,6 @@ static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
 };
 MODULE_DEVICE_TABLE(mdio, dp83tg720_tbl);
 
-// static struct phy_driver dp83tg720_driver[] = {
-// {
-// 	PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID),
-// 	.name		= "TI DP83TG720",
-
-// 	.flags          = PHY_POLL_CABLE_TEST,
-// 	.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,
-// 	.cable_test_start = dp83tg720_cable_test_start,
-// 	.cable_test_get_status = dp83tg720_cable_test_get_status,
-
-// 	.suspend	= genphy_suspend,
-// 	.resume		= genphy_resume,
-// } };
-// module_phy_driver(dp83tg720_driver);
-
-// static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
-// 	{ PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID) },
-// 	{ }
-// };
-// MODULE_DEVICE_TABLE(mdio, dp83tg720_tbl);
-
 MODULE_DESCRIPTION("Texas Instruments DP83TG720 PHY driver");
 MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
 MODULE_LICENSE("GPL");
-- 
2.17.1

[PATCH 5/5] net: phy: dp83tg720: fixed Linux coding standards issues

[Alvaro (Al-vuh-roe) Reyes]

Driver patches was checked against the linux coding standards using scripts/checkpatch.pl.

This patch meets the standards checked by the script.

Signed-off-by: Alvaro (Al-vuh-roe) Reyes <a-reyes1@ti.com>
---
 drivers/net/phy/dp83tg720.c | 71 +++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index 4df6713c51e3..1135dcf5efe6 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -10,7 +10,7 @@
 
 #include "open_alliance_helpers.h"
 
-#define DP83TG720_CS_1_1_PHY_ID				0x2000a284
+#define DP83TG720_CS_1_1_PHY_ID			0x2000a284
 #define DP83TG721_CS_1_0_PHY_ID			0x2000a290
 #define MMD1F							0x1f
 #define MMD1							0x1
@@ -349,10 +349,10 @@ static int dp83tg720_reset(struct phy_device *phydev, bool hw_reset)
 
 	if (hw_reset)
 		ret = phy_write_mmd(phydev, MMD1F, DP83TG720_PHY_RESET_CTRL,
-				DP83TG720_HW_RESET);
+				    DP83TG720_HW_RESET);
 	else
 		ret = phy_write_mmd(phydev, MMD1F, DP83TG720_PHY_RESET_CTRL,
-				DP83TG720_SW_RESET);
+				    DP83TG720_SW_RESET);
 	if (ret)
 		return ret;
 
@@ -377,16 +377,17 @@ static int dp83tg720_phy_reset(struct phy_device *phydev)
 }
 
 static int DP83TG720_write_seq(struct phy_device *phydev,
-			     const struct DP83TG720_init_reg *init_data, int size)
+			       const struct DP83TG720_init_reg *init_data,
+			       int size)
 {
 	int ret;
 	int i;
 
 	for (i = 0; i < size; i++) {
-			ret = phy_write_mmd(phydev, init_data[i].MMD, init_data[i].reg,
-				init_data[i].val);
-			if (ret)
-					return ret;
+		ret = phy_write_mmd(phydev, init_data[i].MMD, init_data[i].reg,
+				    init_data[i].val);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
@@ -635,20 +636,20 @@ static int dp83tg720_chip_init(struct phy_device *phydev)
 	ret = dp83tg720_reset(phydev, true);
 	if (ret)
 		return ret;
-	
+
 	phydev->autoneg = AUTONEG_DISABLE;
-    phydev->speed = SPEED_1000;
+	phydev->speed = SPEED_1000;
 	phydev->duplex = DUPLEX_FULL;
-    linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, phydev->supported);
 
 	switch (DP83TG720->chip) {
 	case DP83TG720_CS1_1:
 		if (DP83TG720->is_master)
 			ret = DP83TG720_write_seq(phydev, DP83TG720_cs1_1_master_init,
-						ARRAY_SIZE(DP83TG720_cs1_1_master_init));
+						  ARRAY_SIZE(DP83TG720_cs1_1_master_init));
 		else
 			ret = DP83TG720_write_seq(phydev, DP83TG720_cs1_1_slave_init,
-						ARRAY_SIZE(DP83TG720_cs1_1_slave_init));
+						  ARRAY_SIZE(DP83TG720_cs1_1_slave_init));
 
 		ret = dp83tg720_reset(phydev, false);
 
@@ -656,10 +657,10 @@ static int dp83tg720_chip_init(struct phy_device *phydev)
 	case DP83TG721_CS1:
 		if (DP83TG720->is_master)
 			ret = DP83TG720_write_seq(phydev, DP83TG721_cs1_master_init,
-						ARRAY_SIZE(DP83TG721_cs1_master_init));
+						  ARRAY_SIZE(DP83TG721_cs1_master_init));
 		else
 			ret = DP83TG720_write_seq(phydev, DP83TG721_cs1_slave_init,
-						ARRAY_SIZE(DP83TG721_cs1_slave_init));
+						  ARRAY_SIZE(DP83TG721_cs1_slave_init));
 
 		ret = dp83tg720_reset(phydev, false);
 
@@ -736,7 +737,7 @@ static int dp83tg720_probe(struct phy_device *phydev)
 	int ret;
 
 	DP83TG720 = devm_kzalloc(&phydev->mdio.dev, sizeof(*DP83TG720),
-			       GFP_KERNEL);
+				 GFP_KERNEL);
 	if (!DP83TG720)
 		return -ENOMEM;
 
@@ -760,33 +761,33 @@ static int dp83tg720_probe(struct phy_device *phydev)
 	return dp83tg720_config_init(phydev);
 }
 
-#define DP83TG720_PHY_DRIVER(_id, _name)                                \
-{                                                                       \
-    PHY_ID_MATCH_EXACT(_id),                                            \
-    .name                   = (_name),                                  \
-    .probe                  = dp83tg720_probe,                          \
-	.soft_reset				= dp83tg720_phy_reset,						\
-    .flags                  = PHY_POLL_CABLE_TEST,                      \
-    .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,                    \
-    .cable_test_start       = dp83tg720_cable_test_start,               \
-    .cable_test_get_status  = dp83tg720_cable_test_get_status,          \
-    .suspend                = genphy_suspend,                           \
-    .resume                 = genphy_resume,                            \
+#define DP83TG720_PHY_DRIVER(_id, _name)				\
+{									\
+	PHY_ID_MATCH_EXACT(_id),					\
+	.name                   = (_name),				\
+	.probe                  = dp83tg720_probe,			\
+	.soft_reset		= dp83tg720_phy_reset,			\
+	.flags                  = PHY_POLL_CABLE_TEST,			\
+	.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,		\
+	.cable_test_start       = dp83tg720_cable_test_start,		\
+	.cable_test_get_status  = dp83tg720_cable_test_get_status,	\
+	.suspend                = genphy_suspend,			\
+	.resume                 = genphy_resume,			\
 }
 
 static struct phy_driver dp83tg720_driver[] = {
-    DP83TG720_PHY_DRIVER(DP83TG720_CS_1_1_PHY_ID, "TI DP83TG720CS1.1"),
+	DP83TG720_PHY_DRIVER(DP83TG720_CS_1_1_PHY_ID, "TI DP83TG720CS1.1"),
 	DP83TG720_PHY_DRIVER(DP83TG721_CS_1_0_PHY_ID, "TI DP83TG721CS1.0"),
 };
 module_phy_driver(dp83tg720_driver);
 
 static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
-    { PHY_ID_MATCH_EXACT(DP83TG720_CS_1_1_PHY_ID) },
+	{ PHY_ID_MATCH_EXACT(DP83TG720_CS_1_1_PHY_ID) },
 	{ PHY_ID_MATCH_EXACT(DP83TG721_CS_1_0_PHY_ID) },
 	{ },
 };
-- 
2.17.1

Re: [PATCH 2/5] net: phy: dp83tg720: Added SGMII Support

[Andrew Lunn]

> +#define MMD1F							0x1f
> +#define MMD1							0x1

MDIO_MMD_VEND2 and MDIO_MMD_PMAPMD. But i don't think MMD1 is used?

> +
>  /* MDIO_MMD_VEND2 registers */
>  #define DP83TG720_MII_REG_10			0x10
>  #define DP83TG720_STS_MII_INT			BIT(7)
> @@ -69,6 +72,13 @@

It looks like the SGMII register goes here, since it is in
MDIO_MMD_VEND2.

	Andrew

Re: [PATCH 3/5] net: phy: dp83tg720: Extending support to DP83TG721 PHY

[Andrew Lunn]

On Thu, Sep 19, 2024 at 02:01:17PM -0700, Alvaro (Al-vuh-roe) Reyes wrote:
> The DP83TG721 is the next revision of the DP83TG720 and will share the
> same driver. Added PHY_ID and probe funtion to check which version is
> being loaded. 

Please don't mix whitespace changes with real code changes. It makes
it harder to see the real changes which need reviewing.

> +enum DP83TG720_chip_type {
> +	DP83TG720_CS1_1,
> +	DP83TG721_CS1,
> +};
> +
> +struct DP83TG720_private {
> +	int chip;

I _think_ this should be enum DP83TG720_chip_type chip;

> +	bool is_master;

I think this can be removed and replaced with
phydev->master_slave_set. You probably want to mirror it into
phydev->master_slave_get.

phydev->master_slave_state normally contains the result of auto-neg,
but i expect this PHY forces it, so again, you probably want to mirror
it here as well. Test it out with ethtool and make sure it reports
what you expect.

> +	struct DP83TG720_private *DP83TG720;

Upper case is very unusual in mainline. It is generally only used for
CPP macros.

> +static struct phy_driver dp83tg720_driver[] = {
> +    DP83TG720_PHY_DRIVER(DP83TG720_CS_1_1_PHY_ID, "TI DP83TG720CS1.1"),
> +	DP83TG720_PHY_DRIVER(DP83TG721_CS_1_0_PHY_ID, "TI DP83TG721CS1.0"),
> +};

Indentation is messed up here. I expect checkpatch says something
about this?

>  module_phy_driver(dp83tg720_driver);
>  
>  static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
> -	{ PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID) },
> -	{ }
> +    { PHY_ID_MATCH_EXACT(DP83TG720_CS_1_1_PHY_ID) },
> +	{ PHY_ID_MATCH_EXACT(DP83TG721_CS_1_0_PHY_ID) },
> +	{ },

Here as well.

>  };
>  MODULE_DEVICE_TABLE(mdio, dp83tg720_tbl);
>  
> +// static struct phy_driver dp83tg720_driver[] = {
> +// {
> +// 	PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID),
> +// 	.name		= "TI DP83TG720",
> +
> +// 	.flags          = PHY_POLL_CABLE_TEST,
> +// 	.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,
> +// 	.cable_test_start = dp83tg720_cable_test_start,
> +// 	.cable_test_get_status = dp83tg720_cable_test_get_status,
> +
> +// 	.suspend	= genphy_suspend,
> +// 	.resume		= genphy_resume,
> +// } };
> +// module_phy_driver(dp83tg720_driver);
> +
> +// static struct mdio_device_id __maybe_unused dp83tg720_tbl[] = {
> +// 	{ PHY_ID_MATCH_MODEL(DP83TG720_PHY_ID) },
> +// 	{ }
> +// };
> +// MODULE_DEVICE_TABLE(mdio, dp83tg720_tbl);

Please don't add code which is commented out.


    Andrew

---
pw-bot: cr

Re: [PATCH 4/5] net: phy: dp83tg720: Added OA script

[Andrew Lunn]

> +struct DP83TG720_init_reg {
> +	int MMD;
> +	int reg;
> +	int val;
> +};
> +
> +/*Refer to SNLA371 for more information*/
> +static const struct DP83TG720_init_reg DP83TG720_cs1_1_master_init[] = {
> +	{0x1F, 0x001F, 0X8000},
> +	{0x1F, 0x0573, 0x0101},
> +	{0x1, 0x0834, 0xC001},

MDIO_MMD_VEND2 etc.

Also 0x834 is BASE-T1 PMA/PMD control. Which is MDIO_PMA_PMD_BT1_CTRL

We also have:
#define MDIO_PMA_PMD_BT1_CTRL_STRAP_B1000 0x0001 /* Select 1000BASE-T1 */
#define MDIO_PMA_PMD_BT1_CTRL_CFG_MST		0x4000 /* MASTER-SLAVE config value */

802.3 says bit 15 is read only, so you don't need to set it.

The rest might be magic which nobody outside of TI will understand,
but you can fully document this.

> +static int dp83tg720_reset(struct phy_device *phydev, bool hw_reset)
> +{
> +	int ret;
> +
> +	if (hw_reset)
> +		ret = phy_write_mmd(phydev, MMD1F, DP83TG720_PHY_RESET_CTRL,
> +				DP83TG720_HW_RESET);
> +	else
> +		ret = phy_write_mmd(phydev, MMD1F, DP83TG720_PHY_RESET_CTRL,
> +				DP83TG720_SW_RESET);
> +	if (ret)
> +		return ret;
> +
> +	mdelay(100);

Does the bit not self clear when it has completed? That would be
common for a reset bit.

> +static int DP83TG720_write_seq(struct phy_device *phydev,
> +			     const struct DP83TG720_init_reg *init_data, int size)
> +{
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < size; i++) {
> +			ret = phy_write_mmd(phydev, init_data[i].MMD, init_data[i].reg,
> +				init_data[i].val);
> +			if (ret)
> +					return ret;
> +	}

More messed up indentation.

> +static int dp83tg720_chip_init(struct phy_device *phydev)
> +{
> +	struct DP83TG720_private *DP83TG720 = phydev->priv;
> +	int ret;
> +
> +	ret = dp83tg720_reset(phydev, true);
> +	if (ret)
> +		return ret;
> +	
> +	phydev->autoneg = AUTONEG_DISABLE;
> +    phydev->speed = SPEED_1000;
> +	phydev->duplex = DUPLEX_FULL;
> +    linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, phydev->supported);

This should not be needed. phylib should be able to figure this out
for itself from the registers. Please check that functions like
genphy_c45_pma_baset1_read_abilities() are doing.

> +
> +	switch (DP83TG720->chip) {
> +	case DP83TG720_CS1_1:
> +		if (DP83TG720->is_master)
> +			ret = DP83TG720_write_seq(phydev, DP83TG720_cs1_1_master_init,
> +						ARRAY_SIZE(DP83TG720_cs1_1_master_init));
> +		else
> +			ret = DP83TG720_write_seq(phydev, DP83TG720_cs1_1_slave_init,
> +						ARRAY_SIZE(DP83TG720_cs1_1_slave_init));
> +
> +		ret = dp83tg720_reset(phydev, false);
> +
> +		return 1;

0 on success, negative error code on error.

	Andrew

Re: [PATCH 5/5] net: phy: dp83tg720: fixed Linux coding standards issues

[Andrew Lunn]

On Thu, Sep 19, 2024 at 02:01:19PM -0700, Alvaro (Al-vuh-roe) Reyes wrote:
> Driver patches was checked against the linux coding standards using scripts/checkpatch.pl.
> 
> This patch meets the standards checked by the script.

This patch should be first, to cleanup any existing issues. New code
you add should already be checkpatch clean as you add it.

These patches need quite a lot of work. Maybe you can find somebody
inside TI who can mentor you and point out issues before posting to
the list.

	Andrew

Re: [PATCH 5/5] net: phy: dp83tg720: fixed Linux coding standards issues

[Nishanth Menon]

On 23:47-20240919, Andrew Lunn wrote:
> On Thu, Sep 19, 2024 at 02:01:19PM -0700, Alvaro (Al-vuh-roe) Reyes wrote:
> > Driver patches was checked against the linux coding standards using scripts/checkpatch.pl.
> > 
> > This patch meets the standards checked by the script.
> 
> This patch should be first, to cleanup any existing issues. New code
> you add should already be checkpatch clean as you add it.
> 
> These patches need quite a lot of work. Maybe you can find somebody
> inside TI who can mentor you and point out issues before posting to
> the list.


Sorry about that Andrew. I will work with the team and try and
straighten this out.

Thank you for your patience.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 4/5] net: phy: dp83tg720: Added OA script

[Oleksij Rempel]

On Thu, Sep 19, 2024 at 11:44:49PM +0200, Andrew Lunn wrote:
> Also 0x834 is BASE-T1 PMA/PMD control. Which is MDIO_PMA_PMD_BT1_CTRL
> 
> We also have:
> #define MDIO_PMA_PMD_BT1_CTRL_STRAP_B1000 0x0001 /* Select 1000BASE-T1 */
> #define MDIO_PMA_PMD_BT1_CTRL_CFG_MST		0x4000 /* MASTER-SLAVE config value */
> 
> 802.3 says bit 15 is read only, so you don't need to set it.
> 
> The rest might be magic which nobody outside of TI will understand,
> but you can fully document this.

Yes, this values will affect products of our customers. It is important
for us to know what and why is changed.

> > +static int dp83tg720_reset(struct phy_device *phydev, bool hw_reset)
> > +{
> > +	int ret;
> > +
> > +	if (hw_reset)
> > +		ret = phy_write_mmd(phydev, MMD1F, DP83TG720_PHY_RESET_CTRL,
> > +				DP83TG720_HW_RESET);
> > +	else
> > +		ret = phy_write_mmd(phydev, MMD1F, DP83TG720_PHY_RESET_CTRL,
> > +				DP83TG720_SW_RESET);
> > +	if (ret)
> > +		return ret;
> > +
> > +	mdelay(100);
> 
> Does the bit not self clear when it has completed? That would be
> common for a reset bit.

Not sure about status bit, but the time seems to be incorrect.
In case of HW reset, about 1ms delay is documented. This time should not
be needed for the SW reset. If it is really needed, please add comment
on where it is described.

With 100ms delay, this chip will be able to establish the link within
10ms (depending on the strap configuration) and then we will disable it again.

Even worse, this script includes already soft reset and power management
state changes. Im strongly against this script in that form.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/5] net: phy: dp83tg720: Extending support to DP83TG721 PHY

[Oleksij Rempel]

On Thu, Sep 19, 2024 at 11:26:49PM +0200, Andrew Lunn wrote:
> On Thu, Sep 19, 2024 at 02:01:17PM -0700, Alvaro (Al-vuh-roe) Reyes wrote:
> > The DP83TG721 is the next revision of the DP83TG720 and will share the
> > same driver. Added PHY_ID and probe funtion to check which version is
> > being loaded. 
> 
> Please don't mix whitespace changes with real code changes. It makes
> it harder to see the real changes which need reviewing.
> 
> > +enum DP83TG720_chip_type {
> > +	DP83TG720_CS1_1,
> > +	DP83TG721_CS1,
> > +};
> > +
> > +struct DP83TG720_private {
> > +	int chip;
> 
> I _think_ this should be enum DP83TG720_chip_type chip;
> 
> > +	bool is_master;
> 
> I think this can be removed and replaced with
> phydev->master_slave_set. You probably want to mirror it into
> phydev->master_slave_get.
> 
> phydev->master_slave_state normally contains the result of auto-neg,
> but i expect this PHY forces it, so again, you probably want to mirror
> it here as well. Test it out with ethtool and make sure it reports
> what you expect.

And we will have device-tree based overwrites for the timing role soon,
so there is no reason to store this values in the priv.

Same for RGMII and SGMII modes, DT is already overwriting it with the
phy-mode property.

> > +	struct DP83TG720_private *DP83TG720;
> 
> Upper case is very unusual in mainline. It is generally only used for
> CPP macros.
> 
> > +static struct phy_driver dp83tg720_driver[] = {
> > +    DP83TG720_PHY_DRIVER(DP83TG720_CS_1_1_PHY_ID, "TI DP83TG720CS1.1"),
> > +	DP83TG720_PHY_DRIVER(DP83TG721_CS_1_0_PHY_ID, "TI DP83TG721CS1.0"),
> > +};

I would prefer not to have DP83TG720_PHY_DRIVER() macros. This devices
are not identical. For example: DP83TG721 have HW time stamping,
DP83TG720 don't. As soon as some one will start implementing it, this
macro will be obsolete.

About names: TI DP83TG720CS1.1 and TI DP83TG721CS1.0 are not known
anywhere outside of TI. If you really won't to use this names, please
add comment describing which final products use core variants

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/5] net: phy: dp83tg720: Changed Macro names

[Oleksij Rempel]

On Thu, Sep 19, 2024 at 02:01:15PM -0700, Alvaro (Al-vuh-roe) Reyes wrote:
> Previous macro referes to DP83TG720S, where this driver works for both
> DP83TG720R & DP83TG720S. Macro changed to DP83TG720 to be more generic.
> 
> Data sheets:
> https://www.ti.com/lit/ds/symlink/dp83tg720s-q1.pdf
> https://www.ti.com/lit/ds/symlink/dp83tg720r-q1.pdf

One one side, this change is not consequent enough - this patch set will
add support for dp83tg721 variants too, but rename reflect support only
for dp83tg720 variants. At same time, TI has company internal names
for this IP cores, which are not used for end product names. Every time, some
one add new chip variant, we will need to rename defines.

On other side, already this refactoring is too much, because it will
make maintenance of stable kernel a nightmare.
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |