2022-09-06 17:13:26

by Sean Anderson

[permalink] [raw]
Subject: [PATCH net-next v5 0/8] net: phy: Add support for rate adaptation

This adds support for phy rate adaptation: when a phy adapts between
differing phy interface and link speeds. It was originally submitted as
part of [1], which is considered "v1" of this series.

Several past discussions [2-4] around adding rate adaptation provide
some context.

Although in earlier versions of this series, userspace could disable
rate adaptation, now it is only possible to determine the current rate
adaptation type. Disabling or otherwise configuring rate adaptation has
been left for future work. However, because currently only
RATE_ADAPT_PAUSE is implemented, it is possible to disable rate
adaptation by modifying the advertisement appropriately.

[1] https://lore.kernel.org/netdev/[email protected]/T/#t
[2] https://lore.kernel.org/netdev/[email protected]/
[3] https://lore.kernel.org/netdev/[email protected]/
[4] https://lore.kernel.org/netdev/[email protected]/

Changes in v5:
- Document phy_rate_adaptation_to_str
- Remove unnecessary comma
- Move phylink_cap_from_speed_duplex to this commit
- Drop patch "Add some helpers for working with mac caps"; it has been
incorperated into the autonegotiation patch.
- Break off patch "net: phy: Add 1000BASE-KX interface mode" for
separate submission.
- Rebase onto net-next/master

Changes in v4:
- Export phy_rate_adaptation_to_str
- Remove phylink_interface_max_speed, which was accidentally added
- Split off the LS1046ARDB 1G fix

Changes in v3:
- Document MAC_(A)SYM_PAUSE
- Modify link settings directly in phylink_link_up, instead of doing
things more indirectly via link_*.
- Add phylink_cap_from_speed_duplex to look up the mac capability
corresponding to the interface's speed.
- Include RATE_ADAPT_CRS; it's a few lines and it doesn't hurt.
- Move unused defines to next commit (where they will be used)
- Remove "Support differing link/interface speed/duplex". It has been
rendered unnecessary due to simplification of the rate adaptation
patches. Thanks Russell!
- Rewrite cover letter to better reflect the opinions of the developers
involved

Changes in v2:
- Use int/defines instead of enum to allow for use in ioctls/netlink
- Add locking to phy_get_rate_adaptation
- Add (read-only) ethtool support for rate adaptation
- Move part of commit message to cover letter, as it gives a good
overview of the whole series, and allows this patch to focus more on
the specifics.
- Use the phy's rate adaptation setting to determine whether to use its
link speed/duplex or the MAC's speed/duplex with MLO_AN_INBAND.
- Always use the rate adaptation setting to determine the interface
speed/duplex (instead of sometimes using the interface mode).
- Determine the interface speed and max mac speed directly instead of
guessing based on the caps.
- Add comments clarifying the register defines
- Reorder variables in aqr107_read_rate

Sean Anderson (8):
net: phylink: Document MAC_(A)SYM_PAUSE
net: phylink: Export phylink_caps_to_linkmodes
net: phylink: Generate caps and convert to linkmodes separately
net: phy: Add support for rate adaptation
net: phylink: Adjust link settings based on rate adaptation
net: phylink: Adjust advertisement based on rate adaptation
net: phy: aquantia: Add some additional phy interfaces
net: phy: aquantia: Add support for rate adaptation

Documentation/networking/ethtool-netlink.rst | 2 +
drivers/net/phy/aquantia_main.c | 68 ++++-
drivers/net/phy/phy-core.c | 21 ++
drivers/net/phy/phy.c | 28 ++
drivers/net/phy/phylink.c | 270 +++++++++++++++++--
include/linux/phy.h | 22 +-
include/linux/phylink.h | 27 +-
include/uapi/linux/ethtool.h | 18 +-
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/ioctl.c | 1 +
net/ethtool/linkmodes.c | 5 +
11 files changed, 429 insertions(+), 34 deletions(-)

--
2.35.1.1320.gc452695387.dirty


2022-09-06 17:14:28

by Sean Anderson

[permalink] [raw]
Subject: [PATCH net-next v5 4/8] net: phy: Add support for rate adaptation

This adds support for rate adaptation to the phy subsystem. The general
idea is that the phy interface runs at one speed, and the MAC throttles
the rate at which it sends packets to the link speed. There's a good
overview of several techniques for achieving this at [1]. This patch
adds support for three: pause-frame based (such as in Aquantia phys),
CRS-based (such as in 10PASS-TS and 2BASE-TL), and open-loop-based (such
as in 10GBASE-W).

