[PATCH net-next v5 0/2] Add support for VSC85xx DT RGMII delays

[PATCH net-next v5 0/2] Add support for VSC85xx DT RGMII delays

[Harini Katakam]

Provide an option to change RGMII delay value via devicetree.

v5:
- Rebase after VSC8501 series is merged, to avoid conflicts
- Rename _internal_delay to use vsc86xx, move declaration and
simplify format of pointer to above
- Acquire DT delay values in vsc85xx_update_rgmii_cntl instead of
vsc85xx_config_init to accommodate all VSC phy versions

v4:
- Remove VSC8531_02 support. Existing code will identify VSC8531_01/02
and there is no unique functionality to be added for either version.
- Correct type of rx/tx_delay to accept correct return value.
- Added Andrew's tag to patch 1

Lore link for v3: 
https://lore.kernel.org/netdev/20230511120808.28646-1-harini.katakam@amd.com/

v3 changes:
- Remove patch 2/3 from v2 as custom mscc properties dont need to be
defined. rx-internal-delay-ps and tx-internal-delay-ps can be used.
- Change RGMII delay precedence as advised by Vladimir:
 phy-mode                       rgmii                          rgmii-rxid/rgmii-id
 --------------------------------------------------------------------------------------------
 rx-internal-delay-ps absent    0.2 ns                         2 ns
 rx-internal-delay-ps present   follow rx-internal-delay-ps    follow rx-internal-delay-ps
- Split VSC8531-02 and RGMII delay config into separate patches.
- Correct vendor ID
- Update commit description and subject everywhere to say RGMII delays
instead of RGMII tuning.

v2 changes:
- Added patch to use a common vendor phy id match
- Removed dt include header patch because delays should be specied in
ps, not register values
- Updated DT binding description and commit for optional delay tuning to
be clearer on the precedence
- Updated dt property name to include vendor instead of phy device name
- Switch both VSC8531 and VSC8531-02 to use exact phy id match as they
share the same model number
- Ensure RCT
- Improve optional property read

Harini Katakam (2):
  phy: mscc: Use PHY_ID_MATCH_VENDOR to minimize PHY ID table
  phy: mscc: Add support for RGMII delay configuration

 drivers/net/phy/mscc/mscc.h      |  1 +
 drivers/net/phy/mscc/mscc_main.c | 51 +++++++++++++++++++-------------
 2 files changed, 31 insertions(+), 21 deletions(-)

-- 
2.17.1

[PATCH net-next v5 1/2] phy: mscc: Use PHY_ID_MATCH_VENDOR to minimize PHY ID table

[Harini Katakam]

All the PHY devices variants specified have the same mask and
hence can be simplified to one vendor look up for 0x00070400.
Any individual config can be identified by PHY_ID_MATCH_EXACT
in the respective structure.

Signed-off-by: Harini Katakam <harini.katakam@amd.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
v5:
Rebased on top latest net-next
v4:
Added Andrew's tag
v3:
Correct vendor ID
v2:
New patch
 drivers/net/phy/mscc/mscc.h      |  1 +
 drivers/net/phy/mscc/mscc_main.c | 16 +---------------
 2 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index defe5cc6d4fc..7a962050a4d4 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -292,6 +292,7 @@ enum rgmii_clock_delay {
 #define PHY_ID_VSC8575			  0x000707d0
 #define PHY_ID_VSC8582			  0x000707b0
 #define PHY_ID_VSC8584			  0x000707c0
+#define PHY_VENDOR_MSCC			0x00070400
 
 #define MSCC_VDDMAC_1500		  1500
 #define MSCC_VDDMAC_1800		  1800
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 28df8a2e4230..fc074bcc894d 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -2678,21 +2678,7 @@ static struct phy_driver vsc85xx_driver[] = {
 module_phy_driver(vsc85xx_driver);
 
 static struct mdio_device_id __maybe_unused vsc85xx_tbl[] = {
-	{ PHY_ID_VSC8501, 0xfffffff0, },
-	{ PHY_ID_VSC8502, 0xfffffff0, },
-	{ PHY_ID_VSC8504, 0xfffffff0, },
-	{ PHY_ID_VSC8514, 0xfffffff0, },
-	{ PHY_ID_VSC8530, 0xfffffff0, },
-	{ PHY_ID_VSC8531, 0xfffffff0, },
-	{ PHY_ID_VSC8540, 0xfffffff0, },
-	{ PHY_ID_VSC8541, 0xfffffff0, },
-	{ PHY_ID_VSC8552, 0xfffffff0, },
-	{ PHY_ID_VSC856X, 0xfffffff0, },
-	{ PHY_ID_VSC8572, 0xfffffff0, },
-	{ PHY_ID_VSC8574, 0xfffffff0, },
-	{ PHY_ID_VSC8575, 0xfffffff0, },
-	{ PHY_ID_VSC8582, 0xfffffff0, },
-	{ PHY_ID_VSC8584, 0xfffffff0, },
+	{ PHY_ID_MATCH_VENDOR(PHY_VENDOR_MSCC) },
 	{ }
 };
 
-- 
2.17.1

[PATCH net-next v5 2/2] phy: mscc: Add support for RGMII delay configuration

[Harini Katakam]

Add support for optional rx/tx-internal-delay-ps from devicetree.
- When rx/tx-internal-delay-ps is/are specified, these take priority
- When either is absent,
1) use 2ns for respective settings if rgmii-id/rxid/txid is/are present
2) use 0.2ns for respective settings if mode is rgmii

