* [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
@ 2024-01-22 21:28 Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 01/13] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
` (13 more replies)
0 siblings, 14 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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
Changes in V5:
- add missing statics for mv88q222x_revb0_init_seq0 and
mv88q222x_revb0_init_seq1
- fix typo in commit message: autonegotiation
- fix ordering of Signed-off-by and Reviewed-by in commit messages
- add interrupt support for link detection
- add suspend / resume ops
- add support for internal temperature sensor
- add cable test support
- call .soft_reset in mv88q2xxx_config_aneg, this makes
mv88q2xxx_config_aneg compatible for Marvell88Q222x devices and
remove mv88q222x_config_aneg which is then just duplicated code
- cleanup mv88q2xxx_config_init and make it compatible with
Marvell88Q222x devices
- move parts from mv88q222x_config_init to mv88q2xxx_config_init
that are applicable for all Marvell88Q2xxx devices.
Dimitri Fedrau (13):
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
net: phy: marvell-88q2xxx: add interrupt support for link detection
net: phy: marvell-88q2xxx: add suspend / resume ops
net: phy: marvell-88q2xxx: add support for temperature sensor
net: phy: marvell-88q2xxx: add cable test support
net: phy: marvell-88q2xxx: make mv88q2xxx_config_aneg generic
net: phy: marvell-88q2xxx: switch to mv88q2xxx_config_aneg
net: phy: marvell-88q2xxx: cleanup mv88q2xxx_config_init
net: phy: marvell-88q2xxx: remove redundant code
drivers/net/phy/marvell-88q2xxx.c | 634 ++++++++++++++++++++++++++++--
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, 618 insertions(+), 30 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 net-next 01/13] net: phy: Add BaseT1 auto-negotiation constants
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 02/13] net: phy: Support 100/1000BT1 linkmode advertisements Dimitri Fedrau
` (12 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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)
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
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] 44+ messages in thread
* [PATCH v5 net-next 02/13] net: phy: Support 100/1000BT1 linkmode advertisements
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 01/13] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 03/13] net: phy: c45: detect 100/1000BASE-T1 " Dimitri Fedrau
` (11 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
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] 44+ messages in thread
* [PATCH v5 net-next 03/13] net: phy: c45: detect 100/1000BASE-T1 linkmode advertisements
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 01/13] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 02/13] net: phy: Support 100/1000BT1 linkmode advertisements Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 04/13] net: phy: marvell-88q2xxx: fix typos Dimitri Fedrau
` (10 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
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] 44+ messages in thread
* [PATCH v5 net-next 04/13] net: phy: marvell-88q2xxx: fix typos
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (2 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 03/13] net: phy: c45: detect 100/1000BASE-T1 " Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 05/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (9 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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] 44+ messages in thread
* [PATCH v5 net-next 05/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (3 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 04/13] net: phy: marvell-88q2xxx: fix typos Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-31 15:08 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 06/13] net: phy: marvell-88q2xxx: add interrupt support for link detection Dimitri Fedrau
` (8 subsequent siblings)
13 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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. Autonegotiation 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..637ef4821e35 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;
+};
+
+static 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 },
+};
+
+static 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] 44+ messages in thread
* [PATCH v5 net-next 06/13] net: phy: marvell-88q2xxx: add interrupt support for link detection
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (4 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 05/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-31 15:12 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 07/13] net: phy: marvell-88q2xxx: add suspend / resume ops Dimitri Fedrau
` (7 subsequent siblings)
13 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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 .config_intr and .handle_interrupt callbacks. Whenever the link
goes up or down an interrupt will be triggered. Interrupts are configured
separately for 100/1000BASET1.
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
drivers/net/phy/marvell-88q2xxx.c | 123 +++++++++++++++++++++++++++++-
1 file changed, 119 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index 637ef4821e35..635827b4a692 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -20,6 +20,19 @@
#define MDIO_MMD_AN_MV_STAT2_100BT1 0x2000
#define MDIO_MMD_AN_MV_STAT2_1000BT1 0x4000
+#define MDIO_MMD_PCS_MV_INT_EN 32784
+#define MDIO_MMD_PCS_MV_INT_EN_LINK_UP 0x0040
+#define MDIO_MMD_PCS_MV_INT_EN_LINK_DOWN 0x0080
+#define MDIO_MMD_PCS_MV_INT_EN_100BT1 0x1000
+
+#define MDIO_MMD_PCS_MV_GPIO_INT_STAT 32785
+#define MDIO_MMD_PCS_MV_GPIO_INT_STAT_LINK_UP 0x0040
+#define MDIO_MMD_PCS_MV_GPIO_INT_STAT_LINK_DOWN 0x0080
+#define MDIO_MMD_PCS_MV_GPIO_INT_STAT_100BT1_GEN 0x1000
+
+#define MDIO_MMD_PCS_MV_GPIO_INT_CTRL 32787
+#define MDIO_MMD_PCS_MV_GPIO_INT_CTRL_TRI_DIS 0x0800
+
#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
@@ -34,6 +47,12 @@
#define MDIO_MMD_PCS_MV_100BT1_STAT2_LINK 0x0004
#define MDIO_MMD_PCS_MV_100BT1_STAT2_ANGE 0x0008
+#define MDIO_MMD_PCS_MV_100BT1_INT_EN 33042
+#define MDIO_MMD_PCS_MV_100BT1_INT_EN_LINKEVENT 0x0400
+
+#define MDIO_MMD_PCS_MV_COPPER_INT_STAT 33043
+#define MDIO_MMD_PCS_MV_COPPER_INT_STAT_LINKEVENT 0x0400
+
#define MDIO_MMD_PCS_MV_RX_STAT 33328
struct mmd_val {
@@ -95,13 +114,15 @@ static int mv88q2xxx_read_link_gbit(struct phy_device *phydev)
/* Read vendor specific Auto-Negotiation status register to get local
* and remote receiver status according to software initialization
- * guide.
+ * guide. However, when not in polling mode the local and remote
+ * receiver status are not evaluated due to the Marvell 88Q2xxx APIs.
*/
ret = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_MMD_AN_MV_STAT);
if (ret < 0) {
return ret;
- } else if ((ret & MDIO_MMD_AN_MV_STAT_LOCAL_RX) &&
- (ret & MDIO_MMD_AN_MV_STAT_REMOTE_RX)) {
+ } else if (((ret & MDIO_MMD_AN_MV_STAT_LOCAL_RX) &&
+ (ret & MDIO_MMD_AN_MV_STAT_REMOTE_RX)) ||
+ !phy_polling_mode(phydev)) {
/* The link state is latched low so that momentary link
* drops can be detected. Do not double-read the status
* in polling mode to detect such short link drops except
@@ -141,7 +162,18 @@ static int mv88q2xxx_read_link_100m(struct phy_device *phydev)
* the link was already down. In case we are not polling,
* we always read the realtime status.
*/
- if (!phy_polling_mode(phydev) || !phydev->link) {
+ if (!phy_polling_mode(phydev)) {
+ phydev->link = false;
+ ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_100BT1_STAT2);
+ if (ret < 0)
+ return ret;
+
+ if (ret & MDIO_MMD_PCS_MV_100BT1_STAT2_LINK)
+ phydev->link = true;
+
+ return 0;
+ } else if (!phydev->link) {
ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
MDIO_MMD_PCS_MV_100BT1_STAT1);
if (ret < 0)
@@ -352,6 +384,79 @@ static int mv88q2xxx_get_sqi_max(struct phy_device *phydev)
return 15;
}
+static int mv88q2xxx_config_intr(struct phy_device *phydev)
+{
+ int ret;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ /* Enable interrupts for 1000BASE-T1 link up and down events
+ * and enable general interrupts for 100BASE-T1.
+ */
+ ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_INT_EN,
+ MDIO_MMD_PCS_MV_INT_EN_LINK_UP |
+ MDIO_MMD_PCS_MV_INT_EN_LINK_DOWN |
+ MDIO_MMD_PCS_MV_INT_EN_100BT1);
+ if (ret < 0)
+ return ret;
+
+ /* Enable interrupts for 100BASE-T1 link events */
+ return phy_write_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_100BT1_INT_EN,
+ MDIO_MMD_PCS_MV_100BT1_INT_EN_LINKEVENT);
+ } else {
+ ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_INT_EN, 0);
+ if (ret < 0)
+ return ret;
+
+ return phy_write_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_100BT1_INT_EN, 0);
+ }
+}
+
+static irqreturn_t mv88q2xxx_handle_interrupt(struct phy_device *phydev)
+{
+ bool trigger_machine = false;
+ int irq;
+
+ /* Before we can acknowledge the 100BT1 general interrupt, that is in
+ * the 1000BT1 interrupt status register, we have to acknowledge any
+ * interrupts that are related to it. Therefore we read first the 100BT1
+ * interrupt status register, followed by reading the 1000BT1 interrupt
+ * status register.
+ */
+
+ irq = phy_read_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_COPPER_INT_STAT);
+ if (irq < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ /* Check link status for 100BT1 */
+ if (irq & MDIO_MMD_PCS_MV_COPPER_INT_STAT_LINKEVENT)
+ trigger_machine = true;
+
+ irq = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_GPIO_INT_STAT);
+ if (irq < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ /* Check link status for 1000BT1 */
+ if ((irq & MDIO_MMD_PCS_MV_GPIO_INT_STAT_LINK_UP) ||
+ (irq & MDIO_MMD_PCS_MV_GPIO_INT_STAT_LINK_DOWN))
+ trigger_machine = true;
+
+ if (!trigger_machine)
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static int mv88q222x_soft_reset(struct phy_device *phydev)
{
int ret;
@@ -418,6 +523,14 @@ static int mv88q222x_revb0_config_init(struct phy_device *phydev)
*/
phydev->pma_extable = MDIO_PMA_EXTABLE_BT1;
+ /* Configure interrupt with default settings, output is driven low for
+ * active interrupt and high for inactive.
+ */
+ if (phy_interrupt_is_valid(phydev))
+ return phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_GPIO_INT_CTRL,
+ MDIO_MMD_PCS_MV_GPIO_INT_CTRL_TRI_DIS);
+
return 0;
}
@@ -444,6 +557,8 @@ static struct phy_driver mv88q2xxx_driver[] = {
.config_init = mv88q222x_revb0_config_init,
.read_status = mv88q2xxx_read_status,
.soft_reset = mv88q222x_soft_reset,
+ .config_intr = mv88q2xxx_config_intr,
+ .handle_interrupt = mv88q2xxx_handle_interrupt,
.set_loopback = genphy_c45_loopback,
.get_sqi = mv88q2xxx_get_sqi,
.get_sqi_max = mv88q2xxx_get_sqi_max,
--
2.39.2
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v5 net-next 07/13] net: phy: marvell-88q2xxx: add suspend / resume ops
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (5 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 06/13] net: phy: marvell-88q2xxx: add interrupt support for link detection Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-31 15:12 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor Dimitri Fedrau
` (6 subsequent siblings)
13 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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 suspend/resume ops for Marvell 88Q2xxx devices.
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
drivers/net/phy/marvell-88q2xxx.c | 34 +++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index 635827b4a692..4cb8fe524795 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -457,6 +457,38 @@ static irqreturn_t mv88q2xxx_handle_interrupt(struct phy_device *phydev)
return IRQ_HANDLED;
}
+static int mv88q2xxx_suspend(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Disable PHY interrupts */
+ if (phy_interrupt_is_valid(phydev)) {
+ phydev->interrupts = PHY_INTERRUPT_DISABLED;
+ ret = mv88q2xxx_config_intr(phydev);
+ if (ret)
+ return ret;
+ }
+
+ return phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1,
+ MDIO_CTRL1_LPOWER);
+}
+
+static int mv88q2xxx_resume(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Enable PHY interrupts */
+ if (phy_interrupt_is_valid(phydev)) {
+ phydev->interrupts = PHY_INTERRUPT_ENABLED;
+ ret = mv88q2xxx_config_intr(phydev);
+ if (ret)
+ return ret;
+ }
+
+ return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1,
+ MDIO_CTRL1_LPOWER);
+}
+
static int mv88q222x_soft_reset(struct phy_device *phydev)
{
int ret;
@@ -562,6 +594,8 @@ static struct phy_driver mv88q2xxx_driver[] = {
.set_loopback = genphy_c45_loopback,
.get_sqi = mv88q2xxx_get_sqi,
.get_sqi_max = mv88q2xxx_get_sqi_max,
+ .suspend = mv88q2xxx_suspend,
+ .resume = mv88q2xxx_resume,
},
};
--
2.39.2
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (6 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 07/13] net: phy: marvell-88q2xxx: add suspend / resume ops Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-30 9:18 ` Russell King (Oracle)
` (2 more replies)
2024-01-22 21:28 ` [PATCH v5 net-next 09/13] net: phy: marvell-88q2xxx: add cable test support Dimitri Fedrau
` (5 subsequent siblings)
13 siblings, 3 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 UTC (permalink / raw)
Cc: Dimitri Fedrau, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jean Delvare, Guenter Roeck, Stefan Eichenberger, netdev,
linux-kernel, linux-hwmon
Marvell 88q2xxx devices have an inbuilt temperature sensor. Add hwmon
support for this sensor.
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
drivers/net/phy/marvell-88q2xxx.c | 152 ++++++++++++++++++++++++++++++
1 file changed, 152 insertions(+)
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index 4cb8fe524795..6900bad275d0 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -5,6 +5,7 @@
#include <linux/ethtool_netlink.h>
#include <linux/marvell_phy.h>
#include <linux/phy.h>
+#include <linux/hwmon.h>
#define PHY_ID_88Q2220_REVB0 (MARVELL_PHY_ID_88Q2220 | 0x1)
@@ -33,6 +34,19 @@
#define MDIO_MMD_PCS_MV_GPIO_INT_CTRL 32787
#define MDIO_MMD_PCS_MV_GPIO_INT_CTRL_TRI_DIS 0x0800
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR1 32833
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_RAW_INT 0x0001
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT 0x0040
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN 0x0080
+
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR2 32834
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR2_DIS_MASK 0xc000
+
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR3 32835
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK 0xff00
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT 8
+#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_MASK 0x00ff
+
#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
@@ -488,6 +502,143 @@ static int mv88q2xxx_resume(struct phy_device *phydev)
return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1,
MDIO_CTRL1_LPOWER);
}
+#ifdef CONFIG_HWMON
+static const struct hwmon_channel_info * const mv88q2xxx_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_ALARM),
+ NULL
+};
+
+static umode_t mv88q2xxx_hwmon_is_visible(const void *data,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (attr) {
+ case hwmon_temp_input:
+ return 0444;
+ case hwmon_temp_max:
+ return 0644;
+ case hwmon_temp_alarm:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static int mv88q2xxx_hwmon_read(struct device *dev,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct phy_device *phydev = dev_get_drvdata(dev);
+ int ret;
+
+ switch (attr) {
+ case hwmon_temp_input:
+ ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TEMP_SENSOR3);
+ if (ret < 0)
+ return ret;
+
+ *val = ((ret & MDIO_MMD_PCS_MV_TEMP_SENSOR3_MASK) - 75) * 1000;
+ return 0;
+ case hwmon_temp_max:
+ ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TEMP_SENSOR3);
+ if (ret < 0)
+ return ret;
+
+ *val = (((ret & MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK) >>
+ MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT) - 75) *
+ 1000;
+ return 0;
+ case hwmon_temp_alarm:
+ ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TEMP_SENSOR1);
+ if (ret < 0)
+ return ret;
+
+ *val = !!(ret & MDIO_MMD_PCS_MV_TEMP_SENSOR1_RAW_INT);
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int mv88q2xxx_hwmon_write(struct device *dev,
+ enum hwmon_sensor_types type, u32 attr,
+ int channel, long val)
+{
+ struct phy_device *phydev = dev_get_drvdata(dev);
+
+ switch (attr) {
+ case hwmon_temp_max:
+ if (val < -75000 || val > 180000)
+ return -EINVAL;
+
+ val = ((val / 1000) + 75) <<
+ MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT;
+ return phy_modify_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TEMP_SENSOR3,
+ MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK,
+ val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static const struct hwmon_ops mv88q2xxx_hwmon_hwmon_ops = {
+ .is_visible = mv88q2xxx_hwmon_is_visible,
+ .read = mv88q2xxx_hwmon_read,
+ .write = mv88q2xxx_hwmon_write,
+};
+
+static const struct hwmon_chip_info mv88q2xxx_hwmon_chip_info = {
+ .ops = &mv88q2xxx_hwmon_hwmon_ops,
+ .info = mv88q2xxx_hwmon_info,
+};
+
+static int mv88q2xxx_hwmon_probe(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ struct device *hwmon;
+ char *hwmon_name;
+ int ret;
+
+ /* Enable temperature sensor interrupt */
+ ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TEMP_SENSOR1,
+ MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN);
+ if (ret < 0)
+ return ret;
+
+ /* Enable temperature sense */
+ ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_TEMP_SENSOR2,
+ MDIO_MMD_PCS_MV_TEMP_SENSOR2_DIS_MASK, 0);
+ if (ret < 0)
+ return ret;
+
+ hwmon_name = devm_hwmon_sanitize_name(dev, dev_name(dev));
+ if (IS_ERR(hwmon_name))
+ return PTR_ERR(hwmon_name);
+
+ hwmon = devm_hwmon_device_register_with_info(dev,
+ hwmon_name,
+ phydev,
+ &mv88q2xxx_hwmon_chip_info,
+ NULL);
+
+ return PTR_ERR_OR_ZERO(hwmon);
+}
+
+#else
+static int mv88q2xxx_hwmon_probe(struct phy_device *phydev)
+{
+ return 0;
+}
+#endif
+static int mv88q2xxx_probe(struct phy_device *phydev)
+{
+ return mv88q2xxx_hwmon_probe(phydev);
+}
static int mv88q222x_soft_reset(struct phy_device *phydev)
{
@@ -583,6 +734,7 @@ static struct phy_driver mv88q2xxx_driver[] = {
{
PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
.name = "mv88q2220",
+ .probe = mv88q2xxx_probe,
.get_features = mv88q2xxx_get_features,
.config_aneg = mv88q222x_config_aneg,
.aneg_done = genphy_c45_aneg_done,
--
2.39.2
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v5 net-next 09/13] net: phy: marvell-88q2xxx: add cable test support
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (7 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-30 9:19 ` Russell King (Oracle)
2024-01-22 21:28 ` [PATCH v5 net-next 10/13] net: phy: marvell-88q2xxx: make mv88q2xxx_config_aneg generic Dimitri Fedrau
` (4 subsequent siblings)
13 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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 cable test support for Marvell 88Q222x devices. Reported distance
granularity is 1m.
1m cable, open:
Cable test started for device eth0.
Cable test completed for device eth0.
Pair A code Open Circuit
Pair A, fault length: 1.00m
1m cable, shorted:
Cable test started for device eth0.
Cable test completed for device eth0.
Pair A code Short within Pair
Pair A, fault length: 1.00m
6m cable, open:
Cable test started for device eth0.
Cable test completed for device eth0.
Pair A code Open Circuit
Pair A, fault length: 6.00m
6m cable, shorted:
Cable test started for device eth0.
Cable test completed for device eth0.
Pair A code Short within Pair
Pair A, fault length: 6.00m
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
drivers/net/phy/marvell-88q2xxx.c | 99 +++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index 6900bad275d0..f59822cf9526 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -69,6 +69,27 @@
#define MDIO_MMD_PCS_MV_RX_STAT 33328
+#define MDIO_MMD_PCS_MV_TDR_RESET 65226
+#define MDIO_MMD_PCS_MV_TDR_RESET_TDR_RST 0x1000
+
+#define MDIO_MMD_PCS_MV_TDR_OFF_SHORT_CABLE 65241
+
+#define MDIO_MMD_PCS_MV_TDR_OFF_LONG_CABLE 65242
+
+#define MDIO_MMD_PCS_MV_TDR_STATUS 65245
+#define MDIO_MMD_PCS_MV_TDR_STATUS_OFF 0x0001
+#define MDIO_MMD_PCS_MV_TDR_STATUS_ON 0x0002
+#define MDIO_MMD_PCS_MV_TDR_STATUS_DIST_MASK 0xff00
+#define MDIO_MMD_PCS_MV_TDR_STATUS_DIST_SHIFT 8
+#define MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_MASK 0x00f0
+#define MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_SHORT 0x0030
+#define MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_OPEN 0x00e0
+#define MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_OK 0x0070
+#define MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_IN_PROGR 0x0080
+#define MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_NOISE 0x0050
+
+#define MDIO_MMD_PCS_MV_TDR_OFF_CUTOFF 65246
+
struct mmd_val {
int devad;
u32 regnum;
@@ -717,6 +738,81 @@ static int mv88q222x_revb0_config_init(struct phy_device *phydev)
return 0;
}
+static int mv88q222x_cable_test_start(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TDR_OFF_CUTOFF, 0x0058);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TDR_OFF_LONG_CABLE, 0x00eb);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TDR_OFF_SHORT_CABLE, 0x010e);
+ if (ret < 0)
+ return ret;
+
+ ret = phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_TDR_RESET,
+ 0x0d90);
+ if (ret < 0)
+ return ret;
+
+ return phy_write_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_TDR_STATUS,
+ MDIO_MMD_PCS_MV_TDR_STATUS_ON);
+}
+
+static int mv88q222x_cable_test_get_status(struct phy_device *phydev,
+ bool *finished)
+{
+ int ret;
+ u32 dist;
+
+ ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_TDR_STATUS);
+ if (ret < 0)
+ return ret;
+
+ *finished = true;
+ /* fault length in meters */
+ dist = ((ret & MDIO_MMD_PCS_MV_TDR_STATUS_DIST_MASK) >>
+ MDIO_MMD_PCS_MV_TDR_STATUS_DIST_SHIFT) * 100;
+ switch (ret & MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_MASK) {
+ case MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_OPEN:
+ ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A,
+ ETHTOOL_A_CABLE_RESULT_CODE_OPEN);
+ ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_A,
+ dist);
+ break;
+ case MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_SHORT:
+ ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A,
+ ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT);
+ ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_A,
+ dist);
+ break;
+ case MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_OK:
+ ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A,
+ ETHTOOL_A_CABLE_RESULT_CODE_OK);
+ break;
+ case MDIO_MMD_PCS_MV_TDR_STATUS_VCT_STAT_IN_PROGR:
+ *finished = false;
+ break;
+ default:
+ ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A,
+ ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC);
+ }
+
+ if (*finished)
+ return phy_write_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_TDR_RESET,
+ MDIO_MMD_PCS_MV_TDR_RESET_TDR_RST | 0xd90);
+
+ return 0;
+}
+
static struct phy_driver mv88q2xxx_driver[] = {
{
.phy_id = MARVELL_PHY_ID_88Q2110,
@@ -734,6 +830,7 @@ static struct phy_driver mv88q2xxx_driver[] = {
{
PHY_ID_MATCH_EXACT(PHY_ID_88Q2220_REVB0),
.name = "mv88q2220",
+ .flags = PHY_POLL_CABLE_TEST,
.probe = mv88q2xxx_probe,
.get_features = mv88q2xxx_get_features,
.config_aneg = mv88q222x_config_aneg,
@@ -744,6 +841,8 @@ static struct phy_driver mv88q2xxx_driver[] = {
.config_intr = mv88q2xxx_config_intr,
.handle_interrupt = mv88q2xxx_handle_interrupt,
.set_loopback = genphy_c45_loopback,
+ .cable_test_start = mv88q222x_cable_test_start,
+ .cable_test_get_status = mv88q222x_cable_test_get_status,
.get_sqi = mv88q2xxx_get_sqi,
.get_sqi_max = mv88q2xxx_get_sqi_max,
.suspend = mv88q2xxx_suspend,
--
2.39.2
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v5 net-next 10/13] net: phy: marvell-88q2xxx: make mv88q2xxx_config_aneg generic
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (8 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 09/13] net: phy: marvell-88q2xxx: add cable test support Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-31 15:19 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 11/13] net: phy: marvell-88q2xxx: switch to mv88q2xxx_config_aneg Dimitri Fedrau
` (3 subsequent siblings)
13 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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
Marvell 88Q2xxx devices follow the same scheme, after configuration they
need a soft reset. Soft resets differ between devices, so we use the
.soft_reset callback instead of creating .config_aneg callbacks for each
device.
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
drivers/net/phy/marvell-88q2xxx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index f59822cf9526..8bd68b3c3ea1 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -362,7 +362,7 @@ static int mv88q2xxx_config_aneg(struct phy_device *phydev)
if (ret)
return ret;
- return mv88q2xxx_soft_reset(phydev);
+ return phydev->drv->soft_reset(phydev);
}
static int mv88q2xxx_config_init(struct phy_device *phydev)
--
2.39.2
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v5 net-next 11/13] net: phy: marvell-88q2xxx: switch to mv88q2xxx_config_aneg
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (9 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 10/13] net: phy: marvell-88q2xxx: make mv88q2xxx_config_aneg generic Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-31 15:20 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 12/13] net: phy: marvell-88q2xxx: cleanup mv88q2xxx_config_init Dimitri Fedrau
` (2 subsequent siblings)
13 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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
Switch to mv88q2xxx_config_aneg for Marvell 88Q2220 devices and remove
the mv88q222x_config_aneg function which is basically a copy of the
mv88q2xxx_config_aneg function.
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
drivers/net/phy/marvell-88q2xxx.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index 8bd68b3c3ea1..e40ea351fcde 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -688,17 +688,6 @@ static int mv88q222x_soft_reset(struct phy_device *phydev)
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;
@@ -833,7 +822,7 @@ static struct phy_driver mv88q2xxx_driver[] = {
.flags = PHY_POLL_CABLE_TEST,
.probe = mv88q2xxx_probe,
.get_features = mv88q2xxx_get_features,
- .config_aneg = mv88q222x_config_aneg,
+ .config_aneg = mv88q2xxx_config_aneg,
.aneg_done = genphy_c45_aneg_done,
.config_init = mv88q222x_revb0_config_init,
.read_status = mv88q2xxx_read_status,
--
2.39.2
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v5 net-next 12/13] net: phy: marvell-88q2xxx: cleanup mv88q2xxx_config_init
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (10 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 11/13] net: phy: marvell-88q2xxx: switch to mv88q2xxx_config_aneg Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-31 15:21 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 13/13] net: phy: marvell-88q2xxx: remove redundant code Dimitri Fedrau
2024-01-30 2:09 ` [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Jakub Kicinski
13 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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
mv88q2xxx_config_init calls genphy_c45_read_pma which is done by
mv88q2xxx_read_status, it calls also mv88q2xxx_config_aneg which is
also called by the PHY state machine. Let the PHY state machine handle
the phydriver ops in their intendend way.
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
drivers/net/phy/marvell-88q2xxx.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index e40ea351fcde..081ed110c87e 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -367,20 +367,13 @@ static int mv88q2xxx_config_aneg(struct phy_device *phydev)
static int mv88q2xxx_config_init(struct phy_device *phydev)
{
- int 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;
- /* Read the current PHY configuration */
- ret = genphy_c45_read_pma(phydev);
- if (ret)
- return ret;
-
- return mv88q2xxx_config_aneg(phydev);
+ return 0;
}
static int mv88q2xxx_get_sqi(struct phy_device *phydev)
--
2.39.2
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v5 net-next 13/13] net: phy: marvell-88q2xxx: remove redundant code
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (11 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 12/13] net: phy: marvell-88q2xxx: cleanup mv88q2xxx_config_init Dimitri Fedrau
@ 2024-01-22 21:28 ` Dimitri Fedrau
2024-01-30 2:09 ` [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Jakub Kicinski
13 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-22 21:28 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
Remove redundant code in mv88q222x_revb0_config_init,
phydev->pma_extable is already set in mv88q2xxx_config_init. Just call
mv88q2xxx_config_init in mv88q222x_revb0_config_init.
Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
drivers/net/phy/marvell-88q2xxx.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index 081ed110c87e..6df580442b6b 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -373,6 +373,14 @@ static int mv88q2xxx_config_init(struct phy_device *phydev)
*/
phydev->pma_extable = MDIO_PMA_EXTABLE_BT1;
+ /* Configure interrupt with default settings, output is driven low for
+ * active interrupt and high for inactive.
+ */
+ if (phy_interrupt_is_valid(phydev))
+ return phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
+ MDIO_MMD_PCS_MV_GPIO_INT_CTRL,
+ MDIO_MMD_PCS_MV_GPIO_INT_CTRL_TRI_DIS);
+
return 0;
}
@@ -703,21 +711,7 @@ static int mv88q222x_revb0_config_init(struct phy_device *phydev)
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;
-
- /* Configure interrupt with default settings, output is driven low for
- * active interrupt and high for inactive.
- */
- if (phy_interrupt_is_valid(phydev))
- return phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
- MDIO_MMD_PCS_MV_GPIO_INT_CTRL,
- MDIO_MMD_PCS_MV_GPIO_INT_CTRL_TRI_DIS);
-
- return 0;
+ return mv88q2xxx_config_init(phydev);
}
static int mv88q222x_cable_test_start(struct phy_device *phydev)
--
2.39.2
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
` (12 preceding siblings ...)
2024-01-22 21:28 ` [PATCH v5 net-next 13/13] net: phy: marvell-88q2xxx: remove redundant code Dimitri Fedrau
@ 2024-01-30 2:09 ` Jakub Kicinski
2024-02-02 18:30 ` Dimitri Fedrau
13 siblings, 1 reply; 44+ messages in thread
From: Jakub Kicinski @ 2024-01-30 2:09 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King
Cc: Dimitri Fedrau, David S. Miller, Eric Dumazet, Paolo Abeni,
Stefan Eichenberger, netdev, linux-kernel
On Mon, 22 Jan 2024 22:28:33 +0100 Dimitri Fedrau wrote:
> Changes in V5:
> - add missing statics for mv88q222x_revb0_init_seq0 and
> mv88q222x_revb0_init_seq1
> - fix typo in commit message: autonegotiation
> - fix ordering of Signed-off-by and Reviewed-by in commit messages
> - add interrupt support for link detection
> - add suspend / resume ops
> - add support for internal temperature sensor
> - add cable test support
> - call .soft_reset in mv88q2xxx_config_aneg, this makes
> mv88q2xxx_config_aneg compatible for Marvell88Q222x devices and
> remove mv88q222x_config_aneg which is then just duplicated code
> - cleanup mv88q2xxx_config_init and make it compatible with
> Marvell88Q222x devices
> - move parts from mv88q222x_config_init to mv88q2xxx_config_init
> that are applicable for all Marvell88Q2xxx devices.
PHY maintainers - could anyone take a look at these patches?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-01-22 21:28 ` [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor Dimitri Fedrau
@ 2024-01-30 9:18 ` Russell King (Oracle)
2024-01-31 11:52 ` Dimitri Fedrau
2024-01-31 15:17 ` Andrew Lunn
2024-02-01 13:18 ` Guenter Roeck
2 siblings, 1 reply; 44+ messages in thread
From: Russell King (Oracle) @ 2024-01-30 9:18 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jean Delvare, Guenter Roeck,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
On Mon, Jan 22, 2024 at 10:28:41PM +0100, Dimitri Fedrau wrote:
int tmp;
> + switch (attr) {
> + case hwmon_temp_input:
> + ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
> + MDIO_MMD_PCS_MV_TEMP_SENSOR3);
> + if (ret < 0)
> + return ret;
> +
> + *val = ((ret & MDIO_MMD_PCS_MV_TEMP_SENSOR3_MASK) - 75) * 1000;
tmp = FIELD_GET(MDIO_MMD_PCS_MV_TEMP_SENSOR3_MASK, ret);
*val = (tmp - 75) * 1000;
> + return 0;
> + case hwmon_temp_max:
> + ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
> + MDIO_MMD_PCS_MV_TEMP_SENSOR3);
> + if (ret < 0)
> + return ret;
> +
> + *val = (((ret & MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK) >>
> + MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT) - 75) *
> + 1000;
tmp = FIELD_GET(MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK,
ret);
*val = (tmp - 75) * 1000;
> + return 0;
> + case hwmon_temp_alarm:
> + ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
> + MDIO_MMD_PCS_MV_TEMP_SENSOR1);
> + if (ret < 0)
> + return ret;
> +
> + *val = !!(ret & MDIO_MMD_PCS_MV_TEMP_SENSOR1_RAW_INT);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int mv88q2xxx_hwmon_write(struct device *dev,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel, long val)
> +{
> + struct phy_device *phydev = dev_get_drvdata(dev);
> +
> + switch (attr) {
> + case hwmon_temp_max:
> + if (val < -75000 || val > 180000)
> + return -EINVAL;
> +
> + val = ((val / 1000) + 75) <<
> + MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT;
val = (val / 1000) + 75;
val = FIELD_PREP(MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK,
val);
... and therefore no need for the _SHIFT constants.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 09/13] net: phy: marvell-88q2xxx: add cable test support
2024-01-22 21:28 ` [PATCH v5 net-next 09/13] net: phy: marvell-88q2xxx: add cable test support Dimitri Fedrau
@ 2024-01-30 9:19 ` Russell King (Oracle)
2024-01-31 12:03 ` Dimitri Fedrau
0 siblings, 1 reply; 44+ messages in thread
From: Russell King (Oracle) @ 2024-01-30 9:19 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
linux-kernel
On Mon, Jan 22, 2024 at 10:28:42PM +0100, Dimitri Fedrau wrote:
> +static int mv88q222x_cable_test_get_status(struct phy_device *phydev,
> + bool *finished)
> +{
> + int ret;
> + u32 dist;
> +
> + ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_TDR_STATUS);
> + if (ret < 0)
> + return ret;
> +
> + *finished = true;
> + /* fault length in meters */
> + dist = ((ret & MDIO_MMD_PCS_MV_TDR_STATUS_DIST_MASK) >>
> + MDIO_MMD_PCS_MV_TDR_STATUS_DIST_SHIFT) * 100;
dist = FIELD_GET(MDIO_MMD_PCS_MV_TDR_STATUS_DIST_MASK, ret) * 100;
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-01-30 9:18 ` Russell King (Oracle)
@ 2024-01-31 11:52 ` Dimitri Fedrau
0 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-31 11:52 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jean Delvare, Guenter Roeck,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
Am Tue, Jan 30, 2024 at 09:18:31AM +0000 schrieb Russell King (Oracle):
> On Mon, Jan 22, 2024 at 10:28:41PM +0100, Dimitri Fedrau wrote:
>
> int tmp;
>
> > + switch (attr) {
> > + case hwmon_temp_input:
> > + ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
> > + MDIO_MMD_PCS_MV_TEMP_SENSOR3);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = ((ret & MDIO_MMD_PCS_MV_TEMP_SENSOR3_MASK) - 75) * 1000;
>
> tmp = FIELD_GET(MDIO_MMD_PCS_MV_TEMP_SENSOR3_MASK, ret);
> *val = (tmp - 75) * 1000;
>
> > + return 0;
> > + case hwmon_temp_max:
> > + ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
> > + MDIO_MMD_PCS_MV_TEMP_SENSOR3);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = (((ret & MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK) >>
> > + MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT) - 75) *
> > + 1000;
>
> tmp = FIELD_GET(MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK,
> ret);
> *val = (tmp - 75) * 1000;
>
> > + return 0;
> > + case hwmon_temp_alarm:
> > + ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
> > + MDIO_MMD_PCS_MV_TEMP_SENSOR1);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = !!(ret & MDIO_MMD_PCS_MV_TEMP_SENSOR1_RAW_INT);
> > + return 0;
> > + default:
> > + return -EOPNOTSUPP;
> > + }
> > +}
> > +
> > +static int mv88q2xxx_hwmon_write(struct device *dev,
> > + enum hwmon_sensor_types type, u32 attr,
> > + int channel, long val)
> > +{
> > + struct phy_device *phydev = dev_get_drvdata(dev);
> > +
> > + switch (attr) {
> > + case hwmon_temp_max:
> > + if (val < -75000 || val > 180000)
> > + return -EINVAL;
> > +
> > + val = ((val / 1000) + 75) <<
> > + MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT;
>
> val = (val / 1000) + 75;
> val = FIELD_PREP(MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK,
> val);
>
> ... and therefore no need for the _SHIFT constants.
>
Will fix it.
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 09/13] net: phy: marvell-88q2xxx: add cable test support
2024-01-30 9:19 ` Russell King (Oracle)
@ 2024-01-31 12:03 ` Dimitri Fedrau
0 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-01-31 12:03 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Stefan Eichenberger, netdev,
linux-kernel
Am Tue, Jan 30, 2024 at 09:19:30AM +0000 schrieb Russell King (Oracle):
> On Mon, Jan 22, 2024 at 10:28:42PM +0100, Dimitri Fedrau wrote:
> > +static int mv88q222x_cable_test_get_status(struct phy_device *phydev,
> > + bool *finished)
> > +{
> > + int ret;
> > + u32 dist;
> > +
> > + ret = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_MMD_PCS_MV_TDR_STATUS);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *finished = true;
> > + /* fault length in meters */
> > + dist = ((ret & MDIO_MMD_PCS_MV_TDR_STATUS_DIST_MASK) >>
> > + MDIO_MMD_PCS_MV_TDR_STATUS_DIST_SHIFT) * 100;
>
> dist = FIELD_GET(MDIO_MMD_PCS_MV_TDR_STATUS_DIST_MASK, ret) * 100;
>
Hi Russell,
sure can fix it. Did you have a chance to review the other patches ? Is
there anything I could do to ease the review process ? I'm new to
submitting patches, maybe I crossed some line without knowing ? Would be
glad to get some feedback. I'm just asking because there was not much
feedback besides your comments.
Best regards,
Dimitri
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 05/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
2024-01-22 21:28 ` [PATCH v5 net-next 05/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
@ 2024-01-31 15:08 ` Andrew Lunn
0 siblings, 0 replies; 44+ messages in thread
From: Andrew Lunn @ 2024-01-31 15:08 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 22, 2024 at 10:28:38PM +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. Autonegotiation is supported.
>
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 06/13] net: phy: marvell-88q2xxx: add interrupt support for link detection
2024-01-22 21:28 ` [PATCH v5 net-next 06/13] net: phy: marvell-88q2xxx: add interrupt support for link detection Dimitri Fedrau
@ 2024-01-31 15:12 ` Andrew Lunn
0 siblings, 0 replies; 44+ messages in thread
From: Andrew Lunn @ 2024-01-31 15:12 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 22, 2024 at 10:28:39PM +0100, Dimitri Fedrau wrote:
> Added .config_intr and .handle_interrupt callbacks. Whenever the link
> goes up or down an interrupt will be triggered. Interrupts are configured
> separately for 100/1000BASET1.
>
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 07/13] net: phy: marvell-88q2xxx: add suspend / resume ops
2024-01-22 21:28 ` [PATCH v5 net-next 07/13] net: phy: marvell-88q2xxx: add suspend / resume ops Dimitri Fedrau
@ 2024-01-31 15:12 ` Andrew Lunn
0 siblings, 0 replies; 44+ messages in thread
From: Andrew Lunn @ 2024-01-31 15:12 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 22, 2024 at 10:28:40PM +0100, Dimitri Fedrau wrote:
> Add suspend/resume ops for Marvell 88Q2xxx devices.
>
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-01-22 21:28 ` [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor Dimitri Fedrau
2024-01-30 9:18 ` Russell King (Oracle)
@ 2024-01-31 15:17 ` Andrew Lunn
2024-02-01 7:11 ` Dimitri Fedrau
2024-02-01 13:18 ` Guenter Roeck
2 siblings, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2024-01-31 15:17 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jean Delvare, Guenter Roeck,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
> +static int mv88q2xxx_hwmon_probe(struct phy_device *phydev)
> +{
> + struct device *dev = &phydev->mdio.dev;
> + struct device *hwmon;
> + char *hwmon_name;
> + int ret;
> +
> + /* Enable temperature sensor interrupt */
> + ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
> + MDIO_MMD_PCS_MV_TEMP_SENSOR1,
> + MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN);
You enable an interrupt, but i don't see any changes to the interrupt
handler to handle any interrupts which are generated?
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 10/13] net: phy: marvell-88q2xxx: make mv88q2xxx_config_aneg generic
2024-01-22 21:28 ` [PATCH v5 net-next 10/13] net: phy: marvell-88q2xxx: make mv88q2xxx_config_aneg generic Dimitri Fedrau
@ 2024-01-31 15:19 ` Andrew Lunn
0 siblings, 0 replies; 44+ messages in thread
From: Andrew Lunn @ 2024-01-31 15:19 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 22, 2024 at 10:28:43PM +0100, Dimitri Fedrau wrote:
> Marvell 88Q2xxx devices follow the same scheme, after configuration they
> need a soft reset. Soft resets differ between devices, so we use the
> .soft_reset callback instead of creating .config_aneg callbacks for each
> device.
>
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 11/13] net: phy: marvell-88q2xxx: switch to mv88q2xxx_config_aneg
2024-01-22 21:28 ` [PATCH v5 net-next 11/13] net: phy: marvell-88q2xxx: switch to mv88q2xxx_config_aneg Dimitri Fedrau
@ 2024-01-31 15:20 ` Andrew Lunn
0 siblings, 0 replies; 44+ messages in thread
From: Andrew Lunn @ 2024-01-31 15:20 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 22, 2024 at 10:28:44PM +0100, Dimitri Fedrau wrote:
> Switch to mv88q2xxx_config_aneg for Marvell 88Q2220 devices and remove
> the mv88q222x_config_aneg function which is basically a copy of the
> mv88q2xxx_config_aneg function.
>
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 12/13] net: phy: marvell-88q2xxx: cleanup mv88q2xxx_config_init
2024-01-22 21:28 ` [PATCH v5 net-next 12/13] net: phy: marvell-88q2xxx: cleanup mv88q2xxx_config_init Dimitri Fedrau
@ 2024-01-31 15:21 ` Andrew Lunn
2024-02-02 17:19 ` Dimitri Fedrau
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2024-01-31 15:21 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 22, 2024 at 10:28:45PM +0100, Dimitri Fedrau wrote:
> mv88q2xxx_config_init calls genphy_c45_read_pma which is done by
> mv88q2xxx_read_status, it calls also mv88q2xxx_config_aneg which is
> also called by the PHY state machine. Let the PHY state machine handle
> the phydriver ops in their intendend way.
>
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-01-31 15:17 ` Andrew Lunn
@ 2024-02-01 7:11 ` Dimitri Fedrau
2024-02-01 13:23 ` Andrew Lunn
2024-02-01 13:34 ` Guenter Roeck
0 siblings, 2 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-01 7:11 UTC (permalink / raw)
To: Andrew Lunn
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jean Delvare, Guenter Roeck,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
Am Wed, Jan 31, 2024 at 04:17:06PM +0100 schrieb Andrew Lunn:
> > +static int mv88q2xxx_hwmon_probe(struct phy_device *phydev)
> > +{
> > + struct device *dev = &phydev->mdio.dev;
> > + struct device *hwmon;
> > + char *hwmon_name;
> > + int ret;
> > +
> > + /* Enable temperature sensor interrupt */
> > + ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
> > + MDIO_MMD_PCS_MV_TEMP_SENSOR1,
> > + MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN);
>
> You enable an interrupt, but i don't see any changes to the interrupt
> handler to handle any interrupts which are generated?
>
Hi Andrew,
you are right. Have to remove these lines. Besides enabling the interrupt
in MDIO_MMD_PCS_MV_TEMP_SENSOR1, there are two further register writes
necessary to make the interrupt propagate. I didn't want it to propagate.
Anyway it's wrong. I couldn't find a good solution to use the temperature
interrupt. Will have a look into this, and probably figuring out how to
do so. But it won't be part of this patch series.
Dimitri
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-01-22 21:28 ` [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor Dimitri Fedrau
2024-01-30 9:18 ` Russell King (Oracle)
2024-01-31 15:17 ` Andrew Lunn
@ 2024-02-01 13:18 ` Guenter Roeck
2024-02-01 13:27 ` Andrew Lunn
2024-02-01 16:17 ` Dimitri Fedrau
2 siblings, 2 replies; 44+ messages in thread
From: Guenter Roeck @ 2024-02-01 13:18 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
On 1/22/24 13:28, Dimitri Fedrau wrote:
> Marvell 88q2xxx devices have an inbuilt temperature sensor. Add hwmon
> support for this sensor.
>
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> ---
> drivers/net/phy/marvell-88q2xxx.c | 152 ++++++++++++++++++++++++++++++
> 1 file changed, 152 insertions(+)
>
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> index 4cb8fe524795..6900bad275d0 100644
> --- a/drivers/net/phy/marvell-88q2xxx.c
> +++ b/drivers/net/phy/marvell-88q2xxx.c
> @@ -5,6 +5,7 @@
> #include <linux/ethtool_netlink.h>
> #include <linux/marvell_phy.h>
> #include <linux/phy.h>
> +#include <linux/hwmon.h>
>
> #define PHY_ID_88Q2220_REVB0 (MARVELL_PHY_ID_88Q2220 | 0x1)
>
> @@ -33,6 +34,19 @@
> #define MDIO_MMD_PCS_MV_GPIO_INT_CTRL 32787
> #define MDIO_MMD_PCS_MV_GPIO_INT_CTRL_TRI_DIS 0x0800
>
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR1 32833
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_RAW_INT 0x0001
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT 0x0040
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN 0x0080
> +
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR2 32834
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR2_DIS_MASK 0xc000
> +
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR3 32835
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_MASK 0xff00
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_INT_THRESH_SHIFT 8
> +#define MDIO_MMD_PCS_MV_TEMP_SENSOR3_MASK 0x00ff
> +
> #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
> @@ -488,6 +502,143 @@ static int mv88q2xxx_resume(struct phy_device *phydev)
> return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1,
> MDIO_CTRL1_LPOWER);
> }
> +#ifdef CONFIG_HWMON
HWMON is tristate, so this may be problematic if the driver is built
into the kernel and hwmon is built as module.
[ ... ]
> +
> +static int mv88q2xxx_hwmon_write(struct device *dev,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel, long val)
> +{
> + struct phy_device *phydev = dev_get_drvdata(dev);
> +
> + switch (attr) {
> + case hwmon_temp_max:
> + if (val < -75000 || val > 180000)
> + return -EINVAL;
> +
Not that it matters much, but we typically use clamp_val() to limit
the range of temperature limits because the valid range differs for
each chip and is otherwise difficult to determine for the user.
Guenter
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 7:11 ` Dimitri Fedrau
@ 2024-02-01 13:23 ` Andrew Lunn
2024-02-01 16:52 ` Dimitri Fedrau
2024-02-01 13:34 ` Guenter Roeck
1 sibling, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2024-02-01 13:23 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jean Delvare, Guenter Roeck,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
> Anyway it's wrong. I couldn't find a good solution to use the temperature
> interrupt. Will have a look into this, and probably figuring out how to
> do so. But it won't be part of this patch series.
I don't know of any PHY driver you can follow, those that do have a
temperature sensor just report the temperature and don't do anything
in addition.
You might need to look at thermal zones, and indicate there has been a
thermal trip point. That could then be used by the thermal subsystem
to increase cooling via a fan, etc. In theory, you could also make the
PHY active to thermal pressure, by forcing the link to renegotiate to
a lower link speed. If you decide to go this route, please try to make
is generic to any PHY. But its going to be quite a disruptive thing,
the link will be lost of a little over a second...
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 13:18 ` Guenter Roeck
@ 2024-02-01 13:27 ` Andrew Lunn
2024-02-01 13:39 ` Guenter Roeck
2024-02-01 16:17 ` Dimitri Fedrau
1 sibling, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2024-02-01 13:27 UTC (permalink / raw)
To: Guenter Roeck
Cc: Dimitri Fedrau, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
> > +#ifdef CONFIG_HWMON
>
> HWMON is tristate, so this may be problematic if the driver is built
> into the kernel and hwmon is built as module.
There should be Kconfig in addition to this, e.g.
config MAXLINEAR_GPHY
tristate "Maxlinear Ethernet PHYs"
select POLYNOMIAL if HWMON
depends on HWMON || HWMON=n
help
Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
GPY241, GPY245 PHYs.
So its forced to being built in, or not built at all.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 7:11 ` Dimitri Fedrau
2024-02-01 13:23 ` Andrew Lunn
@ 2024-02-01 13:34 ` Guenter Roeck
2024-02-01 16:14 ` Dimitri Fedrau
1 sibling, 1 reply; 44+ messages in thread
From: Guenter Roeck @ 2024-02-01 13:34 UTC (permalink / raw)
To: Dimitri Fedrau, Andrew Lunn
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jean Delvare, Stefan Eichenberger,
netdev, linux-kernel, linux-hwmon
On 1/31/24 23:11, Dimitri Fedrau wrote:
> Am Wed, Jan 31, 2024 at 04:17:06PM +0100 schrieb Andrew Lunn:
>>> +static int mv88q2xxx_hwmon_probe(struct phy_device *phydev)
>>> +{
>>> + struct device *dev = &phydev->mdio.dev;
>>> + struct device *hwmon;
>>> + char *hwmon_name;
>>> + int ret;
>>> +
>>> + /* Enable temperature sensor interrupt */
>>> + ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
>>> + MDIO_MMD_PCS_MV_TEMP_SENSOR1,
>>> + MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN);
>>
>> You enable an interrupt, but i don't see any changes to the interrupt
>> handler to handle any interrupts which are generated?
>>
> Hi Andrew,
>
> you are right. Have to remove these lines. Besides enabling the interrupt
> in MDIO_MMD_PCS_MV_TEMP_SENSOR1, there are two further register writes
> necessary to make the interrupt propagate. I didn't want it to propagate.
> Anyway it's wrong. I couldn't find a good solution to use the temperature
> interrupt. Will have a look into this, and probably figuring out how to
> do so. But it won't be part of this patch series.
>
From hwmon perspective, the expected use of such an interrupt would be
to call hwmon_notify_event() with the affected limit attribute as argument.
This would notify the thermal subsystem if the sensor is registered with it
(your patch doesn't set the necessary flag when registering the driver,
so this would not happen), it will send a notification to the sysfs
attribute, and generate a udev event.
Guenter
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 13:27 ` Andrew Lunn
@ 2024-02-01 13:39 ` Guenter Roeck
2024-02-01 16:23 ` Dimitri Fedrau
0 siblings, 1 reply; 44+ messages in thread
From: Guenter Roeck @ 2024-02-01 13:39 UTC (permalink / raw)
To: Andrew Lunn
Cc: Dimitri Fedrau, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
On 2/1/24 05:27, Andrew Lunn wrote:
>>> +#ifdef CONFIG_HWMON
>>
>> HWMON is tristate, so this may be problematic if the driver is built
>> into the kernel and hwmon is built as module.
>
> There should be Kconfig in addition to this, e.g.
>
> config MAXLINEAR_GPHY
> tristate "Maxlinear Ethernet PHYs"
> select POLYNOMIAL if HWMON
> depends on HWMON || HWMON=n
> help
> Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
> GPY241, GPY245 PHYs.
>
> So its forced to being built in, or not built at all.
>
Even then it should be "#if IS_ENABLED(HWMON)" in the code.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 13:34 ` Guenter Roeck
@ 2024-02-01 16:14 ` Dimitri Fedrau
2024-02-01 18:26 ` Guenter Roeck
0 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-01 16:14 UTC (permalink / raw)
To: Guenter Roeck
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
Am Thu, Feb 01, 2024 at 05:34:05AM -0800 schrieb Guenter Roeck:
> On 1/31/24 23:11, Dimitri Fedrau wrote:
> > Am Wed, Jan 31, 2024 at 04:17:06PM +0100 schrieb Andrew Lunn:
> > > > +static int mv88q2xxx_hwmon_probe(struct phy_device *phydev)
> > > > +{
> > > > + struct device *dev = &phydev->mdio.dev;
> > > > + struct device *hwmon;
> > > > + char *hwmon_name;
> > > > + int ret;
> > > > +
> > > > + /* Enable temperature sensor interrupt */
> > > > + ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
> > > > + MDIO_MMD_PCS_MV_TEMP_SENSOR1,
> > > > + MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN);
> > >
> > > You enable an interrupt, but i don't see any changes to the interrupt
> > > handler to handle any interrupts which are generated?
> > >
> > Hi Andrew,
> >
> > you are right. Have to remove these lines. Besides enabling the interrupt
> > in MDIO_MMD_PCS_MV_TEMP_SENSOR1, there are two further register writes
> > necessary to make the interrupt propagate. I didn't want it to propagate.
> > Anyway it's wrong. I couldn't find a good solution to use the temperature
> > interrupt. Will have a look into this, and probably figuring out how to
> > do so. But it won't be part of this patch series.
> >
>
> From hwmon perspective, the expected use of such an interrupt would be
> to call hwmon_notify_event() with the affected limit attribute as argument.
> This would notify the thermal subsystem if the sensor is registered with it
> (your patch doesn't set the necessary flag when registering the driver,
> so this would not happen), it will send a notification to the sysfs
> attribute, and generate a udev event.
>
Thanks, noted it down. Didn't know about the notification to the thermal
subsystem and the generated udev event. :)
Dimitri
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 13:18 ` Guenter Roeck
2024-02-01 13:27 ` Andrew Lunn
@ 2024-02-01 16:17 ` Dimitri Fedrau
1 sibling, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-01 16:17 UTC (permalink / raw)
To: Guenter Roeck
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
Am Thu, Feb 01, 2024 at 05:18:23AM -0800 schrieb Guenter Roeck:
[...]
> > +
> > +static int mv88q2xxx_hwmon_write(struct device *dev,
> > + enum hwmon_sensor_types type, u32 attr,
> > + int channel, long val)
> > +{
> > + struct phy_device *phydev = dev_get_drvdata(dev);
> > +
> > + switch (attr) {
> > + case hwmon_temp_max:
> > + if (val < -75000 || val > 180000)
> > + return -EINVAL;
> > +
>
> Not that it matters much, but we typically use clamp_val() to limit
> the range of temperature limits because the valid range differs for
> each chip and is otherwise difficult to determine for the user.
>
Will fix it.
Dimitri
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 13:39 ` Guenter Roeck
@ 2024-02-01 16:23 ` Dimitri Fedrau
2024-02-01 16:47 ` Andrew Lunn
2024-02-01 18:22 ` Guenter Roeck
0 siblings, 2 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-01 16:23 UTC (permalink / raw)
To: Guenter Roeck
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
Am Thu, Feb 01, 2024 at 05:39:25AM -0800 schrieb Guenter Roeck:
> On 2/1/24 05:27, Andrew Lunn wrote:
> > > > +#ifdef CONFIG_HWMON
> > >
> > > HWMON is tristate, so this may be problematic if the driver is built
> > > into the kernel and hwmon is built as module.
> >
> > There should be Kconfig in addition to this, e.g.
> >
> > config MAXLINEAR_GPHY
> > tristate "Maxlinear Ethernet PHYs"
> > select POLYNOMIAL if HWMON
> > depends on HWMON || HWMON=n
> > help
> > Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
> > GPY241, GPY245 PHYs.
> >
> > So its forced to being built in, or not built at all.
> >
>
> Even then it should be "#if IS_ENABLED(HWMON)" in the code.
>
>
If using "#if IS_ENABLED(HWMON)" do I have to add the dependency in
the KConfig file ? When looking at other PHY drivers, they do.
Dimitri
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 16:23 ` Dimitri Fedrau
@ 2024-02-01 16:47 ` Andrew Lunn
2024-02-01 16:56 ` Dimitri Fedrau
2024-02-01 18:22 ` Guenter Roeck
1 sibling, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2024-02-01 16:47 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Guenter Roeck, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
On Thu, Feb 01, 2024 at 05:23:49PM +0100, Dimitri Fedrau wrote:
> Am Thu, Feb 01, 2024 at 05:39:25AM -0800 schrieb Guenter Roeck:
> > On 2/1/24 05:27, Andrew Lunn wrote:
> > > > > +#ifdef CONFIG_HWMON
> > > >
> > > > HWMON is tristate, so this may be problematic if the driver is built
> > > > into the kernel and hwmon is built as module.
> > >
> > > There should be Kconfig in addition to this, e.g.
> > >
> > > config MAXLINEAR_GPHY
> > > tristate "Maxlinear Ethernet PHYs"
> > > select POLYNOMIAL if HWMON
> > > depends on HWMON || HWMON=n
> > > help
> > > Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
> > > GPY241, GPY245 PHYs.
> > >
> > > So its forced to being built in, or not built at all.
> > >
> >
> > Even then it should be "#if IS_ENABLED(HWMON)" in the code.
> >
> >
> If using "#if IS_ENABLED(HWMON)" do I have to add the dependency in
> the KConfig file ? When looking at other PHY drivers, they do.
Please follow what other drivers do. Its easy to break the build,
resulting is undefined symbols. What we have now works.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 13:23 ` Andrew Lunn
@ 2024-02-01 16:52 ` Dimitri Fedrau
0 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-01 16:52 UTC (permalink / raw)
To: Andrew Lunn
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jean Delvare, Guenter Roeck,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
Am Thu, Feb 01, 2024 at 02:23:15PM +0100 schrieb Andrew Lunn:
> > Anyway it's wrong. I couldn't find a good solution to use the temperature
> > interrupt. Will have a look into this, and probably figuring out how to
> > do so. But it won't be part of this patch series.
>
> I don't know of any PHY driver you can follow, those that do have a
> temperature sensor just report the temperature and don't do anything
> in addition.
>
> You might need to look at thermal zones, and indicate there has been a
> thermal trip point. That could then be used by the thermal subsystem
> to increase cooling via a fan, etc. In theory, you could also make the
> PHY active to thermal pressure, by forcing the link to renegotiate to
> a lower link speed. If you decide to go this route, please try to make
> is generic to any PHY. But its going to be quite a disruptive thing,
> the link will be lost of a little over a second...
>
Making the PHY active to thermal pressure sounds interesting. Will look
into this.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 16:47 ` Andrew Lunn
@ 2024-02-01 16:56 ` Dimitri Fedrau
0 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-01 16:56 UTC (permalink / raw)
To: Andrew Lunn
Cc: Guenter Roeck, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
Am Thu, Feb 01, 2024 at 05:47:21PM +0100 schrieb Andrew Lunn:
> On Thu, Feb 01, 2024 at 05:23:49PM +0100, Dimitri Fedrau wrote:
> > Am Thu, Feb 01, 2024 at 05:39:25AM -0800 schrieb Guenter Roeck:
> > > On 2/1/24 05:27, Andrew Lunn wrote:
> > > > > > +#ifdef CONFIG_HWMON
> > > > >
> > > > > HWMON is tristate, so this may be problematic if the driver is built
> > > > > into the kernel and hwmon is built as module.
> > > >
> > > > There should be Kconfig in addition to this, e.g.
> > > >
> > > > config MAXLINEAR_GPHY
> > > > tristate "Maxlinear Ethernet PHYs"
> > > > select POLYNOMIAL if HWMON
> > > > depends on HWMON || HWMON=n
> > > > help
> > > > Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
> > > > GPY241, GPY245 PHYs.
> > > >
> > > > So its forced to being built in, or not built at all.
> > > >
> > >
> > > Even then it should be "#if IS_ENABLED(HWMON)" in the code.
> > >
> > >
> > If using "#if IS_ENABLED(HWMON)" do I have to add the dependency in
> > the KConfig file ? When looking at other PHY drivers, they do.
>
> Please follow what other drivers do. Its easy to break the build,
> resulting is undefined symbols. What we have now works.
Sure.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 16:23 ` Dimitri Fedrau
2024-02-01 16:47 ` Andrew Lunn
@ 2024-02-01 18:22 ` Guenter Roeck
1 sibling, 0 replies; 44+ messages in thread
From: Guenter Roeck @ 2024-02-01 18:22 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
On Thu, Feb 01, 2024 at 05:23:49PM +0100, Dimitri Fedrau wrote:
> Am Thu, Feb 01, 2024 at 05:39:25AM -0800 schrieb Guenter Roeck:
> > On 2/1/24 05:27, Andrew Lunn wrote:
> > > > > +#ifdef CONFIG_HWMON
> > > >
> > > > HWMON is tristate, so this may be problematic if the driver is built
> > > > into the kernel and hwmon is built as module.
> > >
> > > There should be Kconfig in addition to this, e.g.
> > >
> > > config MAXLINEAR_GPHY
> > > tristate "Maxlinear Ethernet PHYs"
> > > select POLYNOMIAL if HWMON
> > > depends on HWMON || HWMON=n
> > > help
> > > Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
> > > GPY241, GPY245 PHYs.
> > >
> > > So its forced to being built in, or not built at all.
> > >
> >
> > Even then it should be "#if IS_ENABLED(HWMON)" in the code.
> >
> >
> If using "#if IS_ENABLED(HWMON)" do I have to add the dependency in
> the KConfig file ? When looking at other PHY drivers, they do.
Yes, to handle CONFIG_HWMON=m. Note that it is "IS_ENABLED(CONFIG_HWMON)"
^^^^^^^
Guenter
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor
2024-02-01 16:14 ` Dimitri Fedrau
@ 2024-02-01 18:26 ` Guenter Roeck
0 siblings, 0 replies; 44+ messages in thread
From: Guenter Roeck @ 2024-02-01 18:26 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jean Delvare,
Stefan Eichenberger, netdev, linux-kernel, linux-hwmon
On Thu, Feb 01, 2024 at 05:14:35PM +0100, Dimitri Fedrau wrote:
> Am Thu, Feb 01, 2024 at 05:34:05AM -0800 schrieb Guenter Roeck:
> > On 1/31/24 23:11, Dimitri Fedrau wrote:
> > > Am Wed, Jan 31, 2024 at 04:17:06PM +0100 schrieb Andrew Lunn:
> > > > > +static int mv88q2xxx_hwmon_probe(struct phy_device *phydev)
> > > > > +{
> > > > > + struct device *dev = &phydev->mdio.dev;
> > > > > + struct device *hwmon;
> > > > > + char *hwmon_name;
> > > > > + int ret;
> > > > > +
> > > > > + /* Enable temperature sensor interrupt */
> > > > > + ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
> > > > > + MDIO_MMD_PCS_MV_TEMP_SENSOR1,
> > > > > + MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN);
> > > >
> > > > You enable an interrupt, but i don't see any changes to the interrupt
> > > > handler to handle any interrupts which are generated?
> > > >
> > > Hi Andrew,
> > >
> > > you are right. Have to remove these lines. Besides enabling the interrupt
> > > in MDIO_MMD_PCS_MV_TEMP_SENSOR1, there are two further register writes
> > > necessary to make the interrupt propagate. I didn't want it to propagate.
> > > Anyway it's wrong. I couldn't find a good solution to use the temperature
> > > interrupt. Will have a look into this, and probably figuring out how to
> > > do so. But it won't be part of this patch series.
> > >
> >
> > From hwmon perspective, the expected use of such an interrupt would be
> > to call hwmon_notify_event() with the affected limit attribute as argument.
> > This would notify the thermal subsystem if the sensor is registered with it
> > (your patch doesn't set the necessary flag when registering the driver,
> > so this would not happen), it will send a notification to the sysfs
> > attribute, and generate a udev event.
> >
> Thanks, noted it down. Didn't know about the notification to the thermal
> subsystem and the generated udev event. :)
>
Note that you'd have to add something like
HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
to the code to register the sensor as thermal zone.
Guenter
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 12/13] net: phy: marvell-88q2xxx: cleanup mv88q2xxx_config_init
2024-01-31 15:21 ` Andrew Lunn
@ 2024-02-02 17:19 ` Dimitri Fedrau
0 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-02 17:19 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 Wed, Jan 31, 2024 at 04:21:44PM +0100 schrieb Andrew Lunn:
> On Mon, Jan 22, 2024 at 10:28:45PM +0100, Dimitri Fedrau wrote:
> > mv88q2xxx_config_init calls genphy_c45_read_pma which is done by
> > mv88q2xxx_read_status, it calls also mv88q2xxx_config_aneg which is
> > also called by the PHY state machine. Let the PHY state machine handle
> > the phydriver ops in their intendend way.
> >
> > Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
Thanks, for reviewing. Is there a chance that you also have a look on
patch 13 of the series ? It relies on this patch.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
2024-01-30 2:09 ` [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Jakub Kicinski
@ 2024-02-02 18:30 ` Dimitri Fedrau
2024-02-03 16:07 ` Andrew Lunn
0 siblings, 1 reply; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-02 18:30 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Paolo Abeni, Stefan Eichenberger, netdev,
linux-kernel
[...]
Probably late, but there are parts of the code which are based on the
sample code provided by Marvell. The sample code is licensed under BSD
2 Clause. Should I change the license in the driver to dual license ?
Best regards,
Dimitri
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
2024-02-02 18:30 ` Dimitri Fedrau
@ 2024-02-03 16:07 ` Andrew Lunn
2024-02-05 9:32 ` Dimitri Fedrau
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Lunn @ 2024-02-03 16:07 UTC (permalink / raw)
To: Dimitri Fedrau
Cc: Jakub Kicinski, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Paolo Abeni, Stefan Eichenberger, netdev,
linux-kernel
On Fri, Feb 02, 2024 at 07:30:29PM +0100, Dimitri Fedrau wrote:
> [...]
>
> Probably late, but there are parts of the code which are based on the
> sample code provided by Marvell. The sample code is licensed under BSD
> 2 Clause. Should I change the license in the driver to dual license ?
IANAL
You can take BSD code and release it with a GPL license. But it would
be better to indicate it is derived from BSD code. Either make it dual
license, or add a comment about the origin of the code.
Andrew
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY
2024-02-03 16:07 ` Andrew Lunn
@ 2024-02-05 9:32 ` Dimitri Fedrau
0 siblings, 0 replies; 44+ messages in thread
From: Dimitri Fedrau @ 2024-02-05 9:32 UTC (permalink / raw)
To: Andrew Lunn
Cc: Jakub Kicinski, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Paolo Abeni, Stefan Eichenberger, netdev,
linux-kernel
Am Sat, Feb 03, 2024 at 05:07:13PM +0100 schrieb Andrew Lunn:
> On Fri, Feb 02, 2024 at 07:30:29PM +0100, Dimitri Fedrau wrote:
> > [...]
> >
> > Probably late, but there are parts of the code which are based on the
> > sample code provided by Marvell. The sample code is licensed under BSD
> > 2 Clause. Should I change the license in the driver to dual license ?
>
> IANAL
>
> You can take BSD code and release it with a GPL license. But it would
> be better to indicate it is derived from BSD code. Either make it dual
> license, or add a comment about the origin of the code.
>
Is there a reason why the "MODULE_AUTHOR" is not in the driver and it's
also missing in marvell-88x2222.c. I also don't see any copyright
information. I'm just curious, not seeking for any legal advice. :) I
just have to get this figured out to stay out of trouble.
Dimitri
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2024-02-05 9:32 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-22 21:28 [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 01/13] net: phy: Add BaseT1 auto-negotiation constants Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 02/13] net: phy: Support 100/1000BT1 linkmode advertisements Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 03/13] net: phy: c45: detect 100/1000BASE-T1 " Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 04/13] net: phy: marvell-88q2xxx: fix typos Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 05/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Dimitri Fedrau
2024-01-31 15:08 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 06/13] net: phy: marvell-88q2xxx: add interrupt support for link detection Dimitri Fedrau
2024-01-31 15:12 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 07/13] net: phy: marvell-88q2xxx: add suspend / resume ops Dimitri Fedrau
2024-01-31 15:12 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor Dimitri Fedrau
2024-01-30 9:18 ` Russell King (Oracle)
2024-01-31 11:52 ` Dimitri Fedrau
2024-01-31 15:17 ` Andrew Lunn
2024-02-01 7:11 ` Dimitri Fedrau
2024-02-01 13:23 ` Andrew Lunn
2024-02-01 16:52 ` Dimitri Fedrau
2024-02-01 13:34 ` Guenter Roeck
2024-02-01 16:14 ` Dimitri Fedrau
2024-02-01 18:26 ` Guenter Roeck
2024-02-01 13:18 ` Guenter Roeck
2024-02-01 13:27 ` Andrew Lunn
2024-02-01 13:39 ` Guenter Roeck
2024-02-01 16:23 ` Dimitri Fedrau
2024-02-01 16:47 ` Andrew Lunn
2024-02-01 16:56 ` Dimitri Fedrau
2024-02-01 18:22 ` Guenter Roeck
2024-02-01 16:17 ` Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 09/13] net: phy: marvell-88q2xxx: add cable test support Dimitri Fedrau
2024-01-30 9:19 ` Russell King (Oracle)
2024-01-31 12:03 ` Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 10/13] net: phy: marvell-88q2xxx: make mv88q2xxx_config_aneg generic Dimitri Fedrau
2024-01-31 15:19 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 11/13] net: phy: marvell-88q2xxx: switch to mv88q2xxx_config_aneg Dimitri Fedrau
2024-01-31 15:20 ` Andrew Lunn
2024-01-22 21:28 ` [PATCH v5 net-next 12/13] net: phy: marvell-88q2xxx: cleanup mv88q2xxx_config_init Dimitri Fedrau
2024-01-31 15:21 ` Andrew Lunn
2024-02-02 17:19 ` Dimitri Fedrau
2024-01-22 21:28 ` [PATCH v5 net-next 13/13] net: phy: marvell-88q2xxx: remove redundant code Dimitri Fedrau
2024-01-30 2:09 ` [PATCH v5 net-next 00/13] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2220 PHY Jakub Kicinski
2024-02-02 18:30 ` Dimitri Fedrau
2024-02-03 16:07 ` Andrew Lunn
2024-02-05 9:32 ` Dimitri Fedrau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.