netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
@ 2024-01-08  9:36 Dimitri Fedrau
  2024-01-08  9:36 ` [PATCH v4 net-next 1/5] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08  9:36 UTC (permalink / raw)
  Cc: Dimitri Fedrau, Andrew Lunn, Heiner Kallweit, Russell King,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Eichenberger, netdev, linux-kernel

Changes in v2:
	- used defines MDIO_CTRL1_LPOWER and MDIO_PMA_CTRL1_SPEED1000
	  in mv88q222x_config_aneg_preinit
	- use genphy_c45_loopback
	- mv88q2xxx_read_status reads speed, master or slave state when
	  autonegotiation is enabled
	- added defines for magic values in mv88q222x_get_sqi

Changes in v3:
	- mv88q2xxx_read_status includes autonegotiation case
	- add support for 100BT1 and 1000BT1 linkmode advertisement
	- use mv88q2xxx_get_sqi and mv88q2xxx_get_sqi_max, remove
	  mv88q222x_get_sqi and mv88q222x_get_sqi_max
	- fix typo: rename mv88q2xxxx_get_sqi and mv88q2xxxx_get_sqi_max to
	  mv88q2xxx_get_sqi and mv88q2xxx_get_sqi
	- add define MDIO_MMD_PCS_MV_RX_STAT for magic value 0x8230, documented
	  in latest datasheets for both PHYs

Changes in V4:
	- clean up init sequence
	- separate patch for fixing typos in upstreamed code

Dimitri Fedrau (5):
  net: phy: Add BaseT1 auto-negotiation constants
  net: phy: Support 100/1000BT1 linkmode advertisements
  net: phy: c45: detect 100/1000BASE-T1 linkmode advertisements
  net: phy: marvell-88q2xxx: fix typos
  net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY

 drivers/net/phy/marvell-88q2xxx.c | 234 +++++++++++++++++++++++++++---
 drivers/net/phy/phy-c45.c         |   3 +-
 include/linux/marvell_phy.h       |   1 +
 include/linux/mdio.h              |   8 +
 include/uapi/linux/mdio.h         |   2 +
 5 files changed, 230 insertions(+), 18 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 net-next 1/5] net: phy: Add BaseT1 auto-negotiation constants
  2024-01-08  9:36 [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
@ 2024-01-08  9:36 ` Dimitri Fedrau
  2024-01-08 13:49   ` Andrew Lunn
  2024-01-08  9:36 ` [PATCH v4 net-next 2/5] net: phy: Support 100/1000BT1 linkmode advertisements Dimitri Fedrau
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08  9:36 UTC (permalink / raw)
  Cc: Dimitri Fedrau, Andrew Lunn, Heiner Kallweit, Russell King,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Eichenberger, netdev, linux-kernel

Added constants for advertising 100BT1 and 1000BT1 in register BASE-T1
auto-negotiation advertisement register [31:16] (Register 7.515)

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 include/uapi/linux/mdio.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index d03863da180e..020ccc810d23 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -348,6 +348,8 @@
 
 /* BASE-T1 auto-negotiation advertisement register [31:16] */
 #define MDIO_AN_T1_ADV_M_B10L		0x4000	/* device is compatible with 10BASE-T1L */