This patch makes a few assumptions and a few non assumptions about the
types of rate adaptation available. First, it assumes that different phys
may use different forms of rate adaptation. Second, it assumes that phys
can use rate adaptation for any of their supported link speeds (e.g. if a
phy supports 10BASE-T and XGMII, then it can adapt XGMII to 10BASE-T).
Third, it does not assume that all interface modes will use the same form
of rate adaptation. Fourth, it does not assume that all phy devices will
support rate adaptation (even some do). Relaxing or strengthening these
(non-)assumptions could result in a different API. For example, if all
interface modes were assumed to use the same form of rate adaptation, then
a bitmask of interface modes supportting rate adaptation would suffice.

For some better visibility into the process, the current rate adaptation
mode is exposed as part of the ethtool ksettings. For the moment, only
read access is supported. I'm not sure what userspace might want to
configure yet (disable it altogether, disable just one mode, specify the
mode to use, etc.). For the moment, since only pause-based rate
adaptation support is added in the next few commits, rate adaptation can
be disabled altogether by adjusting the advertisement.

Signed-off-by: Sean Anderson <[email protected]>
---
Should the unimplemented adaptation modes be kept in?

Changes in v5:
- Document phy_rate_adaptation_to_str
- Remove unnecessary comma

Changes in v4:
- Export phy_rate_adaptation_to_str

Changes in v2:
- Use int/defines instead of enum to allow for use in ioctls/netlink
- Add locking to phy_get_rate_adaptation
- Add (read-only) ethtool support for rate adaptation
- Move part of commit message to cover letter, as it gives a good
overview of the whole series, and allows this patch to focus more on
the specifics.

Documentation/networking/ethtool-netlink.rst | 2 ++
drivers/net/phy/phy-core.c | 21 +++++++++++++++
drivers/net/phy/phy.c | 28 ++++++++++++++++++++
include/linux/phy.h | 22 ++++++++++++++-
include/uapi/linux/ethtool.h | 18 +++++++++++--
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/ioctl.c | 1 +
net/ethtool/linkmodes.c | 5 ++++
8 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index dbca3e9ec782..65ed29e78499 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -426,6 +426,7 @@ Kernel response contents:
``ETHTOOL_A_LINKMODES_DUPLEX`` u8 duplex mode
``ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG`` u8 Master/slave port mode
``ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE`` u8 Master/slave port state
+ ``ETHTOOL_A_LINKMODES_RATE_ADAPTATION`` u8 PHY rate adaptation
========================================== ====== ==========================

For ``ETHTOOL_A_LINKMODES_OURS``, value represents advertised modes and mask
@@ -449,6 +450,7 @@ Request contents:
``ETHTOOL_A_LINKMODES_SPEED`` u32 link speed (Mb/s)
``ETHTOOL_A_LINKMODES_DUPLEX`` u8 duplex mode
``ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG`` u8 Master/slave port mode
+ ``ETHTOOL_A_LINKMODES_RATE_ADAPTATION`` u8 PHY rate adaptation
``ETHTOOL_A_LINKMODES_LANES`` u32 lanes
========================================== ====== ==========================

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 2a2924bc8f76..b1c9c40fe378 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -74,6 +74,27 @@ const char *phy_duplex_to_str(unsigned int duplex)
}
EXPORT_SYMBOL_GPL(phy_duplex_to_str);

