* [PATCH net-next 1/7] net: ethtool: Add attributes for cable test TDR data
2020-05-17 19:58 [PATCH net-next 0/7] Raw PHY TDR data Andrew Lunn
@ 2020-05-17 19:58 ` Andrew Lunn
2020-05-17 19:58 ` [PATCH net-next 2/7] net: ethtool: Add generic parts of cable test TDR Andrew Lunn
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2020-05-17 19:58 UTC (permalink / raw)
To: David Miller
Cc: netdev, Florian Fainelli, Heiner Kallweit, Chris Healy,
Michal Kubecek, Andrew Lunn
Some Ethernet PHYs can return the raw time domain reflectromatry data.
Add the attributes to allow this data to be requested and returned via
netlink ethtool.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
Documentation/networking/ethtool-netlink.rst | 79 ++++++++++++++++++++
include/uapi/linux/ethtool_netlink.h | 63 ++++++++++++++++
2 files changed, 142 insertions(+)
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index eed46b6aa07d..a504f9a1a6aa 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -205,6 +205,7 @@ Userspace to kernel:
``ETHTOOL_MSG_EEE_SET`` set EEE settings
``ETHTOOL_MSG_TSINFO_GET`` get timestamping info
``ETHTOOL_MSG_CABLE_TEST_ACT`` action start cable test
+ ``ETHTOOL_MSG_CABLE_TEST_TDR_ACT`` action start raw TDR cable test
===================================== ================================
Kernel to userspace:
@@ -237,6 +238,7 @@ Kernel to userspace:
``ETHTOOL_MSG_EEE_NTF`` EEE settings
``ETHTOOL_MSG_TSINFO_GET_REPLY`` timestamping info
``ETHTOOL_MSG_CABLE_TEST_NTF`` Cable test results
+ ``ETHTOOL_MSG_CABLE_TEST_TDR_NTF`` Cable test TDR results
===================================== =================================
``GET`` requests are sent by userspace applications to retrieve device
@@ -1012,6 +1014,82 @@ information.
| | | ``ETHTOOL_A_CABLE_FAULT_LENGTH_CM`` | u32 | length in cm |
+-+-+-----------------------------------------+--------+---------------------+
+CABLE_TEST TDR
+==============
+
+Start a cable test and report raw TDR data
+
+Request contents:
+
+ ==================================== ====== ==========================
+ ``ETHTOOL_A_CABLE_TEST_TDR_HEADER`` nested request header
+ ==================================== ====== ==========================
+
+Notification contents:
+
+Raw TDR data is gathered by sending a pulse down the cable and
+recording the amplitude of the reflected pulse for a given distance.
+
+It can take a number of seconds to collect TDR data, especial if the
+full 100 meters is probed at 1 meter intervals. When the test is
+started a notification will be sent containing just
+ETHTOOL_A_CABLE_TEST_TDR_STATUS with the value
+ETHTOOL_A_CABLE_TEST_NTF_STATUS_STARTED.
+
+When the test has completed a second notification will be sent
+containing ETHTOOL_A_CABLE_TEST_TDR_STATUS with the value
+ETHTOOL_A_CABLE_TEST_NTF_STATUS_COMPLETED and the TDR data.
+
+The message may optionally contain the amplitude of the pulse send
+down the cable. This is measured in mV. A reflection should not be
+bigger than transmitted pulse.
+
+Before the raw TDR data should be an ETHTOOL_A_CABLE_TDR_NEST_STEP
+nest containing information about the distance along the cable for the
+first reading, the last reading, and the step between each
+reading. Distance is measured in meters.
+
+For each step along the cable, a ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE is
+used to report the amplitude of the reflection for a given pair.
+
+ +---------------------------------------------+--------+----------------------+
+ | ``ETHTOOL_A_CABLE_TEST_TDR_HEADER`` | nested | reply header |
+ +---------------------------------------------+--------+----------------------+
+ | ``ETHTOOL_A_CABLE_TEST_TDR_STATUS`` | u8 | completed |
+ +---------------------------------------------+--------+----------------------+
+ | ``ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST`` | nested | all the results |
+ +-+-------------------------------------------+--------+----------------------+
+ | | ``ETHTOOL_A_CABLE_TDR_NEST_PULSE`` | nested | TX Pulse amplitude |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_PULSE_mV`` | s16 | Pulse amplitude |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | ``ETHTOOL_A_CABLE_NEST_STEP`` | nested | TDR step info |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_STEP_FIRST_DISTANCE ``| u32 | First data distance |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_STEP_LAST_DISTANCE `` | u32 | Last data distance |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_STEP_STEP_DISTANCE `` | u32 | distance of each step|
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | ``ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE`` | nested | Reflection amplitude |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_RESULTS_PAIR`` | u8 | pair number |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_AMPLITUDE_mV`` | s16 | Reflection amplitude |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | ``ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE`` | nested | Reflection amplitude |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_RESULTS_PAIR`` | u8 | pair number |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_AMPLITUDE_mV`` | s16 | Reflection amplitude |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | ``ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE`` | nested | Reflection amplitude |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_RESULTS_PAIR`` | u8 | pair number |
+ +-+-+-----------------------------------------+--------+----------------------+
+ | | | ``ETHTOOL_A_CABLE_AMPLITUDE_mV`` | s16 | Reflection amplitude |
+ +-+-+-----------------------------------------+--------+----------------------+
+
Request translation
===================
@@ -1108,4 +1186,5 @@ are netlink only.
``ETHTOOL_GFECPARAM`` n/a
``ETHTOOL_SFECPARAM`` n/a
n/a ''ETHTOOL_MSG_CABLE_TEST_ACT''
+ n/a ''ETHTOOL_MSG_CABLE_TEST_TDR_ACT''
=================================== =====================================
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 2881af411f76..4f223edcefda 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -40,6 +40,7 @@ enum {
ETHTOOL_MSG_EEE_SET,
ETHTOOL_MSG_TSINFO_GET,
ETHTOOL_MSG_CABLE_TEST_ACT,
+ ETHTOOL_MSG_CABLE_TEST_TDR_ACT,
/* add new constants above here */
__ETHTOOL_MSG_USER_CNT,
@@ -76,6 +77,7 @@ enum {
ETHTOOL_MSG_EEE_NTF,
ETHTOOL_MSG_TSINFO_GET_REPLY,
ETHTOOL_MSG_CABLE_TEST_NTF,
+ ETHTOOL_MSG_CABLE_TEST_TDR_NTF,
/* add new constants above here */
__ETHTOOL_MSG_KERNEL_CNT,
@@ -476,6 +478,67 @@ enum {
ETHTOOL_A_CABLE_TEST_NTF_MAX = (__ETHTOOL_A_CABLE_TEST_NTF_CNT - 1)
};
+/* CABLE TEST TDR */
+
+enum {
+ ETHTOOL_A_CABLE_TEST_TDR_UNSPEC,
+ ETHTOOL_A_CABLE_TEST_TDR_HEADER, /* nest - _A_HEADER_* */
+
+ /* add new constants above here */
+ __ETHTOOL_A_CABLE_TEST_TDR_CNT,
+ ETHTOOL_A_CABLE_TEST_TDR_MAX = __ETHTOOL_A_CABLE_TEST_TDR_CNT - 1
+};
+
+/* CABLE TEST TDR NOTIFY */
+
+enum {
+ ETHTOOL_A_CABLE_AMPLITUDE_UNSPEC,
+ ETHTOOL_A_CABLE_AMPLITUDE_PAIR, /* u8 */
+ ETHTOOL_A_CABLE_AMPLITUDE_mV, /* s16 */
+
+ __ETHTOOL_A_CABLE_AMPLITUDE_CNT,
+ ETHTOOL_A_CABLE_AMPLITUDE_MAX = (__ETHTOOL_A_CABLE_AMPLITUDE_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_CABLE_PULSE_UNSPEC,
+ ETHTOOL_A_CABLE_PULSE_mV, /* s16 */
+
+ __ETHTOOL_A_CABLE_PULSE_CNT,
+ ETHTOOL_A_CABLE_PULSE_MAX = (__ETHTOOL_A_CABLE_PULSE_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_CABLE_STEP_UNSPEC,
+ ETHTOOL_A_CABLE_STEP_FIRST_DISTANCE, /* u32 */
+ ETHTOOL_A_CABLE_STEP_LAST_DISTANCE, /* u32 */
+ ETHTOOL_A_CABLE_STEP_STEP_DISTANCE, /* u32 */
+
+ __ETHTOOL_A_CABLE_STEP_CNT,
+ ETHTOOL_A_CABLE_STEP_MAX = (__ETHTOOL_A_CABLE_STEP_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_CABLE_TDR_NEST_UNSPEC,
+ ETHTOOL_A_CABLE_TDR_NEST_STEP, /* nest - ETHTTOOL_A_CABLE_STEP */
+ ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE, /* nest - ETHTOOL_A_CABLE_AMPLITUDE */
+ ETHTOOL_A_CABLE_TDR_NEST_PULSE, /* nest - ETHTOOL_A_CABLE_PULSE */
+
+ __ETHTOOL_A_CABLE_TDR_NEST_CNT,
+ ETHTOOL_A_CABLE_TDR_NEST_MAX = (__ETHTOOL_A_CABLE_TDR_NEST_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_CABLE_TEST_TDR_NTF_UNSPEC,
+ ETHTOOL_A_CABLE_TEST_TDR_NTF_HEADER, /* nest - ETHTOOL_A_HEADER_* */
+ ETHTOOL_A_CABLE_TEST_TDR_NTF_STATUS, /* u8 - _STARTED/_COMPLETE */
+ ETHTOOL_A_CABLE_TEST_TDR_NTF_NEST, /* nest - of results: */
+
+ /* add new constants above here */
+ __ETHTOOL_A_CABLE_TEST_TDR_NTF_CNT,
+ ETHTOOL_A_CABLE_TEST_TDR_NTF_MAX = __ETHTOOL_A_CABLE_TEST_TDR_NTF_CNT - 1
+};
+
/* generic netlink info */
#define ETHTOOL_GENL_NAME "ethtool"
#define ETHTOOL_GENL_VERSION 1
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 2/7] net: ethtool: Add generic parts of cable test TDR
2020-05-17 19:58 [PATCH net-next 0/7] Raw PHY TDR data Andrew Lunn
2020-05-17 19:58 ` [PATCH net-next 1/7] net: ethtool: Add attributes for cable test " Andrew Lunn
@ 2020-05-17 19:58 ` Andrew Lunn
2020-05-17 19:58 ` [PATCH net-next 3/7] net: ethtool: Add helpers for cable test TDR data Andrew Lunn
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2020-05-17 19:58 UTC (permalink / raw)
To: David Miller
Cc: netdev, Florian Fainelli, Heiner Kallweit, Chris Healy,
Michal Kubecek, Andrew Lunn
Add the generic parts of the code used to trigger a cable test and
return raw TDR data. Any PHY driver which support this must implement
the new driver op.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/phy.c | 65 ++++++++++++++++++++++++++++++++-
include/linux/ethtool_netlink.h | 4 +-
include/linux/phy.h | 13 +++++++
net/ethtool/cabletest.c | 65 ++++++++++++++++++++++++++++++---
net/ethtool/netlink.c | 5 +++
net/ethtool/netlink.h | 1 +
6 files changed, 144 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d4bbf79dab6c..1f794c1716cf 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -519,7 +519,7 @@ int phy_start_cable_test(struct phy_device *phydev,
goto out;
}
- err = ethnl_cable_test_alloc(phydev);
+ err = ethnl_cable_test_alloc(phydev, ETHTOOL_MSG_CABLE_TEST_NTF);
if (err)
goto out;
@@ -552,6 +552,69 @@ int phy_start_cable_test(struct phy_device *phydev,
}
EXPORT_SYMBOL(phy_start_cable_test);
+int phy_start_cable_test_tdr(struct phy_device *phydev,
+ struct netlink_ext_ack *extack)
+{
+ struct net_device *dev = phydev->attached_dev;
+ int err = -ENOMEM;
+
+ if (!(phydev->drv &&
+ phydev->drv->cable_test_tdr_start &&
+ phydev->drv->cable_test_get_status)) {
+ NL_SET_ERR_MSG(extack,
+ "PHY driver does not support cable test TDR");
+ return -EOPNOTSUPP;
+ }
+
+ mutex_lock(&phydev->lock);
+ if (phydev->state == PHY_CABLETEST) {
+ NL_SET_ERR_MSG(extack,
+ "PHY already performing a test");
+ err = -EBUSY;
+ goto out;
+ }
+
+ if (phydev->state < PHY_UP ||
+ phydev->state > PHY_CABLETEST) {
+ NL_SET_ERR_MSG(extack,
+ "PHY not configured. Try setting interface up");
+ err = -EBUSY;
+ goto out;
+ }
+
+ err = ethnl_cable_test_alloc(phydev, ETHTOOL_MSG_CABLE_TEST_TDR_NTF);
+ if (err)
+ goto out;
+
+ /* Mark the carrier down until the test is complete */
+ phy_link_down(phydev, true);
+
+ netif_testing_on(dev);
+ err = phydev->drv->cable_test_tdr_start(phydev);
+ if (err) {
+ netif_testing_off(dev);
+ phy_link_up(phydev);
+ goto out_free;
+ }
+
+ phydev->state = PHY_CABLETEST;
+
+ if (phy_polling_mode(phydev))
+ phy_trigger_machine(phydev);
+
+ mutex_unlock(&phydev->lock);
+
+ return 0;
+
+out_free:
+ ethnl_cable_test_free(phydev);
+out:
+ mutex_unlock(&phydev->lock);
+
+ return err;
+}
+EXPORT_SYMBOL(phy_start_cable_test_tdr);
+
static int phy_config_aneg(struct phy_device *phydev)
{
if (phydev->drv->config_aneg)
diff --git a/include/linux/ethtool_netlink.h b/include/linux/ethtool_netlink.h
index e317fc99565e..24817ba252a0 100644
--- a/include/linux/ethtool_netlink.h
+++ b/include/linux/ethtool_netlink.h
@@ -17,13 +17,13 @@ enum ethtool_multicast_groups {
struct phy_device;
#if IS_ENABLED(CONFIG_ETHTOOL_NETLINK)
-int ethnl_cable_test_alloc(struct phy_device *phydev);
+int ethnl_cable_test_alloc(struct phy_device *phydev, u8 cmd);
void ethnl_cable_test_free(struct phy_device *phydev);
void ethnl_cable_test_finished(struct phy_device *phydev);
int ethnl_cable_test_result(struct phy_device *phydev, u8 pair, u8 result);
int ethnl_cable_test_fault_length(struct phy_device *phydev, u8 pair, u32 cm);
#else
-static inline int ethnl_cable_test_alloc(struct phy_device *phydev)
+static inline int ethnl_cable_test_alloc(struct phy_device *phydev, u8 cmd)
{
return -EOPNOTSUPP;
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5d8ff5428010..c35d45fe74ce 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -705,6 +705,10 @@ struct phy_driver {
/* Start a cable test */
int (*cable_test_start)(struct phy_device *dev);
+
+ /* Start a raw TDR cable test */
+ int (*cable_test_tdr_start)(struct phy_device *dev);
+
/* Once per second, or on interrupt, request the status of the
* test.
*/
@@ -1255,6 +1259,8 @@ int phy_reset_after_clk_enable(struct phy_device *phydev);
#if IS_ENABLED(CONFIG_PHYLIB)
int phy_start_cable_test(struct phy_device *phydev,
struct netlink_ext_ack *extack);
+int phy_start_cable_test_tdr(struct phy_device *phydev,
+ struct netlink_ext_ack *extack);
#else
static inline
int phy_start_cable_test(struct phy_device *phydev,
@@ -1263,6 +1269,13 @@ int phy_start_cable_test(struct phy_device *phydev,
NL_SET_ERR_MSG(extack, "Kernel not compiled with PHYLIB support");
return -EOPNOTSUPP;
}
+static inline
+int phy_start_cable_test_tdr(struct phy_device *phydev,
+ struct netlink_ext_ack *extack)
+{
+ NL_SET_ERR_MSG(extack, "Kernel not compiled with PHYLIB support");
+ return -EOPNOTSUPP;
+}
#endif
int phy_cable_test_result(struct phy_device *phydev, u8 pair, u16 result);
diff --git a/net/ethtool/cabletest.c b/net/ethtool/cabletest.c
index 5ba06eabe8c2..44dc4b8e26ac 100644
--- a/net/ethtool/cabletest.c
+++ b/net/ethtool/cabletest.c
@@ -13,7 +13,7 @@ cable_test_act_policy[ETHTOOL_A_CABLE_TEST_MAX + 1] = {
[ETHTOOL_A_CABLE_TEST_HEADER] = { .type = NLA_NESTED },
};
-static int ethnl_cable_test_started(struct phy_device *phydev)
+static int ethnl_cable_test_started(struct phy_device *phydev, u8 cmd)
{
struct sk_buff *skb;
int err = -ENOMEM;
@@ -23,7 +23,7 @@ static int ethnl_cable_test_started(struct phy_device *phydev)
if (!skb)
goto out;
- ehdr = ethnl_bcastmsg_put(skb, ETHTOOL_MSG_CABLE_TEST_NTF);
+ ehdr = ethnl_bcastmsg_put(skb, cmd);
if (!ehdr) {
err = -EMSGSIZE;
goto out;
@@ -86,7 +86,8 @@ int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info)
ethnl_ops_complete(dev);
if (!ret)
- ethnl_cable_test_started(dev->phydev);
+ ethnl_cable_test_started(dev->phydev,
+ ETHTOOL_MSG_CABLE_TEST_NTF);
out_rtnl:
rtnl_unlock();
@@ -95,7 +96,7 @@ int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info)
return ret;
}
-int ethnl_cable_test_alloc(struct phy_device *phydev)
+int ethnl_cable_test_alloc(struct phy_device *phydev, u8 cmd)
{
int err = -ENOMEM;
@@ -103,8 +104,7 @@ int ethnl_cable_test_alloc(struct phy_device *phydev)
if (!phydev->skb)
goto out;
- phydev->ehdr = ethnl_bcastmsg_put(phydev->skb,
- ETHTOOL_MSG_CABLE_TEST_NTF);
+ phydev->ehdr = ethnl_bcastmsg_put(phydev->skb, cmd);
if (!phydev->ehdr) {
err = -EMSGSIZE;
goto out;
@@ -199,3 +199,56 @@ int ethnl_cable_test_fault_length(struct phy_device *phydev, u8 pair, u32 cm)
return ret;
}
EXPORT_SYMBOL_GPL(ethnl_cable_test_fault_length);
+
+static const struct nla_policy
+cable_test_tdr_act_policy[ETHTOOL_A_CABLE_TEST_TDR_MAX + 1] = {
+ [ETHTOOL_A_CABLE_TEST_TDR_UNSPEC] = { .type = NLA_REJECT },
+ [ETHTOOL_A_CABLE_TEST_TDR_HEADER] = { .type = NLA_NESTED },
+};
+
+int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info)
+{
+ struct nlattr *tb[ETHTOOL_A_CABLE_TEST_TDR_MAX + 1];
+ struct ethnl_req_info req_info = {};
+ struct net_device *dev;
+ int ret;
+
+ ret = nlmsg_parse(info->nlhdr, GENL_HDRLEN, tb,
+ ETHTOOL_A_CABLE_TEST_TDR_MAX,
+ cable_test_tdr_act_policy, info->extack);
+ if (ret < 0)
+ return ret;
+
+ ret = ethnl_parse_header_dev_get(&req_info,
+ tb[ETHTOOL_A_CABLE_TEST_TDR_HEADER],
+ genl_info_net(info), info->extack,
+ true);
+ if (ret < 0)
+ return ret;
+
+ dev = req_info.dev;
+ if (!dev->phydev) {
+ ret = -EOPNOTSUPP;
+ goto out_dev_put;
+ }
+
+ rtnl_lock();
+ ret = ethnl_ops_begin(dev);
+ if (ret < 0)
+ goto out_rtnl;
+
+ ret = phy_start_cable_test_tdr(dev->phydev, info->extack);
+
+ ethnl_ops_complete(dev);
+
+ if (!ret)
+ ethnl_cable_test_started(dev->phydev,
+ ETHTOOL_MSG_CABLE_TEST_TDR_NTF);
+
+out_rtnl:
+ rtnl_unlock();
+out_dev_put:
+ dev_put(dev);
+ return ret;
+}
+
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 87bc02da74bc..b236b0d6cccf 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -844,6 +844,11 @@ static const struct genl_ops ethtool_genl_ops[] = {
.flags = GENL_UNS_ADMIN_PERM,
.doit = ethnl_act_cable_test,
},
+ {
+ .cmd = ETHTOOL_MSG_CABLE_TEST_TDR_ACT,
+ .flags = GENL_UNS_ADMIN_PERM,
+ .doit = ethnl_act_cable_test_tdr,
+ },
};
static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index b0eb5d920099..9a96b6e90dc2 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -360,5 +360,6 @@ int ethnl_set_coalesce(struct sk_buff *skb, struct genl_info *info);
int ethnl_set_pause(struct sk_buff *skb, struct genl_info *info);
int ethnl_set_eee(struct sk_buff *skb, struct genl_info *info);
int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
+int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info);
#endif /* _NET_ETHTOOL_NETLINK_H */
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 3/7] net: ethtool: Add helpers for cable test TDR data
2020-05-17 19:58 [PATCH net-next 0/7] Raw PHY TDR data Andrew Lunn
2020-05-17 19:58 ` [PATCH net-next 1/7] net: ethtool: Add attributes for cable test " Andrew Lunn
2020-05-17 19:58 ` [PATCH net-next 2/7] net: ethtool: Add generic parts of cable test TDR Andrew Lunn
@ 2020-05-17 19:58 ` Andrew Lunn
2020-05-17 19:58 ` [PATCH net-next 4/7] net: phy: marvell: Add support for amplitude graph Andrew Lunn
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2020-05-17 19:58 UTC (permalink / raw)
To: David Miller
Cc: netdev, Florian Fainelli, Heiner Kallweit, Chris Healy,
Michal Kubecek, Andrew Lunn
Add helpers for returning raw TDR helpers in netlink messages.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
include/linux/ethtool_netlink.h | 21 +++++++++
net/ethtool/cabletest.c | 79 ++++++++++++++++++++++++++++++++-
2 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/include/linux/ethtool_netlink.h b/include/linux/ethtool_netlink.h
index 24817ba252a0..8fbe4f97ffad 100644
--- a/include/linux/ethtool_netlink.h
+++ b/include/linux/ethtool_netlink.h
@@ -22,6 +22,10 @@ void ethnl_cable_test_free(struct phy_device *phydev);
void ethnl_cable_test_finished(struct phy_device *phydev);
int ethnl_cable_test_result(struct phy_device *phydev, u8 pair, u8 result);
int ethnl_cable_test_fault_length(struct phy_device *phydev, u8 pair, u32 cm);
+int ethnl_cable_test_amplitude(struct phy_device *phydev, u8 pair, s16 mV);
+int ethnl_cable_test_pulse(struct phy_device *phydev, u16 mV);
+int ethnl_cable_test_step(struct phy_device *phydev, u32 first, u32 last,
+ u32 step);
#else
static inline int ethnl_cable_test_alloc(struct phy_device *phydev, u8 cmd)
{
@@ -46,5 +50,22 @@ static inline int ethnl_cable_test_fault_length(struct phy_device *phydev,
{
return -EOPNOTSUPP;
}
+
+static inline int ethnl_cable_test_amplitude(struct phy_device *phydev,
+ u8 pair, s16 mV)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int ethnl_cable_test_pulse(struct phy_device *phydev, u16 mV)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int ethnl_cable_test_step(struct phy_device *phydev, u32 first,
+ u32 last, u32 step)
+{
+ return -EOPNOTSUPP;
+}
#endif /* IS_ENABLED(ETHTOOL_NETLINK) */
#endif /* _LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/net/ethtool/cabletest.c b/net/ethtool/cabletest.c
index 44dc4b8e26ac..c02575e26336 100644
--- a/net/ethtool/cabletest.c
+++ b/net/ethtool/cabletest.c
@@ -100,7 +100,10 @@ int ethnl_cable_test_alloc(struct phy_device *phydev, u8 cmd)
{
int err = -ENOMEM;
- phydev->skb = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+ /* One TDR sample occupies 20 bytes. For a 150 meter cable,
+ * with four pairs, around 12K is needed.
+ */
+ phydev->skb = genlmsg_new(SZ_16K, GFP_KERNEL);
if (!phydev->skb)
goto out;
@@ -252,3 +255,77 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info)
return ret;
}
+int ethnl_cable_test_amplitude(struct phy_device *phydev,
+ u8 pair, s16 mV)
+{
+ struct nlattr *nest;
+ int ret = -EMSGSIZE;
+
+ nest = nla_nest_start(phydev->skb,
+ ETHTOOL_A_CABLE_TDR_NEST_AMPLITUDE);
+ if (!nest)
+ return -EMSGSIZE;
+
+ if (nla_put_u8(phydev->skb, ETHTOOL_A_CABLE_AMPLITUDE_PAIR, pair))
+ goto err;
+ if (nla_put_u16(phydev->skb, ETHTOOL_A_CABLE_AMPLITUDE_mV, mV))
+ goto err;
+
+ nla_nest_end(phydev->skb, nest);
+ return 0;
+
+err:
+ nla_nest_cancel(phydev->skb, nest);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ethnl_cable_test_amplitude);
+
+int ethnl_cable_test_pulse(struct phy_device *phydev, u16 mV)
+{
+ struct nlattr *nest;
+ int ret = -EMSGSIZE;
+
+ nest = nla_nest_start(phydev->skb, ETHTOOL_A_CABLE_TDR_NEST_PULSE);
+ if (!nest)
+ return -EMSGSIZE;
+
+ if (nla_put_u16(phydev->skb, ETHTOOL_A_CABLE_PULSE_mV, mV))
+ goto err;
+
+ nla_nest_end(phydev->skb, nest);
+ return 0;
+
+err:
+ nla_nest_cancel(phydev->skb, nest);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ethnl_cable_test_pulse);
+
+int ethnl_cable_test_step(struct phy_device *phydev, u32 first, u32 last,
+ u32 step)
+{
+ struct nlattr *nest;
+ int ret = -EMSGSIZE;
+
+ nest = nla_nest_start(phydev->skb, ETHTOOL_A_CABLE_TDR_NEST_STEP);
+ if (!nest)
+ return -EMSGSIZE;
+
+ if (nla_put_u32(phydev->skb, ETHTOOL_A_CABLE_STEP_FIRST_DISTANCE,
+ first))
+ goto err;
+
+ if (nla_put_u32(phydev->skb, ETHTOOL_A_CABLE_STEP_LAST_DISTANCE, last))
+ goto err;
+
+ if (nla_put_u32(phydev->skb, ETHTOOL_A_CABLE_STEP_STEP_DISTANCE, step))
+ goto err;
+
+ nla_nest_end(phydev->skb, nest);
+ return 0;
+
+err:
+ nla_nest_cancel(phydev->skb, nest);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ethnl_cable_test_step);
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 4/7] net: phy: marvell: Add support for amplitude graph
2020-05-17 19:58 [PATCH net-next 0/7] Raw PHY TDR data Andrew Lunn
` (2 preceding siblings ...)
2020-05-17 19:58 ` [PATCH net-next 3/7] net: ethtool: Add helpers for cable test TDR data Andrew Lunn
@ 2020-05-17 19:58 ` Andrew Lunn
2020-05-17 20:25 ` Chris Healy
2020-05-17 19:58 ` [PATCH net-next 5/7] net: ethtool: Allow PHY cable test TDR data to configured Andrew Lunn
` (3 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Andrew Lunn @ 2020-05-17 19:58 UTC (permalink / raw)
To: David Miller
Cc: netdev, Florian Fainelli, Heiner Kallweit, Chris Healy,
Michal Kubecek, Andrew Lunn
The Marvell PHYs can measure the amplitude of the returned signal for
a given distance. Implement this option of the cable test
infrastructure.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 227 +++++++++++++++++++++++++++++++++++++-
1 file changed, 226 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 4bc7febf9248..e7994f5b506e 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -42,6 +42,7 @@
#define MII_MARVELL_FIBER_PAGE 0x01
#define MII_MARVELL_MSCR_PAGE 0x02
#define MII_MARVELL_LED_PAGE 0x03
+#define MII_MARVELL_VCT5_PAGE 0x05
#define MII_MARVELL_MISC_TEST_PAGE 0x06
#define MII_MARVELL_VCT7_PAGE 0x07
#define MII_MARVELL_WOL_PAGE 0x11
@@ -164,6 +165,54 @@
#define MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII 0x1 /* SGMII to copper */
#define MII_88E1510_GEN_CTRL_REG_1_RESET 0x8000 /* Soft reset */
+#define MII_VCT5_TX_RX_MDI0_COUPLING 0x10
+#define MII_VCT5_TX_RX_MDI1_COUPLING 0x11
+#define MII_VCT5_TX_RX_MDI2_COUPLING 0x12
+#define MII_VCT5_TX_RX_MDI3_COUPLING 0x13
+#define MII_VCT5_TX_RX_AMPLITUDE_MASK 0x7f00
+#define MII_VCT5_TX_RX_AMPLITUDE_SHIFT 8
+#define MII_VCT5_TX_RX_COUPLING_POSITIVE_REFLECTION BIT(15)
+
+#define MII_VCT5_CTRL 0x17
+#define MII_VCT5_CTRL_ENABLE BIT(15)
+#define MII_VCT5_CTRL_COMPLETE BIT(14)
+#define MII_VCT5_CTRL_TX_SAME_CHANNEL (0x0 << 11)
+#define MII_VCT5_CTRL_TX0_CHANNEL (0x4 << 11)
+#define MII_VCT5_CTRL_TX1_CHANNEL (0x5 << 11)
+#define MII_VCT5_CTRL_TX2_CHANNEL (0x6 << 11)
+#define MII_VCT5_CTRL_TX3_CHANNEL (0x7 << 11)
+#define MII_VCT5_CTRL_SAMPLES_2 (0x0 << 8)
+#define MII_VCT5_CTRL_SAMPLES_4 (0x1 << 8)
+#define MII_VCT5_CTRL_SAMPLES_8 (0x2 << 8)
+#define MII_VCT5_CTRL_SAMPLES_16 (0x3 << 8)
+#define MII_VCT5_CTRL_SAMPLES_32 (0x4 << 8)
+#define MII_VCT5_CTRL_SAMPLES_64 (0x5 << 8)
+#define MII_VCT5_CTRL_SAMPLES_128 (0x6 << 8)
+#define MII_VCT5_CTRL_SAMPLES_DEFAULT (0x6 << 8)
+#define MII_VCT5_CTRL_SAMPLES_256 (0x7 << 8)
+#define MII_VCT5_CTRL_SAMPLES_SHIFT 8
+#define MII_VCT5_CTRL_MODE_MAXIMUM_PEEK (0x0 << 6)
+#define MII_VCT5_CTRL_MODE_FIRST_LAST_PEEK (0x1 << 6)
+#define MII_VCT5_CTRL_MODE_OFFSET (0x2 << 6)
+#define MII_VCT5_CTRL_SAMPLE_POINT (0x3 << 6)
+#define MII_VCT5_CTRL_PEEK_HYST_DEFAULT 3
+
+#define MII_VCT5_SAMPLE_POINT_DISTANCE 0x18
+#define MII_VCT5_TX_PULSE_CTRL 0x1c
+#define MII_VCT5_TX_PULSE_CTRL_DONT_WAIT_LINK_DOWN BIT(12)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_128nS (0x0 << 10)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_96nS (0x1 << 10)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_64nS (0x2 << 10)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_32nS (0x3 << 10)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_SHIFT 10
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_1000mV (0x0 << 8)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_750mV (0x1 << 8)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_500mV (0x2 << 8)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_250mV (0x3 << 8)
+#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_SHIFT 8
+#define MII_VCT5_TX_PULSE_CTRL_MAX_AMP BIT(7)
+#define MII_VCT5_TX_PULSE_CTRL_GT_140m_46_86mV (0x6 << 0)
+
#define MII_VCT7_PAIR_0_DISTANCE 0x10
#define MII_VCT7_PAIR_1_DISTANCE 0x11
#define MII_VCT7_PAIR_2_DISTANCE 0x12
@@ -220,6 +269,7 @@ struct marvell_priv {
u64 stats[ARRAY_SIZE(marvell_hw_stats)];
char *hwmon_name;
struct device *hwmon_dev;
+ bool cable_test_tdr;
};
static int marvell_read_page(struct phy_device *phydev)
@@ -1690,7 +1740,117 @@ static void marvell_get_stats(struct phy_device *phydev,
data[i] = marvell_get_stat(phydev, i);
}
-static int marvell_vct7_cable_test_start(struct phy_device *phydev)
+static int marvell_vct5_wait_complete(struct phy_device *phydev)
+{
+ int i;
+ int val;
+
+ for (i = 0; i < 32; i++) {
+ val = phy_read_paged(phydev, MII_MARVELL_VCT5_PAGE,
+ MII_VCT5_CTRL);
+ if (val < 0)
+ return val;
+
+ if (val & MII_VCT5_CTRL_COMPLETE)
+ return 0;
+
+ usleep_range(1000, 2000);
+ }
+
+ phydev_err(phydev, "Timeout while waiting for cable test to finish\n");
+ return -ETIMEDOUT;
+}
+
+static int marvell_vct5_amplitude(struct phy_device *phydev, int pair)
+{
+ int amplitude;
+ int val;
+ int reg;
+
+ reg = MII_VCT5_TX_RX_MDI0_COUPLING + pair;
+ val = phy_read_paged(phydev, MII_MARVELL_VCT5_PAGE, reg);
+
+ if (val < 0)
+ return 0;
+
+ amplitude = (val & MII_VCT5_TX_RX_AMPLITUDE_MASK) >>
+ MII_VCT5_TX_RX_AMPLITUDE_SHIFT;
+
+ if (!(val & MII_VCT5_TX_RX_COUPLING_POSITIVE_REFLECTION))
+ amplitude = -amplitude;
+
+ return 1000 * amplitude / 128;
+}
+
+static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
+ int meters)
+{
+ int mV_pair0, mV_pair1, mV_pair2, mV_pair3;
+ int distance;
+ u16 reg;
+ int err;
+
+ distance = meters * 1000 / 805;
+
+ err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
+ MII_VCT5_SAMPLE_POINT_DISTANCE,
+ distance);
+ if (err)
+ return err;
+
+ reg = MII_VCT5_CTRL_ENABLE |
+ MII_VCT5_CTRL_TX_SAME_CHANNEL |
+ MII_VCT5_CTRL_SAMPLES_DEFAULT |
+ MII_VCT5_CTRL_SAMPLE_POINT |
+ MII_VCT5_CTRL_PEEK_HYST_DEFAULT;
+ err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
+ MII_VCT5_CTRL, reg);
+ if (err)
+ return err;
+
+ err = marvell_vct5_wait_complete(phydev);
+ if (err)
+ return err;
+
+ mV_pair0 = marvell_vct5_amplitude(phydev, 0);
+ mV_pair1 = marvell_vct5_amplitude(phydev, 1);
+ mV_pair2 = marvell_vct5_amplitude(phydev, 2);
+ mV_pair3 = marvell_vct5_amplitude(phydev, 3);
+
+ ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_A, mV_pair0);
+ ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_B, mV_pair1);
+ ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_C, mV_pair2);
+ ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_D, mV_pair3);
+
+ return 0;
+}
+
+static int marvell_vct5_amplitude_graph(struct phy_device *phydev)
+{
+ int meters;
+ int err;
+ u16 reg;
+
+ reg = MII_VCT5_TX_PULSE_CTRL_GT_140m_46_86mV |
+ MII_VCT5_TX_PULSE_CTRL_DONT_WAIT_LINK_DOWN |
+ MII_VCT5_TX_PULSE_CTRL_MAX_AMP |
+ MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_32nS;
+
+ err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
+ MII_VCT5_TX_PULSE_CTRL, reg);
+ if (err)
+ return err;
+
+ for (meters = 0; meters <= 100; meters++) {
+ err = marvell_vct5_amplitude_distance(phydev, meters);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int marvell_cable_test_start_common(struct phy_device *phydev)
{
int bmcr, bmsr, ret;
@@ -1719,12 +1879,66 @@ static int marvell_vct7_cable_test_start(struct phy_device *phydev)
if (bmsr & BMSR_LSTATUS)
msleep(1500);
+ return 0;
+}
+
+static int marvell_vct7_cable_test_start(struct phy_device *phydev)
+{
+ struct marvell_priv *priv = phydev->priv;
+ int ret;
+
+ ret = marvell_cable_test_start_common(phydev);
+ if (ret)
+ return ret;
+
+ priv->cable_test_tdr = false;
+
+ /* Reset the VCT5 API control to defaults, otherwise
+ * VCT7 does not work correctly.
+ */
+ ret = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
+ MII_VCT5_CTRL,
+ MII_VCT5_CTRL_TX_SAME_CHANNEL |
+ MII_VCT5_CTRL_SAMPLES_DEFAULT |
+ MII_VCT5_CTRL_MODE_MAXIMUM_PEEK |
+ MII_VCT5_CTRL_PEEK_HYST_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
+ MII_VCT5_SAMPLE_POINT_DISTANCE, 0);
+ if (ret)
+ return ret;
+
return phy_write_paged(phydev, MII_MARVELL_VCT7_PAGE,
MII_VCT7_CTRL,
MII_VCT7_CTRL_RUN_NOW |
MII_VCT7_CTRL_CENTIMETERS);
}
+static int marvell_vct5_cable_test_tdr_start(struct phy_device *phydev)
+{
+ struct marvell_priv *priv = phydev->priv;
+ int ret;
+
+ /* Disable VCT7 */
+ ret = phy_write_paged(phydev, MII_MARVELL_VCT7_PAGE,
+ MII_VCT7_CTRL, 0);
+ if (ret)
+ return ret;
+
+ ret = marvell_cable_test_start_common(phydev);
+ if (ret)
+ return ret;
+
+ priv->cable_test_tdr = true;
+ ret = ethnl_cable_test_pulse(phydev, 1000);
+ if (ret)
+ return ret;
+
+ return ethnl_cable_test_step(phydev, 0, 100, 1);
+}
+
static int marvell_vct7_distance_to_length(int distance, bool meter)
{
if (meter)
@@ -1828,8 +2042,15 @@ static int marvell_vct7_cable_test_report(struct phy_device *phydev)
static int marvell_vct7_cable_test_get_status(struct phy_device *phydev,
bool *finished)
{
+ struct marvell_priv *priv = phydev->priv;
int ret;
+ if (priv->cable_test_tdr) {
+ ret = marvell_vct5_amplitude_graph(phydev);
+ *finished = true;
+ return ret;
+ }
+
*finished = false;
ret = phy_read_paged(phydev, MII_MARVELL_VCT7_PAGE,
@@ -2563,6 +2784,7 @@ static struct phy_driver marvell_drivers[] = {
.get_tunable = m88e1011_get_tunable,
.set_tunable = m88e1011_set_tunable,
.cable_test_start = marvell_vct7_cable_test_start,
+ .cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
.cable_test_get_status = marvell_vct7_cable_test_get_status,
},
{
@@ -2588,6 +2810,7 @@ static struct phy_driver marvell_drivers[] = {
.get_tunable = m88e1540_get_tunable,
.set_tunable = m88e1540_set_tunable,
.cable_test_start = marvell_vct7_cable_test_start,
+ .cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
.cable_test_get_status = marvell_vct7_cable_test_get_status,
},
{
@@ -2613,6 +2836,7 @@ static struct phy_driver marvell_drivers[] = {
.get_tunable = m88e1540_get_tunable,
.set_tunable = m88e1540_set_tunable,
.cable_test_start = marvell_vct7_cable_test_start,
+ .cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
.cable_test_get_status = marvell_vct7_cable_test_get_status,
},
{
@@ -2658,6 +2882,7 @@ static struct phy_driver marvell_drivers[] = {
.get_tunable = m88e1540_get_tunable,
.set_tunable = m88e1540_set_tunable,
.cable_test_start = marvell_vct7_cable_test_start,
+ .cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
.cable_test_get_status = marvell_vct7_cable_test_get_status,
},
};
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next 4/7] net: phy: marvell: Add support for amplitude graph
2020-05-17 19:58 ` [PATCH net-next 4/7] net: phy: marvell: Add support for amplitude graph Andrew Lunn
@ 2020-05-17 20:25 ` Chris Healy
2020-05-17 20:51 ` Andrew Lunn
0 siblings, 1 reply; 13+ messages in thread
From: Chris Healy @ 2020-05-17 20:25 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, netdev, Florian Fainelli, Heiner Kallweit, Michal Kubecek
On Sun, May 17, 2020 at 12:59 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> The Marvell PHYs can measure the amplitude of the returned signal for
> a given distance. Implement this option of the cable test
> infrastructure.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> drivers/net/phy/marvell.c | 227 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 226 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index 4bc7febf9248..e7994f5b506e 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -42,6 +42,7 @@
> #define MII_MARVELL_FIBER_PAGE 0x01
> #define MII_MARVELL_MSCR_PAGE 0x02
> #define MII_MARVELL_LED_PAGE 0x03
> +#define MII_MARVELL_VCT5_PAGE 0x05
> #define MII_MARVELL_MISC_TEST_PAGE 0x06
> #define MII_MARVELL_VCT7_PAGE 0x07
> #define MII_MARVELL_WOL_PAGE 0x11
> @@ -164,6 +165,54 @@
> #define MII_88E1510_GEN_CTRL_REG_1_MODE_SGMII 0x1 /* SGMII to copper */
> #define MII_88E1510_GEN_CTRL_REG_1_RESET 0x8000 /* Soft reset */
>
> +#define MII_VCT5_TX_RX_MDI0_COUPLING 0x10
> +#define MII_VCT5_TX_RX_MDI1_COUPLING 0x11
> +#define MII_VCT5_TX_RX_MDI2_COUPLING 0x12
> +#define MII_VCT5_TX_RX_MDI3_COUPLING 0x13
> +#define MII_VCT5_TX_RX_AMPLITUDE_MASK 0x7f00
> +#define MII_VCT5_TX_RX_AMPLITUDE_SHIFT 8
> +#define MII_VCT5_TX_RX_COUPLING_POSITIVE_REFLECTION BIT(15)
> +
> +#define MII_VCT5_CTRL 0x17
> +#define MII_VCT5_CTRL_ENABLE BIT(15)
> +#define MII_VCT5_CTRL_COMPLETE BIT(14)
> +#define MII_VCT5_CTRL_TX_SAME_CHANNEL (0x0 << 11)
> +#define MII_VCT5_CTRL_TX0_CHANNEL (0x4 << 11)
> +#define MII_VCT5_CTRL_TX1_CHANNEL (0x5 << 11)
> +#define MII_VCT5_CTRL_TX2_CHANNEL (0x6 << 11)
> +#define MII_VCT5_CTRL_TX3_CHANNEL (0x7 << 11)
> +#define MII_VCT5_CTRL_SAMPLES_2 (0x0 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_4 (0x1 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_8 (0x2 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_16 (0x3 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_32 (0x4 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_64 (0x5 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_128 (0x6 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_DEFAULT (0x6 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_256 (0x7 << 8)
> +#define MII_VCT5_CTRL_SAMPLES_SHIFT 8
> +#define MII_VCT5_CTRL_MODE_MAXIMUM_PEEK (0x0 << 6)
> +#define MII_VCT5_CTRL_MODE_FIRST_LAST_PEEK (0x1 << 6)
> +#define MII_VCT5_CTRL_MODE_OFFSET (0x2 << 6)
> +#define MII_VCT5_CTRL_SAMPLE_POINT (0x3 << 6)
> +#define MII_VCT5_CTRL_PEEK_HYST_DEFAULT 3
> +
> +#define MII_VCT5_SAMPLE_POINT_DISTANCE 0x18
> +#define MII_VCT5_TX_PULSE_CTRL 0x1c
> +#define MII_VCT5_TX_PULSE_CTRL_DONT_WAIT_LINK_DOWN BIT(12)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_128nS (0x0 << 10)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_96nS (0x1 << 10)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_64nS (0x2 << 10)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_32nS (0x3 << 10)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_SHIFT 10
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_1000mV (0x0 << 8)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_750mV (0x1 << 8)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_500mV (0x2 << 8)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_250mV (0x3 << 8)
> +#define MII_VCT5_TX_PULSE_CTRL_PULSE_AMPLITUDE_SHIFT 8
> +#define MII_VCT5_TX_PULSE_CTRL_MAX_AMP BIT(7)
> +#define MII_VCT5_TX_PULSE_CTRL_GT_140m_46_86mV (0x6 << 0)
> +
> #define MII_VCT7_PAIR_0_DISTANCE 0x10
> #define MII_VCT7_PAIR_1_DISTANCE 0x11
> #define MII_VCT7_PAIR_2_DISTANCE 0x12
> @@ -220,6 +269,7 @@ struct marvell_priv {
> u64 stats[ARRAY_SIZE(marvell_hw_stats)];
> char *hwmon_name;
> struct device *hwmon_dev;
> + bool cable_test_tdr;
> };
>
> static int marvell_read_page(struct phy_device *phydev)
> @@ -1690,7 +1740,117 @@ static void marvell_get_stats(struct phy_device *phydev,
> data[i] = marvell_get_stat(phydev, i);
> }
>
> -static int marvell_vct7_cable_test_start(struct phy_device *phydev)
> +static int marvell_vct5_wait_complete(struct phy_device *phydev)
> +{
> + int i;
> + int val;
> +
> + for (i = 0; i < 32; i++) {
> + val = phy_read_paged(phydev, MII_MARVELL_VCT5_PAGE,
> + MII_VCT5_CTRL);
> + if (val < 0)
> + return val;
> +
> + if (val & MII_VCT5_CTRL_COMPLETE)
> + return 0;
> +
> + usleep_range(1000, 2000);
> + }
> +
> + phydev_err(phydev, "Timeout while waiting for cable test to finish\n");
> + return -ETIMEDOUT;
> +}
> +
> +static int marvell_vct5_amplitude(struct phy_device *phydev, int pair)
> +{
> + int amplitude;
> + int val;
> + int reg;
> +
> + reg = MII_VCT5_TX_RX_MDI0_COUPLING + pair;
> + val = phy_read_paged(phydev, MII_MARVELL_VCT5_PAGE, reg);
> +
> + if (val < 0)
> + return 0;
> +
> + amplitude = (val & MII_VCT5_TX_RX_AMPLITUDE_MASK) >>
> + MII_VCT5_TX_RX_AMPLITUDE_SHIFT;
> +
> + if (!(val & MII_VCT5_TX_RX_COUPLING_POSITIVE_REFLECTION))
> + amplitude = -amplitude;
> +
> + return 1000 * amplitude / 128;
> +}
> +
> +static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
> + int meters)
> +{
> + int mV_pair0, mV_pair1, mV_pair2, mV_pair3;
> + int distance;
> + u16 reg;
> + int err;
> +
> + distance = meters * 1000 / 805;
With this integer based meters representation, it seems to me that we
are artificially reducing the resolution of the distance sampling.
For a 100 meter cable, the Marvell implementation looks to support 124
sample points. This could result in incorrect data reporting as two
adjacent meter numbers would resolve to the same disatance value
entered into the register. (eg - 2 meters = 2 distance 3 meters = 2
distance)
Is there a better way of doing this which would allow for userspace to
use the full resolution of the hardware?
> +
> + err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
> + MII_VCT5_SAMPLE_POINT_DISTANCE,
> + distance);
> + if (err)
> + return err;
> +
> + reg = MII_VCT5_CTRL_ENABLE |
> + MII_VCT5_CTRL_TX_SAME_CHANNEL |
> + MII_VCT5_CTRL_SAMPLES_DEFAULT |
> + MII_VCT5_CTRL_SAMPLE_POINT |
> + MII_VCT5_CTRL_PEEK_HYST_DEFAULT;
> + err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
> + MII_VCT5_CTRL, reg);
> + if (err)
> + return err;
> +
> + err = marvell_vct5_wait_complete(phydev);
> + if (err)
> + return err;
> +
> + mV_pair0 = marvell_vct5_amplitude(phydev, 0);
> + mV_pair1 = marvell_vct5_amplitude(phydev, 1);
> + mV_pair2 = marvell_vct5_amplitude(phydev, 2);
> + mV_pair3 = marvell_vct5_amplitude(phydev, 3);
> +
> + ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_A, mV_pair0);
> + ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_B, mV_pair1);
> + ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_C, mV_pair2);
> + ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_D, mV_pair3);
> +
> + return 0;
> +}
> +
> +static int marvell_vct5_amplitude_graph(struct phy_device *phydev)
> +{
> + int meters;
> + int err;
> + u16 reg;
> +
> + reg = MII_VCT5_TX_PULSE_CTRL_GT_140m_46_86mV |
> + MII_VCT5_TX_PULSE_CTRL_DONT_WAIT_LINK_DOWN |
> + MII_VCT5_TX_PULSE_CTRL_MAX_AMP |
> + MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_32nS;
> +
> + err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
> + MII_VCT5_TX_PULSE_CTRL, reg);
> + if (err)
> + return err;
> +
> + for (meters = 0; meters <= 100; meters++) {
> + err = marvell_vct5_amplitude_distance(phydev, meters);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static int marvell_cable_test_start_common(struct phy_device *phydev)
> {
> int bmcr, bmsr, ret;
>
> @@ -1719,12 +1879,66 @@ static int marvell_vct7_cable_test_start(struct phy_device *phydev)
> if (bmsr & BMSR_LSTATUS)
> msleep(1500);
>
> + return 0;
> +}
> +
> +static int marvell_vct7_cable_test_start(struct phy_device *phydev)
> +{
> + struct marvell_priv *priv = phydev->priv;
> + int ret;
> +
> + ret = marvell_cable_test_start_common(phydev);
> + if (ret)
> + return ret;
> +
> + priv->cable_test_tdr = false;
> +
> + /* Reset the VCT5 API control to defaults, otherwise
> + * VCT7 does not work correctly.
> + */
> + ret = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
> + MII_VCT5_CTRL,
> + MII_VCT5_CTRL_TX_SAME_CHANNEL |
> + MII_VCT5_CTRL_SAMPLES_DEFAULT |
> + MII_VCT5_CTRL_MODE_MAXIMUM_PEEK |
> + MII_VCT5_CTRL_PEEK_HYST_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
> + MII_VCT5_SAMPLE_POINT_DISTANCE, 0);
> + if (ret)
> + return ret;
> +
> return phy_write_paged(phydev, MII_MARVELL_VCT7_PAGE,
> MII_VCT7_CTRL,
> MII_VCT7_CTRL_RUN_NOW |
> MII_VCT7_CTRL_CENTIMETERS);
> }
>
> +static int marvell_vct5_cable_test_tdr_start(struct phy_device *phydev)
> +{
> + struct marvell_priv *priv = phydev->priv;
> + int ret;
> +
> + /* Disable VCT7 */
> + ret = phy_write_paged(phydev, MII_MARVELL_VCT7_PAGE,
> + MII_VCT7_CTRL, 0);
> + if (ret)
> + return ret;
> +
> + ret = marvell_cable_test_start_common(phydev);
> + if (ret)
> + return ret;
> +
> + priv->cable_test_tdr = true;
> + ret = ethnl_cable_test_pulse(phydev, 1000);
> + if (ret)
> + return ret;
> +
> + return ethnl_cable_test_step(phydev, 0, 100, 1);
> +}
> +
> static int marvell_vct7_distance_to_length(int distance, bool meter)
> {
> if (meter)
> @@ -1828,8 +2042,15 @@ static int marvell_vct7_cable_test_report(struct phy_device *phydev)
> static int marvell_vct7_cable_test_get_status(struct phy_device *phydev,
> bool *finished)
> {
> + struct marvell_priv *priv = phydev->priv;
> int ret;
>
> + if (priv->cable_test_tdr) {
> + ret = marvell_vct5_amplitude_graph(phydev);
> + *finished = true;
> + return ret;
> + }
> +
> *finished = false;
>
> ret = phy_read_paged(phydev, MII_MARVELL_VCT7_PAGE,
> @@ -2563,6 +2784,7 @@ static struct phy_driver marvell_drivers[] = {
> .get_tunable = m88e1011_get_tunable,
> .set_tunable = m88e1011_set_tunable,
> .cable_test_start = marvell_vct7_cable_test_start,
> + .cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
> .cable_test_get_status = marvell_vct7_cable_test_get_status,
> },
> {
> @@ -2588,6 +2810,7 @@ static struct phy_driver marvell_drivers[] = {
> .get_tunable = m88e1540_get_tunable,
> .set_tunable = m88e1540_set_tunable,
> .cable_test_start = marvell_vct7_cable_test_start,
> + .cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
> .cable_test_get_status = marvell_vct7_cable_test_get_status,
> },
> {
> @@ -2613,6 +2836,7 @@ static struct phy_driver marvell_drivers[] = {
> .get_tunable = m88e1540_get_tunable,
> .set_tunable = m88e1540_set_tunable,
> .cable_test_start = marvell_vct7_cable_test_start,
> + .cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
> .cable_test_get_status = marvell_vct7_cable_test_get_status,
> },
> {
> @@ -2658,6 +2882,7 @@ static struct phy_driver marvell_drivers[] = {
> .get_tunable = m88e1540_get_tunable,
> .set_tunable = m88e1540_set_tunable,
> .cable_test_start = marvell_vct7_cable_test_start,
> + .cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
> .cable_test_get_status = marvell_vct7_cable_test_get_status,
> },
> };
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 4/7] net: phy: marvell: Add support for amplitude graph
2020-05-17 20:25 ` Chris Healy
@ 2020-05-17 20:51 ` Andrew Lunn
2020-05-18 0:23 ` Chris Healy
2020-05-18 4:49 ` Michal Kubecek
0 siblings, 2 replies; 13+ messages in thread
From: Andrew Lunn @ 2020-05-17 20:51 UTC (permalink / raw)
To: Chris Healy
Cc: David Miller, netdev, Florian Fainelli, Heiner Kallweit, Michal Kubecek
> > +static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
> > + int meters)
> > +{
> > + int mV_pair0, mV_pair1, mV_pair2, mV_pair3;
> > + int distance;
> > + u16 reg;
> > + int err;
> > +
> > + distance = meters * 1000 / 805;
>
> With this integer based meters representation, it seems to me that we
> are artificially reducing the resolution of the distance sampling.
> For a 100 meter cable, the Marvell implementation looks to support 124
> sample points. This could result in incorrect data reporting as two
> adjacent meter numbers would resolve to the same disatance value
> entered into the register. (eg - 2 meters = 2 distance 3 meters = 2
> distance)
>
> Is there a better way of doing this which would allow for userspace to
> use the full resolution of the hardware?
Hi Chris
I don't see a simple solution to this.
PHYs/vendors seem to disagree about the ratio. Atheros use
824. Marvell use 805. I've no idea what Broadcom, aQuantia uses. We
would need to limit the choice of step to multiples of whatever the
vendor picks as its ratio. If the user picks a step of 2m, the driver
needs to return an error and say sorry, please try 2.488 meter steps
for Marvell, 2.427 meter steps on Atheros, and who knows what for
Broadcom. And when the user requests data just for 1-25 meters, the
driver needs to say sorry, try again with 0.824-24.62, or maybe
0.805-24.955. That is not a nice user experience.
It is easy for you to disable this conversion. Do you see a noticeable
difference in the quality of the results?
Andrew
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 4/7] net: phy: marvell: Add support for amplitude graph
2020-05-17 20:51 ` Andrew Lunn
@ 2020-05-18 0:23 ` Chris Healy
2020-05-18 4:49 ` Michal Kubecek
1 sibling, 0 replies; 13+ messages in thread
From: Chris Healy @ 2020-05-18 0:23 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, netdev, Florian Fainelli, Heiner Kallweit, Michal Kubecek
On Sun, May 17, 2020 at 1:51 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > +static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
> > > + int meters)
> > > +{
> > > + int mV_pair0, mV_pair1, mV_pair2, mV_pair3;
> > > + int distance;
> > > + u16 reg;
> > > + int err;
> > > +
> > > + distance = meters * 1000 / 805;
> >
> > With this integer based meters representation, it seems to me that we
> > are artificially reducing the resolution of the distance sampling.
> > For a 100 meter cable, the Marvell implementation looks to support 124
> > sample points. This could result in incorrect data reporting as two
> > adjacent meter numbers would resolve to the same disatance value
> > entered into the register. (eg - 2 meters = 2 distance 3 meters = 2
> > distance)
> >
> > Is there a better way of doing this which would allow for userspace to
> > use the full resolution of the hardware?
>
> Hi Chris
>
> I don't see a simple solution to this.
>
> PHYs/vendors seem to disagree about the ratio. Atheros use
> 824. Marvell use 805. I've no idea what Broadcom, aQuantia uses. We
> would need to limit the choice of step to multiples of whatever the
> vendor picks as its ratio. If the user picks a step of 2m, the driver
> needs to return an error and say sorry, please try 2.488 meter steps
> for Marvell, 2.427 meter steps on Atheros, and who knows what for
> Broadcom. And when the user requests data just for 1-25 meters, the
> driver needs to say sorry, try again with 0.824-24.62, or maybe
> 0.805-24.955. That is not a nice user experience.
>
> It is easy for you to disable this conversion. Do you see a noticeable
> difference in the quality of the results?
>
Could this be resolved by changing the interface slightly such that
the user specifies the range only (in meters or maybe centimetres) and
the driver figures out what to do with it and returns all the data
within that range in a centimetre format?
For example, if we were to provide a range of 10 to 20 meters with the
Marvell PHY, the driver would set the first offset to a register value
of 10/0.805 rounded down to the next lower integer value then
increment through the register values one by one until hitting
20/0.805 rounded up to the next integer value and return the results
for each of these 26 sets of values. With each of these 26 sets of
values, the distance could be returned in a centimetre format thus
providing the data requested in the resolution of the PHY.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 4/7] net: phy: marvell: Add support for amplitude graph
2020-05-17 20:51 ` Andrew Lunn
2020-05-18 0:23 ` Chris Healy
@ 2020-05-18 4:49 ` Michal Kubecek
1 sibling, 0 replies; 13+ messages in thread
From: Michal Kubecek @ 2020-05-18 4:49 UTC (permalink / raw)
To: netdev
Cc: Andrew Lunn, Chris Healy, David Miller, Florian Fainelli,
Heiner Kallweit
On Sun, May 17, 2020 at 10:51:50PM +0200, Andrew Lunn wrote:
> > > +static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
> > > + int meters)
> > > +{
> > > + int mV_pair0, mV_pair1, mV_pair2, mV_pair3;
> > > + int distance;
> > > + u16 reg;
> > > + int err;
> > > +
> > > + distance = meters * 1000 / 805;
> >
> > With this integer based meters representation, it seems to me that we
> > are artificially reducing the resolution of the distance sampling.
> > For a 100 meter cable, the Marvell implementation looks to support 124
> > sample points. This could result in incorrect data reporting as two
> > adjacent meter numbers would resolve to the same disatance value
> > entered into the register. (eg - 2 meters = 2 distance 3 meters = 2
> > distance)
> >
> > Is there a better way of doing this which would allow for userspace to
> > use the full resolution of the hardware?
>
> Hi Chris
>
> I don't see a simple solution to this.
>
> PHYs/vendors seem to disagree about the ratio. Atheros use
> 824. Marvell use 805. I've no idea what Broadcom, aQuantia uses. We
> would need to limit the choice of step to multiples of whatever the
> vendor picks as its ratio. If the user picks a step of 2m, the driver
> needs to return an error and say sorry, please try 2.488 meter steps
> for Marvell, 2.427 meter steps on Atheros, and who knows what for
> Broadcom. And when the user requests data just for 1-25 meters, the
> driver needs to say sorry, try again with 0.824-24.62, or maybe
> 0.805-24.955. That is not a nice user experience.
How about this?
- user would use meters as unit on ethtool command line but non-integer
values like "2.4" would be allowed
- request message would use e.g. cm (for consistency with existing cable
test results)
- driver would round requested values to closest supported (you actually
already round them)
- optionally, actual values used could be returned in request reply or
start notification
Michal
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 5/7] net: ethtool: Allow PHY cable test TDR data to configured
2020-05-17 19:58 [PATCH net-next 0/7] Raw PHY TDR data Andrew Lunn
` (3 preceding siblings ...)
2020-05-17 19:58 ` [PATCH net-next 4/7] net: phy: marvell: Add support for amplitude graph Andrew Lunn
@ 2020-05-17 19:58 ` Andrew Lunn
2020-05-17 19:58 ` [PATCH net-next 6/7] net : phy: marvell: Speedup TDR data retrieval by only changing page once Andrew Lunn
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2020-05-17 19:58 UTC (permalink / raw)
To: David Miller
Cc: netdev, Florian Fainelli, Heiner Kallweit, Chris Healy,
Michal Kubecek, Andrew Lunn
Allow the user to configure where on the cable the TDR data should be
retrieved, in terms of first and last sample, and the step between
samples. Also add the ability to ask for TDR data for just one pair.
If this configuration is not provided, it defaults to 0-150m at 1m
intervals for all pairs.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 49 ++++++++++++++------
drivers/net/phy/phy.c | 6 ++-
include/linux/phy.h | 10 +++--
include/uapi/linux/ethtool_netlink.h | 4 ++
net/ethtool/cabletest.c | 67 +++++++++++++++++++++++++++-
5 files changed, 115 insertions(+), 21 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index e7994f5b506e..f5d9a932db9a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -198,6 +198,7 @@
#define MII_VCT5_CTRL_PEEK_HYST_DEFAULT 3
#define MII_VCT5_SAMPLE_POINT_DISTANCE 0x18
+#define MII_VCT5_SAMPLE_POINT_DISTANCE_MAX 511
#define MII_VCT5_TX_PULSE_CTRL 0x1c
#define MII_VCT5_TX_PULSE_CTRL_DONT_WAIT_LINK_DOWN BIT(12)
#define MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_128nS (0x0 << 10)
@@ -270,6 +271,10 @@ struct marvell_priv {
char *hwmon_name;
struct device *hwmon_dev;
bool cable_test_tdr;
+ u32 first;
+ u32 last;
+ u32 step;
+ s8 pair;
};
static int marvell_read_page(struct phy_device *phydev)
@@ -1783,12 +1788,13 @@ static int marvell_vct5_amplitude(struct phy_device *phydev, int pair)
}
static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
- int meters)
+ int meters, int pair)
{
- int mV_pair0, mV_pair1, mV_pair2, mV_pair3;
int distance;
u16 reg;
int err;
+ int mV;
+ int i;
distance = meters * 1000 / 805;
@@ -1812,21 +1818,20 @@ static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
if (err)
return err;
- mV_pair0 = marvell_vct5_amplitude(phydev, 0);
- mV_pair1 = marvell_vct5_amplitude(phydev, 1);
- mV_pair2 = marvell_vct5_amplitude(phydev, 2);
- mV_pair3 = marvell_vct5_amplitude(phydev, 3);
+ for (i = 0; i < 4; i++) {
+ if (pair != PHY_PAIR_ALL && i != pair)
+ continue;
- ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_A, mV_pair0);
- ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_B, mV_pair1);
- ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_C, mV_pair2);
- ethnl_cable_test_amplitude(phydev, ETHTOOL_A_CABLE_PAIR_D, mV_pair3);
+ mV = marvell_vct5_amplitude(phydev, i);
+ ethnl_cable_test_amplitude(phydev, i, mV);
+ }
return 0;
}
static int marvell_vct5_amplitude_graph(struct phy_device *phydev)
{
+ struct marvell_priv *priv = phydev->priv;
int meters;
int err;
u16 reg;
@@ -1841,8 +1846,11 @@ static int marvell_vct5_amplitude_graph(struct phy_device *phydev)
if (err)
return err;
- for (meters = 0; meters <= 100; meters++) {
- err = marvell_vct5_amplitude_distance(phydev, meters);
+ for (meters = priv->first;
+ meters <= priv->last;
+ meters += priv->step) {
+ err = marvell_vct5_amplitude_distance(phydev, meters,
+ priv->pair);
if (err)
return err;
}
@@ -1916,11 +1924,19 @@ static int marvell_vct7_cable_test_start(struct phy_device *phydev)
MII_VCT7_CTRL_CENTIMETERS);
}
-static int marvell_vct5_cable_test_tdr_start(struct phy_device *phydev)
+static int marvell_vct5_cable_test_tdr_start(struct phy_device *phydev,
+ u32 first, u32 last, u32 step,
+ s8 pair)
{
struct marvell_priv *priv = phydev->priv;
int ret;
+ if (first > MII_VCT5_SAMPLE_POINT_DISTANCE_MAX)
+ return -EINVAL;
+
+ if (last > MII_VCT5_SAMPLE_POINT_DISTANCE_MAX)
+ return -EINVAL;
+
/* Disable VCT7 */
ret = phy_write_paged(phydev, MII_MARVELL_VCT7_PAGE,
MII_VCT7_CTRL, 0);
@@ -1932,11 +1948,16 @@ static int marvell_vct5_cable_test_tdr_start(struct phy_device *phydev)
return ret;
priv->cable_test_tdr = true;
+ priv->first = first;
+ priv->last = last;
+ priv->step = step;
+ priv->pair = pair;
+
ret = ethnl_cable_test_pulse(phydev, 1000);
if (ret)
return ret;
- return ethnl_cable_test_step(phydev, 0, 100, 1);
+ return ethnl_cable_test_step(phydev, first, last, step);
}
static int marvell_vct7_distance_to_length(int distance, bool meter)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1f794c1716cf..92f3f1d1ddaf 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -553,7 +553,8 @@ int phy_start_cable_test(struct phy_device *phydev,
EXPORT_SYMBOL(phy_start_cable_test);
int phy_start_cable_test_tdr(struct phy_device *phydev,
- struct netlink_ext_ack *extack)
+ struct netlink_ext_ack *extack,
+ u32 first, u32 last, u32 step, s8 pair)
{
struct net_device *dev = phydev->attached_dev;
int err = -ENOMEM;
@@ -590,7 +591,8 @@ int phy_start_cable_test_tdr(struct phy_device *phydev,
phy_link_down(phydev, true);
netif_testing_on(dev);
- err = phydev->drv->cable_test_tdr_start(phydev);
+ err = phydev->drv->cable_test_tdr_start(phydev, first, last, step,
+ pair);
if (err) {
netif_testing_off(dev);
phy_link_up(phydev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index c35d45fe74ce..c1a1fa8f33f5 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -706,8 +706,10 @@ struct phy_driver {
/* Start a cable test */
int (*cable_test_start)(struct phy_device *dev);
+#define PHY_PAIR_ALL -1
/* Start a raw TDR cable test */
- int (*cable_test_tdr_start)(struct phy_device *dev);
+ int (*cable_test_tdr_start)(struct phy_device *dev,
+ u32 first, u32 last, u32 step, s8 pair);
/* Once per second, or on interrupt, request the status of the
* test.
@@ -1260,7 +1262,8 @@ int phy_reset_after_clk_enable(struct phy_device *phydev);
int phy_start_cable_test(struct phy_device *phydev,
struct netlink_ext_ack *extack);
int phy_start_cable_test_tdr(struct phy_device *phydev,
- struct netlink_ext_ack *extack);
+ struct netlink_ext_ack *extack,
+ u32 start, u32 last, u32 step, s8 pair);
#else
static inline
int phy_start_cable_test(struct phy_device *phydev,
@@ -1271,7 +1274,8 @@ int phy_start_cable_test(struct phy_device *phydev,
}
static inline
int phy_start_cable_test_tdr(struct phy_device *phydev,
- struct netlink_ext_ack *extack)
+ struct netlink_ext_ack *extack,
+ u32 start, u32 last, u32 step, s8 pair)
{
NL_SET_ERR_MSG(extack, "Kernel not compiled with PHYLIB support");
return -EOPNOTSUPP;
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 4f223edcefda..e41b38567cbe 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -483,6 +483,10 @@ enum {
enum {
ETHTOOL_A_CABLE_TEST_TDR_UNSPEC,
ETHTOOL_A_CABLE_TEST_TDR_HEADER, /* nest - _A_HEADER_* */
+ ETHTOOL_A_CABLE_TEST_TDR_FIRST, /* u32 */
+ ETHTOOL_A_CABLE_TEST_TDR_LAST, /* u32 */
+ ETHTOOL_A_CABLE_TEST_TDR_STEP, /* u32 */
+ ETHTOOL_A_CABLE_TEST_TDR_PAIR, /* u8 */
/* add new constants above here */
__ETHTOOL_A_CABLE_TEST_TDR_CNT,
diff --git a/net/ethtool/cabletest.c b/net/ethtool/cabletest.c
index c02575e26336..e9b1c811ed41 100644
--- a/net/ethtool/cabletest.c
+++ b/net/ethtool/cabletest.c
@@ -5,7 +5,11 @@
#include "netlink.h"
#include "common.h"
-/* CABLE_TEST_ACT */
+/* 802.3 standard allows 100 meters for BaseT cables. However longer
+ * cables might work, depending on the quality of the cables and the
+ * PHY. So allow testing for up to 150 meters.
+ */
+#define MAX_CABLE_LENGTH 150
static const struct nla_policy
cable_test_act_policy[ETHTOOL_A_CABLE_TEST_MAX + 1] = {
@@ -203,18 +207,30 @@ int ethnl_cable_test_fault_length(struct phy_device *phydev, u8 pair, u32 cm)
}
EXPORT_SYMBOL_GPL(ethnl_cable_test_fault_length);
+struct cable_test_tdr_req_info {
+ struct ethnl_req_info base;
+};
+
static const struct nla_policy
cable_test_tdr_act_policy[ETHTOOL_A_CABLE_TEST_TDR_MAX + 1] = {
[ETHTOOL_A_CABLE_TEST_TDR_UNSPEC] = { .type = NLA_REJECT },
[ETHTOOL_A_CABLE_TEST_TDR_HEADER] = { .type = NLA_NESTED },
+ [ETHTOOL_A_CABLE_TEST_TDR_FIRST] = { .type = NLA_U32 },
+ [ETHTOOL_A_CABLE_TEST_TDR_LAST] = { .type = NLA_U32 },
+ [ETHTOOL_A_CABLE_TEST_TDR_STEP] = { .type = NLA_U32 },
+ [ETHTOOL_A_CABLE_TEST_TDR_PAIR] = { .type = NLA_U8 },
};
+/* CABLE_TEST_TDR_ACT */
+
int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info)
{
struct nlattr *tb[ETHTOOL_A_CABLE_TEST_TDR_MAX + 1];
struct ethnl_req_info req_info = {};
struct net_device *dev;
+ u32 first, last, step;
int ret;
+ u8 pair;
ret = nlmsg_parse(info->nlhdr, GENL_HDRLEN, tb,
ETHTOOL_A_CABLE_TEST_TDR_MAX,
@@ -235,12 +251,59 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info)
goto out_dev_put;
}
+ if (tb[ETHTOOL_A_CABLE_TEST_TDR_FIRST])
+ first = nla_get_u32(tb[ETHTOOL_A_CABLE_TEST_TDR_FIRST]);
+ else
+ first = 1;
+ if (tb[ETHTOOL_A_CABLE_TEST_TDR_LAST])
+ last = nla_get_u32(tb[ETHTOOL_A_CABLE_TEST_TDR_LAST]);
+ else
+ last = MAX_CABLE_LENGTH;
+
+ if (tb[ETHTOOL_A_CABLE_TEST_TDR_STEP])
+ step = nla_get_u32(tb[ETHTOOL_A_CABLE_TEST_TDR_STEP]);
+ else
+ step = 1;
+
+ if (tb[ETHTOOL_A_CABLE_TEST_TDR_PAIR]) {
+ pair = nla_get_u8(tb[ETHTOOL_A_CABLE_TEST_TDR_PAIR]);
+ if (pair < ETHTOOL_A_CABLE_PAIR_A ||
+ pair > ETHTOOL_A_CABLE_PAIR_D) {
+ NL_SET_ERR_MSG(info->extack,
+ "invalid pair parameter");
+ return -EINVAL;
+ }
+ } else {
+ pair = PHY_PAIR_ALL;
+ }
+
+ if (first > MAX_CABLE_LENGTH) {
+ NL_SET_ERR_MSG(info->extack, "invalid first parameter");
+ return -EINVAL;
+ }
+
+ if (last > MAX_CABLE_LENGTH) {
+ NL_SET_ERR_MSG(info->extack, "invalid last parameter");
+ return -EINVAL;
+ }
+
+ if (first > last) {
+ NL_SET_ERR_MSG(info->extack, "invalid first/last parameter");
+ return -EINVAL;
+ }
+
+ if (!step) {
+ NL_SET_ERR_MSG(info->extack, "invalid step parameter");
+ return -EINVAL;
+ }
+
rtnl_lock();
ret = ethnl_ops_begin(dev);
if (ret < 0)
goto out_rtnl;
- ret = phy_start_cable_test_tdr(dev->phydev, info->extack);
+ ret = phy_start_cable_test_tdr(dev->phydev, info->extack,
+ first, last, step, pair);
ethnl_ops_complete(dev);
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 6/7] net : phy: marvell: Speedup TDR data retrieval by only changing page once
2020-05-17 19:58 [PATCH net-next 0/7] Raw PHY TDR data Andrew Lunn
` (4 preceding siblings ...)
2020-05-17 19:58 ` [PATCH net-next 5/7] net: ethtool: Allow PHY cable test TDR data to configured Andrew Lunn
@ 2020-05-17 19:58 ` Andrew Lunn
2020-05-17 19:58 ` [PATCH net-next 7/7] net: phy: marvell: Configure TDR pulse based on measurement length Andrew Lunn
2020-05-18 16:38 ` [PATCH net-next 0/7] Raw PHY TDR data Jakub Kicinski
7 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2020-05-17 19:58 UTC (permalink / raw)
To: David Miller
Cc: netdev, Florian Fainelli, Heiner Kallweit, Chris Healy,
Michal Kubecek, Andrew Lunn
Getting the TDR data requires a large number of MDIO bus
transactions. The number can however be reduced if the page is only
changed once. Add the needed locking to allow this, and make use of
unlocked read/write methods where needed.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index f5d9a932db9a..5edf969978bd 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -1751,15 +1751,12 @@ static int marvell_vct5_wait_complete(struct phy_device *phydev)
int val;
for (i = 0; i < 32; i++) {
- val = phy_read_paged(phydev, MII_MARVELL_VCT5_PAGE,
- MII_VCT5_CTRL);
+ val = __phy_read(phydev, MII_VCT5_CTRL);
if (val < 0)
return val;
if (val & MII_VCT5_CTRL_COMPLETE)
return 0;
-
- usleep_range(1000, 2000);
}
phydev_err(phydev, "Timeout while waiting for cable test to finish\n");
@@ -1773,7 +1770,7 @@ static int marvell_vct5_amplitude(struct phy_device *phydev, int pair)
int reg;
reg = MII_VCT5_TX_RX_MDI0_COUPLING + pair;
- val = phy_read_paged(phydev, MII_MARVELL_VCT5_PAGE, reg);
+ val = __phy_read(phydev, reg);
if (val < 0)
return 0;
@@ -1798,9 +1795,8 @@ static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
distance = meters * 1000 / 805;
- err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
- MII_VCT5_SAMPLE_POINT_DISTANCE,
- distance);
+ err = __phy_write(phydev, MII_VCT5_SAMPLE_POINT_DISTANCE,
+ distance);
if (err)
return err;
@@ -1809,8 +1805,7 @@ static int marvell_vct5_amplitude_distance(struct phy_device *phydev,
MII_VCT5_CTRL_SAMPLES_DEFAULT |
MII_VCT5_CTRL_SAMPLE_POINT |
MII_VCT5_CTRL_PEEK_HYST_DEFAULT;
- err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
- MII_VCT5_CTRL, reg);
+ err = __phy_write(phydev, MII_VCT5_CTRL, reg);
if (err)
return err;
@@ -1833,6 +1828,7 @@ static int marvell_vct5_amplitude_graph(struct phy_device *phydev)
{
struct marvell_priv *priv = phydev->priv;
int meters;
+ int page;
int err;
u16 reg;
@@ -1846,16 +1842,27 @@ static int marvell_vct5_amplitude_graph(struct phy_device *phydev)
if (err)
return err;
+ /* Reading the TDR data is very MDIO heavy. We need to optimize
+ * access to keep the time to a minimum. So lock the bus once,
+ * and don't release it until complete. We can then avoid having
+ * to change the page for every access, greatly speeding things
+ * up.
+ */
+ page = phy_select_page(phydev, MII_MARVELL_VCT5_PAGE);
+ if (page < 0)
+ return page;
+
for (meters = priv->first;
meters <= priv->last;
meters += priv->step) {
err = marvell_vct5_amplitude_distance(phydev, meters,
priv->pair);
if (err)
- return err;
+ goto restore_page;
}
- return 0;
+restore_page:
+ return phy_restore_page(phydev, page, err);
}
static int marvell_cable_test_start_common(struct phy_device *phydev)
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 7/7] net: phy: marvell: Configure TDR pulse based on measurement length
2020-05-17 19:58 [PATCH net-next 0/7] Raw PHY TDR data Andrew Lunn
` (5 preceding siblings ...)
2020-05-17 19:58 ` [PATCH net-next 6/7] net : phy: marvell: Speedup TDR data retrieval by only changing page once Andrew Lunn
@ 2020-05-17 19:58 ` Andrew Lunn
2020-05-18 16:38 ` [PATCH net-next 0/7] Raw PHY TDR data Jakub Kicinski
7 siblings, 0 replies; 13+ messages in thread
From: Andrew Lunn @ 2020-05-17 19:58 UTC (permalink / raw)
To: David Miller
Cc: netdev, Florian Fainelli, Heiner Kallweit, Chris Healy,
Michal Kubecek, Andrew Lunn
When performing a TDR measurement for a short distance, the pulse
width should be low, to help differentiate between the outgoing pulse
and any reflection. For longer distances, the pulse should be wider,
to help with attenuation.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/marvell.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 5edf969978bd..7ec57b8a1aed 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -214,6 +214,11 @@
#define MII_VCT5_TX_PULSE_CTRL_MAX_AMP BIT(7)
#define MII_VCT5_TX_PULSE_CTRL_GT_140m_46_86mV (0x6 << 0)
+/* For TDR measurements less than 11 meters, a short pulse should be
+ * used.
+ */
+#define TDR_SHORT_CABLE_LENGTH 11
+
#define MII_VCT7_PAIR_0_DISTANCE 0x10
#define MII_VCT7_PAIR_1_DISTANCE 0x11
#define MII_VCT7_PAIR_2_DISTANCE 0x12
@@ -1828,14 +1833,19 @@ static int marvell_vct5_amplitude_graph(struct phy_device *phydev)
{
struct marvell_priv *priv = phydev->priv;
int meters;
+ u16 width;
int page;
int err;
u16 reg;
+ if (priv->first <= TDR_SHORT_CABLE_LENGTH)
+ width = MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_32nS;
+ else
+ width = MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_128nS;
+
reg = MII_VCT5_TX_PULSE_CTRL_GT_140m_46_86mV |
MII_VCT5_TX_PULSE_CTRL_DONT_WAIT_LINK_DOWN |
- MII_VCT5_TX_PULSE_CTRL_MAX_AMP |
- MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_32nS;
+ MII_VCT5_TX_PULSE_CTRL_MAX_AMP | width;
err = phy_write_paged(phydev, MII_MARVELL_VCT5_PAGE,
MII_VCT5_TX_PULSE_CTRL, reg);
@@ -1859,6 +1869,17 @@ static int marvell_vct5_amplitude_graph(struct phy_device *phydev)
priv->pair);
if (err)
goto restore_page;
+
+ if (meters > TDR_SHORT_CABLE_LENGTH &&
+ width == MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_32nS) {
+ width = MII_VCT5_TX_PULSE_CTRL_PULSE_WIDTH_128nS;
+ reg = MII_VCT5_TX_PULSE_CTRL_GT_140m_46_86mV |
+ MII_VCT5_TX_PULSE_CTRL_DONT_WAIT_LINK_DOWN |
+ MII_VCT5_TX_PULSE_CTRL_MAX_AMP | width;
+ err = __phy_write(phydev, MII_VCT5_TX_PULSE_CTRL, reg);
+ if (err)
+ goto restore_page;
+ }
}
restore_page:
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/7] Raw PHY TDR data
2020-05-17 19:58 [PATCH net-next 0/7] Raw PHY TDR data Andrew Lunn
` (6 preceding siblings ...)
2020-05-17 19:58 ` [PATCH net-next 7/7] net: phy: marvell: Configure TDR pulse based on measurement length Andrew Lunn
@ 2020-05-18 16:38 ` Jakub Kicinski
7 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2020-05-18 16:38 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, netdev, Florian Fainelli, Heiner Kallweit,
Chris Healy, Michal Kubecek
On Sun, 17 May 2020 21:58:44 +0200 Andrew Lunn wrote:
> Some ethernet PHYs allow access to raw TDR data in addition to summary
> diagnostics information. Add support for retrieving this data via
> netlink ethtool. The basic structure in the core is the same as for
> normal phy diagnostics, the PHY driver simply uses different helpers
> to fill the netlink message with different data.
>
> There is a graphical tool under development, as well a ethtool(1)
> which can dump the data as text and JSON.
>
> Thanks for Chris Healy for lots of testing.
Hm, my system can't build this with allmodconfig:
drivers/net/phy/nxp-tja11xx.c:195:37: error: not enough arguments for function ethnl_cable_test_alloc
drivers/net/phy/nxp-tja11xx.c: In function tja11xx_config_aneg_cable_test:
drivers/net/phy/nxp-tja11xx.c:195:8: error: too few arguments to function ethnl_cable_test_alloc
195 | ret = ethnl_cable_test_alloc(phydev);
| ^~~~~~~~~~~~~~~~~~~~~~
In file included from ../drivers/net/phy/nxp-tja11xx.c:8:
include/linux/ethtool_netlink.h:30:19: note: declared here
30 | static inline int ethnl_cable_test_alloc(struct phy_device *phydev, u8 cmd)
| ^~~~~~~~~~~~~~~~~~~~~~
make[4]: *** [drivers/net/phy/nxp-tja11xx.o] Error 1
make[3]: *** [drivers/net/phy] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [drivers/net] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [drivers] Error 2
make: *** [sub-make] Error 2
^ permalink raw reply [flat|nested] 13+ messages in thread