+#define MDIO_AN_T1_ADV_M_1000BT1	0x0080	/* advertise 1000BASE-T1 */
+#define MDIO_AN_T1_ADV_M_100BT1		0x0020	/* advertise 100BASE-T1 */
 #define MDIO_AN_T1_ADV_M_MST		0x0010	/* advertise master preference */
 
 /* BASE-T1 auto-negotiation advertisement register [47:32] */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 net-next 2/5] net: phy: Support 100/1000BT1 linkmode advertisements
  2024-01-08  9:36 [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
  2024-01-08  9:36 ` [PATCH v4 net-next 1/5] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
@ 2024-01-08  9:36 ` Dimitri Fedrau
  2024-01-08  9:36 ` [PATCH v4 net-next 3/5] net: phy: c45: detect 100/1000BASE-T1 " Dimitri Fedrau
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08  9:36 UTC (permalink / raw)
  Cc: Dimitri Fedrau, Andrew Lunn, Heiner Kallweit, Russell King,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Eichenberger, netdev, linux-kernel

Extend helper functions mii_t1_adv_m_mod_linkmode_t and
linkmode_adv_to_mii_t1_adv_m_t to support 100BT1 and 1000BT1 linkmode
advertisements.

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 include/linux/mdio.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 79ceee3c8673..ecd21acc7eed 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -373,6 +373,10 @@ static inline void mii_t1_adv_m_mod_linkmode_t(unsigned long *advertising, u32 l
 {
 	linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
 			 advertising, lpa & MDIO_AN_T1_ADV_M_B10L);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT1_Full_BIT,
+			 advertising, lpa & MDIO_AN_T1_ADV_M_100BT1);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT1_Full_BIT,
+			 advertising, lpa & MDIO_AN_T1_ADV_M_1000BT1);
 }
 
 /**
@@ -409,6 +413,10 @@ static inline u32 linkmode_adv_to_mii_t1_adv_m_t(unsigned long *advertising)
 
 	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, advertising))
 		result |= MDIO_AN_T1_ADV_M_B10L;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT1_Full_BIT, advertising))
+		result |= MDIO_AN_T1_ADV_M_100BT1;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, advertising))
+		result |= MDIO_AN_T1_ADV_M_1000BT1;
 
 	return result;
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 net-next 3/5] net: phy: c45: detect 100/1000BASE-T1 linkmode advertisements
  2024-01-08  9:36 [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
  2024-01-08  9:36 ` [PATCH v4 net-next 1/5] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
  2024-01-08  9:36 ` [PATCH v4 net-next 2/5] net: phy: Support 100/1000BT1 linkmode advertisements Dimitri Fedrau
@ 2024-01-08  9:36 ` Dimitri Fedrau
  2024-01-08  9:36 ` [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos Dimitri Fedrau
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08  9:36 UTC (permalink / raw)
  Cc: Dimitri Fedrau, Andrew Lunn, Heiner Kallweit, Russell King,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Eichenberger, netdev, linux-kernel

Set 100BT1 and 1000BT1 linkmode advertisement bits to adv_l_mask to
enable detection.

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy-c45.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 747d14bf152c..de8f5dc8be12 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -208,7 +208,8 @@ static int genphy_c45_baset1_an_config_aneg(struct phy_device *phydev)
 
 	adv_l_mask = MDIO_AN_T1_ADV_L_FORCE_MS | MDIO_AN_T1_ADV_L_PAUSE_CAP |
 		MDIO_AN_T1_ADV_L_PAUSE_ASYM;
-	adv_m_mask = MDIO_AN_T1_ADV_M_MST | MDIO_AN_T1_ADV_M_B10L;
+	adv_m_mask = MDIO_AN_T1_ADV_M_1000BT1 | MDIO_AN_T1_ADV_M_100BT1 |
+		MDIO_AN_T1_ADV_M_MST | MDIO_AN_T1_ADV_M_B10L;
 
 	switch (phydev->master_slave_set) {
 	case MASTER_SLAVE_CFG_MASTER_FORCE:
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos
  2024-01-08  9:36 [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
                   ` (2 preceding siblings ...)
  2024-01-08  9:36 ` [PATCH v4 net-next 3/5] net: phy: c45: detect 100/1000BASE-T1 " Dimitri Fedrau
@ 2024-01-08  9:36 ` Dimitri Fedrau
  2024-01-08 14:02   ` Andrew Lunn
  2024-01-08 14:05   ` Andrew Lunn
  2024-01-08  9:37 ` [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
  2024-01-08 11:18 ` [PATCH v4 net-next 0/5] " Heiner Kallweit
  5 siblings, 2 replies; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08  9:36 UTC (permalink / raw)
  Cc: Dimitri Fedrau, Andrew Lunn, Heiner Kallweit, Russell King,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Eichenberger, netdev, linux-kernel

Rename mv88q2xxxx_get_sqi to mv88q2xxx_get_sqi and
mv88q2xxxx_get_sqi_max to mv88q2xxx_get_sqi_max.
Fix linebreaks and use everywhere hexadecimal numbers written with
lowercase letters instead of mixing it up.

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
 drivers/net/phy/marvell-88q2xxx.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index 1c3ff77de56b..dcebb4643aff 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -14,7 +14,7 @@
 #define MDIO_MMD_AN_MV_STAT_MS_CONF_FAULT	0x8000
 
 #define MDIO_MMD_PCS_MV_100BT1_STAT1			33032
-#define MDIO_MMD_PCS_MV_100BT1_STAT1_IDLE_ERROR	0x00FF
+#define MDIO_MMD_PCS_MV_100BT1_STAT1_IDLE_ERROR		0x00ff
 #define MDIO_MMD_PCS_MV_100BT1_STAT1_JABBER		0x0100
 #define MDIO_MMD_PCS_MV_100BT1_STAT1_LINK		0x0200
 #define MDIO_MMD_PCS_MV_100BT1_STAT1_LOCAL_RX		0x1000
@@ -27,6 +27,8 @@
 #define MDIO_MMD_PCS_MV_100BT1_STAT2_LINK	0x0004
 #define MDIO_MMD_PCS_MV_100BT1_STAT2_ANGE	0x0008
 
+#define MDIO_MMD_PCS_MV_RX_STAT			33328
+
 static int mv88q2xxx_soft_reset(struct phy_device *phydev)
 {
 	int ret;
@@ -63,7 +65,8 @@ static int mv88q2xxx_read_link_gbit(struct phy_device *phydev)
 		 * the link was already down.
 		 */
 		if (!phy_polling_mode(phydev) || !phydev->link) {
-			ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_STAT);
+			ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+					   MDIO_PCS_1000BT1_STAT);
 			if (ret < 0)
 				return ret;
 			else if (ret & MDIO_PCS_1000BT1_STAT_LINK)
@@ -71,7 +74,8 @@ static int mv88q2xxx_read_link_gbit(struct phy_device *phydev)
 		}
 
 		if (!link) {
-			ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_STAT);
+			ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+					   MDIO_PCS_1000BT1_STAT);
 			if (ret < 0)
 				return ret;
 			else if (ret & MDIO_PCS_1000BT1_STAT_LINK)
@@ -95,7 +99,8 @@ static int mv88q2xxx_read_link_100m(struct phy_device *phydev)
 	 * we always read the realtime status.
 	 */
 	if (!phy_polling_mode(phydev) || !phydev->link) {
-		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_100BT1_STAT1);
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+				   MDIO_MMD_PCS_MV_100BT1_STAT1);
 		if (ret < 0)
 			return ret;
 		else if (ret & MDIO_MMD_PCS_MV_100BT1_STAT1_LINK)
@@ -200,7 +205,7 @@ static int mv88q2xxx_config_init(struct phy_device *phydev)
 	return mv88q2xxx_config_aneg(phydev);
 }
 
-static int mv88q2xxxx_get_sqi(struct phy_device *phydev)
+static int mv88q2xxx_get_sqi(struct phy_device *phydev)
 {
 	int ret;
 
@@ -208,7 +213,8 @@ static int mv88q2xxxx_get_sqi(struct phy_device *phydev)
 		/* Read the SQI from the vendor specific receiver status
 		 * register
 		 */
-		ret = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x8230);
+		ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+				   MDIO_MMD_PCS_MV_RX_STAT);
 		if (ret < 0)
 			return ret;
 
@@ -218,7 +224,7 @@ static int mv88q2xxxx_get_sqi(struct phy_device *phydev)
 		 * but can be found in the Software Initialization Guide. Only
 		 * revisions >= A0 are supported.
 		 */
-		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, 0xFC5D, 0x00FF, 0x00AC);
+		ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, 0xfc5d, 0xff, 0xac);
 		if (ret < 0)
 			return ret;
 
@@ -227,10 +233,10 @@ static int mv88q2xxxx_get_sqi(struct phy_device *phydev)
 			return ret;
 	}
 
-	return ret & 0x0F;
+	return ret & 0x0f;
 }
 
-static int mv88q2xxxx_get_sqi_max(struct phy_device *phydev)
+static int mv88q2xxx_get_sqi_max(struct phy_device *phydev)
 {
 	return 15;
 }