+/**
+ * phy_rate_adaptation_to_str - Return a string describing the rate adaptation
+ *
+ * @rate_adaptation: Type of rate adaptation to describe
+ */
+const char *phy_rate_adaptation_to_str(int rate_adaptation)
+{
+ switch (rate_adaptation) {
+ case RATE_ADAPT_NONE:
+ return "none";
+ case RATE_ADAPT_PAUSE:
+ return "pause";
+ case RATE_ADAPT_CRS:
+ return "crs";
+ case RATE_ADAPT_OPEN_LOOP:
+ return "open-loop";
+ }
+ return "Unsupported (update phy-core.c)";
+}
+EXPORT_SYMBOL_GPL(phy_rate_adaptation_to_str);
+
/**
* phy_interface_num_ports - Return the number of links that can be carried by
* a given MAC-PHY physical link. Returns 0 if this is
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 8d3ee3a6495b..77cbf07852e6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -114,6 +114,33 @@ void phy_print_status(struct phy_device *phydev)
}
EXPORT_SYMBOL(phy_print_status);

+/**
+ * phy_get_rate_adaptation - determine if rate adaptation is supported
+ * @phydev: The phy device to return rate adaptation for
+ * @iface: The interface mode to use
+ *
+ * This determines the type of rate adaptation (if any) that @phy supports
+ * using @iface. @iface may be %PHY_INTERFACE_MODE_NA to determine if any
+ * interface supports rate adaptation.
+ *
+ * Return: The type of rate adaptation @phy supports for @iface, or
+ * %RATE_ADAPT_NONE.
+ */
+int phy_get_rate_adaptation(struct phy_device *phydev,
+ phy_interface_t iface)
+{
+ int ret = RATE_ADAPT_NONE;
+
+ if (phydev->drv->get_rate_adaptation) {
+ mutex_lock(&phydev->lock);
+ ret = phydev->drv->get_rate_adaptation(phydev, iface);
+ mutex_unlock(&phydev->lock);
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_get_rate_adaptation);
+
/**
* phy_config_interrupt - configure the PHY device for the requested interrupts
* @phydev: the phy_device struct
@@ -256,6 +283,7 @@ void phy_ethtool_ksettings_get(struct phy_device *phydev,
cmd->base.duplex = phydev->duplex;
cmd->base.master_slave_cfg = phydev->master_slave_get;
cmd->base.master_slave_state = phydev->master_slave_state;
+ cmd->base.rate_adaptation = phydev->rate_adaptation;
if (phydev->interface == PHY_INTERFACE_MODE_MOCA)
cmd->base.port = PORT_BNC;
else
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 337230c135f7..3e784137b2f3 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -280,7 +280,6 @@ static inline const char *phy_modes(phy_interface_t interface)
}
}

-
#define PHY_INIT_TIMEOUT 100000
#define PHY_FORCE_TIMEOUT 10

@@ -574,6 +573,7 @@ struct macsec_ops;
* @lp_advertising: Current link partner advertised linkmodes
* @eee_broken_modes: Energy efficient ethernet modes which should be prohibited
* @autoneg: Flag autoneg being used
+ * @rate_adaptation: Current rate adaptation mode
* @link: Current link state
* @autoneg_complete: Flag auto negotiation of the link has completed
* @mdix: Current crossover
@@ -641,6 +641,8 @@ struct phy_device {
unsigned irq_suspended:1;
unsigned irq_rerun:1;

+ int rate_adaptation;
+
enum phy_state state;

u32 dev_flags;
@@ -805,6 +807,21 @@ struct phy_driver {
*/
int (*get_features)(struct phy_device *phydev);

+ /**
+ * @get_rate_adaptation: Get the supported type of rate adaptation for a
+ * particular phy interface. This is used by phy consumers to determine
+ * whether to advertise lower-speed modes for that interface. It is
+ * assumed that if a rate adaptation mode is supported on an interface,
+ * then that interface's rate can be adapted to all slower link speeds
+ * supported by the phy. If iface is %PHY_INTERFACE_MODE_NA, and the phy
+ * supports any kind of rate adaptation for any interface, then it must
+ * return that rate adaptation mode (preferring %RATE_ADAPT_PAUSE to
+ * %RATE_ADAPT_CRS). If the interface is not supported, this should
+ * return %RATE_ADAPT_NONE.
+ */
+ int (*get_rate_adaptation)(struct phy_device *phydev,
+ phy_interface_t iface);
+
/* PHY Power Management */
/** @suspend: Suspend the hardware, saving state if needed */
int (*suspend)(struct phy_device *phydev);
@@ -971,6 +988,7 @@ struct phy_fixup {

const char *phy_speed_to_str(int speed);
const char *phy_duplex_to_str(unsigned int duplex);
+const char *phy_rate_adaptation_to_str(int rate_adaptation);

int phy_interface_num_ports(phy_interface_t interface);

@@ -1687,6 +1705,8 @@ int phy_disable_interrupts(struct phy_device *phydev);
void phy_request_interrupt(struct phy_device *phydev);
void phy_free_interrupt(struct phy_device *phydev);
void phy_print_status(struct phy_device *phydev);
+int phy_get_rate_adaptation(struct phy_device *phydev,
+ phy_interface_t iface);
void phy_set_max_speed(struct phy_device *phydev, u32 max_speed);
void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode);
void phy_advertise_supported(struct phy_device *phydev);
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 2d5741fd44bb..49496acbeac9 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1840,6 +1840,20 @@ static inline int ethtool_validate_duplex(__u8 duplex)
#define MASTER_SLAVE_STATE_SLAVE 3
#define MASTER_SLAVE_STATE_ERR 4

+/* These are used to throttle the rate of data on the phy interface when the
+ * native speed of the interface is higher than the link speed. These should
+ * not be used for phy interfaces which natively support multiple speeds (e.g.
+ * MII or SGMII).
+ */
+/* No rate adaptation performed. */
+#define RATE_ADAPT_NONE 0
+/* The phy sends pause frames to throttle the MAC. */
+#define RATE_ADAPT_PAUSE 1
+/* The phy asserts CRS to prevent the MAC from transmitting. */
+#define RATE_ADAPT_CRS 2
+/* The MAC is programmed with a sufficiently-large IPG. */
+#define RATE_ADAPT_OPEN_LOOP 3
+
/* Which connector port. */
#define PORT_TP 0x00
#define PORT_AUI 0x01
@@ -2033,8 +2047,8 @@ enum ethtool_reset_flags {
* reported consistently by PHYLIB. Read-only.
* @master_slave_cfg: Master/slave port mode.
* @master_slave_state: Master/slave port state.
+ * @rate_adaptation: Rate adaptation performed by the PHY
* @reserved: Reserved for future use; see the note on reserved space.
- * @reserved1: Reserved for future use; see the note on reserved space.
* @link_mode_masks: Variable length bitmaps.
*
* If autonegotiation is disabled, the speed and @duplex represent the
@@ -2085,7 +2099,7 @@ struct ethtool_link_settings {
__u8 transceiver;
__u8 master_slave_cfg;
__u8 master_slave_state;
- __u8 reserved1[1];
+ __u8 rate_adaptation;
__u32 reserved[7];
__u32 link_mode_masks[];
/* layout of link_mode_masks fields:
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index d2fb4f7be61b..3a5d81769ff4 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -242,6 +242,7 @@ enum {
ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG, /* u8 */
ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE, /* u8 */
ETHTOOL_A_LINKMODES_LANES, /* u32 */
+ ETHTOOL_A_LINKMODES_RATE_ADAPTATION, /* u8 */

/* add new constants above here */
__ETHTOOL_A_LINKMODES_CNT,
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 9298eb3251cb..6166f3e6b8d5 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -571,6 +571,7 @@ static int ethtool_get_link_ksettings(struct net_device *dev,
= __ETHTOOL_LINK_MODE_MASK_NU32;
link_ksettings.base.master_slave_cfg = MASTER_SLAVE_CFG_UNSUPPORTED;
link_ksettings.base.master_slave_state = MASTER_SLAVE_STATE_UNSUPPORTED;
+ link_ksettings.base.rate_adaptation = RATE_ADAPT_NONE;

return store_link_ksettings_for_user(useraddr, &link_ksettings);
}
diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index 99b29b4fe947..7905bd985c7f 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -70,6 +70,7 @@ static int linkmodes_reply_size(const struct ethnl_req_info *req_base,
+ nla_total_size(sizeof(u32)) /* LINKMODES_SPEED */
+ nla_total_size(sizeof(u32)) /* LINKMODES_LANES */
+ nla_total_size(sizeof(u8)) /* LINKMODES_DUPLEX */
+ + nla_total_size(sizeof(u8)) /* LINKMODES_RATE_ADAPTATION */
+ 0;
ret = ethnl_bitset_size(ksettings->link_modes.advertising,
ksettings->link_modes.supported,
@@ -143,6 +144,10 @@ static int linkmodes_fill_reply(struct sk_buff *skb,
lsettings->master_slave_state))
return -EMSGSIZE;

+ if (nla_put_u8(skb, ETHTOOL_A_LINKMODES_RATE_ADAPTATION,
+ lsettings->rate_adaptation))
+ return -EMSGSIZE;
+
return 0;
}

--
2.35.1.1320.gc452695387.dirty

2022-09-06 17:17:07

by Sean Anderson

[permalink] [raw]
Subject: [PATCH net-next v5 3/8] net: phylink: Generate caps and convert to linkmodes separately

If we call phylink_caps_to_linkmodes directly from
phylink_get_linkmodes, it is difficult to re-use this functionality in
MAC drivers. This is because MAC drivers must then work with an ethtool
linkmode bitmap, instead of with mac capabilities. Instead, let the
caller of phylink_get_linkmodes do the conversion. To reflect this
change, rename the function to phylink_get_capabilities.

Signed-off-by: Sean Anderson <[email protected]>
---

(no changes since v1)

drivers/net/phy/phylink.c | 21 +++++++++++----------
include/linux/phylink.h | 4 ++--
2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index c5c3f0b62d7f..1f022e5d01ba 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -305,17 +305,15 @@ void phylink_caps_to_linkmodes(unsigned long *linkmodes, unsigned long caps)
EXPORT_SYMBOL_GPL(phylink_caps_to_linkmodes);

/**
- * phylink_get_linkmodes() - get acceptable link modes
- * @linkmodes: ethtool linkmode mask (must be already initialised)
+ * phylink_get_capabilities() - get capabilities for a given MAC
* @interface: phy interface mode defined by &typedef phy_interface_t
* @mac_capabilities: bitmask of MAC capabilities
*
- * Set all possible pause, speed and duplex linkmodes in @linkmodes that
- * are supported by the @interface mode and @mac_capabilities. @linkmodes
- * must have been initialised previously.
+ * Get the MAC capabilities that are supported by the @interface mode and
+ * @mac_capabilities.
*/
-void phylink_get_linkmodes(unsigned long *linkmodes, phy_interface_t interface,
- unsigned long mac_capabilities)
+unsigned long phylink_get_capabilities(phy_interface_t interface,
+ unsigned long mac_capabilities)
{
unsigned long caps = MAC_SYM_PAUSE | MAC_ASYM_PAUSE;

@@ -391,9 +389,9 @@ void phylink_get_linkmodes(unsigned long *linkmodes, phy_interface_t interface,
break;
}

- phylink_caps_to_linkmodes(linkmodes, caps & mac_capabilities);
+ return caps & mac_capabilities;
}
-EXPORT_SYMBOL_GPL(phylink_get_linkmodes);
+EXPORT_SYMBOL_GPL(phylink_get_capabilities);

/**
* phylink_generic_validate() - generic validate() callback implementation
@@ -409,11 +407,14 @@ void phylink_generic_validate(struct phylink_config *config,
unsigned long *supported,
struct phylink_link_state *state)
{
+ unsigned long caps;
__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };

phylink_set_port_modes(mask);
phylink_set(mask, Autoneg);
- phylink_get_linkmodes(mask, state->interface, config->mac_capabilities);
+ caps = phylink_get_capabilities(state->interface,
+ config->mac_capabilities);
+ phylink_caps_to_linkmodes(mask, caps);

linkmode_and(supported, supported, mask);
linkmode_and(state->advertising, state->advertising, mask);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 9bb088e0ef3e..c2aa49c692a0 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -535,8 +535,8 @@ void pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
#endif

void phylink_caps_to_linkmodes(unsigned long *linkmodes, unsigned long caps);
-void phylink_get_linkmodes(unsigned long *linkmodes, phy_interface_t interface,
- unsigned long mac_capabilities);
+unsigned long phylink_get_capabilities(phy_interface_t interface,
+ unsigned long mac_capabilities);
void phylink_generic_validate(struct phylink_config *config,
unsigned long *supported,
struct phylink_link_state *state);
--
2.35.1.1320.gc452695387.dirty

2022-09-06 17:18:05

by Sean Anderson

[permalink] [raw]
Subject: [PATCH net-next v5 6/8] net: phylink: Adjust advertisement based on rate adaptation

This adds support for adjusting the advertisement for pause-based rate
adaptation. This may result in a lossy link, since the final link settings
are not adjusted. Asymmetric pause support is necessary. It would be
possible for a MAC supporting only symmetric pause to use pause-based rate
adaptation, but only if pause reception was enabled as well.

Signed-off-by: Sean Anderson <[email protected]>
---

Changes in v5:
- Move phylink_cap_from_speed_duplex to this commit

Changes in v3:
- Add phylink_cap_from_speed_duplex to look up the mac capability
corresponding to the interface's speed.
- Include RATE_ADAPT_CRS; it's a few lines and it doesn't hurt.

Changes in v2:
- Determine the interface speed and max mac speed directly instead of
guessing based on the caps.

drivers/net/phy/phylink.c | 106 ++++++++++++++++++++++++++++++++++++--
include/linux/phylink.h | 3 +-
2 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 8ce110c7be5c..4d73a58f4854 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -373,18 +373,70 @@ void phylink_caps_to_linkmodes(unsigned long *linkmodes, unsigned long caps)
}
EXPORT_SYMBOL_GPL(phylink_caps_to_linkmodes);

+static struct {
+ unsigned long mask;
+ int speed;
+ unsigned int duplex;
+} phylink_caps_params[] = {
+ { MAC_400000FD, SPEED_400000, DUPLEX_FULL },
+ { MAC_200000FD, SPEED_200000, DUPLEX_FULL },
+ { MAC_100000FD, SPEED_100000, DUPLEX_FULL },
+ { MAC_56000FD, SPEED_56000, DUPLEX_FULL },
+ { MAC_50000FD, SPEED_50000, DUPLEX_FULL },
+ { MAC_40000FD, SPEED_40000, DUPLEX_FULL },
+ { MAC_25000FD, SPEED_25000, DUPLEX_FULL },
+ { MAC_20000FD, SPEED_20000, DUPLEX_FULL },
+ { MAC_10000FD, SPEED_10000, DUPLEX_FULL },
+ { MAC_5000FD, SPEED_5000, DUPLEX_FULL },
+ { MAC_2500FD, SPEED_2500, DUPLEX_FULL },
+ { MAC_1000FD, SPEED_1000, DUPLEX_FULL },
+ { MAC_1000HD, SPEED_1000, DUPLEX_HALF },
+ { MAC_100FD, SPEED_100, DUPLEX_FULL },
+ { MAC_100HD, SPEED_100, DUPLEX_HALF },
+ { MAC_10FD, SPEED_10, DUPLEX_FULL },
+ { MAC_10HD, SPEED_10, DUPLEX_HALF },
+};
+
+/**
+ * phylink_cap_from_speed_duplex - Get mac capability from speed/duplex
+ * @speed: the speed to search for
+ * @duplex: the duplex to search for
+ *
+ * Find the mac capability for a given speed and duplex.
+ *
+ * Return: A mask with the mac capability patching @speed and @duplex, or 0 if
+ * there were no matches.
+ */
+static unsigned long phylink_cap_from_speed_duplex(int speed,
+ unsigned int duplex)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(phylink_caps_params); i++) {
+ if (speed == phylink_caps_params[i].speed &&
+ duplex == phylink_caps_params[i].duplex)
+ return phylink_caps_params[i].mask;
+ }
+
+ return 0;
+}
+
/**
* phylink_get_capabilities() - get capabilities for a given MAC
* @interface: phy interface mode defined by &typedef phy_interface_t
* @mac_capabilities: bitmask of MAC capabilities
+ * @rate_adaptation: type of rate adaptation being performed
*
* Get the MAC capabilities that are supported by the @interface mode and
* @mac_capabilities.
*/
unsigned long phylink_get_capabilities(phy_interface_t interface,
- unsigned long mac_capabilities)
+ unsigned long mac_capabilities,
+ int rate_adaptation)
{
+ int max_speed = phylink_interface_max_speed(interface);
unsigned long caps = MAC_SYM_PAUSE | MAC_ASYM_PAUSE;
+ unsigned long adapted_caps = 0;

switch (interface) {
case PHY_INTERFACE_MODE_USXGMII:
@@ -458,7 +510,53 @@ unsigned long phylink_get_capabilities(phy_interface_t interface,
break;
}

- return caps & mac_capabilities;
+ switch (rate_adaptation) {
+ case RATE_ADAPT_OPEN_LOOP:
+ /* TODO */
+ fallthrough;
+ case RATE_ADAPT_NONE:
+ adapted_caps = 0;
+ break;
+ case RATE_ADAPT_PAUSE: {
+ /* The MAC must support asymmetric pause towards the local
+ * device for this. We could allow just symmetric pause, but
+ * then we might have to renegotiate if the link partner
+ * doesn't support pause. This is because there's no way to
+ * accept pause frames without transmitting them if we only
+ * support symmetric pause.
+ */
+ if (!(mac_capabilities & MAC_SYM_PAUSE) ||
+ !(mac_capabilities & MAC_ASYM_PAUSE))
+ break;
+
+ /* We can't adapt if the MAC doesn't support the interface's
+ * max speed at full duplex.
+ */
+ if (mac_capabilities &
+ phylink_cap_from_speed_duplex(max_speed, DUPLEX_FULL)) {
+ /* Although a duplex-adapting phy might exist, we
+ * conservatively remove these modes because the MAC
+ * will not be aware of the half-duplex nature of the
+ * link.
+ */
+ adapted_caps = GENMASK(__fls(caps), __fls(MAC_10HD));
+ adapted_caps &= ~(MAC_1000HD | MAC_100HD | MAC_10HD);
+ }
+ break;
+ }
+ case RATE_ADAPT_CRS:
+ /* The MAC must support half duplex at the interface's max
+ * speed.
+ */
+ if (mac_capabilities &
+ phylink_cap_from_speed_duplex(max_speed, DUPLEX_HALF)) {
+ adapted_caps = GENMASK(__fls(caps), __fls(MAC_10HD));
+ adapted_caps &= mac_capabilities;
+ }
+ break;
+ }
+
+ return (caps & mac_capabilities) | adapted_caps;
}
EXPORT_SYMBOL_GPL(phylink_get_capabilities);

@@ -482,7 +580,8 @@ void phylink_generic_validate(struct phylink_config *config,
phylink_set_port_modes(mask);
phylink_set(mask, Autoneg);
caps = phylink_get_capabilities(state->interface,
- config->mac_capabilities);
+ config->mac_capabilities,
+ state->rate_adaptation);
phylink_caps_to_linkmodes(mask, caps);

linkmode_and(supported, supported, mask);
@@ -1514,6 +1613,7 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
config.interface = PHY_INTERFACE_MODE_NA;
else
config.interface = interface;
+ config.rate_adaptation = phy_get_rate_adaptation(phy, config.interface);

ret = phylink_validate(pl, supported, &config);
if (ret) {
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 1524846e01b4..c67a67620c8e 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -541,7 +541,8 @@ void pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,

void phylink_caps_to_linkmodes(unsigned long *linkmodes, unsigned long caps);
unsigned long phylink_get_capabilities(phy_interface_t interface,
- unsigned long mac_capabilities);
+ unsigned long mac_capabilities,
+ int rate_adaptation);
void phylink_generic_validate(struct phylink_config *config,
unsigned long *supported,
struct phylink_link_state *state);
--
2.35.1.1320.gc452695387.dirty

2022-09-06 17:22:03

by Sean Anderson

[permalink] [raw]
Subject: [PATCH net-next v5 8/8] net: phy: aquantia: Add support for rate adaptation

This adds support for rate adaptation for phys similar to the AQR107. We
assume that all phys using aqr107_read_status support rate adaptation.
However, it could be possible to determine support based on the firmware
revision if there are phys discovered which do not support rate adaptation.
However, as rate adaptation is advertised in the datasheets for these phys,
I suspect it is supported most boards.

Despite the name, the "config" registers are updated with the current rate
adaptation method (if any). Because they appear to be updated
automatically, I don't know if these registers can be used to disable rate
adaptation.

Signed-off-by: Sean Anderson <[email protected]>
---

(no changes since v2)

Changes in v2:
- Add comments clarifying the register defines
- Reorder variables in aqr107_read_rate

drivers/net/phy/aquantia_main.c | 51 ++++++++++++++++++++++++++++++---
1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
index b3a5db487e52..dd73891cf68a 100644
--- a/drivers/net/phy/aquantia_main.c
+++ b/drivers/net/phy/aquantia_main.c
@@ -94,6 +94,19 @@
#define VEND1_GLOBAL_FW_ID_MAJOR GENMASK(15, 8)
#define VEND1_GLOBAL_FW_ID_MINOR GENMASK(7, 0)

+/* The following registers all have similar layouts; first the registers... */
+#define VEND1_GLOBAL_CFG_10M 0x0310
+#define VEND1_GLOBAL_CFG_100M 0x031b
+#define VEND1_GLOBAL_CFG_1G 0x031c
+#define VEND1_GLOBAL_CFG_2_5G 0x031d
+#define VEND1_GLOBAL_CFG_5G 0x031e
+#define VEND1_GLOBAL_CFG_10G 0x031f
+/* ...and now the fields */
+#define VEND1_GLOBAL_CFG_RATE_ADAPT GENMASK(8, 7)
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_NONE 0
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_USX 1
+#define VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE 2
+
#define VEND1_GLOBAL_RSVD_STAT1 0xc885
#define VEND1_GLOBAL_RSVD_STAT1_FW_BUILD_ID GENMASK(7, 4)
#define VEND1_GLOBAL_RSVD_STAT1_PROV_ID GENMASK(3, 0)
@@ -338,40 +351,57 @@ static int aqr_read_status(struct phy_device *phydev)

static int aqr107_read_rate(struct phy_device *phydev)
{
+ u32 config_reg;
int val;

val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_TX_VEND_STATUS1);
if (val < 0)
return val;

+ if (val & MDIO_AN_TX_VEND_STATUS1_FULL_DUPLEX)
+ phydev->duplex = DUPLEX_FULL;
+ else
+ phydev->duplex = DUPLEX_HALF;
+
switch (FIELD_GET(MDIO_AN_TX_VEND_STATUS1_RATE_MASK, val)) {
case MDIO_AN_TX_VEND_STATUS1_10BASET:
phydev->speed = SPEED_10;
+ config_reg = VEND1_GLOBAL_CFG_10M;
break;
case MDIO_AN_TX_VEND_STATUS1_100BASETX:
phydev->speed = SPEED_100;
+ config_reg = VEND1_GLOBAL_CFG_100M;
break;
case MDIO_AN_TX_VEND_STATUS1_1000BASET:
phydev->speed = SPEED_1000;
+ config_reg = VEND1_GLOBAL_CFG_1G;
break;
case MDIO_AN_TX_VEND_STATUS1_2500BASET:
phydev->speed = SPEED_2500;
+ config_reg = VEND1_GLOBAL_CFG_2_5G;
break;
case MDIO_AN_TX_VEND_STATUS1_5000BASET:
phydev->speed = SPEED_5000;
+ config_reg = VEND1_GLOBAL_CFG_5G;
break;
case MDIO_AN_TX_VEND_STATUS1_10GBASET:
phydev->speed = SPEED_10000;
+ config_reg = VEND1_GLOBAL_CFG_10G;
break;
default:
phydev->speed = SPEED_UNKNOWN;
- break;
+ return 0;
}

- if (val & MDIO_AN_TX_VEND_STATUS1_FULL_DUPLEX)
- phydev->duplex = DUPLEX_FULL;
+ val = phy_read_mmd(phydev, MDIO_MMD_VEND1, config_reg);
+ if (val < 0)
+ return val;
+
+ if (FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val) ==
+ VEND1_GLOBAL_CFG_RATE_ADAPT_PAUSE)
+ phydev->rate_adaptation = RATE_ADAPT_PAUSE;
else
- phydev->duplex = DUPLEX_HALF;
+ phydev->rate_adaptation = RATE_ADAPT_NONE;