Signed-off-by: Harini Katakam <harini.katakam@amd.com>
---
v5:
- Rebase on top VSC8501 series, to avoid conflicts
- Rename _internal_delay to use vsc86xx, move declaration and
simplify format of pointer to above
- Acquire DT delay values in vsc85xx_update_rgmii_cntl instead of
vsc85xx_config_init to accommodate all VSC phy versions
v4:
Fix type of rx_delay and tx_delay
Reported by Simon Horman and
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202305140248.lh4LUw2j-lkp@intel.com/
v3 - Patch split:
- Use rx/tx-internal-delay-ps with phy_get_internal_delay
- Change RGMII delay selection precedence
- Update commit description and subject everywhere to say RGMII delays
instead of RGMII tuning.

 drivers/net/phy/mscc/mscc_main.c | 35 ++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index fc074bcc894d..669a4a7a28ce 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -107,6 +107,9 @@ static const struct vsc8531_edge_rate_table edge_table[] = {
 };
 #endif
 
+static const int vsc85xx_internal_delay[] = {200, 800, 1100, 1700, 2000, 2300,
+					     2600, 3400};
+
 static int vsc85xx_phy_read_page(struct phy_device *phydev)
 {
 	return __phy_read(phydev, MSCC_EXT_PAGE_ACCESS);
@@ -525,8 +528,12 @@ static int vsc85xx_update_rgmii_cntl(struct phy_device *phydev, u32 rgmii_cntl,
 {
 	u16 rgmii_rx_delay_pos = ffs(rgmii_rx_delay_mask) - 1;
 	u16 rgmii_tx_delay_pos = ffs(rgmii_tx_delay_mask) - 1;
+	int delay_size = ARRAY_SIZE(vsc85xx_internal_delay);
+	struct device *dev = &phydev->mdio.dev;
 	u16 reg_val = 0;
 	u16 mask = 0;
+	s32 rx_delay;
+	s32 tx_delay;
 	int rc = 0;
 
 	/* For traffic to pass, the VSC8502 family needs the RX_CLK disable bit
@@ -541,12 +548,28 @@ static int vsc85xx_update_rgmii_cntl(struct phy_device *phydev, u32 rgmii_cntl,
 	if (phy_interface_is_rgmii(phydev))
 		mask |= rgmii_rx_delay_mask | rgmii_tx_delay_mask;
 
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
-	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-		reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_rx_delay_pos;
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
-	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-		reg_val |= RGMII_CLK_DELAY_2_0_NS << rgmii_tx_delay_pos;
+	rx_delay = phy_get_internal_delay(phydev, dev, vsc85xx_internal_delay,
+					  delay_size, true);
+	if (rx_delay < 0) {
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
+		    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+			rx_delay = RGMII_CLK_DELAY_2_0_NS;
+		else
+			rx_delay = RGMII_CLK_DELAY_0_2_NS;
+	}
+
+	tx_delay = phy_get_internal_delay(phydev, dev, vsc85xx_internal_delay,
+					  delay_size, false);
+	if (tx_delay < 0) {
+		if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
+		    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+			rx_delay = RGMII_CLK_DELAY_2_0_NS;
+		else
+			rx_delay = RGMII_CLK_DELAY_0_2_NS;
+	}
+
+	reg_val |= rx_delay << rgmii_rx_delay_pos;
+	reg_val |= tx_delay << rgmii_tx_delay_pos;
 
 	if (mask)
 		rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
-- 
2.17.1

Re: [PATCH net-next v5 0/2] Add support for VSC85xx DT RGMII delays

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 29 May 2023 17:50:15 +0530 you wrote:
> Provide an option to change RGMII delay value via devicetree.
> 
> v5:
> - Rebase after VSC8501 series is merged, to avoid conflicts
> - Rename _internal_delay to use vsc86xx, move declaration and
> simplify format of pointer to above
> - Acquire DT delay values in vsc85xx_update_rgmii_cntl instead of
> vsc85xx_config_init to accommodate all VSC phy versions
> 
> [...]

Here is the summary with links:
  - [net-next,v5,1/2] phy: mscc: Use PHY_ID_MATCH_VENDOR to minimize PHY ID table
    https://git.kernel.org/netdev/net-next/c/31605c01fb24
  - [net-next,v5,2/2] phy: mscc: Add support for RGMII delay configuration
    https://git.kernel.org/netdev/net-next/c/dbb050d2bfc8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html