@@ -246,8 +252,8 @@ static struct phy_driver mv88q2xxx_driver[] = {
 		.read_status		= mv88q2xxx_read_status,
 		.soft_reset		= mv88q2xxx_soft_reset,
 		.set_loopback		= genphy_c45_loopback,
-		.get_sqi		= mv88q2xxxx_get_sqi,
-		.get_sqi_max		= mv88q2xxxx_get_sqi_max,
+		.get_sqi		= mv88q2xxx_get_sqi,
+		.get_sqi_max		= mv88q2xxx_get_sqi_max,
 	},
 };
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
  2024-01-08  9:36 [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
                   ` (3 preceding siblings ...)
  2024-01-08  9:36 ` [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos Dimitri Fedrau
@ 2024-01-08  9:37 ` Dimitri Fedrau
  2024-01-08 11:08   ` Simon Horman
  2024-01-11 13:49   ` kernel test robot
  2024-01-08 11:18 ` [PATCH v4 net-next 0/5] " Heiner Kallweit
  5 siblings, 2 replies; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08  9:37 UTC (permalink / raw)
  Cc: Dimitri Fedrau, Andrew Lunn, Heiner Kallweit, Russell King,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Eichenberger, netdev, linux-kernel

Add a driver for the Marvell 88Q2220. This driver allows to detect the
link, switch between 100BASE-T1 and 1000BASE-T1 and switch between
master and slave mode. Autonegoation is supported.

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
 drivers/net/phy/marvell-88q2xxx.c | 206 +++++++++++++++++++++++++++++-
 include/linux/marvell_phy.h       |   1 +
 2 files changed, 201 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index dcebb4643aff..8a0dae82ab2d 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -6,6 +6,8 @@
 #include <linux/marvell_phy.h>
 #include <linux/phy.h>
 
+#define PHY_ID_88Q2220_REVB0	(MARVELL_PHY_ID_88Q2220 | 0x1)
+
 #define MDIO_MMD_AN_MV_STAT			32769
 #define MDIO_MMD_AN_MV_STAT_ANEG		0x0100
 #define MDIO_MMD_AN_MV_STAT_LOCAL_RX		0x1000
@@ -13,6 +15,11 @@
 #define MDIO_MMD_AN_MV_STAT_LOCAL_MASTER	0x4000
 #define MDIO_MMD_AN_MV_STAT_MS_CONF_FAULT	0x8000
 
+#define MDIO_MMD_AN_MV_STAT2			32794
+#define MDIO_MMD_AN_MV_STAT2_AN_RESOLVED	0x0800
+#define MDIO_MMD_AN_MV_STAT2_100BT1		0x2000
+#define MDIO_MMD_AN_MV_STAT2_1000BT1		0x4000
+
 #define MDIO_MMD_PCS_MV_100BT1_STAT1			33032
 #define MDIO_MMD_PCS_MV_100BT1_STAT1_IDLE_ERROR		0x00ff
 #define MDIO_MMD_PCS_MV_100BT1_STAT1_JABBER		0x0100
@@ -29,6 +36,42 @@
 
 #define MDIO_MMD_PCS_MV_RX_STAT			33328
 
+struct mmd_val {
+	int devad;
+	u32 regnum;
+	u16 val;
+};
+
+const struct mmd_val mv88q222x_revb0_init_seq0[] = {
+	{ MDIO_MMD_PCS, 0x8033, 0x6801 },
+	{ MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
+	{ MDIO_MMD_PMAPMD, MDIO_CTRL1,
+	  MDIO_CTRL1_LPOWER | MDIO_PMA_CTRL1_SPEED1000 },
+	{ MDIO_MMD_PCS, 0xfe1b, 0x48 },
+	{ MDIO_MMD_PCS, 0xffe4, 0x6b6 },
+	{ MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0 },
+	{ MDIO_MMD_PCS, MDIO_CTRL1, 0x0 },
+};
+
+const struct mmd_val mv88q222x_revb0_init_seq1[] = {
+	{ MDIO_MMD_PCS, 0xfe79, 0x0 },
+	{ MDIO_MMD_PCS, 0xfe07, 0x125a },
+	{ MDIO_MMD_PCS, 0xfe09, 0x1288 },
+	{ MDIO_MMD_PCS, 0xfe08, 0x2588 },
+	{ MDIO_MMD_PCS, 0xfe11, 0x1105 },
+	{ MDIO_MMD_PCS, 0xfe72, 0x042c },
+	{ MDIO_MMD_PCS, 0xfbba, 0xcb2 },
+	{ MDIO_MMD_PCS, 0xfbbb, 0xc4a },
+	{ MDIO_MMD_AN, 0x8032, 0x2020 },
+	{ MDIO_MMD_AN, 0x8031, 0xa28 },
+	{ MDIO_MMD_AN, 0x8031, 0xc28 },
+	{ MDIO_MMD_PCS, 0xffdb, 0xfc10 },
+	{ MDIO_MMD_PCS, 0xfe1b, 0x58 },
+	{ MDIO_MMD_PCS, 0xfe79, 0x4 },
+	{ MDIO_MMD_PCS, 0xfe5f, 0xe8 },
+	{ MDIO_MMD_PCS, 0xfe05, 0x755c },
+};
+
 static int mv88q2xxx_soft_reset(struct phy_device *phydev)
 {
 	int ret;
@@ -125,24 +168,90 @@ static int mv88q2xxx_read_link_100m(struct phy_device *phydev)
 
 static int mv88q2xxx_read_link(struct phy_device *phydev)
 {
-	int ret;
-
 	/* The 88Q2XXX PHYs do not have the PMA/PMD status register available,
 	 * therefore we need to read the link status from the vendor specific
 	 * registers depending on the speed.
 	 */
+
 	if (phydev->speed == SPEED_1000)
-		ret = mv88q2xxx_read_link_gbit(phydev);
+		return mv88q2xxx_read_link_gbit(phydev);
+	else if (phydev->speed == SPEED_100)
+		return mv88q2xxx_read_link_100m(phydev);
+
+	phydev->link = false;
+	return 0;
+}
+
+static int mv88q2xxx_read_master_slave_state(struct phy_device *phydev)
+{
+	int ret;
+
+	phydev->master_slave_state = MASTER_SLAVE_STATE_UNKNOWN;
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_MMD_AN_MV_STAT);
+	if (ret < 0)
+		return ret;
+
+	if (ret & MDIO_MMD_AN_MV_STAT_LOCAL_MASTER)
+		phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
 	else
-		ret = mv88q2xxx_read_link_100m(phydev);
+		phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
 
-	return ret;
+	return 0;
+}
+
+static int mv88q2xxx_read_aneg_speed(struct phy_device *phydev)
+{
+	int ret;
+
+	phydev->speed = SPEED_UNKNOWN;
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_MMD_AN_MV_STAT2);
+	if (ret < 0)
+		return ret;
+
+	if (!(ret & MDIO_MMD_AN_MV_STAT2_AN_RESOLVED))
+		return 0;
+
+	if (ret & MDIO_MMD_AN_MV_STAT2_100BT1)
+		phydev->speed = SPEED_100;
+	else if (ret & MDIO_MMD_AN_MV_STAT2_1000BT1)
+		phydev->speed = SPEED_1000;
+
+	return 0;
 }
 
 static int mv88q2xxx_read_status(struct phy_device *phydev)
 {
 	int ret;
 
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		/* We have to get the negotiated speed first, otherwise we are
+		 * not able to read the link.
+		 */
+		ret = mv88q2xxx_read_aneg_speed(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = mv88q2xxx_read_link(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = genphy_c45_read_lpa(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = genphy_c45_baset1_read_status(phydev);
+		if (ret < 0)
+			return ret;
+
+		ret = mv88q2xxx_read_master_slave_state(phydev);
+		if (ret < 0)
+			return ret;
+
+		phy_resolve_aneg_linkmode(phydev);
+
+		return 0;
+	}
+
 	ret = mv88q2xxx_read_link(phydev);
 	if (ret < 0)
 		return ret;
@@ -171,7 +280,9 @@ static int mv88q2xxx_get_features(struct phy_device *phydev)
 	 * sequence provided by Marvell. Disable it for now until a proper
 	 * workaround is found or a new PHY revision is released.
 	 */
-	linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
+	if (phydev->drv->phy_id == MARVELL_PHY_ID_88Q2110)
+		linkmode_clear_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+				   phydev->supported);
 
 	return 0;
 }
@@ -241,6 +352,75 @@ static int mv88q2xxx_get_sqi_max(struct phy_device *phydev)
 	return 15;
 }
 
+static int mv88q222x_soft_reset(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Enable RESET of DCL */
+	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x48);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_1000BT1_CTRL,
+			    MDIO_PCS_1000BT1_CTRL_RESET);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, 0xffe4, 0xc);
+	if (ret < 0)
+		return ret;
+
+	/* Disable RESET of DCL */
+	if (phydev->autoneg == AUTONEG_ENABLE || phydev->speed == SPEED_1000)
+		return phy_write_mmd(phydev, MDIO_MMD_PCS, 0xfe1b, 0x58);
+
+	return 0;
+}
+
+static int mv88q222x_config_aneg(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_c45_config_aneg(phydev);
+	if (ret)
+		return ret;
+
+	return mv88q222x_soft_reset(phydev);
+}
+
+static int mv88q222x_revb0_config_init(struct phy_device *phydev)
+{
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(mv88q222x_revb0_init_seq0); i++) {
+		ret = phy_write_mmd(phydev, mv88q222x_revb0_init_seq0[i].devad,
+				    mv88q222x_revb0_init_seq0[i].regnum,
+				    mv88q222x_revb0_init_seq0[i].val);
+		if (ret < 0)
+			return ret;
+	}
+
+	usleep_range(5000, 10000);
+
+	for (i = 0; i < ARRAY_SIZE(mv88q222x_revb0_init_seq1); i++) {
+		ret = phy_write_mmd(phydev, mv88q222x_revb0_init_seq1[i].devad,
+				    mv88q222x_revb0_init_seq1[i].regnum,
+				    mv88q222x_revb0_init_seq1[i].val);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* The 88Q2XXX PHYs do have the extended ability register available, but
+	 * register MDIO_PMA_EXTABLE where they should signalize it does not
+	 * work according to specification. Therefore, we force it here.
+	 */
+	phydev->pma_extable = MDIO_PMA_EXTABLE_BT1;
+
+	return 0;
+}
+
 static struct phy_driver mv88q2xxx_driver[] = {
 	{
 		.phy_id			= MARVELL_PHY_ID_88Q2110,
@@ -255,12 +435,26 @@ static struct phy_driver mv88q2xxx_driver[] = {
 		.get_sqi		= mv88q2xxx_get_sqi,
 		.get_sqi_max		= mv88q2xxx_get_sqi_max,
 	},
+	{
+		PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
+		.name			= "mv88q2220",
+		.get_features		= mv88q2xxx_get_features,
+		.config_aneg		= mv88q222x_config_aneg,
+		.aneg_done		= genphy_c45_aneg_done,
+		.config_init		= mv88q222x_revb0_config_init,
+		.read_status		= mv88q2xxx_read_status,
+		.soft_reset		= mv88q222x_soft_reset,
+		.set_loopback		= genphy_c45_loopback,
+		.get_sqi		= mv88q2xxx_get_sqi,
+		.get_sqi_max		= mv88q2xxx_get_sqi_max,
+	},
 };
 
 module_phy_driver(mv88q2xxx_driver);
 
 static struct mdio_device_id __maybe_unused mv88q2xxx_tbl[] = {
 	{ MARVELL_PHY_ID_88Q2110, MARVELL_PHY_ID_MASK },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0), },
 	{ /*sentinel*/ }
 };
 MODULE_DEVICE_TABLE(mdio, mv88q2xxx_tbl);
