| Message ID | 20220401093909.3341836-2-horatiu.vultur@microchip.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ethtool: Extend to set PHY latencies | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next |
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
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
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; }
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(+)