return 0;
}
@@ -612,6 +642,16 @@ static void aqr107_link_change_notify(struct phy_device *phydev)
phydev_info(phydev, "Aquantia 1000Base-T2 mode active\n");
}

+static int aqr107_get_rate_adaptation(struct phy_device *phydev,
+ phy_interface_t iface)
+{
+ if (iface == PHY_INTERFACE_MODE_10GBASER ||
+ iface == PHY_INTERFACE_MODE_2500BASEX ||
+ iface == PHY_INTERFACE_MODE_NA)
+ return RATE_ADAPT_PAUSE;
+ return RATE_ADAPT_NONE;
+}
+
static int aqr107_suspend(struct phy_device *phydev)
{
return phy_set_bits_mmd(phydev, MDIO_MMD_VEND1, MDIO_CTRL1,
@@ -673,6 +713,7 @@ static struct phy_driver aqr_driver[] = {
PHY_ID_MATCH_MODEL(PHY_ID_AQR107),
.name = "Aquantia AQR107",
.probe = aqr107_probe,
+ .get_rate_adaptation = aqr107_get_rate_adaptation,
.config_init = aqr107_config_init,
.config_aneg = aqr_config_aneg,
.config_intr = aqr_config_intr,
@@ -691,6 +732,7 @@ static struct phy_driver aqr_driver[] = {
PHY_ID_MATCH_MODEL(PHY_ID_AQCS109),
.name = "Aquantia AQCS109",
.probe = aqr107_probe,
+ .get_rate_adaptation = aqr107_get_rate_adaptation,
.config_init = aqcs109_config_init,
.config_aneg = aqr_config_aneg,
.config_intr = aqr_config_intr,
@@ -717,6 +759,7 @@ static struct phy_driver aqr_driver[] = {
PHY_ID_MATCH_MODEL(PHY_ID_AQR113C),
.name = "Aquantia AQR113C",
.probe = aqr107_probe,
+ .get_rate_adaptation = aqr107_get_rate_adaptation,
.config_init = aqr107_config_init,
.config_aneg = aqr_config_aneg,
.config_intr = aqr_config_intr,
--
2.35.1.1320.gc452695387.dirty

2022-09-07 10:08:40

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net-next v5 3/8] net: phylink: Generate caps and convert to linkmodes separately

On Tue, Sep 06, 2022 at 12:18:47PM -0400, Sean Anderson wrote:
> @@ -409,11 +407,14 @@ void phylink_generic_validate(struct phylink_config *config,
> unsigned long *supported,
> struct phylink_link_state *state)
> {
> + unsigned long caps;
> __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };

net code requires reverse christmas tree for variable declarations.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2022-09-07 17:03:55

by Sean Anderson

[permalink] [raw]
Subject: Re: [PATCH net-next v5 3/8] net: phylink: Generate caps and convert to linkmodes separately

On 9/7/22 5:42 AM, Russell King (Oracle) wrote:
> On Tue, Sep 06, 2022 at 12:18:47PM -0400, Sean Anderson wrote:
>> @@ -409,11 +407,14 @@ void phylink_generic_validate(struct phylink_config *config,
>> unsigned long *supported,
>> struct phylink_link_state *state)
>> {
>> + unsigned long caps;
>> __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>
> net code requires reverse christmas tree for variable declarations.
>

OK