diff --git a/include/linux/marvell_phy.h b/include/linux/marvell_phy.h
index 9b54c4f0677f..693eba9869e4 100644
--- a/include/linux/marvell_phy.h
+++ b/include/linux/marvell_phy.h
@@ -26,6 +26,7 @@
 #define MARVELL_PHY_ID_88E2110		0x002b09b0
 #define MARVELL_PHY_ID_88X2222		0x01410f10
 #define MARVELL_PHY_ID_88Q2110		0x002b0980
+#define MARVELL_PHY_ID_88Q2220		0x002b0b20
 
 /* Marvel 88E1111 in Finisar SFP module with modified PHY ID */
 #define MARVELL_PHY_ID_88E1111_FINISAR	0x01ff0cc0
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
  2024-01-08  9:37 ` [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
@ 2024-01-08 11:08   ` Simon Horman
  2024-01-08 11:16     ` Dimitri Fedrau
  2024-01-11 13:49   ` kernel test robot
  1 sibling, 1 reply; 17+ messages in thread
From: Simon Horman @ 2024-01-08 11:08 UTC (permalink / raw)
  To: Dimitri Fedrau
  Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Stefan Eichenberger,
	netdev, linux-kernel

On Mon, Jan 08, 2024 at 10:37:00AM +0100, Dimitri Fedrau wrote:
> Add a driver for the Marvell 88Q2220. This driver allows to detect the
> link, switch between 100BASE-T1 and 1000BASE-T1 and switch between
> master and slave mode. Autonegoation is supported.

nit: Autonegotiation

> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> ---
>  drivers/net/phy/marvell-88q2xxx.c | 206 +++++++++++++++++++++++++++++-
>  include/linux/marvell_phy.h       |   1 +
>  2 files changed, 201 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c

...

> @@ -29,6 +36,42 @@
>  
>  #define MDIO_MMD_PCS_MV_RX_STAT			33328
>  
> +struct mmd_val {
> +	int devad;
> +	u32 regnum;
> +	u16 val;
> +};
> +
> +const struct mmd_val mv88q222x_revb0_init_seq0[] = {
> +	{ MDIO_MMD_PCS, 0x8033, 0x6801 },
> +	{ MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
> +	{ MDIO_MMD_PMAPMD, MDIO_CTRL1,
> +	  MDIO_CTRL1_LPOWER | MDIO_PMA_CTRL1_SPEED1000 },
> +	{ MDIO_MMD_PCS, 0xfe1b, 0x48 },
> +	{ MDIO_MMD_PCS, 0xffe4, 0x6b6 },
> +	{ MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0 },
> +	{ MDIO_MMD_PCS, MDIO_CTRL1, 0x0 },
> +};
> +
> +const struct mmd_val mv88q222x_revb0_init_seq1[] = {
> +	{ MDIO_MMD_PCS, 0xfe79, 0x0 },
> +	{ MDIO_MMD_PCS, 0xfe07, 0x125a },
> +	{ MDIO_MMD_PCS, 0xfe09, 0x1288 },
> +	{ MDIO_MMD_PCS, 0xfe08, 0x2588 },
> +	{ MDIO_MMD_PCS, 0xfe11, 0x1105 },
> +	{ MDIO_MMD_PCS, 0xfe72, 0x042c },
> +	{ MDIO_MMD_PCS, 0xfbba, 0xcb2 },
> +	{ MDIO_MMD_PCS, 0xfbbb, 0xc4a },
> +	{ MDIO_MMD_AN, 0x8032, 0x2020 },
> +	{ MDIO_MMD_AN, 0x8031, 0xa28 },
> +	{ MDIO_MMD_AN, 0x8031, 0xc28 },
> +	{ MDIO_MMD_PCS, 0xffdb, 0xfc10 },
> +	{ MDIO_MMD_PCS, 0xfe1b, 0x58 },
> +	{ MDIO_MMD_PCS, 0xfe79, 0x4 },
> +	{ MDIO_MMD_PCS, 0xfe5f, 0xe8 },
> +	{ MDIO_MMD_PCS, 0xfe05, 0x755c },
> +};

nit: mv88q222x_revb0_init_seq0 and mv88q222x_revb0_init_seq1 seem
    to only be used in this file. Perhaps they should be static.

...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
  2024-01-08 11:08   ` Simon Horman
@ 2024-01-08 11:16     ` Dimitri Fedrau
  0 siblings, 0 replies; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08 11:16 UTC (permalink / raw)
  To: Simon Horman
  Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Stefan Eichenberger,
	netdev, linux-kernel

Am Mon, Jan 08, 2024 at 11:08:44AM +0000 schrieb Simon Horman:
> On Mon, Jan 08, 2024 at 10:37:00AM +0100, Dimitri Fedrau wrote:
> > Add a driver for the Marvell 88Q2220. This driver allows to detect the
> > link, switch between 100BASE-T1 and 1000BASE-T1 and switch between
> > master and slave mode. Autonegoation is supported.
> 
> nit: Autonegotiation
> 
Will fix it in V5.
> > Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> > ---
> >  drivers/net/phy/marvell-88q2xxx.c | 206 +++++++++++++++++++++++++++++-
> >  include/linux/marvell_phy.h       |   1 +
> >  2 files changed, 201 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> 
> ...
> 
> > @@ -29,6 +36,42 @@
> >  
> >  #define MDIO_MMD_PCS_MV_RX_STAT			33328
> >  
> > +struct mmd_val {
> > +	int devad;
> > +	u32 regnum;
> > +	u16 val;
> > +};
> > +
> > +const struct mmd_val mv88q222x_revb0_init_seq0[] = {
> > +	{ MDIO_MMD_PCS, 0x8033, 0x6801 },
> > +	{ MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
> > +	{ MDIO_MMD_PMAPMD, MDIO_CTRL1,
> > +	  MDIO_CTRL1_LPOWER | MDIO_PMA_CTRL1_SPEED1000 },
> > +	{ MDIO_MMD_PCS, 0xfe1b, 0x48 },
> > +	{ MDIO_MMD_PCS, 0xffe4, 0x6b6 },
> > +	{ MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0 },
> > +	{ MDIO_MMD_PCS, MDIO_CTRL1, 0x0 },
> > +};
> > +
> > +const struct mmd_val mv88q222x_revb0_init_seq1[] = {
> > +	{ MDIO_MMD_PCS, 0xfe79, 0x0 },
> > +	{ MDIO_MMD_PCS, 0xfe07, 0x125a },
> > +	{ MDIO_MMD_PCS, 0xfe09, 0x1288 },
> > +	{ MDIO_MMD_PCS, 0xfe08, 0x2588 },
> > +	{ MDIO_MMD_PCS, 0xfe11, 0x1105 },
> > +	{ MDIO_MMD_PCS, 0xfe72, 0x042c },
> > +	{ MDIO_MMD_PCS, 0xfbba, 0xcb2 },
> > +	{ MDIO_MMD_PCS, 0xfbbb, 0xc4a },
> > +	{ MDIO_MMD_AN, 0x8032, 0x2020 },
> > +	{ MDIO_MMD_AN, 0x8031, 0xa28 },
> > +	{ MDIO_MMD_AN, 0x8031, 0xc28 },
> > +	{ MDIO_MMD_PCS, 0xffdb, 0xfc10 },
> > +	{ MDIO_MMD_PCS, 0xfe1b, 0x58 },
> > +	{ MDIO_MMD_PCS, 0xfe79, 0x4 },
> > +	{ MDIO_MMD_PCS, 0xfe5f, 0xe8 },
> > +	{ MDIO_MMD_PCS, 0xfe05, 0x755c },
> > +};
> 
> nit: mv88q222x_revb0_init_seq0 and mv88q222x_revb0_init_seq1 seem
>     to only be used in this file. Perhaps they should be static.
> 
> ...

You are right, will fix it in V5. Thanks.

Best regards,
Dimitri

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
  2024-01-08  9:36 [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
                   ` (4 preceding siblings ...)
  2024-01-08  9:37 ` [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
@ 2024-01-08 11:18 ` Heiner Kallweit
  2024-01-08 11:24   ` Dimitri Fedrau
  5 siblings, 1 reply; 17+ messages in thread
From: Heiner Kallweit @ 2024-01-08 11:18 UTC (permalink / raw)
  To: Dimitri Fedrau
  Cc: Andrew Lunn, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
	linux-kernel

On 08.01.2024 10:36, Dimitri Fedrau wrote:
> Changes in v2:
> 	- used defines MDIO_CTRL1_LPOWER and MDIO_PMA_CTRL1_SPEED1000
> 	  in mv88q222x_config_aneg_preinit
> 	- use genphy_c45_loopback
> 	- mv88q2xxx_read_status reads speed, master or slave state when
> 	  autonegotiation is enabled
> 	- added defines for magic values in mv88q222x_get_sqi
> 
> Changes in v3:
> 	- mv88q2xxx_read_status includes autonegotiation case
> 	- add support for 100BT1 and 1000BT1 linkmode advertisement
> 	- use mv88q2xxx_get_sqi and mv88q2xxx_get_sqi_max, remove
> 	  mv88q222x_get_sqi and mv88q222x_get_sqi_max
> 	- fix typo: rename mv88q2xxxx_get_sqi and mv88q2xxxx_get_sqi_max to
> 	  mv88q2xxx_get_sqi and mv88q2xxx_get_sqi
> 	- add define MDIO_MMD_PCS_MV_RX_STAT for magic value 0x8230, documented
> 	  in latest datasheets for both PHYs
> 
> Changes in V4:
> 	- clean up init sequence
> 	- separate patch for fixing typos in upstreamed code
> 
> Dimitri Fedrau (5):
>   net: phy: Add BaseT1 auto-negotiation constants
>   net: phy: Support 100/1000BT1 linkmode advertisements
>   net: phy: c45: detect 100/1000BASE-T1 linkmode advertisements
>   net: phy: marvell-88q2xxx: fix typos
>   net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
> 
>  drivers/net/phy/marvell-88q2xxx.c | 234 +++++++++++++++++++++++++++---
>  drivers/net/phy/phy-c45.c         |   3 +-
>  include/linux/marvell_phy.h       |   1 +
>  include/linux/mdio.h              |   8 +
>  include/uapi/linux/mdio.h         |   2 +
>  5 files changed, 230 insertions(+), 18 deletions(-)
> 
net-next is closed. Let's see whether the maintainers still accept your series.
Otherwise you may have to resubmit once net-next opens again.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
  2024-01-08 11:18 ` [PATCH v4 net-next 0/5] " Heiner Kallweit
@ 2024-01-08 11:24   ` Dimitri Fedrau
  2024-01-08 11:28     ` Heiner Kallweit
  0 siblings, 1 reply; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08 11:24 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Andrew Lunn, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
	linux-kernel

Am Mon, Jan 08, 2024 at 12:18:30PM +0100 schrieb Heiner Kallweit:
> On 08.01.2024 10:36, Dimitri Fedrau wrote:
> > Changes in v2:
> > 	- used defines MDIO_CTRL1_LPOWER and MDIO_PMA_CTRL1_SPEED1000
> > 	  in mv88q222x_config_aneg_preinit
> > 	- use genphy_c45_loopback
> > 	- mv88q2xxx_read_status reads speed, master or slave state when
> > 	  autonegotiation is enabled
> > 	- added defines for magic values in mv88q222x_get_sqi
> > 
> > Changes in v3:
> > 	- mv88q2xxx_read_status includes autonegotiation case
> > 	- add support for 100BT1 and 1000BT1 linkmode advertisement
> > 	- use mv88q2xxx_get_sqi and mv88q2xxx_get_sqi_max, remove
> > 	  mv88q222x_get_sqi and mv88q222x_get_sqi_max
> > 	- fix typo: rename mv88q2xxxx_get_sqi and mv88q2xxxx_get_sqi_max to
> > 	  mv88q2xxx_get_sqi and mv88q2xxx_get_sqi
> > 	- add define MDIO_MMD_PCS_MV_RX_STAT for magic value 0x8230, documented
> > 	  in latest datasheets for both PHYs
> > 
> > Changes in V4:
> > 	- clean up init sequence
> > 	- separate patch for fixing typos in upstreamed code
> > 
> > Dimitri Fedrau (5):
> >   net: phy: Add BaseT1 auto-negotiation constants
> >   net: phy: Support 100/1000BT1 linkmode advertisements
> >   net: phy: c45: detect 100/1000BASE-T1 linkmode advertisements
> >   net: phy: marvell-88q2xxx: fix typos
> >   net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
> > 
> >  drivers/net/phy/marvell-88q2xxx.c | 234 +++++++++++++++++++++++++++---
> >  drivers/net/phy/phy-c45.c         |   3 +-
> >  include/linux/marvell_phy.h       |   1 +
> >  include/linux/mdio.h              |   8 +
> >  include/uapi/linux/mdio.h         |   2 +
> >  5 files changed, 230 insertions(+), 18 deletions(-)
> > 
> net-next is closed. Let's see whether the maintainers still accept your series.
> Otherwise you may have to resubmit once net-next opens again.
>
Hi Heiner,

thanks for the information, next time I will check if net-next is
closed.

Best regards,
Dimitri Fedrau

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
  2024-01-08 11:24   ` Dimitri Fedrau
@ 2024-01-08 11:28     ` Heiner Kallweit
  0 siblings, 0 replies; 17+ messages in thread
From: Heiner Kallweit @ 2024-01-08 11:28 UTC (permalink / raw)
  To: Dimitri Fedrau
  Cc: Andrew Lunn, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
	linux-kernel

On 08.01.2024 12:24, Dimitri Fedrau wrote:
> Am Mon, Jan 08, 2024 at 12:18:30PM +0100 schrieb Heiner Kallweit:
>> On 08.01.2024 10:36, Dimitri Fedrau wrote:
>>> Changes in v2:
>>> 	- used defines MDIO_CTRL1_LPOWER and MDIO_PMA_CTRL1_SPEED1000
>>> 	  in mv88q222x_config_aneg_preinit
>>> 	- use genphy_c45_loopback
>>> 	- mv88q2xxx_read_status reads speed, master or slave state when
>>> 	  autonegotiation is enabled
>>> 	- added defines for magic values in mv88q222x_get_sqi
>>>
>>> Changes in v3:
>>> 	- mv88q2xxx_read_status includes autonegotiation case
>>> 	- add support for 100BT1 and 1000BT1 linkmode advertisement
>>> 	- use mv88q2xxx_get_sqi and mv88q2xxx_get_sqi_max, remove
>>> 	  mv88q222x_get_sqi and mv88q222x_get_sqi_max
>>> 	- fix typo: rename mv88q2xxxx_get_sqi and mv88q2xxxx_get_sqi_max to
>>> 	  mv88q2xxx_get_sqi and mv88q2xxx_get_sqi
>>> 	- add define MDIO_MMD_PCS_MV_RX_STAT for magic value 0x8230, documented
>>> 	  in latest datasheets for both PHYs
>>>
>>> Changes in V4:
>>> 	- clean up init sequence
>>> 	- separate patch for fixing typos in upstreamed code
>>>
>>> Dimitri Fedrau (5):
>>>   net: phy: Add BaseT1 auto-negotiation constants
>>>   net: phy: Support 100/1000BT1 linkmode advertisements
>>>   net: phy: c45: detect 100/1000BASE-T1 linkmode advertisements
>>>   net: phy: marvell-88q2xxx: fix typos
>>>   net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
>>>
>>>  drivers/net/phy/marvell-88q2xxx.c | 234 +++++++++++++++++++++++++++---
>>>  drivers/net/phy/phy-c45.c         |   3 +-
>>>  include/linux/marvell_phy.h       |   1 +
>>>  include/linux/mdio.h              |   8 +
>>>  include/uapi/linux/mdio.h         |   2 +
>>>  5 files changed, 230 insertions(+), 18 deletions(-)
>>>
>> net-next is closed. Let's see whether the maintainers still accept your series.
>> Otherwise you may have to resubmit once net-next opens again.
>>
> Hi Heiner,
> 
> thanks for the information, next time I will check if net-next is
> closed.
> 
https://lwn.net/Articles/727558/

> Best regards,
> Dimitri Fedrau


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 1/5] net: phy: Add BaseT1 auto-negotiation constants
  2024-01-08  9:36 ` [PATCH v4 net-next 1/5] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
@ 2024-01-08 13:49   ` Andrew Lunn
  2024-01-08 14:25     ` Dimitri Fedrau
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Lunn @ 2024-01-08 13:49 UTC (permalink / raw)
  To: Dimitri Fedrau
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
	linux-kernel

On Mon, Jan 08, 2024 at 10:36:56AM +0100, Dimitri Fedrau wrote:
> Added constants for advertising 100BT1 and 1000BT1 in register BASE-T1
> auto-negotiation advertisement register [31:16] (Register 7.515)
> 
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Sorry, more nit-picking :-(

Signed-off-by, Reviewed-by:, Fixes: etc should all be together. No
blanks lines between them. And Signed-off-by: comes last.

Given the missing statics, it looks like you will need to repost in
two weeks times.

       Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos
  2024-01-08  9:36 ` [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos Dimitri Fedrau
@ 2024-01-08 14:02   ` Andrew Lunn
  2024-01-08 14:31     ` Dimitri Fedrau
  2024-01-08 14:05   ` Andrew Lunn
  1 sibling, 1 reply; 17+ messages in thread
From: Andrew Lunn @ 2024-01-08 14:02 UTC (permalink / raw)
  To: Dimitri Fedrau
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
	linux-kernel

On Mon, Jan 08, 2024 at 10:36:59AM +0100, Dimitri Fedrau wrote:
> Rename mv88q2xxxx_get_sqi to mv88q2xxx_get_sqi and
> mv88q2xxxx_get_sqi_max to mv88q2xxx_get_sqi_max.
> Fix linebreaks and use everywhere hexadecimal numbers written with
> lowercase letters instead of mixing it up.

You could split is up into three patches. Its probably not worth it
now, but its something to remember for the future.

Ideally you want lots of small patches which are obviously correct.  A
patch just containing a rename mv88q2xxxx_get_XXX to
mv88q2xxx_get_sqi_XXX etc, should be obviously correct, and just takes
a few seconds to review.

A patch adding a few line breaks should again take a few seconds to
review.

Upper case to lower case is easy to review.

When it is all mixed together, in a bigger patch it takes a bit more
effort to review, a bit more effort is needed to look for typ0s etc.
Its can be faster and easier to review 10 very simple patches than 3
big patches...

    Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos
  2024-01-08  9:36 ` [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos Dimitri Fedrau
  2024-01-08 14:02   ` Andrew Lunn
@ 2024-01-08 14:05   ` Andrew Lunn
  1 sibling, 0 replies; 17+ messages in thread
From: Andrew Lunn @ 2024-01-08 14:05 UTC (permalink / raw)
  To: Dimitri Fedrau
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
	linux-kernel

On Mon, Jan 08, 2024 at 10:36:59AM +0100, Dimitri Fedrau wrote:
> Rename mv88q2xxxx_get_sqi to mv88q2xxx_get_sqi and
> mv88q2xxxx_get_sqi_max to mv88q2xxx_get_sqi_max.
> Fix linebreaks and use everywhere hexadecimal numbers written with
> lowercase letters instead of mixing it up.
> 
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>

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

    Andrew

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 1/5] net: phy: Add BaseT1 auto-negotiation constants
  2024-01-08 13:49   ` Andrew Lunn
@ 2024-01-08 14:25     ` Dimitri Fedrau
  0 siblings, 0 replies; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08 14:25 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
	linux-kernel

Am Mon, Jan 08, 2024 at 02:49:02PM +0100 schrieb Andrew Lunn:
> On Mon, Jan 08, 2024 at 10:36:56AM +0100, Dimitri Fedrau wrote:
> > Added constants for advertising 100BT1 and 1000BT1 in register BASE-T1
> > auto-negotiation advertisement register [31:16] (Register 7.515)
> > 
> > Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> > 
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> Sorry, more nit-picking :-(
>
> Signed-off-by, Reviewed-by:, Fixes: etc should all be together. No
> blanks lines between them. And Signed-off-by: comes last.
>
I'm fine with it. :-) Will fix this in V5.

> Given the missing statics, it looks like you will need to repost in
> two weeks times.
> 

That's okay. I will repost in two weeks when net-next is open. Do you
mind if I add further patches to the series ?

>        Andrew

Best regards,
Dimitri

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos
  2024-01-08 14:02   ` Andrew Lunn
@ 2024-01-08 14:31     ` Dimitri Fedrau
  0 siblings, 0 replies; 17+ messages in thread
From: Dimitri Fedrau @ 2024-01-08 14:31 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
	linux-kernel

Am Mon, Jan 08, 2024 at 03:02:18PM +0100 schrieb Andrew Lunn:
> On Mon, Jan 08, 2024 at 10:36:59AM +0100, Dimitri Fedrau wrote:
> > Rename mv88q2xxxx_get_sqi to mv88q2xxx_get_sqi and
> > mv88q2xxxx_get_sqi_max to mv88q2xxx_get_sqi_max.
> > Fix linebreaks and use everywhere hexadecimal numbers written with
> > lowercase letters instead of mixing it up.
> 
> You could split is up into three patches. Its probably not worth it
> now, but its something to remember for the future.
> 
> Ideally you want lots of small patches which are obviously correct.  A
> patch just containing a rename mv88q2xxxx_get_XXX to
> mv88q2xxx_get_sqi_XXX etc, should be obviously correct, and just takes
> a few seconds to review.
> 
> A patch adding a few line breaks should again take a few seconds to
> review.
> 
> Upper case to lower case is easy to review.
> 
> When it is all mixed together, in a bigger patch it takes a bit more
> effort to review, a bit more effort is needed to look for typ0s etc.
> Its can be faster and easier to review 10 very simple patches than 3
> big patches...
>
It makes totally sense to me. Haven't thought about it. Thanks for
your explanation. I will keep it in mind for future patches.

>     Andrew

Best regards,
Dimitri

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
  2024-01-08  9:37 ` [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
  2024-01-08 11:08   ` Simon Horman
@ 2024-01-11 13:49   ` kernel test robot
  1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-01-11 13:49 UTC (permalink / raw)
  To: Dimitri Fedrau
  Cc: oe-kbuild-all, Dimitri Fedrau, Andrew Lunn, Heiner Kallweit,
	Russell King, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Stefan Eichenberger, netdev, linux-kernel

Hi Dimitri,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Dimitri-Fedrau/net-phy-Add-BaseT1-auto-negotiation-constants/20240108-174130
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240108093702.13476-6-dima.fedrau%40gmail.com
patch subject: [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
config: i386-randconfig-063-20240111 (https://download.01.org/0day-ci/archive/20240111/202401112120.tfRSOQJm-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240111/202401112120.tfRSOQJm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401112120.tfRSOQJm-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/phy/marvell-88q2xxx.c:45:22: sparse: sparse: symbol 'mv88q222x_revb0_init_seq0' was not declared. Should it be static?
>> drivers/net/phy/marvell-88q2xxx.c:56:22: sparse: sparse: symbol 'mv88q222x_revb0_init_seq1' was not declared. Should it be static?

vim +/mv88q222x_revb0_init_seq0 +45 drivers/net/phy/marvell-88q2xxx.c

    44	
  > 45	const struct mmd_val mv88q222x_revb0_init_seq0[] = {
    46		{ MDIO_MMD_PCS, 0x8033, 0x6801 },
    47		{ MDIO_MMD_AN, MDIO_AN_T1_CTRL, 0x0 },
    48		{ MDIO_MMD_PMAPMD, MDIO_CTRL1,
    49		  MDIO_CTRL1_LPOWER | MDIO_PMA_CTRL1_SPEED1000 },
    50		{ MDIO_MMD_PCS, 0xfe1b, 0x48 },
    51		{ MDIO_MMD_PCS, 0xffe4, 0x6b6 },
    52		{ MDIO_MMD_PMAPMD, MDIO_CTRL1, 0x0 },
    53		{ MDIO_MMD_PCS, MDIO_CTRL1, 0x0 },
    54	};
    55	
  > 56	const struct mmd_val mv88q222x_revb0_init_seq1[] = {
    57		{ MDIO_MMD_PCS, 0xfe79, 0x0 },
    58		{ MDIO_MMD_PCS, 0xfe07, 0x125a },
    59		{ MDIO_MMD_PCS, 0xfe09, 0x1288 },
    60		{ MDIO_MMD_PCS, 0xfe08, 0x2588 },
    61		{ MDIO_MMD_PCS, 0xfe11, 0x1105 },
    62		{ MDIO_MMD_PCS, 0xfe72, 0x042c },
    63		{ MDIO_MMD_PCS, 0xfbba, 0xcb2 },
    64		{ MDIO_MMD_PCS, 0xfbbb, 0xc4a },
    65		{ MDIO_MMD_AN, 0x8032, 0x2020 },
    66		{ MDIO_MMD_AN, 0x8031, 0xa28 },
    67		{ MDIO_MMD_AN, 0x8031, 0xc28 },
    68		{ MDIO_MMD_PCS, 0xffdb, 0xfc10 },
    69		{ MDIO_MMD_PCS, 0xfe1b, 0x58 },
    70		{ MDIO_MMD_PCS, 0xfe79, 0x4 },
    71		{ MDIO_MMD_PCS, 0xfe5f, 0xe8 },
    72		{ MDIO_MMD_PCS, 0xfe05, 0x755c },
    73	};
    74	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-01-11 13:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-08  9:36 [PATCH v4 net-next 0/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
2024-01-08  9:36 ` [PATCH v4 net-next 1/5] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
2024-01-08 13:49   ` Andrew Lunn
2024-01-08 14:25     ` Dimitri Fedrau
2024-01-08  9:36 ` [PATCH v4 net-next 2/5] net: phy: Support 100/1000BT1 linkmode advertisements Dimitri Fedrau
2024-01-08  9:36 ` [PATCH v4 net-next 3/5] net: phy: c45: detect 100/1000BASE-T1 " Dimitri Fedrau
2024-01-08  9:36 ` [PATCH v4 net-next 4/5] net: phy: marvell-88q2xxx: fix typos Dimitri Fedrau
2024-01-08 14:02   ` Andrew Lunn
2024-01-08 14:31     ` Dimitri Fedrau
2024-01-08 14:05   ` Andrew Lunn
2024-01-08  9:37 ` [PATCH v4 net-next 5/5] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
2024-01-08 11:08   ` Simon Horman
2024-01-08 11:16     ` Dimitri Fedrau
2024-01-11 13:49   ` kernel test robot
2024-01-08 11:18 ` [PATCH v4 net-next 0/5] " Heiner Kallweit
2024-01-08 11:24   ` Dimitri Fedrau
2024-01-08 11:28     ` Heiner Kallweit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).