[RFC PATCH net-next 0/2] ethtool: Extend to set PHY latencies

[RFC PATCH net-next 0/2] ethtool: Extend to set PHY latencies

[Horatiu Vultur]

Because there will always be a strugged to configure correct latencies, then
don't allow the kernel or the DT to have these values but allow the userspace
to configure them.
Therefore extend ethtool to allow to configure the latencies both per speed
and direction.

Horatiu Vultur (2):
  ethtool: Extend to allow to set PHY latencies
  net: phy: micrel: Implement set/get_tunable

 drivers/net/phy/micrel.c     | 93 ++++++++++++++++++++++++++++++++++++
 include/uapi/linux/ethtool.h |  6 +++
 net/ethtool/common.c         |  6 +++
 net/ethtool/ioctl.c          | 10 ++++
 4 files changed, 115 insertions(+)

-- 
2.33.0

[RFC PATCH net-next 1/2] ethtool: Extend to allow to set PHY latencies

[Horatiu Vultur]

Extend ethtool uapi to allow to configure the latencies for the PHY.
Allow to configure the latency per speed and per direction.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 include/uapi/linux/ethtool.h |  6 ++++++
 net/ethtool/common.c         |  6 ++++++
 net/ethtool/ioctl.c          | 10 ++++++++++
 3 files changed, 22 insertions(+)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 7bc4b8def12c..f120904a4e43 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -296,6 +296,12 @@ enum phy_tunable_id {
 	ETHTOOL_PHY_DOWNSHIFT,
 	ETHTOOL_PHY_FAST_LINK_DOWN,
 	ETHTOOL_PHY_EDPD,
+	ETHTOOL_PHY_LATENCY_RX_10MBIT,
+	ETHTOOL_PHY_LATENCY_TX_10MBIT,
+	ETHTOOL_PHY_LATENCY_RX_100MBIT,
+	ETHTOOL_PHY_LATENCY_TX_100MBIT,
+	ETHTOOL_PHY_LATENCY_RX_1000MBIT,
+	ETHTOOL_PHY_LATENCY_TX_1000MBIT,
 	/*
 	 * Add your fresh new phy tunable attribute above and remember to update
 	 * phy_tunable_strings[] in net/ethtool/common.c
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 0c5210015911..e0fec9eec047 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -98,6 +98,12 @@ phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
 	[ETHTOOL_PHY_DOWNSHIFT]	= "phy-downshift",
 	[ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down",
 	[ETHTOOL_PHY_EDPD]	= "phy-energy-detect-power-down",
+	[ETHTOOL_PHY_LATENCY_RX_10MBIT] = "phy-latency-rx-10mbit",
+	[ETHTOOL_PHY_LATENCY_TX_10MBIT] = "phy-latency-tx-10mbit",
+	[ETHTOOL_PHY_LATENCY_RX_100MBIT] = "phy-latency-rx-100mbit",
+	[ETHTOOL_PHY_LATENCY_TX_100MBIT] = "phy-lantecy-tx-100mbit",
+	[ETHTOOL_PHY_LATENCY_RX_1000MBIT] = "phy-latency-rx-1000mbit",
+	[ETHTOOL_PHY_LATENCY_TX_1000MBIT] = "phy-latency-tx-1000mbit",
 };
 
 #define __LINK_MODE_NAME(speed, type, duplex) \
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 326e14ee05db..a1caee4ef5b9 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2605,6 +2605,16 @@ static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
 		    tuna->type_id != ETHTOOL_TUNABLE_U16)
 			return -EINVAL;
 		break;
+	case ETHTOOL_PHY_LATENCY_RX_10MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_10MBIT:
+	case ETHTOOL_PHY_LATENCY_RX_100MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_100MBIT:
+	case ETHTOOL_PHY_LATENCY_RX_1000MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_1000MBIT:
+		if (tuna->len != sizeof(s32) ||
+		    tuna->type_id != ETHTOOL_TUNABLE_S32)
+			return -EINVAL;
+		break;
 	default:
 		return -EINVAL;
 	}
-- 
2.33.0

[RFC PATCH net-next 2/2] net: phy: micrel: Implement set/get_tunable

[Horatiu Vultur]

Implement set/get_tunable to set/get the PHY latencies.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/micrel.c | 93 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index fc53b71dc872..f537e61cb61d 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -99,6 +99,15 @@
 #define PTP_TIMESTAMP_EN_PDREQ_			BIT(2)
 #define PTP_TIMESTAMP_EN_PDRES_			BIT(3)
 
+#define PTP_RX_LATENCY_1000			0x0224
+#define PTP_TX_LATENCY_1000			0x0225
+
+#define PTP_RX_LATENCY_100			0x0222
+#define PTP_TX_LATENCY_100			0x0223
+
+#define PTP_RX_LATENCY_10			0x0220
+#define PTP_TX_LATENCY_10			0x0221
+
 #define PTP_TX_PARSE_L2_ADDR_EN			0x0284
 #define PTP_RX_PARSE_L2_ADDR_EN			0x0244
 
@@ -2591,6 +2600,88 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
 	return 0;
 }
 
+static void lan8814_get_latency(struct phy_device *phydev, u32 id, s32 *latency)
+{
+	switch (id) {
+	case ETHTOOL_PHY_LATENCY_RX_10MBIT:
+		*latency = lanphy_read_page_reg(phydev, 5, PTP_RX_LATENCY_10);
+		break;
+	case ETHTOOL_PHY_LATENCY_TX_10MBIT:
+		*latency = lanphy_read_page_reg(phydev, 5, PTP_TX_LATENCY_10);
+		break;
+	case ETHTOOL_PHY_LATENCY_RX_100MBIT:
+		*latency = lanphy_read_page_reg(phydev, 5, PTP_RX_LATENCY_100);
+		break;
+	case ETHTOOL_PHY_LATENCY_TX_100MBIT:
+		*latency = lanphy_read_page_reg(phydev, 5, PTP_TX_LATENCY_100);
+		break;
+	case ETHTOOL_PHY_LATENCY_RX_1000MBIT:
+		*latency = lanphy_read_page_reg(phydev, 5, PTP_RX_LATENCY_1000);
+		break;
+	case ETHTOOL_PHY_LATENCY_TX_1000MBIT:
+		*latency = lanphy_read_page_reg(phydev, 5, PTP_TX_LATENCY_1000);
+		break;
+	}
+}
+
+static int lan8814_get_tunable(struct phy_device *phydev,
+			       struct ethtool_tunable *tuna, void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_LATENCY_RX_10MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_10MBIT:
+	case ETHTOOL_PHY_LATENCY_RX_100MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_100MBIT:
+	case ETHTOOL_PHY_LATENCY_RX_1000MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_1000MBIT:
+		lan8814_get_latency(phydev, tuna->id, data);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static void lan8814_set_latency(struct phy_device *phydev, u32 id, s32 latency)
+{
+	switch (id) {
+	case ETHTOOL_PHY_LATENCY_RX_10MBIT:
+		lanphy_write_page_reg(phydev, 5, PTP_RX_LATENCY_10, latency);
+		break;
+	case ETHTOOL_PHY_LATENCY_TX_10MBIT:
+		lanphy_write_page_reg(phydev, 5, PTP_TX_LATENCY_10, latency);
+		break;
+	case ETHTOOL_PHY_LATENCY_RX_100MBIT:
+		lanphy_write_page_reg(phydev, 5, PTP_RX_LATENCY_100, latency);
+		break;
+	case ETHTOOL_PHY_LATENCY_TX_100MBIT:
+		lanphy_write_page_reg(phydev, 5, PTP_TX_LATENCY_100, latency);
+		break;
+	case ETHTOOL_PHY_LATENCY_RX_1000MBIT:
+		lanphy_write_page_reg(phydev, 5, PTP_RX_LATENCY_1000, latency);
+		break;
+	case ETHTOOL_PHY_LATENCY_TX_1000MBIT:
+		lanphy_write_page_reg(phydev, 5, PTP_TX_LATENCY_1000, latency);
+		break;
+	}
+}
+
+static int lan8814_set_tunable(struct phy_device *phydev,
+			       struct ethtool_tunable *tuna, const void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_LATENCY_RX_10MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_10MBIT:
+	case ETHTOOL_PHY_LATENCY_RX_100MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_100MBIT:
+	case ETHTOOL_PHY_LATENCY_RX_1000MBIT:
+	case ETHTOOL_PHY_LATENCY_TX_1000MBIT:
+		lan8814_set_latency(phydev, tuna->id, *(const s32 *)data);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int lan8814_config_init(struct phy_device *phydev)
 {
 	int val;
@@ -2827,6 +2918,8 @@ static struct phy_driver ksphy_driver[] = {
 	.probe		= lan8814_probe,
 	.soft_reset	= genphy_soft_reset,
 	.read_status	= ksz9031_read_status,
+	.get_tunable	= lan8814_get_tunable,
+	.set_tunable	= lan8814_set_tunable,
 	.get_sset_count	= kszphy_get_sset_count,
 	.get_strings	= kszphy_get_strings,
 	.get_stats	= kszphy_get_stats,
-- 
2.33.0

Re: [RFC PATCH net-next 1/2] ethtool: Extend to allow to set PHY latencies

[Andrew Lunn]

On Fri, Apr 01, 2022 at 11:39:08AM +0200, Horatiu Vultur wrote:
> Extend ethtool uapi to allow to configure the latencies for the PHY.
> Allow to configure the latency per speed and per direction.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  include/uapi/linux/ethtool.h |  6 ++++++
>  net/ethtool/common.c         |  6 ++++++
>  net/ethtool/ioctl.c          | 10 ++++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 7bc4b8def12c..f120904a4e43 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -296,6 +296,12 @@ enum phy_tunable_id {
>  	ETHTOOL_PHY_DOWNSHIFT,
>  	ETHTOOL_PHY_FAST_LINK_DOWN,
>  	ETHTOOL_PHY_EDPD,
> +	ETHTOOL_PHY_LATENCY_RX_10MBIT,
> +	ETHTOOL_PHY_LATENCY_TX_10MBIT,
> +	ETHTOOL_PHY_LATENCY_RX_100MBIT,
> +	ETHTOOL_PHY_LATENCY_TX_100MBIT,
> +	ETHTOOL_PHY_LATENCY_RX_1000MBIT,
> +	ETHTOOL_PHY_LATENCY_TX_1000MBIT,

How does this scale with 2.5G, 5G, 10G, 14G, 40G, etc.

Could half duplex differ to full duplex? What about 1000BaseT vs
1000BaseT1 and 1000BaseT2? The Aquantia/Marvell PHY can do both
1000BaseT and 1000BaseT2 and will downshift from 4 pairs to 2 pairs if
you have the correct magic in its firmware blobs.

A more generic API would pass a link mode, a direction and a
latency. The driver can then return -EOPNOTSUPP for a mode it does not
support.

	Andrew

Re: [RFC PATCH net-next 1/2] ethtool: Extend to allow to set PHY latencies

[Horatiu Vultur]

The 04/01/2022 14:36, Andrew Lunn wrote:
> 
> On Fri, Apr 01, 2022 at 11:39:08AM +0200, Horatiu Vultur wrote:
> > Extend ethtool uapi to allow to configure the latencies for the PHY.
> > Allow to configure the latency per speed and per direction.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  include/uapi/linux/ethtool.h |  6 ++++++
> >  net/ethtool/common.c         |  6 ++++++
> >  net/ethtool/ioctl.c          | 10 ++++++++++
> >  3 files changed, 22 insertions(+)
> >
> > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > index 7bc4b8def12c..f120904a4e43 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -296,6 +296,12 @@ enum phy_tunable_id {
> >       ETHTOOL_PHY_DOWNSHIFT,
> >       ETHTOOL_PHY_FAST_LINK_DOWN,
> >       ETHTOOL_PHY_EDPD,
> > +     ETHTOOL_PHY_LATENCY_RX_10MBIT,
> > +     ETHTOOL_PHY_LATENCY_TX_10MBIT,
> > +     ETHTOOL_PHY_LATENCY_RX_100MBIT,
> > +     ETHTOOL_PHY_LATENCY_TX_100MBIT,
> > +     ETHTOOL_PHY_LATENCY_RX_1000MBIT,
> > +     ETHTOOL_PHY_LATENCY_TX_1000MBIT,
> 
> How does this scale with 2.5G, 5G, 10G, 14G, 40G, etc.
> 
> Could half duplex differ to full duplex? What about 1000BaseT vs
> 1000BaseT1 and 1000BaseT2? The Aquantia/Marvell PHY can do both
> 1000BaseT and 1000BaseT2 and will downshift from 4 pairs to 2 pairs if
> you have the correct magic in its firmware blobs.
> 
> A more generic API would pass a link mode, a direction and a
> latency. The driver can then return -EOPNOTSUPP for a mode it does not
> support.

Yes, I can see your point, the proposed solution is not scalable.
I will try implement something like you suggested.

> 
>         Andrew

-- 
/Horatiu