THIS PATCH SET DOES NOT WORK, AND IT ISN'T INTENDED TO WORK.
Changes in v2:
- replace phy_check_cdr_lock() with phy_get_status(PHY_STATUS_CDR_LOCK)
- rename PHY_MODE_ETHERNET_PHY to PHY_MODE_ETHTOOL
- stop describing the AN/LT block in the device tree and make use of the
fact that it is discoverable. Add phy_get_status(PHY_STATUS_PCVT_ADDR)
to the generic PHY layer to discover it.
- add the 25GBase-KR-S and 25GBase-CR-S link modes. Proper treatment of
RS-FEC and BASE-R FEC is still TODO (will also require new API in the
generic PHY layer).
- rework the implementation from a phylib phy_device to a phylink_pcs
component (library code). The phy-mode is still "internal". It may or
may have not been the right thing to do. There are some things to say
about that on patch 08/15.
- support multi-lane link modes - tested with 40GBase-KR4
- solve the pre-configuration and register access problem for 1000Base-KX
by having mtip_get_mdiodev() pre-configure the SerDes lane and
protocol converter for the highest supported backplane link mode.
The original cover letter for the RFC v1 at
https://patchwork.kernel.org/project/netdevbpf/cover/[email protected]/
is still attached below.
=================================================
I've been tasked with maintaining the copper backplane support on
Layerscape SoCs. There was a previous attempt to upstream that, which is
located here:
https://lore.kernel.org/lkml/[email protected]/
Nonetheless, I am presenting here a complete rewrite based on my
understanding. The quality of implementation is not great, and there are
probably bugs which I've yet to identify. The reason for this RFC is to
collect feedback for the overall design from networking PHY and generic
PHY maintainers.
The newly proposed generic PHY API has the mtip_backplane.c driver as a
user (a consumer), but its actual hardware implementation (which should
go in drivers/phy/freescale/phy-fsl-lynx-28g.c), is not included in this
patch set. That would just have just uselessly distracted the reviewers'
attention. This is why the patch set, as posted, does not work.
I recommend reviewing from the bottom up - dt-bindings first, those give
an overall picture of the AN/LT block and its integration in the SoC.
Future work consists of:
- supporting multi-lane link modes
- advertising more than a single link mode, and performing dynamic
SerDes protocol switching towards that link mode. Strictly speaking,
the hardware was intended to be used with a single link mode advertised
over C73 (the link mode pre-configured through the RCW - Reset
Configuration Word), and that is quite apparent in its design. With
some inventive workarounds which I've yet to try out, it might be
possible to circumvent the design limitations and advertise any link
mode that the SerDes PLLs can sustain. This is in an exploratory stage
and in the end it might not come to fruition, but I've identified some
aspects which require forethought in the driver's design.
Both these features hit a wall given the current driver design, which is
"how do we access the AN/LT block's registers?".
The hardware designers were inconsistent in the way that they've
integrated this AN/LT blocks for 1G/10G ports, vs how they did it for
25G/40G/50G/100G ports. There is always one single AN/LT block per
SerDes lane, but for 1G/10G modes, hardware design wanted to make it
appear as if the AN/LT is part of the PCS. So it inherently responds to
the same MDIO address as the PCS. Whereas in the >25G modes, it responds
to an MDIO address defined by a different control register, and that
address is also different from the PCS' MDIO address.
In the current dt-bindings, I am requesting DT writers to put a
phy-handle towards the MDIO address of the AN/LT block (which depends on
a lot of things, see the dt-bindings document for details).
As opposed to regular ports where the PCS and SerDes PHY are controlled
by the MAC driver (drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c),
with backplane, those same components are controlled by the
mtip_backplane.c driver. The backplane AN/LT driver has a pcs-handle
towards the PCS, and it treats it as a raw MDIO device when polling for
link status.
This gets obviously problematic when switching SerDes protocols, because
the location of the backplane AN/LT block changes on the MDIO bus with
the new SerDes protocol, and what's in the device tree becomes invalid
(the phydev->mdio.addr would need to be updated). It's the same AN/LT
block, and after a SerDes protocol change it has been reset, but it moved.
The SerDes control registers for the MDIO addresses of the PCS and AN/LT blocks:
- SGMIInCR1[MDEV_PORT]
- SXGMIInCR1[MDEV_PORT]
- ANLTnCR1[MDEV_PORT]
- E25GnCR1[MDEV_PORT]
- E40GnCR1[MDEV_PORT]
- E50GnCR1[MDEV_PORT]
- E100GnCR1[MDEV_PORT]
are in premise all configurable, but it's an open question to me as to
how Linux should configure them. Currently, we treat all those addresses
as read-only and populate the device trees based on their default values.
There's an additional (related) problem for 1000base-KX. The lane starts
up at power-on reset in 1000base-X mode (non-backplane) and this means
that the PCS which responds to MDIO commands is the one used for
SGMII/1000Base-X by drivers/net/pcs/pcs-lynx.c. That responds to C22
MDIO commands. The PCS for 1000Base-KX is different, and it responds to
C45 MDIO commands. It also incorporates the AN/LT block at this C45 MDIO
address. In the current mtip_backplane.c driver design, the lane switches
to backplane mode (thus enabling the respective PCS) once the link mode
was resolved to 1000base-KX through C73 autoneg, using phy_set_mode_ext().
But that is too late with 1000base-KX, since the AN/LT block won't
respond to C45 transactions either, if the port isn't pre-configured for
1000base-KX. So C73 autoneg won't work to begin with... Somebody needs
to pre-configure the SerDes lane for backplane mode, so that the
mtip_backplane.c driver can probe, but it's not clear who and based on
what.
Finally, I reckon that in the end, the PCS should be driven using a
struct phylink_pcs, by the lynx_pcs driver, and not as an mdio_device
using raw accesses by the mtip_backplane.c. In premise, this AN/LT IP
core can be integrated, to my knowledge, with any PCS and any SerDes,
so that should be also possible in the driver design. Yet, there's a
problem: in the 1G/10G modes, there would be 2 drivers for the same
mdio_device: one for the PCS and the other for the AN/LT block (PHY).
True, they are at different MMDs, but bindings multiple Linux drivers to
mdio_devices per MMD is not a thing, I guess?
Interrupt support is also missing, and that's very far away on my TODO
list. AFAIU, PCS/ANLT interrupts are routed by the SoC to the attached
MAC's IEVENT register, which isn't enabled as an interrupt source on any
Layerscape platform yet. I've structured the code using an irqpoll
thread for now, so it is more-or-less just as event-driven as an IRQ
based driver would be.
As a side note, I have analyzed the feedback previously given to Florinel,
especially the one from Russell:
https://lore.kernel.org/lkml/[email protected]/
"This uses phylib, which is a problem ..."
and I still haven't changed that here. Without going into a wall of text
explanation, I'm not fully convinced that phylib isn't, in fact, the
best place for a backplane AN/LT driver to live. At least in the initial
implementation shown here, I'm not sure that going straight for a
phylink implementation of the AN/LT block would have solved any of the
problems that I described above.
Vladimir Oltean (15):
phy: introduce phy_get_status() and use it to report CDR lock
phy: introduce the PHY_MODE_ETHTOOL mode for phy_set_mode_ext()
phy: ethernet: add configuration interface for copper backplane
Ethernet PHYs
phy: allow querying the address of protocol converters through
phy_get_status()
net: add 25GBase-KR-S and 25GBase-CR-S to ethtool link mode UAPI
net: mii: add C73 base page helpers
net: phylink: centralize phy_interface_mode_is_8023z() &&
phylink_autoneg_inband() checks
net: phylink: allow PCS to handle C73 autoneg for phy-mode =
"internal"
net: ethtool: introduce ethtool_link_mode_str()
net: phylink: support all ethtool port modes with inband modes
net: phylink: support the 25GBase-KR-S and 25GBase-CR-S link modes too
net: phylink: add the 25G link modes to
phylink_c73_priority_resolution[]
dt-bindings: lynx-pcs: add properties for backplane mode
net: pcs: mtip_backplane: add driver for MoreThanIP backplane AN/LT
core
net: pcs: lynx: use MTIP AN/LT block for copper backplanes
.../bindings/net/pcs/fsl,lynx-pcs.yaml | 15 +-
drivers/net/mii.c | 34 +-
drivers/net/pcs/Kconfig | 8 +
drivers/net/pcs/Makefile | 1 +
drivers/net/pcs/mtip_backplane.c | 2022 +++++++++++++++++
drivers/net/pcs/mtip_backplane.h | 87 +
drivers/net/pcs/pcs-lynx.c | 135 ++
drivers/net/phy/phy-core.c | 2 +-
drivers/net/phy/phylink.c | 53 +-
drivers/phy/phy-core.c | 31 +
include/linux/ethtool.h | 6 +
include/linux/mii.h | 105 +
include/linux/phy/phy-ethernet.h | 292 +++
include/linux/phy/phy.h | 83 +
include/linux/phylink.h | 1 +
include/uapi/linux/ethtool.h | 2 +
net/ethtool/common.c | 12 +
17 files changed, 2876 insertions(+), 13 deletions(-)
create mode 100644 drivers/net/pcs/mtip_backplane.c
create mode 100644 drivers/net/pcs/mtip_backplane.h
create mode 100644 include/linux/phy/phy-ethernet.h
--
2.34.1
Some phylink and phylib based systems might want to operate on backplane
media types ("K" in the name), and thus, picking a phy_interface_t for
them becomes a challenge.
phy_interface_t is a description of the connection between the MAC and
the PHY, or if a MAC-side PCS is present, the connection between that
and the next link segment (which can be remote).
A MAC-side PCS is so far considered to be a PCS handling link modes with
optional C37 autoneg. But C73 autoneg (for backplanes and SFP28 modules)
is not at the same level in the OSI layering, so that existing model may
or may not apply.
(a) If we say that the PCS is MAC-side for C73 modes as well, the
implication seems to be that the phy-mode should be one of
PHY_INTERFACE_MODE_10GBASEKR, PHY_INTERFACE_MODE_1000BASEKX, etc.
Similar to PHY_INTERFACE_MODE_1000BASEX which imitates the link mode
ETHTOOL_LINK_MODE_1000baseX_Full_BIT.
(b) If we say that the PCS is not MAC-side, but rather that the
phylink_pcs represents an entire non-phylib backplane PHY which may
negotiate one of many link modes (like a copper phylib PHY), then
the phy-mode should probably be one of PHY_INTERFACE_MODE_XGMII,
XLGMII etc. Or rather, because there is no MII pinout per se and the
backplane PHY / phylink_pcs is internal, we can also use
PHY_INTERFACE_MODE_INTERNAL.
The trouble with (a), in my opinion, is that if we let the phy_interface_t
follow the link mode like in the case of Base-X fiber modes, we have to
consider the fact that C73 PHYs can advertise multiple link modes, so
the phy_interface_t selection will be arbitrary, and any phy_interface_t
selection will have to leave in the "supported" and "advertised" masks
of link modes all the other backplane modes. This may be hard to justify.
That is the reasoning based on which I selected this phy-mode to
describe the setup in Layerscape SoCs which have integrated backplane
autoneg support. The changes in phylink permit the managed =
"in-band-status" fwnode property to be extended for C73 autoneg, which
is then controllable through ethtool. With phy-mode = "internal" in an
in-band autoneg mode, we advertise all backplane link modes. The list is
not exhaustive and may be extended in the future.
Link: https://lore.kernel.org/netdev/[email protected]/
Link: https://lore.kernel.org/netdev/[email protected]/
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: patch is new
drivers/net/phy/phylink.c | 19 ++++++++++++++++++-
include/linux/phylink.h | 1 +
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 548130d77302..88ace7e203c3 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -972,6 +972,21 @@ static int phylink_parse_mode(struct phylink *pl,
phylink_set(pl->supported, 100000baseDR2_Full);
break;
+ case PHY_INTERFACE_MODE_INTERNAL:
+ phylink_set(pl->supported, 1000baseKX_Full);
+ phylink_set(pl->supported, 10000baseKX4_Full);
+ phylink_set(pl->supported, 10000baseKR_Full);
+ phylink_set(pl->supported, 25000baseCR_Full);
+ phylink_set(pl->supported, 25000baseKR_Full);
+ phylink_set(pl->supported, 25000baseCR_S_Full);
+ phylink_set(pl->supported, 25000baseKR_S_Full);
+ phylink_set(pl->supported, 40000baseKR4_Full);
+ phylink_set(pl->supported, 50000baseKR2_Full);
+ phylink_set(pl->supported, 50000baseKR_Full);
+ phylink_set(pl->supported, 100000baseKR4_Full);
+ phylink_set(pl->supported, 100000baseKR2_Full);
+ break;
+
default:
phylink_err(pl,
"incorrect link mode %s for in-band status\n",
@@ -1109,7 +1124,9 @@ static void phylink_mac_config(struct phylink *pl,
static bool phylink_pcs_handles_an(phy_interface_t iface, unsigned int mode)
{
- return phy_interface_mode_is_8023z(iface) && phylink_autoneg_inband(mode);
+ return (phy_interface_mode_is_8023z(iface) ||
+ iface == PHY_INTERFACE_MODE_INTERNAL) &&
+ phylink_autoneg_inband(mode);
}
static void phylink_pcs_an_restart(struct phylink *pl)
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 2b886ea654bb..7e8e26001587 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -141,6 +141,7 @@ static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
case PHY_INTERFACE_MODE_1000BASEX:
case PHY_INTERFACE_MODE_2500BASEX:
+ case PHY_INTERFACE_MODE_INTERNAL:
/* 1000base-X is designed for use media-side for Fibre
* connections, and thus the Autoneg bit needs to be
* taken into account. We also do this for 2500base-X
--
2.34.1
When the Lynx PCS is deployed on a copper backplane link, it must be
prepared to handle clause 73 autoneg and clause 72 link training, which
it can do using a dedicated AN/LT block. The latter doesn't need to be
described in the device tree, because it is discoverable from the SerDes
lanes.
The media type that is deployed on the link is not discoverable though,
so the introduction of a fsl,backplane-mode boolean property appears
necessary to determine whether the AN/LT block should be employed, or
left bypassed.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: patch is new
.../devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml b/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
index fbedf696c555..40fbcd80ee2a 100644
--- a/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
+++ b/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
@@ -16,11 +16,24 @@ description: |
properties:
compatible:
- const: fsl,lynx-pcs
+ enum:
+ - fsl,lx2160a-lynx-pcs
+ - fsl,lynx-pcs
reg:
maxItems: 1
+ phys:
+ maxItems: 4
+ description:
+ phandle for the SerDes lanes that act as PMA/PMD layer when the PCS is
+ part of a copper backplane PHY.
+
+ fsl,backplane-mode:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ Indicates that the PCS is deployed over a copper backplane link.
+
required:
- compatible
- reg
--
2.34.1
If the fsl,backplane-mode device tree property is present, then the Lynx
PCS makes use of the AN/LT block to advertise the supported backplane
link modes using clause 73 autoneg.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: code is new
drivers/net/pcs/Kconfig | 1 +
drivers/net/pcs/pcs-lynx.c | 135 +++++++++++++++++++++++++++++++++++++
2 files changed, 136 insertions(+)
diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index 24a033e93bdd..be561c465b4a 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -13,6 +13,7 @@ config MTIP_BACKPLANE_PHY
SoCs.
config PCS_XPCS
+ depends on MTIP_BACKPLANE_PHY || MTIP_BACKPLANE_PHY=n
tristate
select PHYLINK
help
diff --git a/drivers/net/pcs/pcs-lynx.c b/drivers/net/pcs/pcs-lynx.c
index dc3962b2aa6b..1352f08edcf3 100644
--- a/drivers/net/pcs/pcs-lynx.c
+++ b/drivers/net/pcs/pcs-lynx.c
@@ -4,10 +4,14 @@
*/
#include <linux/mdio.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
#include <linux/phylink.h>
#include <linux/pcs-lynx.h>
#include <linux/property.h>
+#include "mtip_backplane.h"
+
#define SGMII_CLOCK_PERIOD_NS 8 /* PCS is clocked at 125 MHz */
#define LINK_TIMER_VAL(ns) ((u32)((ns) / SGMII_CLOCK_PERIOD_NS))
@@ -20,9 +24,15 @@
#define IF_MODE_SPEED_MSK GENMASK(3, 2)
#define IF_MODE_HALF_DUPLEX BIT(4)
+#define PRIMARY_LANE 0
+#define MAX_NUM_LANES 4
+
struct lynx_pcs {
struct phylink_pcs pcs;
struct mdio_device *mdio;
+ struct mtip_backplane *anlt[MAX_NUM_LANES];
+ int num_lanes;
+ bool backplane_mode;
};
enum sgmii_speed {
@@ -100,6 +110,9 @@ static void lynx_pcs_get_state(struct phylink_pcs *pcs,
case PHY_INTERFACE_MODE_10GBASER:
phylink_mii_c45_pcs_get_state(lynx->mdio, state);
break;
+ case PHY_INTERFACE_MODE_INTERNAL:
+ mtip_backplane_get_state(lynx->anlt[PRIMARY_LANE], state);
+ break;
default:
break;
}
@@ -168,6 +181,17 @@ static int lynx_pcs_config_usxgmii(struct mdio_device *pcs,
ADVERTISE_SGMII | ADVERTISE_LPACK);
}
+static int lynx_pcs_config_backplane(struct phylink_pcs *pcs,
+ unsigned int neg_mode,
+ const unsigned long *advertising)
+{
+ bool autoneg = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
+ struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
+
+ return mtip_backplane_config_aneg(lynx->anlt[PRIMARY_LANE], autoneg,
+ advertising);
+}
+
static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
phy_interface_t ifmode,
const unsigned long *advertising, bool permit)
@@ -193,6 +217,8 @@ static int lynx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
case PHY_INTERFACE_MODE_10GBASER:
/* Nothing to do here for 10GBASER */
break;
+ case PHY_INTERFACE_MODE_INTERNAL:
+ return lynx_pcs_config_backplane(pcs, neg_mode, advertising);
default:
return -EOPNOTSUPP;
}
@@ -204,6 +230,9 @@ static void lynx_pcs_an_restart(struct phylink_pcs *pcs)
{
struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
+ if (lynx->backplane_mode)
+ return mtip_backplane_an_restart(lynx->anlt[PRIMARY_LANE]);
+
phylink_mii_c22_pcs_an_restart(lynx->mdio);
}
@@ -306,16 +335,111 @@ static void lynx_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
}
}
+static int lynx_pcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
+ const struct phylink_link_state *state)
+{
+ struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
+
+ if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
+ return 0;
+
+ return mtip_backplane_validate(lynx->anlt[PRIMARY_LANE], supported);
+}
+
+static int lynx_pcs_enable(struct phylink_pcs *pcs)
+{
+ struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
+ int err;
+
+ if (lynx->backplane_mode) {
+ err = mtip_backplane_resume(lynx->anlt[PRIMARY_LANE]);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static void lynx_pcs_disable(struct phylink_pcs *pcs)
+{
+ struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
+
+ if (lynx->backplane_mode)
+ mtip_backplane_suspend(lynx->anlt[PRIMARY_LANE]);
+}
+
static const struct phylink_pcs_ops lynx_pcs_phylink_ops = {
.pcs_get_state = lynx_pcs_get_state,
.pcs_config = lynx_pcs_config,
.pcs_an_restart = lynx_pcs_an_restart,
.pcs_link_up = lynx_pcs_link_up,
+ .pcs_validate = lynx_pcs_validate,
+ .pcs_enable = lynx_pcs_enable,
+ .pcs_disable = lynx_pcs_disable,
};
+static int lynx_pcs_parse_fwnode(struct lynx_pcs *lynx)
+{
+ struct fwnode_handle *node = lynx->mdio->dev.fwnode;
+ enum mtip_model model = MTIP_MODEL_AUTODETECT;
+ struct device_node *np = to_of_node(node);
+ struct mdio_device *mdio = lynx->mdio;
+ struct device *dev = &mdio->dev;
+ struct phy *phy;
+ int i, err;
+
+ if (!node)
+ return 0;
+
+ lynx->backplane_mode = fwnode_property_present(node, "fsl,backplane-mode");
+ if (!lynx->backplane_mode)
+ return 0;
+
+ if (fwnode_device_is_compatible(node, "fsl,lx2160a-lynx-pcs"))
+ model = MTIP_MODEL_LX2160A;
+
+ lynx->num_lanes = of_count_phandle_with_args(np, "phys", "#phy-cells");
+ if (lynx->num_lanes < 0)
+ return lynx->num_lanes;
+
+ if (WARN_ON(lynx->num_lanes > MAX_NUM_LANES))
+ return -EINVAL;
+
+ for (i = 0; i < lynx->num_lanes; i++) {
+ phy = devm_of_phy_get_by_index(dev, np, i);
+ if (IS_ERR(phy))
+ return dev_err_probe(dev, PTR_ERR(phy),
+ "Failed to get SerDes PHY %d\n", i);
+
+ lynx->anlt[i] = mtip_backplane_create(mdio, phy, model);
+ if (IS_ERR(lynx->anlt[i])) {
+ err = PTR_ERR(lynx->anlt[i]);
+
+ while (i-- > 0)
+ mtip_backplane_destroy(lynx->anlt[i]);
+
+ return err;
+ }
+ }
+
+ for (i = 1; i < lynx->num_lanes; i++) {
+ err = mtip_backplane_add_subordinate(lynx->anlt[PRIMARY_LANE],
+ lynx->anlt[i]);
+ if (WARN_ON(err)) {
+ /* Too many SerDes lanes in the device tree? */
+ for (i = 0; i < lynx->num_lanes; i++)
+ mtip_backplane_destroy(lynx->anlt[i]);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
{
struct lynx_pcs *lynx;
+ int err;
lynx = kzalloc(sizeof(*lynx), GFP_KERNEL);
if (!lynx)
@@ -327,6 +451,12 @@ static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
lynx->pcs.neg_mode = true;
lynx->pcs.poll = true;
+ err = lynx_pcs_parse_fwnode(lynx);
+ if (err) {
+ kfree(lynx);
+ return ERR_PTR(err);
+ }
+
return lynx_to_phylink_pcs(lynx);
}
@@ -392,6 +522,11 @@ EXPORT_SYMBOL_GPL(lynx_pcs_create_fwnode);
void lynx_pcs_destroy(struct phylink_pcs *pcs)
{
struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
+ int i;
+
+ if (lynx->backplane_mode)
+ for (i = 0; i < lynx->num_lanes; i++)
+ mtip_backplane_destroy(lynx->anlt[i]);
mdio_device_put(lynx->mdio);
kfree(lynx);
--
2.34.1
In networking, we have 2 distinct data types:
- phy_interface_t describes the link between a MAC (or MAC-side PCS) and
an attached PHY or SFP cage
- enum ethtool_link_mode_bit_indices describes the link between a local
PHY and a remote PHY (for example, gigabit RJ45 twisted copper pairs)
Currently, phy_set_mode_ext(PHY_MODE_ETHERNET) takes arguments of the
phy_interface_t type, and there is no way to pass an argument of the
enum ethtool_link_mode_bit_indices type. The new PHY_MODE_ETHTOOL
intends to address that.
It is true that there is currently some overlap between these data
types, namely:
phy_interface_t enum ethtool_link_mode_bit_indices
-----------------------------------------------------------------
PHY_INTERFACE_MODE_10GKR ETHTOOL_LINK_MODE_10000baseKR_Full_BIT
PHY_INTERFACE_MODE_1000BASEKX ETHTOOL_LINK_MODE_1000baseKX_Full_BIT
but those overlaps were deemed to be mistakes, and PHY-to-PHY link modes
should only be added to ethtool_link_mode_bit_indices going forward.
Thus, I believe that the distinction is necessary, rather than hacking
more improper PHY modes. Some of the PHY-to-PHY link modes which may be
added in the future (to ethtool_link_mode_bit_indices and not to
phy_interface_t) are:
ETHTOOL_LINK_MODE_100000baseKP4_Full_BIT
ETHTOOL_LINK_MODE_100000baseCR10_Full_BIT
ETHTOOL_LINK_MODE_25000baseKR_S_Full_BIT
ETHTOOL_LINK_MODE_25000baseCR_S_Full_BIT
One user of PHY_MODE_ETHTOOL will be the MTIP backplane AN/LT + Lynx
SerDes PHY combo, where the backplane autoneg protocol (IEEE 802.3
clause 73) selects the operating PHY-to-PHY link mode.
There are electrical differences between the PHY-to-PHY backplane link
modes (like ETHTOOL_LINK_MODE_10000baseKR_Full_BIT) and their
non-backplane counterparts (like PHY_INTERFACE_MODE_10GBASER), namely
the number of TX signal equalization taps and their configurability.
This further justifies distinguishing between them in the generic PHY
API.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: rename PHY_MODE_ETHERNET_PHY to PHY_MODE_ETHTOOL at Russell's
suggestion
include/linux/phy/phy.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6be348f1fa0e..72ef4afcda81 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -39,6 +39,7 @@ enum phy_mode {
PHY_MODE_UFS_HS_B,
PHY_MODE_PCIE,
PHY_MODE_ETHERNET,
+ PHY_MODE_ETHTOOL,
PHY_MODE_MIPI_DPHY,
PHY_MODE_SATA,
PHY_MODE_LVDS,
@@ -52,7 +53,7 @@ enum phy_media {
};
enum phy_status_type {
- /* Valid for PHY_MODE_ETHERNET */
+ /* Valid for PHY_MODE_ETHERNET and PHY_MODE_ETHTOOL */
PHY_STATUS_CDR_LOCK,
};
--
2.34.1
For each networking SerDes lane on certain Layerscape SoCs, there is a
block, based on an IP core from MoreThanIP, which optionally handles
IEEE 802.3 clause 73 and clause 72, i.e. backplane auto-negotiation and
link training.
The hardware integration between the SerDes lane and this AN/LT block is
rather weak. For this reason, there is no automatic link training
performed in hardware, but rather, software needs to implement a custom,
SerDes-specific link training algorithm and use the AN/LT registers to
communicate it with the link partner. This driver is an inapt attempt to
do just that.
Since the MTIP AN/LT block may be, in premise, integrated in non-NXP
SoCs as well, the implementation is as generic as possible.
In fact, it is not a driver per se, but it is presented as library code
which can be instantiated from the lynx phylink_pcs support code.
Initial support is present only for the LX2160A SoC. Here, the register
map of the IP block was a bit mangled, and we don't have any PHY ID for
auto-detection. But, the location of the AN/LT block is detectable by
querying the SerDes for the ANLTnCR1[MDEV_PORT] fields.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2:
- support multi-lane link modes: see mtip_backplane_add_subordinate() as
an entry point for what that support entails
- replace phylib integration with phylink_pcs integration
- auto-detect the location of the AN/LT block using the SerDes'
MDEV_PORT registers, rather than hardcoding in the device tree. There
are now no dt-bindings for the AN/LT block, just the Lynx PCS bindings
were extended.
- mtip_lt_frame_lock() was reading the wrong register, leading to
incorrectly proceeding to link training when the other side wasn't yet
ready for it
- use mdiodev->addr instead of dev_name(dev) in the kthread workers'
name, as it is shorter and it would have been impossible to
distinguish names otherwise. But now it is not unique for different
ports...
- some reinterpretations of link training status fields, as well as a
small rework of the control flow in mtip_local_tx_lt_work() and
mtip_remote_tx_lt_work()
- also advertise 25GBase-KR-S when advertising 25GBase-KR. The FEC/RS-FEC
resolution and application is still TODO.
- some more defensive workarounds in mtip_irqpoll_work() which end up
restarting autoneg in circumstances where the AN/LT block enters a
strange state
- don't call mtip_start_irqpoll() since mtip_backplane_create(), but
wait until mtip_backplane_resume() - aka disable polling when the link
is administratively down
drivers/net/pcs/Kconfig | 7 +
drivers/net/pcs/Makefile | 1 +
drivers/net/pcs/mtip_backplane.c | 2022 ++++++++++++++++++++++++++++++
drivers/net/pcs/mtip_backplane.h | 87 ++
4 files changed, 2117 insertions(+)
create mode 100644 drivers/net/pcs/mtip_backplane.c
create mode 100644 drivers/net/pcs/mtip_backplane.h
diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index 87cf308fc6d8..24a033e93bdd 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -5,6 +5,13 @@
menu "PCS device drivers"
+config MTIP_BACKPLANE_PHY
+ tristate "MoreThanIP copper backplane PHYs"
+ help
+ Enable support for the MoreThanIP copper backplane Auto-Negotiation
+ and Link Training blocks, as implemented on the QorIQ and Layerscape
+ SoCs.
+
config PCS_XPCS
tristate
select PHYLINK
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index fb1694192ae6..08f9102c3fba 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for Linux PCS drivers
+obj-$(CONFIG_MTIP_BACKPLANE_PHY) += mtip_backplane.o
pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-nxp.o pcs-xpcs-wx.o
obj-$(CONFIG_PCS_XPCS) += pcs_xpcs.o
diff --git a/drivers/net/pcs/mtip_backplane.c b/drivers/net/pcs/mtip_backplane.c
new file mode 100644
index 000000000000..a4eb8b470215
--- /dev/null
+++ b/drivers/net/pcs/mtip_backplane.c
@@ -0,0 +1,2022 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Driver for MoreThanIP copper backplane AN/LT (Auto-Negotiation and
+ * Link Training) core
+ *
+ * Copyright 2023 NXP
+ */
+
+#include <linux/kernel.h>
+#include <linux/mii.h>
+#include <linux/module.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/phylink.h>
+#include <linux/phy/phy.h>
+
+#include "mtip_backplane.h"
+
+#define BP_ETH_STAT_ALWAYS_1 BIT(0)
+#define BP_ETH_STAT_1GKX BIT(1)
+#define BP_ETH_STAT_10GKX4 BIT(2)
+#define BP_ETH_STAT_10GKR BIT(3)
+#define BP_ETH_STAT_FC_FEC BIT(4)
+#define BP_ETH_STAT_40GKR4 BIT(5)
+#define BP_ETH_STAT_40GCR4 BIT(6)
+#define BP_ETH_STAT_RS_FEC BIT(7)
+#define BP_ETH_STAT_100GCR10 BIT(8)
+#define BP_ETH_STAT_100GKP4 BIT(9)
+#define BP_ETH_STAT_100GKR4 BIT(10)
+#define BP_ETH_STAT_100GCR4 BIT(11)
+#define BP_ETH_STAT_25GKR_S BIT(12)
+#define BP_ETH_STAT_25GKR BIT(13)
+
+#define BP_ETH_STAT_PARALLEL_DETECT (BP_ETH_STAT_ALWAYS_1 | \
+ BP_ETH_STAT_1GKX | \
+ BP_ETH_STAT_10GKX4)
+
+#define LT_CTRL_RESTART_TRAINING BIT(0)
+#define LT_CTRL_TRAINING_ENABLE BIT(1)
+
+#define LT_STAT_RX_STATUS BIT(0)
+#define LT_STAT_FRAME_LOCK BIT(1)
+#define LT_STAT_STARTUP_PROTOCOL_STATUS BIT(2)
+#define LT_STAT_TRAINING_FAILURE BIT(3)
+#define LT_STAT_SIGNAL_DETECT BIT(15)
+
+#define LT_COM1_MASK GENMASK(1, 0)
+#define LT_COZ_MASK GENMASK(3, 2)
+#define LT_COP1_MASK GENMASK(5, 4)
+#define LT_COM1(x) ((x) & LT_COM1_MASK)
+#define LT_COM1_X(x) ((x) & LT_COM1_MASK)
+#define LT_COZ(x) (((x) << 2) & LT_COZ_MASK)
+#define LT_COZ_X(x) (((x) & LT_COZ_MASK) >> 2)
+#define LT_COP1(x) (((x) << 4) & LT_COP1_MASK)
+#define LT_COP1_X(x) (((x) & LT_COP1_MASK) >> 4)
+
+#define LT_COEF_STAT_MASK (LT_COM1_MASK | LT_COZ_MASK | LT_COP1_MASK)
+#define LT_COEF_STAT_ALL_NOT_UPDATED(x) (((x) & LT_COEF_STAT_MASK) == 0)
+#define LT_COEF_STAT_ANY_UPDATED(x) (((x) & LT_COEF_STAT_MASK) != 0)
+
+#define LT_COEF_UPD_MASK (LT_COM1_MASK | LT_COZ_MASK | LT_COP1_MASK)
+#define LT_COEF_UPD_ALL_HOLD (LT_COM1(COEF_UPD_HOLD) | \
+ LT_COZ(COEF_UPD_HOLD) | \
+ LT_COP1(COEF_UPD_HOLD))
+
+#define LT_COEF_UPD_ANYTHING(x) ((x) != 0)
+#define LT_COEF_UPD_NOTHING(x) ((x) == 0)
+
+#define LT_COEF_UPD_INIT BIT(12)
+#define LT_COEF_UPD_PRESET BIT(13)
+
+#define LT_COEF_STAT_RX_READY BIT(15)
+
+#define C73_ADV_0(x) (u16)((x) & GENMASK(15, 0))
+#define C73_ADV_1(x) (u16)(((x) & GENMASK(31, 16)) >> 16)
+#define C73_ADV_2(x) (u16)(((x) & GENMASK_ULL(47, 32)) >> 32)
+
+#define IRQPOLL_INTERVAL (HZ / 4)
+
+#define MTIP_CDR_SLEEP_US 100
+#define MTIP_CDR_TIMEOUT_US 500000
+
+#define MTIP_LT_END_SLEEP_US 10
+#define MTIP_LT_END_TIMEOUT_US 300000
+
+#define MTIP_LT_RESTART_SLEEP_US 10
+#define MTIP_LT_RESTART_TIMEOUT_US 1000000
+
+#define MTIP_FRAME_LOCK_SLEEP_US 10
+#define MTIP_FRAME_LOCK_TIMEOUT_US 1000000
+
+#define MTIP_RESET_SLEEP_US 100
+#define MTIP_RESET_TIMEOUT_US 100000
+
+#define MTIP_BP_ETH_STAT_SLEEP_US 10
+#define MTIP_BP_ETH_STAT_TIMEOUT_US 100
+
+#define MTIP_COEF_STAT_SLEEP_US 10
+#define MTIP_COEF_STAT_TIMEOUT_US 500000
+
+#define MTIP_LT_TIMEOUT_MS 1000
+#define MTIP_AN_TIMEOUT_MS 10000
+
+#define MTIP_MAX_NUM_SUBORDINATES 3
+
+enum mtip_an_reg {
+ AN_CTRL,
+ AN_STAT,
+ AN_ADV_0,
+ AN_ADV_1,
+ AN_ADV_2,
+ AN_LPA_0,
+ AN_LPA_1,
+ AN_LPA_2,
+ AN_MS_CNT,
+ AN_ADV_XNP_0,
+ AN_ADV_XNP_1,
+ AN_ADV_XNP_2,
+ AN_LPA_XNP_0,
+ AN_LPA_XNP_1,
+ AN_LPA_XNP_2,
+ AN_BP_ETH_STAT,
+};
+
+enum mtip_lt_reg {
+ LT_CTRL,
+ LT_STAT,
+ LT_LP_COEF,
+ LT_LP_STAT,
+ LT_LD_COEF,
+ LT_LD_STAT,
+ LT_TRAIN_PATTERN,
+ LT_RX_PATTERN,
+ LT_RX_PATTERN_ERR,
+ LT_RX_PATTERN_BEGIN,
+};
+
+struct mtip_irqpoll {
+ struct mutex lock;
+ struct delayed_work work;
+ u16 old_an_stat;
+ u16 old_pcs_stat;
+ bool link;
+ bool link_ack;
+ bool cdr_locked;
+ bool run_once;
+};
+
+struct mtip_lt_work {
+ struct mtip_backplane *priv;
+ struct kthread_work work;
+};
+
+struct mtip_backplane;
+
+struct mtip_backplane {
+ struct mdio_device *mdiodev;
+ struct mdio_device *pcs_mdiodev;
+ union {
+ struct mtip_backplane *subordinate[MTIP_MAX_NUM_SUBORDINATES];
+ struct mtip_backplane *coordinator;
+ };
+ bool is_subordinate;
+ int num_subordinates;
+ struct phylink_pcs *pcs;
+ struct phy *serdes;
+ const u16 *an_regs;
+ const u16 *lt_regs;
+ int lt_mmd;
+ enum ethtool_link_mode_bit_indices link_mode;
+ bool link_mode_resolved;
+ bool lane_powered_on;
+ bool any_request_received;
+ unsigned long last_lt_done;
+ unsigned long last_an_restart;
+ struct mtip_irqpoll irqpoll;
+ struct kthread_worker *local_tx_lt_worker;
+ struct kthread_worker *remote_tx_lt_worker;
+ /* Serialized by an_restart_lock */
+ bool an_restart_pending;
+ bool an_enabled;
+ /* Used for orderly shutdown of LT threads. Modified without any
+ * locking. Set to true only by the irqpoll thread, set to false
+ * by irqpoll and by the LT threads.
+ */
+ bool lt_stop_request;
+ bool lt_enabled;
+ bool local_tx_lt_done;
+ bool remote_tx_lt_done;
+ /* Serialize concurrent attempts from the local TX and remote TX
+ * kthreads to finalize their side of the link training
+ */
+ struct mutex lt_lock;
+ struct mutex an_restart_lock;
+};
+
+/* Auto-Negotiation Control and Status Registers are on page 0: 0x0 */
+static const u16 mtip_lx2160a_an_regs[] = {
+ [AN_CTRL] = 0,
+ [AN_STAT] = 1,
+ [AN_ADV_0] = 2,
+ [AN_ADV_1] = 3,
+ [AN_ADV_2] = 4,
+ [AN_LPA_0] = 5,
+ [AN_LPA_1] = 6,
+ [AN_LPA_2] = 7,
+ [AN_MS_CNT] = 8,
+ [AN_ADV_XNP_0] = 9,
+ [AN_ADV_XNP_1] = 10,
+ [AN_ADV_XNP_2] = 11,
+ [AN_LPA_XNP_0] = 12,
+ [AN_LPA_XNP_1] = 13,
+ [AN_LPA_XNP_2] = 14,
+ [AN_BP_ETH_STAT] = 15,
+};
+
+/* Link Training Control and Status Registers are on page 1: 256 = 0x100 */
+static const u16 mtip_lx2160a_lt_regs[] = {
+ [LT_CTRL] = 0x100,
+ [LT_STAT] = 0x101,
+ [LT_LP_COEF] = 0x102,
+ [LT_LP_STAT] = 0x103,
+ [LT_LD_COEF] = 0x104,
+ [LT_LD_STAT] = 0x105,
+ [LT_TRAIN_PATTERN] = 0x108,
+ [LT_RX_PATTERN] = 0x109,
+ [LT_RX_PATTERN_ERR] = 0x10a,
+ [LT_RX_PATTERN_BEGIN] = 0x10b,
+};
+
+/* Keep sorted in order of decreasing link speeds */
+static const enum ethtool_link_mode_bit_indices mtip_backplane_link_modes[] = {
+ ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
+ ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+ ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
+ ETHTOOL_LINK_MODE_25000baseKR_S_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+};
+
+static bool
+link_mode_needs_training(enum ethtool_link_mode_bit_indices link_mode)
+{
+ if (link_mode == ETHTOOL_LINK_MODE_1000baseKX_Full_BIT)
+ return false;
+
+ return true;
+}
+
+static int for_each_lane(int (*cb)(struct mtip_backplane *priv),
+ struct mtip_backplane *priv)
+{
+ int i, err;
+
+ err = cb(priv);
+ if (err)
+ return err;
+
+ for (i = 0; i < priv->num_subordinates; i++) {
+ err = cb(priv->subordinate[i]);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int for_each_lane_args(int (*cb)(struct mtip_backplane *priv,
+ void *args),
+ struct mtip_backplane *priv, void *args)
+{
+ int i, err;
+
+ err = cb(priv, args);
+ if (err)
+ return err;
+
+ for (i = 0; i < priv->num_subordinates; i++) {
+ err = cb(priv->subordinate[i], args);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int mtip_read_an(struct mtip_backplane *priv, enum mtip_an_reg reg)
+{
+ struct mdio_device *mdiodev = priv->mdiodev;
+
+ return mdiodev_c45_read(mdiodev, MDIO_MMD_AN, priv->an_regs[reg]);
+}
+
+static int mtip_write_an(struct mtip_backplane *priv, enum mtip_an_reg reg,
+ u16 val)
+{
+ struct mdio_device *mdiodev = priv->mdiodev;
+
+ return mdiodev_c45_write(mdiodev, MDIO_MMD_AN, priv->an_regs[reg],
+ val);
+}
+
+static int mtip_modify_an(struct mtip_backplane *priv, enum mtip_an_reg reg,
+ u16 mask, u16 set)
+{
+ struct mdio_device *mdiodev = priv->mdiodev;
+
+ return mdiodev_c45_modify(mdiodev, MDIO_MMD_AN, priv->an_regs[reg],
+ mask, set);
+}
+
+static int mtip_read_lt(struct mtip_backplane *priv, enum mtip_lt_reg reg)
+{
+ struct mdio_device *mdiodev = priv->mdiodev;
+
+ return mdiodev_c45_read(mdiodev, priv->lt_mmd, priv->lt_regs[reg]);
+}
+
+static int mtip_write_lt(struct mtip_backplane *priv, enum mtip_lt_reg reg,
+ u16 val)
+{
+ struct mdio_device *mdiodev = priv->mdiodev;
+
+ return mdiodev_c45_write(mdiodev, priv->lt_mmd, priv->lt_regs[reg],
+ val);
+}
+
+static int mtip_modify_lt(struct mtip_backplane *priv, enum mtip_lt_reg reg,
+ u16 mask, u16 set)
+{
+ struct mdio_device *mdiodev = priv->mdiodev;
+
+ return mdiodev_c45_modify(mdiodev, priv->lt_mmd, priv->lt_regs[reg],
+ mask, set);
+}
+
+static int mtip_read_pcs(struct mtip_backplane *priv, int reg)
+{
+ struct mdio_device *mdiodev = priv->pcs_mdiodev;
+
+ return mdiodev_c45_read(mdiodev, MDIO_MMD_PCS, reg);
+}
+
+static int mtip_reset_pcs(struct mtip_backplane *priv)
+{
+ struct mdio_device *mdiodev = priv->pcs_mdiodev;
+ int err, val;
+
+ err = mdiodev_c45_modify(mdiodev, MDIO_MMD_PCS, MDIO_CTRL1,
+ MDIO_CTRL1_RESET, MDIO_CTRL1_RESET);
+ if (err < 0)
+ return err;
+
+ err = read_poll_timeout(mdiodev_c45_read, val,
+ val < 0 || !(val & MDIO_CTRL1_RESET),
+ MTIP_RESET_SLEEP_US, MTIP_RESET_TIMEOUT_US,
+ false, mdiodev, MDIO_MMD_PCS, MDIO_CTRL1);
+
+ return (val < 0) ? val : err;
+}
+
+static int mtip_reset_an(struct mtip_backplane *priv)
+{
+ int err, val;
+
+ err = mtip_modify_an(priv, AN_CTRL, MDIO_CTRL1_RESET,
+ MDIO_CTRL1_RESET);
+ if (err < 0)
+ return err;
+
+ err = read_poll_timeout(mtip_read_an, val,
+ val < 0 || !(val & MDIO_CTRL1_RESET),
+ MTIP_RESET_SLEEP_US, MTIP_RESET_TIMEOUT_US,
+ false, priv, AN_CTRL);
+
+ return (val < 0) ? val : err;
+}
+
+static int mtip_check_cdr_lock(struct mtip_backplane *priv,
+ bool *all_lanes_have_cdr_lock)
+{
+ union phy_status_opts opts = {};
+ int i, err;
+
+ err = phy_get_status(priv->serdes, PHY_STATUS_CDR_LOCK, &opts);
+ if (err)
+ return err;
+
+ *all_lanes_have_cdr_lock = opts.cdr.cdr_locked;
+
+ /* Until C73 resolves a link mode, only the primary lane needs
+ * to have CDR lock. The others may even be powered off.
+ */
+ if (!priv->link_mode_resolved || !*all_lanes_have_cdr_lock)
+ return 0;
+
+ for (i = 0; i < priv->num_subordinates; i++) {
+ err = phy_get_status(priv->subordinate[i]->serdes,
+ PHY_STATUS_CDR_LOCK, &opts);
+ if (err)
+ return err;
+
+ if (!opts.cdr.cdr_locked) {
+ *all_lanes_have_cdr_lock = false;
+ return 0;
+ }
+ }
+
+ return 0;
+}
+
+static int mtip_wait_for_cdr_lock(struct mtip_backplane *priv)
+{
+ bool cdr_locked;
+ int err, val;
+
+ err = read_poll_timeout(mtip_check_cdr_lock, val,
+ val < 0 || cdr_locked,
+ MTIP_CDR_SLEEP_US, MTIP_CDR_TIMEOUT_US,
+ false, priv, &cdr_locked);
+
+ return (val < 0) ? val : err;
+}
+
+int mtip_backplane_validate(struct mtip_backplane *priv,
+ unsigned long *supported)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mtip_supported) = {};
+ const enum ethtool_link_mode_bit_indices *link_modes;
+ int i, err;
+
+ linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, mtip_supported);
+ linkmode_set_bit(ETHTOOL_LINK_MODE_Backplane_BIT, mtip_supported);
+ linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, mtip_supported);
+ linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, mtip_supported);
+
+ link_modes = mtip_backplane_link_modes;
+
+ /* Ask the SerDes driver what link modes are supported,
+ * based on the current PLL configuration.
+ */
+ for (i = 0; i < ARRAY_SIZE(mtip_backplane_link_modes); i++) {
+ err = phy_validate(priv->serdes, PHY_MODE_ETHTOOL,
+ link_modes[i], NULL);
+ if (err)
+ continue;
+
+ linkmode_set_bit(link_modes[i], mtip_supported);
+ }
+
+ linkmode_and(supported, supported, mtip_supported);
+
+ return 0;
+}
+EXPORT_SYMBOL(mtip_backplane_validate);
+
+static void mtip_start_irqpoll(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+
+ if (priv->is_subordinate)
+ return;
+
+ schedule_delayed_work(&irqpoll->work, 0);
+}
+
+static void mtip_stop_irqpoll(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+
+ if (priv->is_subordinate)
+ return;
+
+ cancel_delayed_work_sync(&irqpoll->work);
+}
+
+static void mtip_run_irqpoll_once(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+
+ if (priv->is_subordinate)
+ return;
+
+ irqpoll->run_once = true;
+ schedule_delayed_work(&irqpoll->work, 0);
+}
+
+int mtip_backplane_suspend(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ mtip_stop_irqpoll(priv);
+
+ err = phy_power_off(priv->serdes);
+ if (err) {
+ dev_err(dev, "Failed to power off SerDes: %pe\n",
+ ERR_PTR(err));
+ return err;
+ }
+
+ priv->lane_powered_on = false;
+
+ /* The link will drop via the CDR lock check after we power off the
+ * SerDes lane, and that is not latched low. So we need to schedule
+ * the irqpoll thread once more, so that we don't miss the event.
+ */
+ mtip_run_irqpoll_once(priv);
+
+ return 0;
+}
+EXPORT_SYMBOL(mtip_backplane_suspend);
+
+int mtip_backplane_resume(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ err = phy_power_on(priv->serdes);
+ if (err) {
+ dev_err(dev, "Failed to power on SerDes: %pe\n", ERR_PTR(err));
+ return err;
+ }
+
+ priv->lane_powered_on = true;
+
+ if (!priv->is_subordinate)
+ mtip_start_irqpoll(priv);
+
+ return 0;
+}
+EXPORT_SYMBOL(mtip_backplane_resume);
+
+/* Our LT_LP_STAT register updates only after receiving training frames, so
+ * unless we wait for it to lock, there is a risk that after a renegotiation,
+ * we act upon information from the previous link training process.
+ */
+static int mtip_lt_frame_lock(struct mtip_backplane *priv)
+{
+ int err, val;
+
+ err = read_poll_timeout(mtip_read_lt, val,
+ val < 0 || (val & LT_STAT_FRAME_LOCK),
+ MTIP_FRAME_LOCK_SLEEP_US, MTIP_FRAME_LOCK_TIMEOUT_US,
+ false, priv, LT_STAT);
+
+ return (val < 0) ? val : err;
+}
+
+static int mtip_restart_lt(struct mtip_backplane *priv, bool enable)
+{
+ u16 mask = LT_CTRL_RESTART_TRAINING | LT_CTRL_TRAINING_ENABLE;
+ u16 set = LT_CTRL_RESTART_TRAINING;
+ int err, val;
+
+ if (enable)
+ set |= LT_CTRL_TRAINING_ENABLE;
+
+ err = mtip_modify_lt(priv, LT_CTRL, mask, set);
+ if (err < 0)
+ return err;
+
+ err = read_poll_timeout(mtip_read_lt, val,
+ val < 0 || !(val & LT_CTRL_RESTART_TRAINING),
+ MTIP_LT_RESTART_SLEEP_US, MTIP_LT_RESTART_TIMEOUT_US,
+ false, priv, LT_CTRL);
+
+ return (val < 0) ? val : err;
+}
+
+/* Enable the lane datapath by disconnecting it from the AN/LT block
+ * and connecting it to the PCS. This is called both from the irqpoll thread,
+ * as well as from the last link training kthread to finish.
+ */
+static int mtip_finish_lt(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ if (!priv->lt_enabled)
+ return 0;
+
+ err = mtip_restart_lt(priv, false);
+ if (err) {
+ dev_err(dev, "Failed to disable link training: %pe\n",
+ ERR_PTR(err));
+ return err;
+ }
+
+ /* Subsequent attempts to disable LT will time out, so stop them */
+ priv->lt_enabled = false;
+
+ return 0;
+}
+
+static struct mtip_irqpoll *mtip_get_irqpoll(struct mtip_backplane *priv)
+{
+ if (priv->is_subordinate)
+ return &priv->coordinator->irqpoll;
+
+ return &priv->irqpoll;
+}
+
+static int mtip_stop_lt(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = mtip_get_irqpoll(priv);
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ lockdep_assert_held(&irqpoll->lock);
+
+ kthread_flush_worker(priv->remote_tx_lt_worker);
+ kthread_flush_worker(priv->local_tx_lt_worker);
+
+ err = mtip_finish_lt(priv);
+ if (err)
+ return err;
+
+ dev_dbg(dev, "Link training disabled\n");
+
+ return 0;
+}
+
+static int mtip_reset_lt(struct mtip_backplane *priv)
+{
+ int err;
+
+ /* Don't allow AN to complete without training */
+ err = mtip_modify_lt(priv, LT_STAT, LT_STAT_RX_STATUS, 0);
+ if (err < 0)
+ return err;
+
+ err = mtip_write_lt(priv, LT_LD_COEF, 0);
+ if (err < 0)
+ return err;
+
+ err = mtip_write_lt(priv, LT_LD_STAT, 0);
+ if (err < 0)
+ return err;
+
+ priv->any_request_received = false;
+
+ return 0;
+}
+
+/* Reset state when detecting that the previously determined link mode
+ * is no longer valid
+ */
+static int mtip_state_reset(struct mtip_backplane *priv)
+{
+ int i, err;
+
+ priv->link_mode_resolved = false;
+
+ err = for_each_lane(mtip_stop_lt, priv);
+ if (err)
+ return err;
+
+ err = for_each_lane(mtip_reset_lt, priv);
+ if (err < 0)
+ return err;
+
+ /* 802.3-2018 clause 73.5.1 recommends: "For any multi-lane PHY, DME
+ * pages shall be transmitted only on lane 0. The transmitters on other
+ * lanes should be disabled". Let's do that and only keep them enabled
+ * when the link mode has been resolved.
+ */
+ for (i = 0; i < priv->num_subordinates; i++) {
+ if (priv->subordinate[i]->lane_powered_on) {
+ err = mtip_backplane_suspend(priv->subordinate[i]);
+ if (err)
+ return err;
+ }
+ }
+
+ priv->local_tx_lt_done = false;
+ priv->remote_tx_lt_done = false;
+ priv->lt_stop_request = false;
+
+ /* If we received a new base page and the local link training threads
+ * also requested an autoneg restart, it's pointless to fulfill it now.
+ * Clear the request here, after the link training was stopped, so that
+ * we don't race with it.
+ */
+ priv->an_restart_pending = false;
+
+ return 0;
+}
+
+/* Make sure we don't act on old event bits from previous runs when
+ * we restart autoneg.
+ */
+static int mtip_unlatch_an_stat(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+ int val;
+
+ lockdep_assert_held(&irqpoll->lock);
+
+ val = mtip_read_an(priv, AN_STAT);
+ if (val < 0)
+ return val;
+
+ /* Discard the current AN status, it will become invalid soon */
+ irqpoll->old_an_stat = 0;
+
+ return 0;
+}
+
+/* Suppress a "PCS link dropped, restarting autoneg" event when initiating
+ * an autoneg restart locally.
+ */
+static int mtip_unlatch_pcs_stat(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+
+ irqpoll->old_pcs_stat = 0;
+
+ return 0;
+}
+
+static int mtip_read_adv(struct mtip_backplane *priv, u64 *base_page)
+{
+ int val;
+
+ val = mtip_read_an(priv, AN_ADV_0);
+ if (val < 0)
+ return val;
+
+ *base_page = (u64)val;
+
+ val = mtip_read_an(priv, AN_ADV_1);
+ if (val < 0)
+ return val;
+
+ *base_page |= (u64)val << 16;
+
+ val = mtip_read_an(priv, AN_ADV_2);
+ if (val < 0)
+ return val;
+
+ *base_page |= (u64)val << 32;
+
+ return 0;
+}
+
+static int mtip_write_adv(struct mtip_backplane *priv, u64 base_page)
+{
+ int val;
+
+ val = mtip_write_an(priv, AN_ADV_0, C73_ADV_0(base_page));
+ if (val < 0)
+ return val;
+
+ val = mtip_write_an(priv, AN_ADV_1, C73_ADV_1(base_page));
+ if (val < 0)
+ return val;
+
+ val = mtip_write_an(priv, AN_ADV_2, C73_ADV_2(base_page));
+ if (val < 0)
+ return val;
+
+ return 0;
+}
+
+static int mtip_read_lpa(struct mtip_backplane *priv, u64 *base_page)
+{
+ int val;
+
+ val = mtip_read_an(priv, AN_LPA_0);
+ if (val < 0)
+ return val;
+
+ *base_page = (u64)val;
+
+ val = mtip_read_an(priv, AN_LPA_1);
+ if (val < 0)
+ return val;
+
+ *base_page |= (u64)val << 16;
+
+ val = mtip_read_an(priv, AN_LPA_2);
+ if (val < 0)
+ return val;
+
+ *base_page |= (u64)val << 32;
+
+ return 0;
+}
+
+static int mtip_config_an_adv(struct mtip_backplane *priv,
+ const unsigned long *advertising)
+{
+ u64 base_page = linkmode_adv_to_c73_base_page(advertising);
+ u8 nonce;
+
+ /* The transmitted nonce must not be equal with the one transmitted by
+ * the link partner, otherwise AN will not complete (nonce_match=true).
+ */
+ get_random_bytes(&nonce, sizeof(nonce));
+
+ base_page |= C73_BASE_PAGE_TRANSMITTED_NONCE(nonce);
+ /* According to Annex 28A, set Selector to "IEEE 802.3" */
+ base_page |= C73_BASE_PAGE_SELECTOR(1);
+ /* C73_BASE_PAGE_ACK and C73_BASE_PAGE_ECHOED_NONCE seem to have
+ * a life of their own, regardless of what we set them to.
+ */
+
+ return mtip_write_adv(priv, base_page);
+}
+
+static int mtip_an_restart(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ dev_dbg(dev, "Link training requests autoneg restart\n");
+
+ err = mtip_state_reset(priv);
+ if (err)
+ return err;
+
+ /* Make sure AN is temporarily disabled, so that we can safely
+ * unlatch the previous status without losing real events
+ */
+ err = mtip_reset_an(priv);
+ if (err < 0)
+ return err;
+
+ err = mtip_unlatch_an_stat(priv);
+ if (err)
+ return err;
+
+ err = mtip_unlatch_pcs_stat(priv);
+ if (err)
+ return err;
+
+ err = mtip_modify_an(priv, AN_CTRL,
+ MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART,
+ MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART);
+ if (err < 0)
+ return err;
+
+ priv->last_an_restart = jiffies;
+ priv->an_restart_pending = false;
+
+ return 0;
+}
+
+void mtip_backplane_an_restart(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ mutex_lock(&irqpoll->lock);
+
+ if (!priv->an_enabled)
+ goto out_unlock;
+
+ err = mtip_an_restart(priv);
+ if (err)
+ dev_err(dev, "Failed to restart backplane autoneg: %pe\n",
+ ERR_PTR(err));
+
+out_unlock:
+ mutex_unlock(&irqpoll->lock);
+}
+EXPORT_SYMBOL(mtip_backplane_an_restart);
+
+/* The reason for deferral is that the irqpoll thread waits for the LT kthreads
+ * to finish with irqpoll->lock held, and AN restart also requires holding the
+ * irqpoll->lock. So the kthreads cannot directly restart autoneg without
+ * deadlocking with the irqpoll thread, they must signal to the irqpoll thread
+ * to do so.
+ */
+static void mtip_an_restart_from_lt(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ struct mtip_backplane *coordinator;
+
+ dev_dbg(dev, "Link training requests autoneg restart\n");
+
+ coordinator = priv->is_subordinate ? priv->coordinator : priv;
+
+ mutex_lock(&coordinator->an_restart_lock);
+ coordinator->an_restart_pending = true;
+ mutex_unlock(&coordinator->an_restart_lock);
+}
+
+static int mtip_wait_for_lt_end(struct mtip_backplane *priv)
+{
+ int err, val;
+
+ err = read_poll_timeout(mtip_read_lt, val,
+ val < 0 || !(val & LT_STAT_STARTUP_PROTOCOL_STATUS),
+ MTIP_LT_END_SLEEP_US, MTIP_LT_END_TIMEOUT_US,
+ false, priv, LT_STAT);
+
+ return (val < 0) ? val : err;
+}
+
+static int mtip_finalize_lt(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ union phy_configure_opts opts = {
+ .ethernet = {
+ .type = C72_LT_DONE,
+ },
+ };
+ int err, val;
+
+ lockdep_assert_held(&priv->lt_lock);
+
+ if (!priv->local_tx_lt_done || !priv->remote_tx_lt_done)
+ return 0;
+
+ priv->last_lt_done = jiffies;
+
+ /* Let the local state machine know we're done */
+ err = mtip_modify_lt(priv, LT_STAT, LT_STAT_RX_STATUS,
+ LT_STAT_RX_STATUS);
+ if (err < 0) {
+ dev_err(dev, "Failed to update LT_STAT: %pe\n", ERR_PTR(err));
+ return err;
+ }
+
+ /* Give some time for the LP to see our training frames
+ * with "RX ready", before disabling link training.
+ */
+ err = mtip_wait_for_lt_end(priv);
+ if (err) {
+ /* With 25G, this can often be seen to fail, but it seems
+ * inconsequential, so ignore it
+ */
+ dev_warn(dev,
+ "Failed to wait for the Start-up Protocol Status bit to clear: %pe, LT_STAT = 0x%x\n",
+ ERR_PTR(err), mtip_read_lt(priv, LT_STAT));
+ }
+
+ err = mtip_finish_lt(priv);
+ if (err)
+ return err;
+
+ val = mtip_read_lt(priv, LT_STAT);
+ if (val < 0)
+ return val;
+
+ if (!(val & LT_STAT_SIGNAL_DETECT)) {
+ dev_err(dev, "Link training did not succeed: LT_STAT = 0x%x\n",
+ val);
+ return -ENETDOWN;
+ }
+
+ return phy_configure(priv->serdes, &opts);
+}
+
+static int mtip_tx_c72_coef_update(struct mtip_backplane *priv,
+ const struct c72_coef_update *upd,
+ struct c72_coef_status *stat)
+{
+ char upd_buf[C72_COEF_UPDATE_BUFSIZ], stat_buf[C72_COEF_STATUS_BUFSIZ];
+ struct device *dev = &priv->mdiodev->dev;
+ int err, val;
+ u16 ld_coef;
+
+ c72_coef_update_print(upd, upd_buf);
+
+ err = read_poll_timeout(mtip_read_lt, val,
+ val < 0 || LT_COEF_STAT_ALL_NOT_UPDATED(val),
+ MTIP_COEF_STAT_SLEEP_US, MTIP_COEF_STAT_TIMEOUT_US,
+ false, priv, LT_LP_STAT);
+ if (val < 0)
+ return val;
+ if (err) {
+ dev_err(dev,
+ "LP did not set coefficient status to NOT_UPDATED, couldn't send %s; LP_STAT = 0x%x\n",
+ upd_buf, val);
+ return err;
+ }
+
+ ld_coef = LT_COM1(upd->com1) | LT_COZ(upd->coz) | LT_COP1(upd->cop1);
+ if (upd->init)
+ ld_coef |= LT_COEF_UPD_INIT;
+ if (upd->preset)
+ ld_coef |= LT_COEF_UPD_PRESET;
+
+ err = mtip_write_lt(priv, LT_LD_COEF, ld_coef);
+ if (err < 0)
+ return err;
+
+ err = read_poll_timeout(mtip_read_lt, val,
+ val < 0 || LT_COEF_STAT_ANY_UPDATED(val),
+ MTIP_COEF_STAT_SLEEP_US, MTIP_COEF_STAT_TIMEOUT_US,
+ false, priv, LT_LP_STAT);
+ if (val < 0)
+ return val;
+ if (err) {
+ dev_err(dev,
+ "LP did not update coefficient status for %s; LP_STAT = 0x%x\n",
+ upd_buf, val);
+ return err;
+ }
+
+ stat->com1 = LT_COM1_X(val);
+ stat->coz = LT_COZ_X(val);
+ stat->cop1 = LT_COP1_X(val);
+ c72_coef_status_print(stat, stat_buf);
+
+ ld_coef = LT_COM1(COEF_UPD_HOLD) | LT_COZ(COEF_UPD_HOLD) |
+ LT_COP1(COEF_UPD_HOLD);
+ err = mtip_write_lt(priv, LT_LD_COEF, ld_coef);
+ if (err < 0)
+ return err;
+
+ dev_dbg(dev, "Sent update: %s, got status: %s\n", upd_buf, stat_buf);
+
+ return 0;
+}
+
+static int mtip_c72_process_request(struct mtip_backplane *priv,
+ const struct c72_coef_update *upd,
+ struct c72_coef_status *stat)
+{
+ union phy_configure_opts opts = {
+ .ethernet = {
+ .type = C72_LOCAL_TX,
+ .local_tx = {
+ .update = *upd,
+ },
+ },
+ };
+ int err;
+
+ err = phy_configure(priv->serdes, &opts);
+ if (err)
+ return err;
+
+ *stat = opts.ethernet.local_tx.status;
+ priv->any_request_received = true;
+
+ return 0;
+}
+
+static int mtip_read_lt_lp_coef_if_not_ready(struct mtip_backplane *priv,
+ bool *rx_ready)
+{
+ int val;
+
+ val = mtip_read_lt(priv, LT_LP_STAT);
+ if (val < 0)
+ return val;
+
+ *rx_ready = !!(val & LT_COEF_STAT_RX_READY);
+ if (*rx_ready)
+ return 0;
+
+ return mtip_read_lt(priv, LT_LP_COEF);
+}
+
+static int mtip_rx_c72_coef_update(struct mtip_backplane *priv,
+ struct c72_coef_update *upd,
+ bool *rx_ready)
+{
+ char upd_buf[C72_COEF_UPDATE_BUFSIZ], stat_buf[C72_COEF_STATUS_BUFSIZ];
+ struct device *dev = &priv->mdiodev->dev;
+ struct c72_coef_status stat = {};
+ int err, val;
+
+ err = read_poll_timeout(mtip_read_lt_lp_coef_if_not_ready,
+ val, val < 0 || *rx_ready || LT_COEF_UPD_ANYTHING(val),
+ MTIP_COEF_STAT_SLEEP_US, MTIP_COEF_STAT_TIMEOUT_US,
+ false, priv, rx_ready);
+ if (val < 0)
+ return val;
+ if (*rx_ready) {
+ if (!priv->any_request_received)
+ dev_warn(dev,
+ "LP says its RX is ready, but there was no coefficient request (LP_STAT = 0x%x, LD_STAT = 0x%x)\n",
+ mtip_read_lt(priv, LT_LP_STAT),
+ mtip_read_lt(priv, LT_LD_STAT));
+ else
+ dev_dbg(dev, "LP says its RX is ready\n");
+ return 0;
+ }
+ if (err) {
+ dev_err(dev,
+ "LP did not request coefficient updates; LP_COEF = 0x%x\n",
+ val);
+ return err;
+ }
+
+ upd->com1 = LT_COM1_X(val);
+ upd->coz = LT_COZ_X(val);
+ upd->cop1 = LT_COP1_X(val);
+ upd->init = !!(val & LT_COEF_UPD_INIT);
+ upd->preset = !!(val & LT_COEF_UPD_PRESET);
+ c72_coef_update_print(upd, upd_buf);
+
+ if ((upd->com1 || upd->coz || upd->cop1) + upd->init + upd->preset > 1) {
+ dev_warn(dev, "Ignoring illegal request: %s (LP_COEF = 0x%x)\n",
+ upd_buf, val);
+ return 0;
+ }
+
+ err = mtip_c72_process_request(priv, upd, &stat);
+ if (err)
+ return err;
+
+ c72_coef_status_print(&stat, stat_buf);
+ dev_dbg(dev, "Received update: %s, responded with status: %s\n",
+ upd_buf, stat_buf);
+
+ err = mtip_modify_lt(priv, LT_LD_STAT, LT_COEF_STAT_MASK,
+ LT_COM1(stat.com1) | LT_COZ(stat.coz) |
+ LT_COP1(stat.cop1));
+ if (err < 0)
+ return err;
+
+ err = read_poll_timeout(mtip_read_lt, val,
+ val < 0 || LT_COEF_UPD_NOTHING(val),
+ MTIP_COEF_STAT_SLEEP_US, MTIP_COEF_STAT_TIMEOUT_US,
+ false, priv, LT_LP_COEF);
+ if (val < 0)
+ return val;
+ if (err) {
+ dev_err(dev, "LP did not revert to HOLD; LP_COEF = 0x%x\n",
+ val);
+ return err;
+ }
+
+ err = mtip_modify_lt(priv, LT_LD_STAT, LT_COEF_STAT_MASK,
+ LT_COM1(COEF_STAT_NOT_UPDATED) |
+ LT_COZ(COEF_STAT_NOT_UPDATED) |
+ LT_COP1(COEF_STAT_NOT_UPDATED));
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int mtip_local_tx_lt_done(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ mutex_lock(&priv->lt_lock);
+
+ priv->local_tx_lt_done = true;
+
+ err = mtip_finalize_lt(priv);
+ if (!err)
+ dev_dbg(dev, "Link training for local TX done\n");
+
+ mutex_unlock(&priv->lt_lock);
+
+ return err;
+}
+
+static int mtip_remote_tx_lt_done(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ mutex_lock(&priv->lt_lock);
+
+ priv->remote_tx_lt_done = true;
+
+ err = mtip_finalize_lt(priv);
+ if (!err)
+ dev_dbg(dev, "Link training for remote TX done\n");
+
+ mutex_unlock(&priv->lt_lock);
+
+ return err;
+}
+
+/* This is our hardware-based 500 ms timer for the link training */
+static int mtip_lt_in_progress(struct mtip_backplane *priv)
+{
+ int val;
+
+ val = mtip_read_lt(priv, LT_STAT);
+ if (val < 0)
+ return val;
+
+ return !!(val & LT_STAT_TRAINING_FAILURE) ? -ETIMEDOUT : 0;
+}
+
+/* Make adjustments to the local TX according to link partner requests,
+ * so that its RX improves
+ */
+static void mtip_local_tx_lt_work(struct kthread_work *work)
+{
+ struct mtip_lt_work *lt_work = container_of(work, struct mtip_lt_work,
+ work);
+ struct mtip_backplane *priv = lt_work->priv;
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ while (!READ_ONCE(priv->lt_stop_request)) {
+ struct c72_coef_update upd = {};
+ bool rx_ready;
+
+ err = mtip_lt_in_progress(priv);
+ if (err) {
+ dev_err(dev, "Local TX LT failed: %pe\n", ERR_PTR(err));
+ break;
+ }
+
+ err = mtip_rx_c72_coef_update(priv, &upd, &rx_ready);
+ if (err)
+ goto out;
+
+ if (rx_ready) {
+ err = mtip_local_tx_lt_done(priv);
+ if (err) {
+ dev_err(dev, "Failed to finalize local LT: %pe\n",
+ ERR_PTR(err));
+ goto out;
+ }
+ break;
+ }
+ }
+
+out:
+ if (err && !READ_ONCE(priv->lt_stop_request))
+ mtip_an_restart_from_lt(priv);
+
+ kfree(lt_work);
+}
+
+/* Train the link partner TX, so that the local RX quality improves */
+static void mtip_remote_tx_lt_work(struct kthread_work *work)
+{
+ struct mtip_lt_work *lt_work = container_of(work, struct mtip_lt_work,
+ work);
+ struct mtip_backplane *priv = lt_work->priv;
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ while (true) {
+ struct c72_coef_status status = {};
+ union phy_configure_opts opts = {
+ .ethernet = {
+ .type = C72_REMOTE_TX,
+ },
+ };
+
+ if (READ_ONCE(priv->lt_stop_request))
+ goto out;
+
+ err = mtip_lt_in_progress(priv);
+ if (err) {
+ dev_err(dev, "Remote TX LT failed: %pe\n", ERR_PTR(err));
+ goto out;
+ }
+
+ err = phy_configure(priv->serdes, &opts);
+ if (err) {
+ dev_err(dev,
+ "Failed to get remote TX training request from SerDes: %pe\n",
+ ERR_PTR(err));
+ goto out;
+ }
+
+ if (opts.ethernet.remote_tx.rx_ready)
+ break;
+
+ err = mtip_tx_c72_coef_update(priv, &opts.ethernet.remote_tx.update,
+ &status);
+ if (opts.ethernet.remote_tx.cb)
+ opts.ethernet.remote_tx.cb(opts.ethernet.remote_tx.cb_priv,
+ err, opts.ethernet.remote_tx.update,
+ status);
+ if (err)
+ goto out;
+ }
+
+ /* Let the link partner know we're done */
+ err = mtip_modify_lt(priv, LT_LD_STAT, LT_COEF_STAT_RX_READY,
+ LT_COEF_STAT_RX_READY);
+ if (err < 0) {
+ dev_err(dev, "Failed to update LT_LD_STAT: %pe\n",
+ ERR_PTR(err));
+ goto out;
+ }
+
+ err = mtip_remote_tx_lt_done(priv);
+ if (err) {
+ dev_err(dev, "Failed to finalize remote LT: %pe\n",
+ ERR_PTR(err));
+ goto out;
+ }
+
+out:
+ if (err && !READ_ONCE(priv->lt_stop_request))
+ mtip_an_restart_from_lt(priv);
+
+ kfree(lt_work);
+}
+
+static int mtip_start_lt(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = mtip_get_irqpoll(priv);
+ struct device *dev = &priv->mdiodev->dev;
+ struct mtip_lt_work *remote_tx_lt_work;
+ struct mtip_lt_work *local_tx_lt_work;
+ int err;
+
+ lockdep_assert_held(&irqpoll->lock);
+
+ local_tx_lt_work = kzalloc(sizeof(*local_tx_lt_work), GFP_KERNEL);
+ if (!local_tx_lt_work) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ remote_tx_lt_work = kzalloc(sizeof(*remote_tx_lt_work), GFP_KERNEL);
+ if (!remote_tx_lt_work) {
+ err = -ENOMEM;
+ goto out_free_local_tx_lt;
+ }
+
+ err = mtip_reset_lt(priv);
+ if (err)
+ goto out_free_remote_tx_lt;
+
+ err = mtip_restart_lt(priv, true);
+ if (err)
+ goto out_free_remote_tx_lt;
+
+ priv->lt_enabled = true;
+
+ dev_dbg(dev, "Link training enabled\n");
+
+ err = mtip_lt_frame_lock(priv);
+ if (err) {
+ dev_err(dev,
+ "Failed to acquire training frame delineation: %pe\n",
+ ERR_PTR(err));
+ goto out_stop_lt;
+ }
+
+ local_tx_lt_work->priv = priv;
+ kthread_init_work(&local_tx_lt_work->work, mtip_local_tx_lt_work);
+ kthread_queue_work(priv->local_tx_lt_worker, &local_tx_lt_work->work);
+
+ remote_tx_lt_work->priv = priv;
+ kthread_init_work(&remote_tx_lt_work->work, mtip_remote_tx_lt_work);
+ kthread_queue_work(priv->remote_tx_lt_worker, &remote_tx_lt_work->work);
+
+ return 0;
+
+out_stop_lt:
+ mtip_finish_lt(priv);
+out_free_remote_tx_lt:
+ kfree(remote_tx_lt_work);
+out_free_local_tx_lt:
+ kfree(local_tx_lt_work);
+out:
+ dev_err(dev, "Failed to start link training: %pe\n", ERR_PTR(err));
+ return err;
+}
+
+/* Allow the datapath to come up without link training */
+static int mtip_bypass_lt(struct mtip_backplane *priv)
+{
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ err = mtip_modify_lt(priv, LT_STAT, LT_STAT_RX_STATUS,
+ LT_STAT_RX_STATUS);
+ if (err < 0) {
+ dev_err(dev, "Failed to bypass link training: %pe\n",
+ ERR_PTR(err));
+ return err;
+ }
+
+ return 0;
+}
+
+static void mtip_update_link_latch(struct mtip_backplane *priv,
+ bool cdr_locked, bool phy_los,
+ bool an_complete, bool pcs_lstat)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+ struct device *dev = &priv->mdiodev->dev;
+
+ lockdep_assert_held(&irqpoll->lock);
+
+ /* Update irqpoll->link if true, or if false
+ * and mtip_read_status() saw that already.
+ */
+ if (irqpoll->link || irqpoll->link_ack) {
+ irqpoll->link = phy_los && cdr_locked && an_complete && pcs_lstat;
+ irqpoll->link_ack = false;
+ }
+
+ dev_dbg(dev, "PCS link%s: %d, PHY LOS: %d, CDR locked: %d, AN complete: %d\n",
+ priv->link_mode_resolved ? "" : " (ignored)",
+ pcs_lstat, phy_los, cdr_locked, an_complete);
+}
+
+static bool mtip_cached_an_complete(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+
+ lockdep_assert_held(&irqpoll->lock);
+
+ return !!(irqpoll->old_an_stat & MDIO_AN_STAT1_COMPLETE);
+}
+
+static bool mtip_read_link_unlatch(struct mtip_backplane *priv)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+ bool old_link = irqpoll->link;
+
+ lockdep_assert_held(&irqpoll->lock);
+
+ /* A change to the link status may have occurred while a link
+ * loss was latched, so update it again after reading it
+ */
+ irqpoll->link = !!(irqpoll->old_an_stat & MDIO_STAT1_LSTATUS) &&
+ !!(irqpoll->old_an_stat & MDIO_AN_STAT1_COMPLETE) &&
+ !!(irqpoll->old_pcs_stat & MDIO_STAT1_LSTATUS) &&
+ irqpoll->cdr_locked;
+ irqpoll->link_ack = true;
+
+ return old_link;
+}
+
+static u16 mtip_expected_bp_eth_stat(enum ethtool_link_mode_bit_indices link_mode)
+{
+ switch (link_mode) {
+ case ETHTOOL_LINK_MODE_1000baseKX_Full_BIT:
+ return BP_ETH_STAT_ALWAYS_1 | BP_ETH_STAT_1GKX;
+ case ETHTOOL_LINK_MODE_10000baseKR_Full_BIT:
+ return BP_ETH_STAT_ALWAYS_1 | BP_ETH_STAT_10GKR;
+ case ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT:
+ return BP_ETH_STAT_ALWAYS_1 | BP_ETH_STAT_40GKR4;
+ case ETHTOOL_LINK_MODE_25000baseKR_Full_BIT:
+ return BP_ETH_STAT_ALWAYS_1 | BP_ETH_STAT_25GKR;
+ case ETHTOOL_LINK_MODE_25000baseKR_S_Full_BIT:
+ return BP_ETH_STAT_ALWAYS_1 | BP_ETH_STAT_25GKR_S;
+ case ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT:
+ return BP_ETH_STAT_ALWAYS_1 | BP_ETH_STAT_100GKR4;
+ default:
+ return 0;
+ }
+}
+
+static int mtip_wait_bp_eth_stat(struct mtip_backplane *priv,
+ enum ethtool_link_mode_bit_indices link_mode)
+{
+ u16 expected = mtip_expected_bp_eth_stat(link_mode);
+ struct device *dev = &priv->mdiodev->dev;
+ int err, val;
+
+ err = read_poll_timeout(mtip_read_an, val,
+ val < 0 || val == expected,
+ MTIP_BP_ETH_STAT_SLEEP_US,
+ MTIP_BP_ETH_STAT_TIMEOUT_US,
+ false, priv, AN_BP_ETH_STAT);
+ if (val < 0)
+ return val;
+
+ if (err) {
+ dev_warn(dev,
+ "BP_ETH_STAT did not become 0x%x to indicate resolved link mode %s, instead it shows 0x%x%s\n",
+ expected, ethtool_link_mode_str(link_mode), val,
+ val == BP_ETH_STAT_PARALLEL_DETECT ? " (parallel detection)" : "");
+ return err;
+ }
+
+ return 0;
+}
+
+static int mtip_switch_protocol(struct mtip_backplane *priv, void *args)
+{
+ const enum ethtool_link_mode_bit_indices *resolved = args;
+ struct device *dev = &priv->mdiodev->dev;
+ int err;
+
+ err = phy_set_mode_ext(priv->serdes, PHY_MODE_ETHTOOL, *resolved);
+ if (err) {
+ dev_err(dev, "phy_set_mode_ext(%s) returned %pe\n",
+ ethtool_link_mode_str(*resolved), ERR_PTR(err));
+ return err;
+ }
+
+ return 0;
+}
+
+static int mtip_c73_page_received(struct mtip_backplane *priv, bool *restart_an)
+{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
+ enum ethtool_link_mode_bit_indices resolved;
+ struct device *dev = &priv->mdiodev->dev;
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+ u64 base_page, lpa;
+ int i, err;
+
+ err = mtip_state_reset(priv);
+ if (err)
+ return err;
+
+ err = mtip_read_adv(priv, &base_page);
+ if (err < 0)
+ return err;
+
+ err = mtip_read_lpa(priv, &lpa);
+ if (err < 0)
+ return err;
+
+ if (lpa & C73_BASE_PAGE_NP)
+ dev_warn(dev, "Next Page exchange not implemented!\n");
+
+ mii_c73_mod_linkmode_lpa_t(advertising, base_page);
+ mii_c73_mod_linkmode_lpa_t(lp_advertising, lpa);
+ linkmode_and(common, advertising, lp_advertising);
+
+ err = linkmode_c73_priority_resolution(common, &resolved);
+ if (err) {
+ dev_warn(dev, "C73 page received, no common link mode\n");
+ *restart_an = true;
+ return 0;
+ }
+
+ err = mtip_wait_bp_eth_stat(priv, resolved);
+ if (err) {
+ *restart_an = true;
+ return 0;
+ }
+
+ dev_dbg(dev,
+ "C73 page received, LD %04x:%04x:%04x, LP %04x:%04x:%04x, resolved link mode %s\n",
+ C73_ADV_2(base_page), C73_ADV_1(base_page), C73_ADV_0(base_page),
+ C73_ADV_2(lpa), C73_ADV_1(lpa), C73_ADV_0(lpa),
+ ethtool_link_mode_str(resolved));
+
+ err = for_each_lane_args(mtip_switch_protocol, priv, &resolved);
+ if (err)
+ return err;
+
+ for (i = 0; i < priv->num_subordinates; i++) {
+ if (!priv->subordinate[i]->lane_powered_on) {
+ err = mtip_backplane_resume(priv->subordinate[i]);
+ if (err)
+ return err;
+ }
+ }
+
+ err = mtip_wait_for_cdr_lock(priv);
+ if (err) {
+ dev_warn(dev, "Failed to reacquire CDR lock after protocol change: %pe\n",
+ ERR_PTR(err));
+ *restart_an = true;
+ return 0;
+ }
+
+ if (link_mode_needs_training(resolved)) {
+ err = for_each_lane(mtip_start_lt, priv);
+ if (err) { // FIXME return err
+ /* mtip_an_restart() -> mtip_state_reset()
+ * will clean up and stop the kthreads for the lanes
+ * where link training may have already started
+ */
+ *restart_an = true;
+ return 0;
+ }
+ } else {
+ err = for_each_lane(mtip_bypass_lt, priv);
+ if (err)
+ return err;
+ }
+
+ priv->link_mode = resolved;
+ priv->link_mode_resolved = true;
+
+ return 0;
+}
+
+static void mtip_c73_remote_fault(struct mtip_backplane *priv, bool fault)
+{
+ struct device *dev = &priv->mdiodev->dev;
+
+ dev_err(dev, "Remote fault: %d\n", fault);
+}
+
+static bool mtip_are_all_lanes_trained(struct mtip_backplane *priv)
+{
+ int i;
+
+ if (!priv->local_tx_lt_done || !priv->remote_tx_lt_done)
+ return false;
+
+ for (i = 0; i < priv->num_subordinates; i++) {
+ if (!priv->subordinate[i]->local_tx_lt_done ||
+ !priv->subordinate[i]->remote_tx_lt_done)
+ return false;
+ }
+
+ return true;
+}
+
+static void mtip_irqpoll_work(struct work_struct *work)
+{
+ struct mtip_irqpoll *irqpoll = container_of(work, struct mtip_irqpoll, work.work);
+ struct mtip_backplane *priv = container_of(irqpoll, struct mtip_backplane, irqpoll);
+ struct device *dev = &priv->mdiodev->dev;
+ bool all_lanes_have_cdr_lock;
+ bool restart_an = false;
+ bool new_page = false;
+ int val, err = 0;
+ int pcs_stat = 0;
+
+ /* Check for AN restart requests from the link training kthreads */
+ mutex_lock(&priv->an_restart_lock);
+ if (priv->an_restart_pending) {
+ restart_an = true;
+ priv->an_restart_pending = false;
+ }
+ mutex_unlock(&priv->an_restart_lock);
+
+ /* Then enter the irqpoll logic per se
+ * (PCS MDIO_STAT1, AN/LT MDIO_STAT1 and CDR lock)
+ */
+ mutex_lock(&irqpoll->lock);
+
+ err = mtip_check_cdr_lock(priv, &all_lanes_have_cdr_lock);
+ if (err)
+ goto out_unlock;
+
+ if (priv->link_mode_resolved) {
+ pcs_stat = mtip_read_pcs(priv, MDIO_STAT1);
+ if (pcs_stat < 0) {
+ err = pcs_stat;
+ goto out_unlock;
+ }
+ }
+
+ val = mtip_read_an(priv, AN_STAT);
+ if (val < 0) {
+ err = val;
+ goto out_unlock;
+ }
+
+ if ((irqpoll->cdr_locked != all_lanes_have_cdr_lock) ||
+ ((irqpoll->old_an_stat ^ val) & (MDIO_STAT1_LSTATUS |
+ MDIO_AN_STAT1_COMPLETE)) ||
+ ((irqpoll->old_pcs_stat ^ pcs_stat) & MDIO_STAT1_LSTATUS)) {
+ mtip_update_link_latch(priv, all_lanes_have_cdr_lock,
+ !!(val & MDIO_STAT1_LSTATUS),
+ !!(val & MDIO_AN_STAT1_COMPLETE),
+ !!(pcs_stat & MDIO_STAT1_LSTATUS));
+ }
+
+ /* The manual says that this bit is latched high, but experimentation
+ * shows that reads will not unlatch it while link training is in
+ * progress; only reading it after link training has completed will.
+ * Only act upon bit transitions, to avoid processing a false "page
+ * received" event during link training.
+ */
+ if (((irqpoll->old_an_stat ^ val) & MDIO_AN_STAT1_PAGE) &&
+ (val & MDIO_AN_STAT1_PAGE) && !restart_an) {
+ /* When we had a link and the LP retriggers autoneg, we first
+ * detect a new base page and resolve the link mode properly,
+ * then we see that the PCS link dropped and we retrigger
+ * autoneg again. Avoid that.
+ */
+ new_page = true;
+
+ err = mtip_c73_page_received(priv, &restart_an);
+ if (err)
+ goto out_unlock;
+ }
+
+ if ((irqpoll->old_an_stat ^ val) & MDIO_AN_STAT1_RFAULT)
+ mtip_c73_remote_fault(priv, val & MDIO_AN_STAT1_RFAULT);
+
+ /* Checks that result in AN restart should go at the end */
+
+ /* Make sure the lane goes back into DME page exchange mode
+ * after a link drop
+ */
+ if (priv->link_mode_resolved && !new_page &&
+ (irqpoll->old_pcs_stat & MDIO_STAT1_LSTATUS) &&
+ !(pcs_stat & MDIO_STAT1_LSTATUS)) {
+ dev_dbg(dev, "PCS link dropped, restarting autoneg\n");
+ restart_an = true;
+ }
+
+ /* Paranoid workaround for undetermined issue */
+ if (!priv->link_mode_resolved && (val & MDIO_AN_STAT1_COMPLETE) &&
+ priv->an_enabled && time_after(jiffies, priv->last_an_restart +
+ msecs_to_jiffies(MTIP_AN_TIMEOUT_MS))) {
+ dev_err(dev,
+ "Hardware says AN has completed, but we never saw a base page, and that's bogus\n");
+ restart_an = true;
+ }
+
+ /* Sometimes, after a renegotiation, it can be seen that link training
+ * failures on one side trigger an autoneg restart (as they should),
+ * but that does not get acted upon by the other side. It appears that
+ * the other side is in a strange state where it has completed link
+ * training and it's waiting for something. As seen in the
+ * MDIO_AN_STAT1_PAGE workaround above, we will fail to detect new base
+ * pages received during link training, so we won't be able to exit
+ * that state. Detect it and exit it if 1 second has passed since link
+ * training has completed, but the 'autoneg done' bit hasn't asserted.
+ */
+ if (mtip_are_all_lanes_trained(priv) && !(val & MDIO_AN_STAT1_COMPLETE) &&
+ time_after(jiffies, priv->last_lt_done +
+ msecs_to_jiffies(MTIP_LT_TIMEOUT_MS))) {
+ dev_err(dev, "AN did not complete after link training completed\n");
+ restart_an = true;
+ }
+
+ if (priv->link_mode_resolved && priv->lt_enabled) {
+ int expected = mtip_expected_bp_eth_stat(priv->link_mode);
+ int bp_eth_stat = mtip_read_an(priv, AN_BP_ETH_STAT);
+
+ if (bp_eth_stat != expected) {
+ dev_err(dev, "BP_ETH_STAT 0x%x changed from expected value of 0x%x\n",
+ bp_eth_stat, expected);
+ restart_an = true;
+ }
+ }
+
+ if (restart_an) {
+ err = mtip_an_restart(priv);
+ if (err)
+ goto out_unlock;
+
+ /* don't overwrite what was set by mtip_unlatch_an_stat() */
+ goto ignore_an_and_pcs_stat;
+ }
+
+ irqpoll->old_an_stat = val;
+ irqpoll->old_pcs_stat = pcs_stat;
+ignore_an_and_pcs_stat:
+ irqpoll->cdr_locked = all_lanes_have_cdr_lock;
+
+out_unlock:
+ mutex_unlock(&irqpoll->lock);
+
+ if (err) {
+ dev_err(dev,
+ "Error detected from irqpoll thread: %pe, exiting...\n",
+ ERR_PTR(err));
+ return;
+ }
+
+ if (!irqpoll->run_once)
+ schedule_delayed_work(&irqpoll->work, IRQPOLL_INTERVAL);
+
+ irqpoll->run_once = false;
+}
+
+int mtip_backplane_config_aneg(struct mtip_backplane *priv, bool autoneg,
+ const unsigned long *advertising)
+{
+ u16 mask = MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART;
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+ int err;
+
+ mutex_lock(&irqpoll->lock);
+
+ if (autoneg) {
+ err = mtip_config_an_adv(priv, advertising);
+ if (err < 0)
+ goto out_unlock;
+
+ err = mtip_an_restart(priv);
+ if (err)
+ goto out_unlock;
+
+ priv->an_enabled = true;
+ } else {
+ err = mtip_modify_an(priv, AN_CTRL, mask, 0);
+ if (err < 0)
+ goto out_unlock;
+
+ priv->an_enabled = false;
+ }
+
+out_unlock:
+ mutex_unlock(&irqpoll->lock);
+
+ return err;
+}
+EXPORT_SYMBOL(mtip_backplane_config_aneg);
+
+static int mtip_resolve_aneg_linkmode(struct mtip_backplane *priv,
+ struct phylink_link_state *state)
+{
+ u64 base_page;
+ int err;
+
+ linkmode_zero(state->lp_advertising);
+
+ err = mtip_read_lpa(priv, &base_page);
+ if (err)
+ return err;
+
+ mii_c73_mod_linkmode_lpa_t(state->lp_advertising, base_page);
+ phylink_resolve_c73(state);
+
+ return 0;
+}
+
+void mtip_backplane_get_state(struct mtip_backplane *priv,
+ struct phylink_link_state *state)
+{
+ struct mtip_irqpoll *irqpoll = &priv->irqpoll;
+ struct device *dev = &priv->mdiodev->dev;
+ u64 base_page;
+ int err = 0;
+
+ mutex_lock(&irqpoll->lock);
+
+ state->speed = SPEED_UNKNOWN;
+ state->duplex = DUPLEX_UNKNOWN;
+ state->pause = 0;
+
+ err = mtip_read_adv(priv, &base_page);
+ if (err)
+ goto out_unlock;
+
+ state->link = mtip_read_link_unlatch(priv);
+ if (!state->link)
+ goto out_unlock;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ state->advertising)) {
+ state->an_complete = mtip_cached_an_complete(priv);
+
+ if (state->an_complete)
+ err = mtip_resolve_aneg_linkmode(priv, state);
+
+ state->link = state->link && mtip_are_all_lanes_trained(priv);
+ }
+
+out_unlock:
+ mutex_unlock(&irqpoll->lock);
+
+ if (err)
+ dev_err(dev, "Failed to get backplane state: %pe\n",
+ ERR_PTR(err));
+}
+EXPORT_SYMBOL(mtip_backplane_get_state);
+
+int mtip_backplane_add_subordinate(struct mtip_backplane *priv,
+ struct mtip_backplane *subordinate)
+{
+ if (priv->num_subordinates == MTIP_MAX_NUM_SUBORDINATES)
+ return -ERANGE;
+
+ subordinate->is_subordinate = true;
+ subordinate->coordinator = priv;
+ priv->subordinate[priv->num_subordinates++] = subordinate;
+
+ return 0;
+}
+EXPORT_SYMBOL(mtip_backplane_add_subordinate);
+
+static struct mdio_device *
+mtip_get_mdiodev_for_link_mode(struct mii_bus *bus, struct phy *serdes,
+ enum ethtool_link_mode_bit_indices link_mode)
+{
+ union phy_status_opts opts = {
+ .pcvt = {
+ .type = PHY_PCVT_ETHERNET_ANLT,
+ },
+ };
+ int err;
+
+ err = phy_set_mode_ext(serdes, PHY_MODE_ETHTOOL, link_mode);
+ if (err)
+ return ERR_PTR(err);
+
+ err = phy_get_status(serdes, PHY_STATUS_PCVT_ADDR, &opts);
+ if (err)
+ return ERR_PTR(err);
+
+ return mdio_device_create(bus, opts.pcvt.addr.mdio);
+}
+
+static struct mdio_device *mtip_get_mdiodev(struct mii_bus *bus,
+ struct phy *serdes)
+{
+ const enum ethtool_link_mode_bit_indices *link_modes;
+ int i, err;
+
+ link_modes = mtip_backplane_link_modes;
+
+ /* Preconfigure the SerDes lane for the highest supported link mode,
+ * make sure the backplane AN/LT + PCS are enabled, and get the MDIO
+ * address of our device so that we can access its registers.
+ */
+ for (i = 0; i < ARRAY_SIZE(mtip_backplane_link_modes); i++) {
+ err = phy_validate(serdes, PHY_MODE_ETHTOOL,
+ link_modes[i], NULL);
+ if (err)
+ continue;
+
+ return mtip_get_mdiodev_for_link_mode(bus, serdes,
+ link_modes[i]);
+ }
+
+ dev_err(&serdes->dev, "No backplane link modes supported!\n");
+
+ return ERR_PTR(-ENODEV);
+}
+
+static void mtip_irqpoll_init(struct mtip_backplane *priv,
+ struct mtip_irqpoll *irqpoll)
+{
+ mutex_init(&irqpoll->lock);
+ INIT_DELAYED_WORK(&irqpoll->work, mtip_irqpoll_work);
+}
+
+struct mtip_backplane *mtip_backplane_create(struct mdio_device *pcs_mdiodev,
+ struct phy *serdes,
+ enum mtip_model model)
+{
+ struct mii_bus *bus = pcs_mdiodev->bus;
+ struct mtip_backplane *priv;
+ struct mdio_device *mdiodev;
+ struct device *dev;
+ int err;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ mdiodev = mtip_get_mdiodev(bus, serdes);
+ if (IS_ERR(mdiodev)) {
+ err = PTR_ERR(mdiodev);
+ goto out_free_priv;
+ }
+
+ dev = &mdiodev->dev;
+ priv->pcs_mdiodev = pcs_mdiodev;
+ priv->mdiodev = mdiodev;
+ priv->serdes = serdes;
+
+ switch (model) {
+ case MTIP_MODEL_LX2160A:
+ priv->an_regs = mtip_lx2160a_an_regs;
+ priv->lt_regs = mtip_lx2160a_lt_regs;
+ priv->lt_mmd = MDIO_MMD_AN;
+ break;
+ default:
+ /* TODO */
+ err = -EINVAL;
+ goto out_free_mdiodev;
+ }
+
+ err = mtip_reset_pcs(priv);
+ if (err < 0)
+ goto out_free_mdiodev;
+
+ err = mtip_reset_an(priv);
+ if (err < 0)
+ goto out_free_mdiodev;
+
+ mtip_irqpoll_init(priv, &priv->irqpoll);
+ mutex_init(&priv->an_restart_lock);
+ mutex_init(&priv->lt_lock);
+
+ priv->local_tx_lt_worker = kthread_create_worker(0, "%d_local_tx_lt",
+ mdiodev->addr);
+ if (IS_ERR(priv->local_tx_lt_worker)) {
+ err = PTR_ERR(priv->local_tx_lt_worker);
+ goto out_free_priv;
+ }
+
+ priv->remote_tx_lt_worker = kthread_create_worker(0, "%d_remote_tx_lt",
+ mdiodev->addr);
+ if (IS_ERR(priv->remote_tx_lt_worker)) {
+ err = PTR_ERR(priv->remote_tx_lt_worker);
+ goto out_destroy_local_tx_lt;
+ }
+
+ err = phy_init(priv->serdes);
+ if (err) {
+ dev_err(dev, "Failed to initialize SerDes: %pe\n",
+ ERR_PTR(err));
+ goto out_destroy_remote_tx_lt;
+ }
+
+ return priv;
+
+out_destroy_remote_tx_lt:
+ kthread_destroy_worker(priv->remote_tx_lt_worker);
+out_destroy_local_tx_lt:
+ kthread_destroy_worker(priv->local_tx_lt_worker);
+out_free_mdiodev:
+ mdio_device_put(priv->mdiodev);
+out_free_priv:
+ kfree(priv);
+out:
+ return ERR_PTR(err);
+}
+EXPORT_SYMBOL(mtip_backplane_create);
+
+void mtip_backplane_destroy(struct mtip_backplane *priv)
+{
+ phy_exit(priv->serdes);
+ kthread_destroy_worker(priv->remote_tx_lt_worker);
+ kthread_destroy_worker(priv->local_tx_lt_worker);
+ mdio_device_put(priv->mdiodev);
+ kfree(priv);
+}
+EXPORT_SYMBOL(mtip_backplane_destroy);
+
+MODULE_AUTHOR("Vladimir Oltean <[email protected]>");
+MODULE_DESCRIPTION("MTIP Backplane PHY driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/pcs/mtip_backplane.h b/drivers/net/pcs/mtip_backplane.h
new file mode 100644
index 000000000000..d418630017d3
--- /dev/null
+++ b/drivers/net/pcs/mtip_backplane.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright 2023 NXP
+ */
+#ifndef _MTIP_BACKPLANE_H
+#define _MTIP_BACKPLANE_H
+
+struct mdio_device;
+struct mtip_backplane;
+struct phy;
+
+enum mtip_model {
+ MTIP_MODEL_AUTODETECT,
+ MTIP_MODEL_LX2160A,
+};
+
+#if IS_ENABLED(CONFIG_MTIP_BACKPLANE_PHY)
+
+int mtip_backplane_config_aneg(struct mtip_backplane *priv, bool autoneg,
+ const unsigned long *advertising);
+void mtip_backplane_an_restart(struct mtip_backplane *priv);
+void mtip_backplane_get_state(struct mtip_backplane *priv,
+ struct phylink_link_state *state);
+int mtip_backplane_suspend(struct mtip_backplane *priv);
+int mtip_backplane_resume(struct mtip_backplane *priv);
+int mtip_backplane_validate(struct mtip_backplane *priv,
+ unsigned long *supported);
+int mtip_backplane_add_subordinate(struct mtip_backplane *priv,
+ struct mtip_backplane *subordinate);
+struct mtip_backplane *mtip_backplane_create(struct mdio_device *pcs_mdiodev,
+ struct phy *serdes,
+ enum mtip_model model);
+void mtip_backplane_destroy(struct mtip_backplane *priv);
+
+#else
+
+static inline int mtip_backplane_config_aneg(struct mtip_backplane *priv,
+ bool autoneg,
+ const unsigned long *advertising)
+{
+ return -ENODEV;
+}
+
+static inline void mtip_backplane_an_restart(struct mtip_backplane *priv)
+{
+}
+
+static inline void mtip_backplane_get_state(struct mtip_backplane *priv,
+ struct phylink_link_state *state)
+{
+}
+
+static inline int mtip_backplane_suspend(struct mtip_backplane *priv)
+{
+ return -ENODEV;
+}
+
+static inline int mtip_backplane_resume(struct mtip_backplane *priv)
+{
+ return -ENODEV;
+}
+
+static inline int mtip_backplane_validate(struct mtip_backplane *priv,
+ unsigned long *supported)
+{
+ return -ENODEV;
+}
+
+static inline int mtip_backplane_add_subordinate(struct mtip_backplane *priv,
+ struct mtip_backplane *subordinate)
+{
+ return -ENODEV;
+}
+
+static inline struct mtip_backplane *mtip_backplane_create(struct mdio_device *pcs_mdiodev,
+ struct phy *serdes,
+ enum mtip_model model)
+{
+ return -ENODEV;
+}
+
+static inline void mtip_backplane_destroy(struct mtip_backplane *priv)
+{
+}
+
+#endif
+
+#endif
--
2.34.1
Treat the newly introduced subsets of 25GBase-KR and 25GBase-CR the same
way as the fully-featured link modes. The difference only consists in
RS-FEC support.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: patch is new
drivers/net/phy/phylink.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 6415c7b91053..157984dd81de 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -321,6 +321,8 @@ void phylink_caps_to_linkmodes(unsigned long *linkmodes, unsigned long caps)
if (caps & MAC_25000FD) {
__set_bit(ETHTOOL_LINK_MODE_25000baseCR_Full_BIT, linkmodes);
__set_bit(ETHTOOL_LINK_MODE_25000baseKR_Full_BIT, linkmodes);
+ __set_bit(ETHTOOL_LINK_MODE_25000baseCR_S_Full_BIT, linkmodes);
+ __set_bit(ETHTOOL_LINK_MODE_25000baseKR_S_Full_BIT, linkmodes);
__set_bit(ETHTOOL_LINK_MODE_25000baseSR_Full_BIT, linkmodes);
}
@@ -919,6 +921,8 @@ static int phylink_parse_mode(struct phylink *pl,
case PHY_INTERFACE_MODE_25GBASER:
phylink_set(pl->supported, 25000baseCR_Full);
phylink_set(pl->supported, 25000baseKR_Full);
+ phylink_set(pl->supported, 25000baseCR_S_Full);
+ phylink_set(pl->supported, 25000baseKR_S_Full);
phylink_set(pl->supported, 25000baseSR_Full);
fallthrough;
case PHY_INTERFACE_MODE_USXGMII:
@@ -948,6 +952,8 @@ static int phylink_parse_mode(struct phylink *pl,
case PHY_INTERFACE_MODE_XLGMII:
phylink_set(pl->supported, 25000baseCR_Full);
phylink_set(pl->supported, 25000baseKR_Full);
+ phylink_set(pl->supported, 25000baseCR_S_Full);
+ phylink_set(pl->supported, 25000baseKR_S_Full);
phylink_set(pl->supported, 25000baseSR_Full);
phylink_set(pl->supported, 40000baseKR4_Full);
phylink_set(pl->supported, 40000baseCR4_Full);
--
2.34.1
Allow phylink_resolve_c73() to resolve backplane (KR) or SFP28 (CR)
link speeds of 25Gbps.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: patch is new
drivers/net/phy/phylink.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 157984dd81de..484db2a5b62b 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -3382,6 +3382,10 @@ static struct {
/* 100GBASE-KP4 and 100GBASE-CR10 not supported */
{ ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT, SPEED_40000 },
{ ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT, SPEED_40000 },
+ { ETHTOOL_LINK_MODE_25000baseKR_Full_BIT, SPEED_25000 },
+ { ETHTOOL_LINK_MODE_25000baseCR_Full_BIT, SPEED_25000 },
+ { ETHTOOL_LINK_MODE_25000baseKR_S_Full_BIT, SPEED_25000 },
+ { ETHTOOL_LINK_MODE_25000baseCR_S_Full_BIT, SPEED_25000 },
{ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, SPEED_10000 },
{ ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT, SPEED_10000 },
/* 5GBASE-KR not supported */
--
2.34.1
The bit stream handled by a SerDes lane needs protocol converters to be
usable for Ethernet. On Freescale/NXP SoCs, those protocol converters
are located on the internal MDIO buses of the Ethernet MACs that need
them.
The location on that MDIO bus, on these SoCs, is not fixed, but given by
some control registers of the SerDes block itself.
Because no one modifies those addresses from the power-on default, so
far we've relied on hardcoding the default values in the device trees,
resulting in something like this:
pcs_mdio1: mdio@8c07000 {
compatible = "fsl,fman-memac-mdio";
pcs1: ethernet-phy@0 {
reg = <0>;
};
};
where the "reg" of "pcs1" can actually be retrieved from "serdes_1".
That was for the PCS. For AN/LT blocks, that can also be done, but the
MAC to PCS to AN/LT block mapping is non-trivial and extremely easy to
get wrong, which will confuse and frustrate any device tree writers.
The proposal is to take advantage of the fact that these protocol
converters *are* discoverable, and to side-step that entire device tree
mapping issue by not putting them in the device tree at all. So, one of
the consumers of the SerDes PHY uses the phy_get_status() API to figure
out the address on the MDIO bus, it also has a reference to the MDIO bus
=> it can create the mdio_device in a non OF-based manner.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: patch is new
include/linux/phy/phy.h | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index f1f03fa66943..ee721067517b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -56,6 +56,33 @@ enum phy_media {
enum phy_status_type {
/* Valid for PHY_MODE_ETHERNET and PHY_MODE_ETHTOOL */
PHY_STATUS_CDR_LOCK,
+ PHY_STATUS_PCVT_ADDR,
+};
+
+/* enum phy_pcvt_type - PHY protocol converter type
+ *
+ * @PHY_PCVT_ETHERNET_PCS: Ethernet Physical Coding Sublayer, top-most layer of
+ * an Ethernet PHY. Connects through MII to the MAC,
+ * and handles link status detection and the conversion
+ * of MII signals to link-specific code words (8b/10b,
+ * 64b/66b etc).
+ * @PHY_PCVT_ETHERNET_ANLT: Ethernet Auto-Negotiation and Link Training,
+ * bottom-most layer of an Ethernet PHY, beneath the
+ * PMA and PMD. Its activity is only visible on the
+ * physical medium, and it is responsible for
+ * selecting the most adequate PCS/PMA/PMD set that
+ * can operate on that medium.
+ */
+enum phy_pcvt_type {
+ PHY_PCVT_ETHERNET_PCS,
+ PHY_PCVT_ETHERNET_ANLT,
+};
+
+struct phy_status_opts_pcvt {
+ enum phy_pcvt_type type;
+ union {
+ unsigned int mdio;
+ } addr;
};
/* If the CDR (Clock and Data Recovery) block is able to lock onto the RX bit
@@ -71,9 +98,11 @@ struct phy_status_opts_cdr {
* union phy_status_opts - Opaque generic phy status
*
* @cdr: Configuration set applicable for PHY_STATUS_CDR_LOCK.
+ * @pcvt: Configuration set applicable for PHY_STATUS_PCVT_ADDR.
*/
union phy_status_opts {
struct phy_status_opts_cdr cdr;
+ struct phy_status_opts_pcvt pcvt;
};
/**
--
2.34.1
Allow driver code to print stuff like the resolved link mode to the
kernel log, by giving it access to the link_mode_names[] ethtool
internal array which already holds this info.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: patch is new
include/linux/ethtool.h | 6 ++++++
net/ethtool/common.c | 6 ++++++
2 files changed, 12 insertions(+)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 62b61527bcc4..7b144f2b7dfd 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -988,6 +988,12 @@ void
ethtool_params_from_link_mode(struct ethtool_link_ksettings *link_ksettings,
enum ethtool_link_mode_bit_indices link_mode);
+/**
+ * ethtool_link_mode_str - Get name of a given link mode, in string format
+ * @link_mode: the link mode represented in integer format
+ */
+const char *ethtool_link_mode_str(enum ethtool_link_mode_bit_indices link_mode);
+
/**
* ethtool_get_phc_vclocks - Derive phc vclocks information, and caller
* is responsible to free memory of vclock_index
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 2b3ddea465af..9a79548adb68 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -691,3 +691,9 @@ ethtool_params_from_link_mode(struct ethtool_link_ksettings *link_ksettings,
link_ksettings->base.duplex = link_info->duplex;
}
EXPORT_SYMBOL_GPL(ethtool_params_from_link_mode);
+
+const char *ethtool_link_mode_str(enum ethtool_link_mode_bit_indices link_mode)
+{
+ return link_mode_names[link_mode];
+}
+EXPORT_SYMBOL(ethtool_link_mode_str);
--
2.34.1
In Layerscape and QorIQ SoCs, compliance with the backplane Ethernet
protocol is bolted on top of the SerDes lanes using an external IP core,
that is modeled as an Ethernet PHY. This means that dynamic tuning of
the electrical equalization parameters of the link needs to be
communicated with the consumer of the generic PHY.
Create a small layer of glue API between a networking PHY (dealing with
the AN/LT logic for backplanes) and a generic PHY by extending the
phy_configure() API with a new struct phy_configure_opts_ethernet.
There are 2 directions of interest. In the "local TX training", the
generic PHY consumer gets requests over the wire from the link partner
regarding changes we should make to our TX equalization. In the "remote
TX training" direction, the generic PHY is the producer of requests,
based on its RX status, and the generic PHY consumer polls for these
requests until we are happy. Each request is also sent (externally to
the generic PHY layer) to the link partner board, for it to adjust its
TX equalization.
struct phy_configure_opts_ethernet is valid when phy_set_mode_ext() has
been called with PHY_MODE_ETHERNET or PHY_MODE_ETHTOOL, same as with
other union phy_configure_opts types.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2:
- rename "xgkr" to "ethernet" in phy_configure_opts_ethernet, to match
other modes like PHY_MODE_LVDS and phy_configure_opts_lvds.
- add kerneldocs
include/linux/phy/phy-ethernet.h | 292 +++++++++++++++++++++++++++++++
include/linux/phy/phy.h | 4 +
2 files changed, 296 insertions(+)
create mode 100644 include/linux/phy/phy-ethernet.h
diff --git a/include/linux/phy/phy-ethernet.h b/include/linux/phy/phy-ethernet.h
new file mode 100644
index 000000000000..5260e132de21
--- /dev/null
+++ b/include/linux/phy/phy-ethernet.h
@@ -0,0 +1,292 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2023 NXP
+ */
+
+#ifndef __PHY_ETHERNET_H_
+#define __PHY_ETHERNET_H_
+
+struct phy;
+
+/**
+ * enum coef_status - Status report field of one TX equalization tap
+ * (coefficient), according to IEEE 802.3-2018 clause
+ * 72.6.10.2.4.5 Coefficient (k) status
+ *
+ * @COEF_STAT_NOT_UPDATED: The default state for a given tap is not_updated.
+ * An increment or decrement request will only be acted
+ * upon when the state of the tap is not_updated.
+ * @COEF_STAT_UPDATED: The tap transitions to this state after it has
+ * successfully responded to an update request different
+ * from HOLD.
+ * @COEF_STAT_MIN: The tap transitions to this state when it has received a DEC
+ * request, but it has already reached its minimum limit and it
+ * cannot fulfill the request.
+ * @COEF_STAT_MAX: The tap transitions to this state when it has received an
+ * INC request, but it has already reached its maximum limit
+ * and it cannot fulfill the request.
+ *
+ * After any non-HOLD update request which results in an update of the
+ * coefficient status, the update request must return to the HOLD state.
+ * Then, the status field returns to NOT_UPDATED and the process can repeat.
+ */
+enum coef_status {
+ COEF_STAT_NOT_UPDATED = 0,
+ COEF_STAT_UPDATED = 1,
+ COEF_STAT_MIN = 2,
+ COEF_STAT_MAX = 3,
+};
+
+/**
+ * enum coef_update - Requested update field of one TX equalization tap
+ * (coefficient), according to IEEE 802.3-2018 clause
+ * 72.6.10.2.3.3 Coefficient (k) update
+ *
+ * @COEF_UPD_HOLD: The default state for a given tap is hold, which corresponds
+ * to no change in the coefficient.
+ * @COEF_UPD_INC: Request the link partner to increase the equalization of the
+ * given tap by one step, which implies a change to the waveform
+ * voltages within the limits defined in IEEE 802.3-2018 Table
+ * 72-7 "Transmitter output waveform requirements related to
+ * coefficient update"
+ * @COEF_UPD_DEC: See @COEF_UPD_DEC
+ *
+ * Coefficient increment/decrement shall not be sent in combination with
+ * initialize or preset.
+ */
+enum coef_update {
+ COEF_UPD_HOLD = 0,
+ COEF_UPD_INC = 1,
+ COEF_UPD_DEC = 2,
+};
+
+/**
+ * struct c72_coef_update - C72 coefficient request
+ *
+ * @com1: corresponds to "Coefficient (-1) update" ("minus 1")
+ * @coz: corresponds to "Coefficient (0) update" ("zero")
+ * @cop1: corresponds to "Coefficient (+1) update" ("plus 1")
+ * @preset: when set, preset coefficients are requested
+ * @init: when set, coefficient initialization is requested
+ *
+ * This is the input structure for a local TX link training step, and is based
+ * on the definitions from IEEE 802.3-2018 clause 72.6.10.2.3 "Coefficient
+ * update field". It carries correction information from the local receiver
+ * to the link partner transmit equalizer. The structure consists of preset
+ * controls, initialization controls, and coefficient updates for three
+ * transmit equalizer taps.
+ */
+struct c72_coef_update {
+ enum coef_update com1;
+ enum coef_update coz;
+ enum coef_update cop1;
+ bool preset;
+ bool init;
+};
+
+/**
+ * struct c72_coef_status - response to C72 coefficient request
+ *
+ * @com1: corresponds to "Coefficient (-1) status" ("minus 1")
+ * @coz: corresponds to "Coefficient (0) status" ("zero")
+ * @cop1: corresponds to "Coefficient (+1) status" ("plus 1")
+ *
+ * This is the output structure for a local TX link training step, and is based
+ * on the definitions from IEEE 802.3-2018 clause 72.6.10.2.4 "Status report
+ * field". The "Receiver ready" (bit 15 as defined by IEEE) of the status
+ * report is deliberately not part of this structure, since it is logically
+ * part of the remote TX link training procedure, and is decoupled from the
+ * local TX link training.
+ */
+struct c72_coef_status {
+ enum coef_status com1;
+ enum coef_status coz;
+ enum coef_status cop1;
+};
+
+/**
+ * enum ethernet_phy_configure_type - Configuration types for an Ethernet phy
+ *
+ * @C72_LOCAL_TX: Execute a C72 link training step for the local transmitter.
+ * @C72_REMOTE_TX: Execute a C72 link training step for the remote transmitter.
+ * @C72_LT_DONE: Finalize C72 link training.
+ *
+ * The @C72_LOCAL_TX, @C72_REMOTE_TX and @C72_LT_DONE types apply to Ethernet
+ * phys supporting media types with the IEEE 802.3 clause 72: 10GBase-KR,
+ * 40GBase-KR etc.
+ */
+enum ethernet_phy_configure_type {
+ C72_LOCAL_TX,
+ C72_REMOTE_TX,
+ C72_LT_DONE,
+};
+
+/**
+ * struct c72_phy_configure_local_tx - configuration set for C72 local TX
+ * link training
+ * @update: input structure containing a C72 coefficient update request
+ * from the link partner.
+ * @status: output structure containing the response of the local PHY to
+ * the given update request
+ *
+ * Adjust the TX equalization of the local PHY in response to a C72 coefficient
+ * update request from the link partner. Used when @ethernet_phy_configure_type
+ * is set to @C72_LOCAL_TX.
+ */
+struct c72_phy_configure_local_tx {
+ struct c72_coef_update update;
+ struct c72_coef_status status;
+};
+
+/**
+ * struct c72_phy_configure_remote_tx - configuration set for C72 remote TX
+ * link training
+ *
+ * @rx_ready: output boolean set by phy when it does not need any transmitter
+ * adjustments from the link partner
+ * @update: output structure containing a request to the link partner
+ * transmitter, based on information from the local receiver
+ * @cb: optional callback to see how the link partner reacted to the
+ * update request (which is echoed back unmodified). The
+ * coefficient status is only valid if there was no error
+ * during its propagation.
+ * @cb_priv: private structure for the callback @cb.
+ *
+ * Query the phy RX quality in order to compute a C72 coefficient update
+ * request to the link partner to improve that. The phy consumer is responsible
+ * for taking the computed request and transmitting it to the link partner, and
+ * then calling the optional phy callback before making any other query.
+ *
+ * Used when @ethernet_phy_configure_type is set to @C72_REMOTE_TX.
+ *
+ * WARNING: the phy implementation may be stateful, i.e. the number of previous
+ * requests and their received status may modify the phy's state and it might
+ * influence the next computed request.
+ */
+struct c72_phy_configure_remote_tx {
+ bool rx_ready;
+ struct c72_coef_update update;
+ void (*cb)(void *cb_priv, int err, struct c72_coef_update update,
+ struct c72_coef_status status);
+ void *cb_priv;
+};
+
+/**
+ * struct phy_configure_opts_ethernet - Ethernet PHY configuration set
+ *
+ * This structure is used to represent the configuration state of an Ethernet
+ * PHY (of various media types).
+ */
+struct phy_configure_opts_ethernet {
+ enum ethernet_phy_configure_type type;
+ union {
+ struct c72_phy_configure_local_tx local_tx;
+ struct c72_phy_configure_remote_tx remote_tx;
+ };
+};
+
+/**
+ * coef_update_opposite - return the opposite of one C72 coefficient update
+ * request
+ *
+ * @update: original coefficient update
+ *
+ * Helper to transform the update request of one equalization tap into a
+ * request of the same tap in the opposite direction. May be used by C72
+ * phy remote TX link training algorithms.
+ */
+static inline enum coef_update coef_update_opposite(enum coef_update update)
+{
+ switch (update) {
+ case COEF_UPD_INC:
+ return COEF_UPD_DEC;
+ case COEF_UPD_DEC:
+ return COEF_UPD_INC;
+ default:
+ return COEF_UPD_HOLD;
+ }
+}
+
+/**
+ * coef_update_clamp - clamp one C72 coefficient update request
+ *
+ * @update: pointer to coefficient update
+ * @status: response from link partner to a previous update request to the
+ * same tap, based on which we are clamping this one
+ *
+ * Helper which may be used by C72 phy remote TX link training algorithms to
+ * clamp coefficient updates for a tap. When the link partner responded with
+ * MAX or MIN to a previous update request for the same tap, future requests
+ * are likely to result in the same response, so just transform them into HOLD,
+ * which represents the lack of an update request.
+ */
+static inline void coef_update_clamp(enum coef_update *update,
+ enum coef_status status)
+{
+ if (*update == COEF_UPD_INC && status == COEF_STAT_MAX)
+ *update = COEF_UPD_HOLD;
+ if (*update == COEF_UPD_DEC && status == COEF_STAT_MIN)
+ *update = COEF_UPD_HOLD;
+}
+
+/* Other helpers */
+static inline bool coef_update_is_all_hold(const struct c72_coef_update *update)
+{
+ return update->coz == COEF_UPD_HOLD &&
+ update->com1 == COEF_UPD_HOLD &&
+ update->cop1 == COEF_UPD_HOLD;
+}
+
+#define C72_COEF_UPDATE_BUFSIZ 64
+#define C72_COEF_STATUS_BUFSIZ 64
+
+static inline const char *coef_update_to_string(enum coef_update coef)
+{
+ switch (coef) {
+ case COEF_UPD_HOLD:
+ return "HOLD";
+ case COEF_UPD_INC:
+ return "INC";
+ case COEF_UPD_DEC:
+ return "DEC";
+ default:
+ return "unknown";
+ }
+}
+
+static inline const char *coef_status_to_string(enum coef_status coef)
+{
+ switch (coef) {
+ case COEF_STAT_NOT_UPDATED:
+ return "NOT_UPDATED";
+ case COEF_STAT_UPDATED:
+ return "UPDATED";
+ case COEF_STAT_MIN:
+ return "MIN";
+ case COEF_STAT_MAX:
+ return "MAX";
+ default:
+ return "unknown";
+ }
+}
+
+static void inline c72_coef_update_print(const struct c72_coef_update *update,
+ char buf[C72_COEF_UPDATE_BUFSIZ])
+{
+ sprintf(buf, "INIT %d, PRESET %d, C(-1) %s, C(0) %s, C(+1) %s",
+ update->init, update->preset,
+ coef_update_to_string(update->com1),
+ coef_update_to_string(update->coz),
+ coef_update_to_string(update->cop1));
+}
+
+static inline void c72_coef_status_print(const struct c72_coef_status *status,
+ char buf[C72_COEF_STATUS_BUFSIZ])
+{
+ sprintf(buf, "C(-1) %s, C(0) %s, C(+1) %s",
+ coef_status_to_string(status->com1),
+ coef_status_to_string(status->coz),
+ coef_status_to_string(status->cop1));
+}
+
+#endif /* __PHY_ETHERNET_H_ */
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 72ef4afcda81..f1f03fa66943 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -17,6 +17,7 @@
#include <linux/regulator/consumer.h>
#include <linux/phy/phy-dp.h>
+#include <linux/phy/phy-ethernet.h>
#include <linux/phy/phy-lvds.h>
#include <linux/phy/phy-mipi-dphy.h>
@@ -84,11 +85,14 @@ union phy_status_opts {
* the DisplayPort protocol.
* @lvds: Configuration set applicable for phys supporting
* the LVDS phy mode.
+ * @ethernet: Configuration set applicable for phys supporting
+ * the ethernet and ethtool phy mode.
*/
union phy_configure_opts {
struct phy_configure_opts_mipi_dphy mipi_dphy;
struct phy_configure_opts_dp dp;
struct phy_configure_opts_lvds lvds;
+ struct phy_configure_opts_ethernet ethernet;
};
/**
--
2.34.1
IEEE 802.3 clause 73 defines auto-negotiation for backplanes and copper
cable assemblies. This is a medium type (link mode) just like Ethernet
over twisted pairs (BASE-T / BASE-TX) and over two wires for automotive
(BASE-T1) for which the PHY library currently contains support.
As a minimal framework for backplane PHY drivers, introduce a set of
helpers that parse and interpret the base pages that are exchanged by
PHYs during the clause 73 negotiation.
The placement in the "legacy" mii code is perhaps not the best, but I
tried to put them somewhere accessible by phylib, phylink and non-phylib
drivers. Note that phylink also has its own phylink_resolve_c73() which
is more or less similar in purpose, but:
- it requires constructing a struct phylink_link_state which is deeply
embedded with the phylink API and that may not be desirable for
drivers
- the presence of some link modes in phylink's own
phylink_c73_priority_resolution[] is "interesting", like
ETHTOOL_LINK_MODE_2500baseX_Full_BIT, which is not a backplane mode
negotiable through C73 at all. That comes from the xpcs driver which
may have a non-standard C73 autoneg, and this makes it difficult for
me to e.g. refactor phylink_resolve_c73() to use the more generic
linkmode_c73_priority_resolution(). Also see the attached link where I
had previously pointed this out.
Link: https://lore.kernel.org/netdev/20230516090009.ssq3uedjl53kzsjr@skbuf/
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: add 25GBase-KR-S and 25GBase-CR-S
drivers/net/mii.c | 34 +++++++++++++-
include/linux/mii.h | 105 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 22680f47385d..03e8b0877600 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -648,6 +648,38 @@ int generic_mii_ioctl(struct mii_if_info *mii_if,
return rc;
}
+static const enum ethtool_link_mode_bit_indices c73_linkmodes[] = {
+ ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT,
+ ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
+ /* ETHTOOL_LINK_MODE_100000baseKP4_Full_BIT not supported */
+ /* ETHTOOL_LINK_MODE_100000baseCR10_Full_BIT not supported */
+ ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT,
+ ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+ ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
+ ETHTOOL_LINK_MODE_25000baseCR_Full_BIT,
+ ETHTOOL_LINK_MODE_25000baseKR_S_Full_BIT,
+ ETHTOOL_LINK_MODE_25000baseCR_S_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+ ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+};
+
+int
+linkmode_c73_priority_resolution(const unsigned long *modes,
+ enum ethtool_link_mode_bit_indices *resolved)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(c73_linkmodes); i++) {
+ if (linkmode_test_bit(c73_linkmodes[i], modes)) {
+ *resolved = c73_linkmodes[i];
+ return 0;
+ }
+ }
+
+ return -ENOPROTOOPT;
+}
+
MODULE_AUTHOR ("Jeff Garzik <[email protected]>");
MODULE_DESCRIPTION ("MII hardware support library");
MODULE_LICENSE("GPL");
@@ -662,4 +694,4 @@ EXPORT_SYMBOL(mii_check_link);
EXPORT_SYMBOL(mii_check_media);
EXPORT_SYMBOL(mii_check_gmii_support);
EXPORT_SYMBOL(generic_mii_ioctl);
-
+EXPORT_SYMBOL(linkmode_c73_priority_resolution);
diff --git a/include/linux/mii.h b/include/linux/mii.h
index d5a959ce4877..4b141e9acd08 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -13,6 +13,36 @@
#include <linux/linkmode.h>
#include <uapi/linux/mii.h>
+/* 802.3-2018 clause 73.6 Link codeword encoding */
+#define C73_BASE_PAGE_SELECTOR(x) ((x) & GENMASK(4, 0))
+#define C73_BASE_PAGE_ECHOED_NONCE(x) (((x) << 5) & GENMASK(9, 5))
+#define C73_BASE_PAGE_ECHOED_NONCE_X(x) (((x) & GENMASK(9, 5)) >> 5)
+#define C73_BASE_PAGE_ECHOED_NONCE_MSK GENMASK(9, 5)
+#define C73_BASE_PAGE_PAUSE BIT(10)
+#define C73_BASE_PAGE_ASM_DIR BIT(11)
+#define C73_BASE_PAGE_RF BIT(13)
+#define C73_BASE_PAGE_ACK BIT(14)
+#define C73_BASE_PAGE_NP BIT(15)
+#define C73_BASE_PAGE_TRANSMITTED_NONCE(x) (((x) << 16) & GENMASK(20, 16))
+#define C73_BASE_PAGE_TRANSMITTED_NONCE_X(x) (((x) & GENMASK(20, 16)) >> 16)
+#define C73_BASE_PAGE_TRANSMITTED_NONCE_MSK GENMASK(20, 16)
+#define C73_BASE_PAGE_A(x) BIT(21 + (x))
+#define C73_BASE_PAGE_TECH_ABL_1000BASEKX C73_BASE_PAGE_A(0)
+#define C73_BASE_PAGE_TECH_ABL_10GBASEKX4 C73_BASE_PAGE_A(1)
+#define C73_BASE_PAGE_TECH_ABL_10GBASEKR C73_BASE_PAGE_A(2)
+#define C73_BASE_PAGE_TECH_ABL_40GBASEKR4 C73_BASE_PAGE_A(3)
+#define C73_BASE_PAGE_TECH_ABL_40GBASECR4 C73_BASE_PAGE_A(4)
+#define C73_BASE_PAGE_TECH_ABL_100GBASECR10 C73_BASE_PAGE_A(5)
+#define C73_BASE_PAGE_TECH_ABL_100GBASEKP4 C73_BASE_PAGE_A(6)
+#define C73_BASE_PAGE_TECH_ABL_100GBASEKR4 C73_BASE_PAGE_A(7)
+#define C73_BASE_PAGE_TECH_ABL_100GBASECR4 C73_BASE_PAGE_A(8)
+#define C73_BASE_PAGE_TECH_ABL_25GBASEKRS C73_BASE_PAGE_A(9)
+#define C73_BASE_PAGE_TECH_ABL_25GBASEKR C73_BASE_PAGE_A(10)
+#define C73_BASE_PAGE_25G_RS_FEC_REQ BIT_ULL(44)
+#define C73_BASE_PAGE_25G_BASER_FEC_REQ BIT_ULL(45)
+#define C73_BASE_PAGE_10G_BASER_FEC_ABL BIT_ULL(46)
+#define C73_BASE_PAGE_10G_BASER_FEC_REQ BIT_ULL(47)
+
struct ethtool_cmd;
struct mii_if_info {
@@ -47,6 +77,10 @@ extern int generic_mii_ioctl(struct mii_if_info *mii_if,
struct mii_ioctl_data *mii_data, int cmd,
unsigned int *duplex_changed);
+extern int
+linkmode_c73_priority_resolution(const unsigned long *modes,
+ enum ethtool_link_mode_bit_indices *resolved);
+
static inline struct mii_ioctl_data *if_mii(struct ifreq *rq)
{
@@ -506,6 +540,77 @@ static inline u16 linkmode_adv_to_mii_adv_x(const unsigned long *linkmodes,
return adv;
}
+static inline u64 linkmode_adv_to_c73_base_page(const unsigned long *advertising)
+{
+ u64 result = 0;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+ advertising))
+ result |= C73_BASE_PAGE_TECH_ABL_1000BASEKX;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+ advertising))
+ result |= C73_BASE_PAGE_TECH_ABL_10GBASEKX4;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+ advertising))
+ result |= C73_BASE_PAGE_TECH_ABL_10GBASEKR;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+ advertising))
+ result |= C73_BASE_PAGE_TECH_ABL_40GBASEKR4;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT,
+ advertising))
+ result |= C73_BASE_PAGE_TECH_ABL_40GBASECR4;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
+ advertising))
+ result |= C73_BASE_PAGE_TECH_ABL_100GBASEKR4;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT,
+ advertising))
+ result |= C73_BASE_PAGE_TECH_ABL_100GBASECR4;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
+ advertising))
+ result |= C73_BASE_PAGE_TECH_ABL_25GBASEKR;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising))
+ result |= C73_BASE_PAGE_PAUSE;
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising))
+ result |= C73_BASE_PAGE_ASM_DIR;
+
+ return result;
+}
+
+static inline void mii_c73_mod_linkmode_lpa_t(unsigned long *advertising,
+ u64 base_page)
+{
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT, advertising,
+ base_page & C73_BASE_PAGE_TECH_ABL_1000BASEKX);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT, advertising,
+ base_page & C73_BASE_PAGE_TECH_ABL_10GBASEKX4);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, advertising,
+ base_page & C73_BASE_PAGE_TECH_ABL_10GBASEKR);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT, advertising,
+ base_page & C73_BASE_PAGE_TECH_ABL_40GBASEKR4);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT, advertising,
+ base_page & C73_BASE_PAGE_TECH_ABL_40GBASECR4);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT, advertising,
+ base_page & C73_BASE_PAGE_TECH_ABL_100GBASEKR4);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT, advertising,
+ base_page & C73_BASE_PAGE_TECH_ABL_100GBASECR4);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_25000baseKR_Full_BIT, advertising,
+ base_page & C73_BASE_PAGE_TECH_ABL_25GBASEKR);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising,
+ base_page & C73_BASE_PAGE_PAUSE);
+
+ linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising,
+ base_page & C73_BASE_PAGE_ASM_DIR);
+}
+
/**
* mii_advertise_flowctrl - get flow control advertisement flags
* @cap: Flow control capabilities (FLOW_CTRL_RX, FLOW_CTRL_TX or both)
--
2.34.1
On 23/09/2023 15:49, Vladimir Oltean wrote:
> When the Lynx PCS is deployed on a copper backplane link, it must be
> prepared to handle clause 73 autoneg and clause 72 link training, which
> it can do using a dedicated AN/LT block. The latter doesn't need to be
> described in the device tree, because it is discoverable from the SerDes
> lanes.
>
> The media type that is deployed on the link is not discoverable though,
> so the introduction of a fsl,backplane-mode boolean property appears
> necessary to determine whether the AN/LT block should be employed, or
> left bypassed.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> v1->v2: patch is new
>
> .../devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml b/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> index fbedf696c555..40fbcd80ee2a 100644
> --- a/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> +++ b/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> @@ -16,11 +16,24 @@ description: |
>
> properties:
> compatible:
> - const: fsl,lynx-pcs
> + enum:
> + - fsl,lx2160a-lynx-pcs
> + - fsl,lynx-pcs
>
> reg:
> maxItems: 1
>
> + phys:
> + maxItems: 4
> + description:
> + phandle for the SerDes lanes that act as PMA/PMD layer when the PCS is
> + part of a copper backplane PHY.
> +
> + fsl,backplane-mode:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + Indicates that the PCS is deployed over a copper backplane link.
> +
Please extend also existing example. If these do not apply to lynx-pcs,
then they should be disallowed in allOf:if:then.
Best regards,
Krzysztof
Some modules, like the MTIP AN/LT block used as a copper backplane PHY
driver, need this extra information from the SerDes PHY as another
source of "link up" information.
Namely, the 25GBase-R PCS does not have a MDIO_CTRL1_LPOWER bit
implemented in its MDIO_MMD_PCS:MDIO_CTRL1 register. That bit is
typically set from phy_suspend() or phylink_pcs_disable() implementations,
and that is supposed to cause a link drop event on the link partner.
But here it does not happen.
By implementing the networking phylink_pcs_disable() as phy_power_off(),
we are able to actually power down the lane in a way that is visible to
the remote end. Where it is visible is the CDR lock, so we introduce
PHY_STATUS_TYPE_CDR_LOCK as an extra link indication, we are able to
detect that condition and signal it to upper layers of the network
stack.
A more high-level and generic phy_get_status() operation was chosen
instead of the more specific phy_get_cdr_lock() alternative, because I
saw this as being more in the spirit of the generic PHY API.
Also, phy_get_status() is more extensible and reusable for other
purposes as well.
Signed-off-by: Vladimir Oltean <[email protected]>
---
v1->v2: reimplement phy_check_cdr_lock() as something more generic
drivers/phy/phy-core.c | 31 ++++++++++++++++++++++++++
include/linux/phy/phy.h | 49 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 96a0b1e111f3..3b7e04a59a00 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -553,6 +553,37 @@ int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
}
EXPORT_SYMBOL_GPL(phy_validate);
+/**
+ * phy_get_status() - Query various parameters of a PHY
+ * @phy: the phy returned by phy_get()
+ * @type: type of the status being queried
+ * @opts: pointer to union of status structures, determined by type
+ *
+ * phy_init() must have been called on the phy. The status is relative to the
+ * current phy mode, that can be changed using phy_set_mode(). Not all status
+ * types may be relevant to all phy modes.
+ *
+ * Return: %0 if successful, a negative error code otherwise
+ */
+int phy_get_status(struct phy *phy, enum phy_status_type type,
+ union phy_status_opts *opts)
+{
+ int ret;
+
+ if (!phy)
+ return -EINVAL;
+
+ if (!phy->ops->get_status)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&phy->mutex);
+ ret = phy->ops->get_status(phy, type, opts);
+ mutex_unlock(&phy->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_get_status);
+
/**
* _of_phy_get() - lookup and obtain a reference to a phy by phandle
* @np: device_node for which to get the phy
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index f6d607ef0e80..6be348f1fa0e 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -51,6 +51,29 @@ enum phy_media {
PHY_MEDIA_DAC,
};
+enum phy_status_type {
+ /* Valid for PHY_MODE_ETHERNET */
+ PHY_STATUS_CDR_LOCK,
+};
+
+/* If the CDR (Clock and Data Recovery) block is able to lock onto the RX bit
+ * stream, it means that the stream contains valid bit transitions for the
+ * configured protocol. This indicates that a link partner is physically
+ * present and powered on.
+ */
+struct phy_status_opts_cdr {
+ bool cdr_locked;
+};
+
+/**
+ * union phy_status_opts - Opaque generic phy status
+ *
+ * @cdr: Configuration set applicable for PHY_STATUS_CDR_LOCK.
+ */
+union phy_status_opts {
+ struct phy_status_opts_cdr cdr;
+};
+
/**
* union phy_configure_opts - Opaque generic phy configuration
*
@@ -78,6 +101,7 @@ union phy_configure_opts {
* @set_speed: set the speed of the phy (optional)
* @reset: resetting the phy
* @calibrate: calibrate the phy
+ * @get_status: get the mode-specific status of the phy
* @release: ops to be performed while the consumer relinquishes the PHY
* @owner: the module owner containing the ops
*/
@@ -122,6 +146,20 @@ struct phy_ops {
union phy_configure_opts *opts);
int (*reset)(struct phy *phy);
int (*calibrate)(struct phy *phy);
+
+ /**
+ * @get_status:
+ *
+ * Optional.
+ *
+ * Used to query the mode-specific status of the phy. Must have no side
+ * effects.
+ *
+ * Returns: 0 if the operation was successful, negative error code
+ * otherwise.
+ */
+ int (*get_status)(struct phy *phy, enum phy_status_type type,
+ union phy_status_opts *opts);
void (*release)(struct phy *phy);
struct module *owner;
};
@@ -236,6 +274,8 @@ int phy_set_speed(struct phy *phy, int speed);
int phy_configure(struct phy *phy, union phy_configure_opts *opts);
int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
union phy_configure_opts *opts);
+int phy_get_status(struct phy *phy, enum phy_status_type type,
+ union phy_status_opts *opts);
static inline enum phy_mode phy_get_mode(struct phy *phy)
{
@@ -414,6 +454,15 @@ static inline int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
return -ENOSYS;
}
+static inline int phy_get_status(struct phy *phy, enum phy_status_type type,
+ union phy_status_opts *opts)
+{
+ if (!phy)
+ return 0;
+
+ return -ENOSYS;
+}
+
static inline int phy_get_bus_width(struct phy *phy)
{
return -ENOSYS;
--
2.34.1
On Sat, Sep 23, 2023 at 04:48:52PM +0300, Vladimir Oltean wrote:
> In Layerscape and QorIQ SoCs, compliance with the backplane Ethernet
> protocol is bolted on top of the SerDes lanes using an external IP core,
> that is modeled as an Ethernet PHY. This means that dynamic tuning of
> the electrical equalization parameters of the link needs to be
> communicated with the consumer of the generic PHY.
>
> Create a small layer of glue API between a networking PHY (dealing with
> the AN/LT logic for backplanes) and a generic PHY by extending the
> phy_configure() API with a new struct phy_configure_opts_ethernet.
>
> There are 2 directions of interest. In the "local TX training", the
> generic PHY consumer gets requests over the wire from the link partner
> regarding changes we should make to our TX equalization. In the "remote
> TX training" direction, the generic PHY is the producer of requests,
> based on its RX status, and the generic PHY consumer polls for these
> requests until we are happy. Each request is also sent (externally to
> the generic PHY layer) to the link partner board, for it to adjust its
> TX equalization.
>
> struct phy_configure_opts_ethernet is valid when phy_set_mode_ext() has
> been called with PHY_MODE_ETHERNET or PHY_MODE_ETHTOOL, same as with
> other union phy_configure_opts types.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
...
> +/**
> + * struct phy_configure_opts_ethernet - Ethernet PHY configuration set
nit: please include documentation of the structure members - type,
local_tx, and remote_tx - here.
> + *
> + * This structure is used to represent the configuration state of an Ethernet
> + * PHY (of various media types).
> + */
> +struct phy_configure_opts_ethernet {
> + enum ethernet_phy_configure_type type;
> + union {
> + struct c72_phy_configure_local_tx local_tx;
> + struct c72_phy_configure_remote_tx remote_tx;
> + };
> +};
...
On Sat, Sep 23, 2023 at 04:49:03PM +0300, Vladimir Oltean wrote:
...
> +static int mtip_rx_c72_coef_update(struct mtip_backplane *priv,
> + struct c72_coef_update *upd,
> + bool *rx_ready)
> +{
> + char upd_buf[C72_COEF_UPDATE_BUFSIZ], stat_buf[C72_COEF_STATUS_BUFSIZ];
> + struct device *dev = &priv->mdiodev->dev;
> + struct c72_coef_status stat = {};
> + int err, val;
> +
> + err = read_poll_timeout(mtip_read_lt_lp_coef_if_not_ready,
> + val, val < 0 || *rx_ready || LT_COEF_UPD_ANYTHING(val),
> + MTIP_COEF_STAT_SLEEP_US, MTIP_COEF_STAT_TIMEOUT_US,
> + false, priv, rx_ready);
> + if (val < 0)
> + return val;
> + if (*rx_ready) {
> + if (!priv->any_request_received)
> + dev_warn(dev,
> + "LP says its RX is ready, but there was no coefficient request (LP_STAT = 0x%x, LD_STAT = 0x%x)\n",
> + mtip_read_lt(priv, LT_LP_STAT),
> + mtip_read_lt(priv, LT_LD_STAT));
> + else
> + dev_dbg(dev, "LP says its RX is ready\n");
> + return 0;
> + }
> + if (err) {
> + dev_err(dev,
> + "LP did not request coefficient updates; LP_COEF = 0x%x\n",
> + val);
> + return err;
> + }
> +
> + upd->com1 = LT_COM1_X(val);
> + upd->coz = LT_COZ_X(val);
> + upd->cop1 = LT_COP1_X(val);
> + upd->init = !!(val & LT_COEF_UPD_INIT);
> + upd->preset = !!(val & LT_COEF_UPD_PRESET);
> +
Hi Vladimir,
I'm unsure if this can actually happen.
But if the while loop runs zero times then err is used uninitialised here.
As flagged by Smatch.
> + mtip_an_restart_from_lt(priv);
> +
> + kfree(lt_work);
> +}
> +
> +/* Train the link partner TX, so that the local RX quality improves */
> +static void mtip_remote_tx_lt_work(struct kthread_work *work)
> +{
> + struct mtip_lt_work *lt_work = container_of(work, struct mtip_lt_work,
> + work);
> + struct mtip_backplane *priv = lt_work->priv;
> + struct device *dev = &priv->mdiodev->dev;
> + int err;
> +
> + while (true) {
> + struct c72_coef_status status = {};
> + union phy_configure_opts opts = {
> + .ethernet = {
> + .type = C72_REMOTE_TX,
> + },
> + };
> +
> + if (READ_ONCE(priv->lt_stop_request))
> + goto out;
Likewise, I'm unsure if this can happen.
But if the condition above is met on the first iteration of
the loop then the out label will use err without it being initialised.
Also flagged by Smatch.
> +
> + err = mtip_lt_in_progress(priv);
> + if (err) {
> + dev_err(dev, "Remote TX LT failed: %pe\n", ERR_PTR(err));
> + goto out;
> + }
> +
> + err = phy_configure(priv->serdes, &opts);
> + if (err) {
> + dev_err(dev,
> + "Failed to get remote TX training request from SerDes: %pe\n",
> + ERR_PTR(err));
> + goto out;
> + }
> +
> + if (opts.ethernet.remote_tx.rx_ready)
> + break;
> +
> + err = mtip_tx_c72_coef_update(priv, &opts.ethernet.remote_tx.update,
> + &status);
> + if (opts.ethernet.remote_tx.cb)
> + opts.ethernet.remote_tx.cb(opts.ethernet.remote_tx.cb_priv,
> + err, opts.ethernet.remote_tx.update,
> + status);
> + if (err)
> + goto out;
> + }
> +
> + /* Let the link partner know we're done */
> + err = mtip_modify_lt(priv, LT_LD_STAT, LT_COEF_STAT_RX_READY,
> + LT_COEF_STAT_RX_READY);
> + if (err < 0) {
> + dev_err(dev, "Failed to update LT_LD_STAT: %pe\n",
> + ERR_PTR(err));
> + goto out;
> + }
> +
> + err = mtip_remote_tx_lt_done(priv);
> + if (err) {
> + dev_err(dev, "Failed to finalize remote LT: %pe\n",
> + ERR_PTR(err));
> + goto out;
> + }
> +
> +out:
> + if (err && !READ_ONCE(priv->lt_stop_request))
> + mtip_an_restart_from_lt(priv);
> +
> + kfree(lt_work);
> +}
...
On Sat, Sep 23, 2023 at 04:48:52PM +0300, Vladimir Oltean wrote:
...
> +/**
> + * coef_update_opposite - return the opposite of one C72 coefficient update
> + * request
> + *
> + * @update: original coefficient update
> + *
> + * Helper to transform the update request of one equalization tap into a
> + * request of the same tap in the opposite direction. May be used by C72
> + * phy remote TX link training algorithms.
> + */
> +static inline enum coef_update coef_update_opposite(enum coef_update update)
Hi Vladimir,
another nit from me.
Please put the inline keyword first.
Likewise elsewhere in this patch.
Tooling, including gcc-13 with W=1, complains about this.
> +{
> + switch (update) {
> + case COEF_UPD_INC:
> + return COEF_UPD_DEC;
> + case COEF_UPD_DEC:
> + return COEF_UPD_INC;
> + default:
> + return COEF_UPD_HOLD;
> + }
> +}
...
On 23-09-23, 16:48, Vladimir Oltean wrote:
> The bit stream handled by a SerDes lane needs protocol converters to be
> usable for Ethernet. On Freescale/NXP SoCs, those protocol converters
> are located on the internal MDIO buses of the Ethernet MACs that need
> them.
>
> The location on that MDIO bus, on these SoCs, is not fixed, but given by
> some control registers of the SerDes block itself.
>
> Because no one modifies those addresses from the power-on default, so
> far we've relied on hardcoding the default values in the device trees,
> resulting in something like this:
>
> pcs_mdio1: mdio@8c07000 {
> compatible = "fsl,fman-memac-mdio";
>
> pcs1: ethernet-phy@0 {
> reg = <0>;
> };
> };
>
> where the "reg" of "pcs1" can actually be retrieved from "serdes_1".
>
> That was for the PCS. For AN/LT blocks, that can also be done, but the
> MAC to PCS to AN/LT block mapping is non-trivial and extremely easy to
> get wrong, which will confuse and frustrate any device tree writers.
>
> The proposal is to take advantage of the fact that these protocol
> converters *are* discoverable, and to side-step that entire device tree
> mapping issue by not putting them in the device tree at all. So, one of
> the consumers of the SerDes PHY uses the phy_get_status() API to figure
> out the address on the MDIO bus, it also has a reference to the MDIO bus
> => it can create the mdio_device in a non OF-based manner.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> v1->v2: patch is new
>
> include/linux/phy/phy.h | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index f1f03fa66943..ee721067517b 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -56,6 +56,33 @@ enum phy_media {
> enum phy_status_type {
> /* Valid for PHY_MODE_ETHERNET and PHY_MODE_ETHTOOL */
> PHY_STATUS_CDR_LOCK,
> + PHY_STATUS_PCVT_ADDR,
> +};
> +
> +/* enum phy_pcvt_type - PHY protocol converter type
It is not a generic protocol converter but an ethernet phy protocol
converter, so i guess we should add that here (we are generic phy and
not ethernet phy here!
> + *
> + * @PHY_PCVT_ETHERNET_PCS: Ethernet Physical Coding Sublayer, top-most layer of
> + * an Ethernet PHY. Connects through MII to the MAC,
> + * and handles link status detection and the conversion
> + * of MII signals to link-specific code words (8b/10b,
> + * 64b/66b etc).
> + * @PHY_PCVT_ETHERNET_ANLT: Ethernet Auto-Negotiation and Link Training,
> + * bottom-most layer of an Ethernet PHY, beneath the
> + * PMA and PMD. Its activity is only visible on the
> + * physical medium, and it is responsible for
> + * selecting the most adequate PCS/PMA/PMD set that
> + * can operate on that medium.
> + */
> +enum phy_pcvt_type {
> + PHY_PCVT_ETHERNET_PCS,
> + PHY_PCVT_ETHERNET_ANLT,
> +};
> +
> +struct phy_status_opts_pcvt {
> + enum phy_pcvt_type type;
> + union {
> + unsigned int mdio;
> + } addr;
> };
>
> /* If the CDR (Clock and Data Recovery) block is able to lock onto the RX bit
> @@ -71,9 +98,11 @@ struct phy_status_opts_cdr {
> * union phy_status_opts - Opaque generic phy status
> *
> * @cdr: Configuration set applicable for PHY_STATUS_CDR_LOCK.
> + * @pcvt: Configuration set applicable for PHY_STATUS_PCVT_ADDR.
> */
> union phy_status_opts {
> struct phy_status_opts_cdr cdr;
> + struct phy_status_opts_pcvt pcvt;
> };
>
> /**
> --
> 2.34.1
--
~Vinod
Hi Vinod,
On Fri, Sep 29, 2023 at 09:53:22PM +0530, Vinod Koul wrote:
> On 23-09-23, 16:48, Vladimir Oltean wrote:
> > The bit stream handled by a SerDes lane needs protocol converters to be
> > usable for Ethernet. On Freescale/NXP SoCs, those protocol converters
> > are located on the internal MDIO buses of the Ethernet MACs that need
> > them.
> >
> > The location on that MDIO bus, on these SoCs, is not fixed, but given by
> > some control registers of the SerDes block itself.
> >
> > Because no one modifies those addresses from the power-on default, so
> > far we've relied on hardcoding the default values in the device trees,
> > resulting in something like this:
> >
> > pcs_mdio1: mdio@8c07000 {
> > compatible = "fsl,fman-memac-mdio";
> >
> > pcs1: ethernet-phy@0 {
> > reg = <0>;
> > };
> > };
> >
> > where the "reg" of "pcs1" can actually be retrieved from "serdes_1".
> >
> > That was for the PCS. For AN/LT blocks, that can also be done, but the
> > MAC to PCS to AN/LT block mapping is non-trivial and extremely easy to
> > get wrong, which will confuse and frustrate any device tree writers.
> >
> > The proposal is to take advantage of the fact that these protocol
> > converters *are* discoverable, and to side-step that entire device tree
> > mapping issue by not putting them in the device tree at all. So, one of
> > the consumers of the SerDes PHY uses the phy_get_status() API to figure
> > out the address on the MDIO bus, it also has a reference to the MDIO bus
> > => it can create the mdio_device in a non OF-based manner.
> >
> > Signed-off-by: Vladimir Oltean <[email protected]>
> > ---
> > v1->v2: patch is new
> >
> > include/linux/phy/phy.h | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index f1f03fa66943..ee721067517b 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -56,6 +56,33 @@ enum phy_media {
> > enum phy_status_type {
> > /* Valid for PHY_MODE_ETHERNET and PHY_MODE_ETHTOOL */
> > PHY_STATUS_CDR_LOCK,
> > + PHY_STATUS_PCVT_ADDR,
> > +};
> > +
> > +/* enum phy_pcvt_type - PHY protocol converter type
>
> It is not a generic protocol converter but an ethernet phy protocol
> converter, so i guess we should add that here (we are generic phy and
> not ethernet phy here!
Can you please show, using a diff, what modification you would like to see?
In my mind, enum phy_pcvt_type could also contain non-Ethernet protocol
converter types, and so, I didn't want to add "ethernet_" in it.
The "ETHERNET_" prefix will be part of the individual enum values that
are applicable to Ethernet.
> > + *
> > + * @PHY_PCVT_ETHERNET_PCS: Ethernet Physical Coding Sublayer, top-most layer of
> > + * an Ethernet PHY. Connects through MII to the MAC,
> > + * and handles link status detection and the conversion
> > + * of MII signals to link-specific code words (8b/10b,
> > + * 64b/66b etc).
> > + * @PHY_PCVT_ETHERNET_ANLT: Ethernet Auto-Negotiation and Link Training,
> > + * bottom-most layer of an Ethernet PHY, beneath the
> > + * PMA and PMD. Its activity is only visible on the
> > + * physical medium, and it is responsible for
> > + * selecting the most adequate PCS/PMA/PMD set that
> > + * can operate on that medium.
> > + */
> > +enum phy_pcvt_type {
> > + PHY_PCVT_ETHERNET_PCS,
> > + PHY_PCVT_ETHERNET_ANLT,
> > +};
> > +
> > +struct phy_status_opts_pcvt {
> > + enum phy_pcvt_type type;
> > + union {
> > + unsigned int mdio;
> > + } addr;
> > };
> >
> > /* If the CDR (Clock and Data Recovery) block is able to lock onto the RX bit
> > @@ -71,9 +98,11 @@ struct phy_status_opts_cdr {
> > * union phy_status_opts - Opaque generic phy status
> > *
> > * @cdr: Configuration set applicable for PHY_STATUS_CDR_LOCK.
> > + * @pcvt: Configuration set applicable for PHY_STATUS_PCVT_ADDR.
> > */
> > union phy_status_opts {
> > struct phy_status_opts_cdr cdr;
> > + struct phy_status_opts_pcvt pcvt;
> > };
> >
> > /**
> > --
> > 2.34.1
>
> --
> ~Vinod
Hi Simon,
On Thu, Sep 28, 2023 at 09:05:36PM +0200, Simon Horman wrote:
> On Sat, Sep 23, 2023 at 04:48:52PM +0300, Vladimir Oltean wrote:
>
> ...
>
> > +/**
> > + * coef_update_opposite - return the opposite of one C72 coefficient update
> > + * request
> > + *
> > + * @update: original coefficient update
> > + *
> > + * Helper to transform the update request of one equalization tap into a
> > + * request of the same tap in the opposite direction. May be used by C72
> > + * phy remote TX link training algorithms.
> > + */
> > +static inline enum coef_update coef_update_opposite(enum coef_update update)
>
> Hi Vladimir,
>
> another nit from me.
>
> Please put the inline keyword first.
> Likewise elsewhere in this patch.
>
> Tooling, including gcc-13 with W=1, complains about this.
Thanks for pointing this out. I guess you are talking about the c72_coef_update_print()
function, whose prototype is mistakenly "static void inline" instead of
"static inline void". I cannot find the problem with the quoted coef_update_opposite().
On Mon, Oct 02, 2023 at 04:11:10PM +0300, Vladimir Oltean wrote:
> Hi Simon,
>
> On Thu, Sep 28, 2023 at 09:05:36PM +0200, Simon Horman wrote:
> > On Sat, Sep 23, 2023 at 04:48:52PM +0300, Vladimir Oltean wrote:
> >
> > ...
> >
> > > +/**
> > > + * coef_update_opposite - return the opposite of one C72 coefficient update
> > > + * request
> > > + *
> > > + * @update: original coefficient update
> > > + *
> > > + * Helper to transform the update request of one equalization tap into a
> > > + * request of the same tap in the opposite direction. May be used by C72
> > > + * phy remote TX link training algorithms.
> > > + */
> > > +static inline enum coef_update coef_update_opposite(enum coef_update update)
> >
> > Hi Vladimir,
> >
> > another nit from me.
> >
> > Please put the inline keyword first.
> > Likewise elsewhere in this patch.
> >
> > Tooling, including gcc-13 with W=1, complains about this.
>
> Thanks for pointing this out. I guess you are talking about the c72_coef_update_print()
> function, whose prototype is mistakenly "static void inline" instead of
> "static inline void". I cannot find the problem with the quoted coef_update_opposite().
Yes, you are right.
Sorry for my error.
On 9/23/23 06:48, Vladimir Oltean wrote:
> Some modules, like the MTIP AN/LT block used as a copper backplane PHY
> driver, need this extra information from the SerDes PHY as another
> source of "link up" information.
>
> Namely, the 25GBase-R PCS does not have a MDIO_CTRL1_LPOWER bit
> implemented in its MDIO_MMD_PCS:MDIO_CTRL1 register. That bit is
> typically set from phy_suspend() or phylink_pcs_disable() implementations,
> and that is supposed to cause a link drop event on the link partner.
> But here it does not happen.
>
> By implementing the networking phylink_pcs_disable() as phy_power_off(),
> we are able to actually power down the lane in a way that is visible to
> the remote end. Where it is visible is the CDR lock, so we introduce
> PHY_STATUS_TYPE_CDR_LOCK as an extra link indication, we are able to
> detect that condition and signal it to upper layers of the network
> stack.
>
> A more high-level and generic phy_get_status() operation was chosen
> instead of the more specific phy_get_cdr_lock() alternative, because I
> saw this as being more in the spirit of the generic PHY API.
> Also, phy_get_status() is more extensible and reusable for other
> purposes as well.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
Hi Krzysztof,
On Sun, Sep 24, 2023 at 01:49:24PM +0200, Krzysztof Kozlowski wrote:
> On 23/09/2023 15:49, Vladimir Oltean wrote:
> > When the Lynx PCS is deployed on a copper backplane link, it must be
> > prepared to handle clause 73 autoneg and clause 72 link training, which
> > it can do using a dedicated AN/LT block. The latter doesn't need to be
> > described in the device tree, because it is discoverable from the SerDes
> > lanes.
> >
> > The media type that is deployed on the link is not discoverable though,
> > so the introduction of a fsl,backplane-mode boolean property appears
> > necessary to determine whether the AN/LT block should be employed, or
> > left bypassed.
> >
> > Signed-off-by: Vladimir Oltean <[email protected]>
> > ---
> > v1->v2: patch is new
> >
> > .../devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml b/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> > index fbedf696c555..40fbcd80ee2a 100644
> > --- a/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> > +++ b/Documentation/devicetree/bindings/net/pcs/fsl,lynx-pcs.yaml
> > @@ -16,11 +16,24 @@ description: |
> >
> > properties:
> > compatible:
> > - const: fsl,lynx-pcs
> > + enum:
> > + - fsl,lx2160a-lynx-pcs
> > + - fsl,lynx-pcs
> >
> > reg:
> > maxItems: 1
> >
> > + phys:
> > + maxItems: 4
> > + description:
> > + phandle for the SerDes lanes that act as PMA/PMD layer when the PCS is
> > + part of a copper backplane PHY.
> > +
> > + fsl,backplane-mode:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + Indicates that the PCS is deployed over a copper backplane link.
> > +
>
> Please extend also existing example.
Ok. Snippet for attention (a dtsi I was working with - applies over fsl-lx2160a-qds.dts):
&dpmac2 {
phy-connection-type = "internal";
managed = "in-band-status";
/delete-property/ phys;
};
&pcs_mdio2 {
status = "okay";
};
&pcs2 {
fsl,backplane-mode;
phys = <&serdes_1 3>, /* lane D */
<&serdes_1 2>, /* lane C */
<&serdes_1 1>, /* lane B */
<&serdes_1 0>; /* lane A */
};
The thing is that the RFC v2 bindings are still very much WIP. For v3,
I will try to remove the "phys" property from the pcs node, and process
the ones from the MAC node (client of PCS).
For example, arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-itx.dtsi
has "phys" under &dpmac7:
&dpmac7 {
sfp = <&sfp0>;
managed = "in-band-status";
phys = <&serdes_1 3>;
};
but &dpmac7 also has pcs-handle = <&pcs7>; in fsl-lx2160a.dtsi.
Thus, if I'm able to pass the "phys" phandle from &dpmac7 to &pcs7
through code (argument to lynx_pcs_create_fwnode()), then the location
of the "phys" property could be the same regardless of use case
(backplane or not), and the dt-bindings of the lynx pcs would be simpler.
The only dilemma that has stopped me from doing it is that the dpmac node
may have other "phys" in the signal path (for example external redrivers/
retimers). With up to 4 SerDes lanes per MAC and with optional retimer
phys on each lane, it becomes a question of how can we distinguish the
SerDes phys from the other phys (the PCS wants the SerDes PHY)? Would it
be okay to add a phy-names property, and parse it for "serdes-%d" to
indicate a SerDes PHY, and anything else can be named in any other way
("retimer-%d")?
> If these do not apply to lynx-pcs, then they should be disallowed in
> allOf:if:then.
They do: the "fsl,lynx-pcs" compatible string + the bool "fsl,backplane-mode"
is supposed to instantiate a MTIP_MODEL_AUTODETECT backplane AN/LT block
(as opposed to LX2160A where it isn't autodetectable). It's just that I
didn't get to implement support for the other models yet.
On 9/23/23 06:48, Vladimir Oltean wrote:
> In networking, we have 2 distinct data types:
>
> - phy_interface_t describes the link between a MAC (or MAC-side PCS) and
> an attached PHY or SFP cage
>
> - enum ethtool_link_mode_bit_indices describes the link between a local
> PHY and a remote PHY (for example, gigabit RJ45 twisted copper pairs)
>
> Currently, phy_set_mode_ext(PHY_MODE_ETHERNET) takes arguments of the
> phy_interface_t type, and there is no way to pass an argument of the
> enum ethtool_link_mode_bit_indices type. The new PHY_MODE_ETHTOOL
> intends to address that.
>
> It is true that there is currently some overlap between these data
> types, namely:
>
> phy_interface_t enum ethtool_link_mode_bit_indices
> -----------------------------------------------------------------
> PHY_INTERFACE_MODE_10GKR ETHTOOL_LINK_MODE_10000baseKR_Full_BIT
> PHY_INTERFACE_MODE_1000BASEKX ETHTOOL_LINK_MODE_1000baseKX_Full_BIT
>
> but those overlaps were deemed to be mistakes, and PHY-to-PHY link modes
> should only be added to ethtool_link_mode_bit_indices going forward.
> Thus, I believe that the distinction is necessary, rather than hacking
> more improper PHY modes. Some of the PHY-to-PHY link modes which may be
> added in the future (to ethtool_link_mode_bit_indices and not to
> phy_interface_t) are:
>
> ETHTOOL_LINK_MODE_100000baseKP4_Full_BIT
> ETHTOOL_LINK_MODE_100000baseCR10_Full_BIT
> ETHTOOL_LINK_MODE_25000baseKR_S_Full_BIT
> ETHTOOL_LINK_MODE_25000baseCR_S_Full_BIT
>
> One user of PHY_MODE_ETHTOOL will be the MTIP backplane AN/LT + Lynx
> SerDes PHY combo, where the backplane autoneg protocol (IEEE 802.3
> clause 73) selects the operating PHY-to-PHY link mode.
>
> There are electrical differences between the PHY-to-PHY backplane link
> modes (like ETHTOOL_LINK_MODE_10000baseKR_Full_BIT) and their
> non-backplane counterparts (like PHY_INTERFACE_MODE_10GBASER), namely
> the number of TX signal equalization taps and their configurability.
> This further justifies distinguishing between them in the generic PHY
> API.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> v1->v2: rename PHY_MODE_ETHERNET_PHY to PHY_MODE_ETHTOOL at Russell's
> suggestion
>
> include/linux/phy/phy.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 6be348f1fa0e..72ef4afcda81 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -39,6 +39,7 @@ enum phy_mode {
> PHY_MODE_UFS_HS_B,
> PHY_MODE_PCIE,
> PHY_MODE_ETHERNET,
> + PHY_MODE_ETHTOOL,
Not feeling very comfortable with using ETHTOOL here because that is a
Linux sub-subsystem name as opposed to the other enumeration values
which are electrical modes of operation and/or industry standards names.
How about PHY_MODE_ETHERNET_EXPLICIT or PHY_MODE_ETHERNET_LINKMODE?
--
Florian
Hi Russell,
On Sat, Sep 23, 2023 at 04:48:57PM +0300, Vladimir Oltean wrote:
> Some phylink and phylib based systems might want to operate on backplane
> media types ("K" in the name), and thus, picking a phy_interface_t for
> them becomes a challenge.
>
> phy_interface_t is a description of the connection between the MAC and
> the PHY, or if a MAC-side PCS is present, the connection between that
> and the next link segment (which can be remote).
>
> A MAC-side PCS is so far considered to be a PCS handling link modes with
> optional C37 autoneg. But C73 autoneg (for backplanes and SFP28 modules)
> is not at the same level in the OSI layering, so that existing model may
> or may not apply.
>
> (a) If we say that the PCS is MAC-side for C73 modes as well, the
> implication seems to be that the phy-mode should be one of
> PHY_INTERFACE_MODE_10GBASEKR, PHY_INTERFACE_MODE_1000BASEKX, etc.
> Similar to PHY_INTERFACE_MODE_1000BASEX which imitates the link mode
> ETHTOOL_LINK_MODE_1000baseX_Full_BIT.
>
> (b) If we say that the PCS is not MAC-side, but rather that the
> phylink_pcs represents an entire non-phylib backplane PHY which may
> negotiate one of many link modes (like a copper phylib PHY), then
> the phy-mode should probably be one of PHY_INTERFACE_MODE_XGMII,
> XLGMII etc. Or rather, because there is no MII pinout per se and the
> backplane PHY / phylink_pcs is internal, we can also use
> PHY_INTERFACE_MODE_INTERNAL.
>
> The trouble with (a), in my opinion, is that if we let the phy_interface_t
> follow the link mode like in the case of Base-X fiber modes, we have to
> consider the fact that C73 PHYs can advertise multiple link modes, so
> the phy_interface_t selection will be arbitrary, and any phy_interface_t
> selection will have to leave in the "supported" and "advertised" masks
> of link modes all the other backplane modes. This may be hard to justify.
>
> That is the reasoning based on which I selected this phy-mode to
> describe the setup in Layerscape SoCs which have integrated backplane
> autoneg support. The changes in phylink permit the managed =
> "in-band-status" fwnode property to be extended for C73 autoneg, which
> is then controllable through ethtool. With phy-mode = "internal" in an
> in-band autoneg mode, we advertise all backplane link modes. The list is
> not exhaustive and may be extended in the future.
>
> Link: https://lore.kernel.org/netdev/[email protected]/
> Link: https://lore.kernel.org/netdev/[email protected]/
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> v1->v2: patch is new
>
> drivers/net/phy/phylink.c | 19 ++++++++++++++++++-
> include/linux/phylink.h | 1 +
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 548130d77302..88ace7e203c3 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -972,6 +972,21 @@ static int phylink_parse_mode(struct phylink *pl,
> phylink_set(pl->supported, 100000baseDR2_Full);
> break;
>
> + case PHY_INTERFACE_MODE_INTERNAL:
> + phylink_set(pl->supported, 1000baseKX_Full);
> + phylink_set(pl->supported, 10000baseKX4_Full);
> + phylink_set(pl->supported, 10000baseKR_Full);
> + phylink_set(pl->supported, 25000baseCR_Full);
> + phylink_set(pl->supported, 25000baseKR_Full);
> + phylink_set(pl->supported, 25000baseCR_S_Full);
> + phylink_set(pl->supported, 25000baseKR_S_Full);
> + phylink_set(pl->supported, 40000baseKR4_Full);
> + phylink_set(pl->supported, 50000baseKR2_Full);
> + phylink_set(pl->supported, 50000baseKR_Full);
> + phylink_set(pl->supported, 100000baseKR4_Full);
> + phylink_set(pl->supported, 100000baseKR2_Full);
> + break;
> +
> default:
> phylink_err(pl,
> "incorrect link mode %s for in-band status\n",
> @@ -1109,7 +1124,9 @@ static void phylink_mac_config(struct phylink *pl,
>
> static bool phylink_pcs_handles_an(phy_interface_t iface, unsigned int mode)
> {
> - return phy_interface_mode_is_8023z(iface) && phylink_autoneg_inband(mode);
> + return (phy_interface_mode_is_8023z(iface) ||
> + iface == PHY_INTERFACE_MODE_INTERNAL) &&
> + phylink_autoneg_inband(mode);
> }
>
> static void phylink_pcs_an_restart(struct phylink *pl)
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index 2b886ea654bb..7e8e26001587 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -141,6 +141,7 @@ static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
>
> case PHY_INTERFACE_MODE_1000BASEX:
> case PHY_INTERFACE_MODE_2500BASEX:
> + case PHY_INTERFACE_MODE_INTERNAL:
> /* 1000base-X is designed for use media-side for Fibre
> * connections, and thus the Autoneg bit needs to be
> * taken into account. We also do this for 2500base-X
> --
> 2.34.1
>
What do you think about this change?
Hi Vladimir,
On Mon, Oct 02, 2023 at 05:17:43PM +0300, Vladimir Oltean wrote:
> Hi Russell,
>
> On Sat, Sep 23, 2023 at 04:48:57PM +0300, Vladimir Oltean wrote:
> > Some phylink and phylib based systems might want to operate on backplane
> > media types ("K" in the name), and thus, picking a phy_interface_t for
> > them becomes a challenge.
> >
> > phy_interface_t is a description of the connection between the MAC and
> > the PHY, or if a MAC-side PCS is present, the connection between that
> > and the next link segment (which can be remote).
> >
> > A MAC-side PCS is so far considered to be a PCS handling link modes with
> > optional C37 autoneg. But C73 autoneg (for backplanes and SFP28 modules)
> > is not at the same level in the OSI layering, so that existing model may
> > or may not apply.
> >
> > (a) If we say that the PCS is MAC-side for C73 modes as well, the
> > implication seems to be that the phy-mode should be one of
> > PHY_INTERFACE_MODE_10GBASEKR, PHY_INTERFACE_MODE_1000BASEKX, etc.
> > Similar to PHY_INTERFACE_MODE_1000BASEX which imitates the link mode
> > ETHTOOL_LINK_MODE_1000baseX_Full_BIT.
> >
> > (b) If we say that the PCS is not MAC-side, but rather that the
> > phylink_pcs represents an entire non-phylib backplane PHY which may
> > negotiate one of many link modes (like a copper phylib PHY), then
> > the phy-mode should probably be one of PHY_INTERFACE_MODE_XGMII,
> > XLGMII etc. Or rather, because there is no MII pinout per se and the
> > backplane PHY / phylink_pcs is internal, we can also use
> > PHY_INTERFACE_MODE_INTERNAL.
> >
> > The trouble with (a), in my opinion, is that if we let the phy_interface_t
> > follow the link mode like in the case of Base-X fiber modes, we have to
> > consider the fact that C73 PHYs can advertise multiple link modes, so
> > the phy_interface_t selection will be arbitrary, and any phy_interface_t
> > selection will have to leave in the "supported" and "advertised" masks
> > of link modes all the other backplane modes. This may be hard to justify.
> >
> > That is the reasoning based on which I selected this phy-mode to
> > describe the setup in Layerscape SoCs which have integrated backplane
> > autoneg support. The changes in phylink permit the managed =
> > "in-band-status" fwnode property to be extended for C73 autoneg, which
> > is then controllable through ethtool. With phy-mode = "internal" in an
> > in-band autoneg mode, we advertise all backplane link modes. The list is
> > not exhaustive and may be extended in the future.
> >
> > Link: https://lore.kernel.org/netdev/[email protected]/
> > Link: https://lore.kernel.org/netdev/[email protected]/
> > Signed-off-by: Vladimir Oltean <[email protected]>
> > ---
> > v1->v2: patch is new
> >
> > drivers/net/phy/phylink.c | 19 ++++++++++++++++++-
> > include/linux/phylink.h | 1 +
> > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index 548130d77302..88ace7e203c3 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -972,6 +972,21 @@ static int phylink_parse_mode(struct phylink *pl,
> > phylink_set(pl->supported, 100000baseDR2_Full);
> > break;
> >
> > + case PHY_INTERFACE_MODE_INTERNAL:
> > + phylink_set(pl->supported, 1000baseKX_Full);
> > + phylink_set(pl->supported, 10000baseKX4_Full);
> > + phylink_set(pl->supported, 10000baseKR_Full);
> > + phylink_set(pl->supported, 25000baseCR_Full);
> > + phylink_set(pl->supported, 25000baseKR_Full);
> > + phylink_set(pl->supported, 25000baseCR_S_Full);
> > + phylink_set(pl->supported, 25000baseKR_S_Full);
> > + phylink_set(pl->supported, 40000baseKR4_Full);
> > + phylink_set(pl->supported, 50000baseKR2_Full);
> > + phylink_set(pl->supported, 50000baseKR_Full);
> > + phylink_set(pl->supported, 100000baseKR4_Full);
> > + phylink_set(pl->supported, 100000baseKR2_Full);
> > + break;
I wonder whether this should just set all link modes, much like
phylink_get_capabilities() allows.
I'm also wondering whether the contents of this switch() statement
should now just do:
case PHY_INTERFACE_... (for each supported mode):
cals = ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
caps = phylink_get_capabilities(interface, caps,
RATE_MATCH_NONE);
phylink_caps_to_linkmodes(pl->supported, caps);
break;
rather than duplicating the logic.
That said, 10GBASER and 10GKR are treated slightly differently because
of the problem with PHYs like 88x3310, and I think it's now difficult
to undo that bit of history.
> > +
> > default:
> > phylink_err(pl,
> > "incorrect link mode %s for in-band status\n",
> > @@ -1109,7 +1124,9 @@ static void phylink_mac_config(struct phylink *pl,
> >
> > static bool phylink_pcs_handles_an(phy_interface_t iface, unsigned int mode)
> > {
> > - return phy_interface_mode_is_8023z(iface) && phylink_autoneg_inband(mode);
> > + return (phy_interface_mode_is_8023z(iface) ||
> > + iface == PHY_INTERFACE_MODE_INTERNAL) &&
> > + phylink_autoneg_inband(mode);
Is this true also for DSA devices that use "internal" mode? I'm
wondering whether this will cause the PHY to be ignored/remain
unattached in DSA switches because of the changes in patch 7.
> > }
> >
> > static void phylink_pcs_an_restart(struct phylink *pl)
> > diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> > index 2b886ea654bb..7e8e26001587 100644
> > --- a/include/linux/phylink.h
> > +++ b/include/linux/phylink.h
> > @@ -141,6 +141,7 @@ static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
> >
> > case PHY_INTERFACE_MODE_1000BASEX:
> > case PHY_INTERFACE_MODE_2500BASEX:
> > + case PHY_INTERFACE_MODE_INTERNAL:
> > /* 1000base-X is designed for use media-side for Fibre
> > * connections, and thus the Autoneg bit needs to be
> > * taken into account. We also do this for 2500base-X
Thinking about DSA cases, I don't think this change would be an issue
because where DSA uses "internal" there isn't a PCS, so this won't
matter.
Note that as there is now no need for anything outside phylink.c to
reference this function, I have plans at some point to move it into
the .c file rather than keeping it as an inline in the header file.
It was temporarily necessary while introducing it to be in the
header.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Sat, Sep 23, 2023 at 04:48:58PM +0300, Vladimir Oltean wrote:
> Allow driver code to print stuff like the resolved link mode to the
> kernel log, by giving it access to the link_mode_names[] ethtool
> internal array which already holds this info.
Looks good to me.
> Signed-off-by: Vladimir Oltean <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Thanks!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Sat, Sep 23, 2023 at 04:49:00PM +0300, Vladimir Oltean wrote:
> Treat the newly introduced subsets of 25GBase-KR and 25GBase-CR the same
> way as the fully-featured link modes. The difference only consists in
> RS-FEC support.
As mentioned in the patch adding these new linkmodes, I wonder whether
this should be part of that patch. Is there a reason to keep it
separate?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Sat, Sep 23, 2023 at 04:49:04PM +0300, Vladimir Oltean wrote:
> +static int lynx_pcs_parse_fwnode(struct lynx_pcs *lynx)
> +{
> + struct fwnode_handle *node = lynx->mdio->dev.fwnode;
> + enum mtip_model model = MTIP_MODEL_AUTODETECT;
> + struct device_node *np = to_of_node(node);
> + struct mdio_device *mdio = lynx->mdio;
> + struct device *dev = &mdio->dev;
> + struct phy *phy;
> + int i, err;
> +
> + if (!node)
> + return 0;
> +
> + lynx->backplane_mode = fwnode_property_present(node, "fsl,backplane-mode");
> + if (!lynx->backplane_mode)
> + return 0;
> +
> + if (fwnode_device_is_compatible(node, "fsl,lx2160a-lynx-pcs"))
> + model = MTIP_MODEL_LX2160A;
> +
> + lynx->num_lanes = of_count_phandle_with_args(np, "phys", "#phy-cells");
> + if (lynx->num_lanes < 0)
> + return lynx->num_lanes;
Is it possible for ->num_lanes to be zero at this point? If that is
possible, then ->anlt[PRIMARY_LANE] will be NULL but ->backplane_mode
will be set, so won't that cause the mtip_* calls above to pass a
NULL pointer into those functions? Is that safe? Should we trap that
case here?
If that's correct, then I don't see any point in storing
->backplane_mode, since we can then use ->num_lanes > PRIMARY_LANE
or similar instead.
> +
> + if (WARN_ON(lynx->num_lanes > MAX_NUM_LANES))
> + return -EINVAL;
Do we need to use WARN_ON() here, or would it be better to print a short
error-level message?
> +
> + for (i = 0; i < lynx->num_lanes; i++) {
> + phy = devm_of_phy_get_by_index(dev, np, i);
> + if (IS_ERR(phy))
> + return dev_err_probe(dev, PTR_ERR(phy),
> + "Failed to get SerDes PHY %d\n", i);
> +
> + lynx->anlt[i] = mtip_backplane_create(mdio, phy, model);
> + if (IS_ERR(lynx->anlt[i])) {
> + err = PTR_ERR(lynx->anlt[i]);
> +
> + while (i-- > 0)
> + mtip_backplane_destroy(lynx->anlt[i]);
> +
> + return err;
> + }
> + }
> +
> + for (i = 1; i < lynx->num_lanes; i++) {
> + err = mtip_backplane_add_subordinate(lynx->anlt[PRIMARY_LANE],
> + lynx->anlt[i]);
> + if (WARN_ON(err)) {
Again, does this need to be a backtrace-producing WARN_ON()?
> + /* Too many SerDes lanes in the device tree? */
> + for (i = 0; i < lynx->num_lanes; i++)
> + mtip_backplane_destroy(lynx->anlt[i]);
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
> {
> struct lynx_pcs *lynx;
> + int err;
>
> lynx = kzalloc(sizeof(*lynx), GFP_KERNEL);
> if (!lynx)
> @@ -327,6 +451,12 @@ static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
> lynx->pcs.neg_mode = true;
> lynx->pcs.poll = true;
>
> + err = lynx_pcs_parse_fwnode(lynx);
> + if (err) {
> + kfree(lynx);
> + return ERR_PTR(err);
> + }
> +
> return lynx_to_phylink_pcs(lynx);
> }
>
> @@ -392,6 +522,11 @@ EXPORT_SYMBOL_GPL(lynx_pcs_create_fwnode);
> void lynx_pcs_destroy(struct phylink_pcs *pcs)
> {
> struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
> + int i;
> +
> + if (lynx->backplane_mode)
> + for (i = 0; i < lynx->num_lanes; i++)
> + mtip_backplane_destroy(lynx->anlt[i]);
Won't ->num_lanes only be non-zero when ->backplane_mode is set, so
isn't the test for ->backplane_mode redundant here?
>
> mdio_device_put(lynx->mdio);
> kfree(lynx);
> --
> 2.34.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Sat, Sep 23, 2023 at 04:49:01PM +0300, Vladimir Oltean wrote:
> Allow phylink_resolve_c73() to resolve backplane (KR) or SFP28 (CR)
> link speeds of 25Gbps.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
Shouldn't this also be part of patch 5?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
On Tue, Oct 03, 2023 at 12:06:18PM +0100, Russell King (Oracle) wrote:
> I wonder whether this should just set all link modes, much like
> phylink_get_capabilities() allows.
>
> I'm also wondering whether the contents of this switch() statement
> should now just do:
>
> case PHY_INTERFACE_... (for each supported mode):
> cals = ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
> caps = phylink_get_capabilities(interface, caps,
> RATE_MATCH_NONE);
> phylink_caps_to_linkmodes(pl->supported, caps);
> break;
>
> rather than duplicating the logic.
You mean something like this?
[PATCH] net: phylink: reimplement population of pl->supported for in-band
phylink_parse_mode() populates all possible supported link modes for a
given phy_interface_t, for the case where a phylib phy may be absent and
we can't retrieve the supported link modes from that.
Russell points out that since the introduction of the generic validation
helpers phylink_get_capabilities() and phylink_caps_to_linkmodes(), we
can rewrite this procedure to populate the pl->supported mask, so that
instead of spelling out the link modes, we derive an intermediary
mac_capabilities bit field, and we convert that to the equivalent link
modes.
Suggested-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
drivers/net/phy/phylink.c | 71 +++------------------------------------
1 file changed, 5 insertions(+), 66 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 548130d77302..88a4b726a9fc 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -864,6 +864,7 @@ static int phylink_parse_mode(struct phylink *pl,
{
struct fwnode_handle *dn;
const char *managed;
+ unsigned long caps;
dn = fwnode_get_named_child_node(fwnode, "fixed-link");
if (dn || fwnode_property_present(fwnode, "fixed-link"))
@@ -896,80 +897,18 @@ static int phylink_parse_mode(struct phylink *pl,
case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_TXID:
case PHY_INTERFACE_MODE_RTBI:
- phylink_set(pl->supported, 10baseT_Half);
- phylink_set(pl->supported, 10baseT_Full);
- phylink_set(pl->supported, 100baseT_Half);
- phylink_set(pl->supported, 100baseT_Full);
- phylink_set(pl->supported, 1000baseT_Half);
- phylink_set(pl->supported, 1000baseT_Full);
- break;
-
case PHY_INTERFACE_MODE_1000BASEX:
- phylink_set(pl->supported, 1000baseX_Full);
- break;
-
case PHY_INTERFACE_MODE_2500BASEX:
- phylink_set(pl->supported, 2500baseX_Full);
- break;
-
case PHY_INTERFACE_MODE_5GBASER:
- phylink_set(pl->supported, 5000baseT_Full);
- break;
-
case PHY_INTERFACE_MODE_25GBASER:
- phylink_set(pl->supported, 25000baseCR_Full);
- phylink_set(pl->supported, 25000baseKR_Full);
- phylink_set(pl->supported, 25000baseSR_Full);
- fallthrough;
case PHY_INTERFACE_MODE_USXGMII:
case PHY_INTERFACE_MODE_10GKR:
case PHY_INTERFACE_MODE_10GBASER:
- phylink_set(pl->supported, 10baseT_Half);
- phylink_set(pl->supported, 10baseT_Full);
- phylink_set(pl->supported, 100baseT_Half);
- phylink_set(pl->supported, 100baseT_Full);
- phylink_set(pl->supported, 1000baseT_Half);
- phylink_set(pl->supported, 1000baseT_Full);
- phylink_set(pl->supported, 1000baseX_Full);
- phylink_set(pl->supported, 1000baseKX_Full);
- phylink_set(pl->supported, 2500baseT_Full);
- phylink_set(pl->supported, 2500baseX_Full);
- phylink_set(pl->supported, 5000baseT_Full);
- phylink_set(pl->supported, 10000baseT_Full);
- phylink_set(pl->supported, 10000baseKR_Full);
- phylink_set(pl->supported, 10000baseKX4_Full);
- phylink_set(pl->supported, 10000baseCR_Full);
- phylink_set(pl->supported, 10000baseSR_Full);
- phylink_set(pl->supported, 10000baseLR_Full);
- phylink_set(pl->supported, 10000baseLRM_Full);
- phylink_set(pl->supported, 10000baseER_Full);
- break;
-
case PHY_INTERFACE_MODE_XLGMII:
- phylink_set(pl->supported, 25000baseCR_Full);
- phylink_set(pl->supported, 25000baseKR_Full);
- phylink_set(pl->supported, 25000baseSR_Full);
- phylink_set(pl->supported, 40000baseKR4_Full);
- phylink_set(pl->supported, 40000baseCR4_Full);
- phylink_set(pl->supported, 40000baseSR4_Full);
- phylink_set(pl->supported, 40000baseLR4_Full);
- phylink_set(pl->supported, 50000baseCR2_Full);
- phylink_set(pl->supported, 50000baseKR2_Full);
- phylink_set(pl->supported, 50000baseSR2_Full);
- phylink_set(pl->supported, 50000baseKR_Full);
- phylink_set(pl->supported, 50000baseSR_Full);
- phylink_set(pl->supported, 50000baseCR_Full);
- phylink_set(pl->supported, 50000baseLR_ER_FR_Full);
- phylink_set(pl->supported, 50000baseDR_Full);
- phylink_set(pl->supported, 100000baseKR4_Full);
- phylink_set(pl->supported, 100000baseSR4_Full);
- phylink_set(pl->supported, 100000baseCR4_Full);
- phylink_set(pl->supported, 100000baseLR4_ER4_Full);
- phylink_set(pl->supported, 100000baseKR2_Full);
- phylink_set(pl->supported, 100000baseSR2_Full);
- phylink_set(pl->supported, 100000baseCR2_Full);
- phylink_set(pl->supported, 100000baseLR2_ER2_FR2_Full);
- phylink_set(pl->supported, 100000baseDR2_Full);
+ caps = ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
+ caps = phylink_get_capabilities(pl->link_config.interface, caps,
+ RATE_MATCH_NONE);
+ phylink_caps_to_linkmodes(pl->supported, caps);
break;
default:
I've superficially tested it on 2 boards (LS1028A-RDB and Turris MOX)
and I didn't notice regressions.
But...
> That said, 10GBASER and 10GKR are treated slightly differently because
> of the problem with PHYs like 88x3310, and I think it's now difficult
> to undo that bit of history.
I don't really understand this. Can you please show me where and how
PHY_INTERFACE_MODE_10GBASER and PHY_INTERFACE_MODE_10GKR are treated
differently in the kernel?
> > > +
> > > default:
> > > phylink_err(pl,
> > > "incorrect link mode %s for in-band status\n",
> > > @@ -1109,7 +1124,9 @@ static void phylink_mac_config(struct phylink *pl,
> > >
> > > static bool phylink_pcs_handles_an(phy_interface_t iface, unsigned int mode)
> > > {
> > > - return phy_interface_mode_is_8023z(iface) && phylink_autoneg_inband(mode);
> > > + return (phy_interface_mode_is_8023z(iface) ||
> > > + iface == PHY_INTERFACE_MODE_INTERNAL) &&
> > > + phylink_autoneg_inband(mode);
>
> Is this true also for DSA devices that use "internal" mode? I'm
> wondering whether this will cause the PHY to be ignored/remain
> unattached in DSA switches because of the changes in patch 7.
phylink_pcs_handles_an() shouldn't return true for those, no. For that
to happen, phylink_autoneg_inband() should also return true for that link.
Which I believe it never does, because prior to this change, phylink_parse_mode()
would have errored out with -EINVAL and the "incorrect link mode %s for in-band status\n"
text for phy-mode = "internal".
> > > }
> > >
> > > static void phylink_pcs_an_restart(struct phylink *pl)
> > > diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> > > index 2b886ea654bb..7e8e26001587 100644
> > > --- a/include/linux/phylink.h
> > > +++ b/include/linux/phylink.h
> > > @@ -141,6 +141,7 @@ static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
> > >
> > > case PHY_INTERFACE_MODE_1000BASEX:
> > > case PHY_INTERFACE_MODE_2500BASEX:
> > > + case PHY_INTERFACE_MODE_INTERNAL:
> > > /* 1000base-X is designed for use media-side for Fibre
> > > * connections, and thus the Autoneg bit needs to be
> > > * taken into account. We also do this for 2500base-X
>
> Thinking about DSA cases, I don't think this change would be an issue
> because where DSA uses "internal" there isn't a PCS, so this won't
> matter.
I agree, and also see the justification above.
> Note that as there is now no need for anything outside phylink.c to
> reference this function, I have plans at some point to move it into
> the .c file rather than keeping it as an inline in the header file.
> It was temporarily necessary while introducing it to be in the
> header.
Ok. Do you want me to move phylink_pcs_neg_mode() to phylink.c as part
of this series?
Hi Florian,
On Mon, Oct 02, 2023 at 12:19:57PM -0700, Florian Fainelli wrote:
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index 6be348f1fa0e..72ef4afcda81 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -39,6 +39,7 @@ enum phy_mode {
> > PHY_MODE_UFS_HS_B,
> > PHY_MODE_PCIE,
> > PHY_MODE_ETHERNET,
> > + PHY_MODE_ETHTOOL,
>
> Not feeling very comfortable with using ETHTOOL here because that is a Linux
> sub-subsystem name as opposed to the other enumeration values which are
> electrical modes of operation and/or industry standards names.
>
> How about PHY_MODE_ETHERNET_EXPLICIT or PHY_MODE_ETHERNET_LINKMODE?
> --
> Florian
>
I agree with your sentiment. I can rename this to PHY_MODE_ETHERNET_LINKMODE.
Hi Russell,
On Tue, Oct 03, 2023 at 12:31:46PM +0100, Russell King (Oracle) wrote:
> On Sat, Sep 23, 2023 at 04:49:00PM +0300, Vladimir Oltean wrote:
> > Treat the newly introduced subsets of 25GBase-KR and 25GBase-CR the same
> > way as the fully-featured link modes. The difference only consists in
> > RS-FEC support.
>
> As mentioned in the patch adding these new linkmodes, I wonder whether
> this should be part of that patch. Is there a reason to keep it
> separate?
I can squash this into the other CR-S/KR-S patch.
On Tue, Oct 03, 2023 at 02:16:25PM +0100, Russell King (Oracle) wrote:
> On Sat, Sep 23, 2023 at 04:49:01PM +0300, Vladimir Oltean wrote:
> > Allow phylink_resolve_c73() to resolve backplane (KR) or SFP28 (CR)
> > link speeds of 25Gbps.
> >
> > Signed-off-by: Vladimir Oltean <[email protected]>
>
> Shouldn't this also be part of patch 5?
Not really, no.
Apart from adding the 25000baseKR_S_Full and 25000baseCR_S_Full link
modes (which are indeed newly added in patch 5) to phylink_c73_priority_resolution[],
it also adds the pre-existing 25000baseKR_Full and 25000baseCR_Full link
modes. Without this, phylink fails to resolve the 25G backplane or SFP28
speeds, and it just reports "Link is up - unknown/unknown".
The patch splitting may have been confusing. I had 2 options, either:
(a) - create one patch which adds the missing pre-existing 25G backplane/
SFP28 modes to phylink_c73_priority_resolution[]
- add the CR-S/KR-S link modes to phylink_c73_priority_resolution[]
as part of the general CR-S/KR-S addition
or
(b) - first add the CR-S/KR-S link modes everywhere where phylink
already uses 25GBase-KR/25GBase-CR
- extend the phylink_c73_priority_resolution[] for all 4 link modes
at the same time
I opted for (b) but I can also go with (a) if you prefer it that way.
Hi Russell,
On Tue, Oct 03, 2023 at 02:14:41PM +0100, Russell King (Oracle) wrote:
> On Sat, Sep 23, 2023 at 04:49:04PM +0300, Vladimir Oltean wrote:
> > +static int lynx_pcs_parse_fwnode(struct lynx_pcs *lynx)
> > +{
> > + struct fwnode_handle *node = lynx->mdio->dev.fwnode;
> > + enum mtip_model model = MTIP_MODEL_AUTODETECT;
> > + struct device_node *np = to_of_node(node);
> > + struct mdio_device *mdio = lynx->mdio;
> > + struct device *dev = &mdio->dev;
> > + struct phy *phy;
> > + int i, err;
> > +
> > + if (!node)
> > + return 0;
> > +
> > + lynx->backplane_mode = fwnode_property_present(node, "fsl,backplane-mode");
> > + if (!lynx->backplane_mode)
> > + return 0;
> > +
> > + if (fwnode_device_is_compatible(node, "fsl,lx2160a-lynx-pcs"))
> > + model = MTIP_MODEL_LX2160A;
> > +
> > + lynx->num_lanes = of_count_phandle_with_args(np, "phys", "#phy-cells");
> > + if (lynx->num_lanes < 0)
> > + return lynx->num_lanes;
>
> Is it possible for ->num_lanes to be zero at this point? If that is
> possible, then ->anlt[PRIMARY_LANE] will be NULL but ->backplane_mode
> will be set, so won't that cause the mtip_* calls above to pass a
> NULL pointer into those functions? Is that safe? Should we trap that
> case here?
Assuming the dt-bindings as proposed here, that case would be an invalid
device tree ("fsl,backplane-mode" present but "phys" isn't present),
which I indeed failed to catch.
But in my reply to Krzysztof here:
https://lore.kernel.org/netdev/20231002121958.xybzovgjzldfiae2@skbuf/
I said that for v3, I'm looking to move the "phys" property from the PCS
to the MAC (it's already in the MAC for the non-backplane use cases).
On LS1028A (ENETC, Felix), the lynx-pcs is not OF-based (we use
lynx_pcs_create_mdiodev()), but it would be good to support the
1000Base-KX link mode there also. As I'm starting to look beyond
LX2160A, I'm starting to see why adding extra dt-bindings to the
lynx-pcs (both "phys" and "fsl,backplane-mode") will be problematic
if there is no OF node to speak of.
I will leave a separate comment with some new ideas.
> If that's correct, then I don't see any point in storing
> ->backplane_mode, since we can then use ->num_lanes > PRIMARY_LANE
> or similar instead.
Well, in v3, my plan is for the caller of lynx_pcs_create() (aka the MAC)
to always pass an array of phys (the ones from its own OF node). In that
case, we would indeed need the "fsl,backplane-mode" property in the PCS,
because otherwise, with your proposal, the PCS would instantiate the
AN/LT block even when it's not expected.
> > +
> > + if (WARN_ON(lynx->num_lanes > MAX_NUM_LANES))
> > + return -EINVAL;
>
> Do we need to use WARN_ON() here, or would it be better to print a short
> error-level message?
Admittedly I may not have the best intuition here, but I didn't want to
over-complicate the code with error messages that can only be triggered
with invalid device trees.
> > +
> > + for (i = 0; i < lynx->num_lanes; i++) {
> > + phy = devm_of_phy_get_by_index(dev, np, i);
> > + if (IS_ERR(phy))
> > + return dev_err_probe(dev, PTR_ERR(phy),
> > + "Failed to get SerDes PHY %d\n", i);
> > +
> > + lynx->anlt[i] = mtip_backplane_create(mdio, phy, model);
> > + if (IS_ERR(lynx->anlt[i])) {
> > + err = PTR_ERR(lynx->anlt[i]);
> > +
> > + while (i-- > 0)
> > + mtip_backplane_destroy(lynx->anlt[i]);
> > +
> > + return err;
> > + }
> > + }
> > +
> > + for (i = 1; i < lynx->num_lanes; i++) {
> > + err = mtip_backplane_add_subordinate(lynx->anlt[PRIMARY_LANE],
> > + lynx->anlt[i]);
> > + if (WARN_ON(err)) {
>
> Again, does this need to be a backtrace-producing WARN_ON()?
mtip_backplane_add_subordinate() will only return -ERANGE if called too
many times (more than MTIP_MAX_NUM_SUBORDINATES times, aka more than
"MAX_NUM_LANES - 1" times).
Given the way that the code is constructed, it is technically impossible
for that to happen, but only because MTIP_MAX_NUM_SUBORDINATES is
hand-crafted to be 3 and MAX_NUM_LANES to be 4. I think that if I define
MTIP_MAX_NUM_SUBORDINATES in terms of MAX_NUM_LANES - 1, I can simply
make mtip_backplane_add_subordinate() return void.
What I want to avoid is to add error handling for errors which cannot
take place. Which is where the WARN_ON() came from.
> > + /* Too many SerDes lanes in the device tree? */
> > + for (i = 0; i < lynx->num_lanes; i++)
> > + mtip_backplane_destroy(lynx->anlt[i]);
> > + return err;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
> > {
> > struct lynx_pcs *lynx;
> > + int err;
> >
> > lynx = kzalloc(sizeof(*lynx), GFP_KERNEL);
> > if (!lynx)
> > @@ -327,6 +451,12 @@ static struct phylink_pcs *lynx_pcs_create(struct mdio_device *mdio)
> > lynx->pcs.neg_mode = true;
> > lynx->pcs.poll = true;
> >
> > + err = lynx_pcs_parse_fwnode(lynx);
> > + if (err) {
> > + kfree(lynx);
> > + return ERR_PTR(err);
> > + }
> > +
> > return lynx_to_phylink_pcs(lynx);
> > }
> >
> > @@ -392,6 +522,11 @@ EXPORT_SYMBOL_GPL(lynx_pcs_create_fwnode);
> > void lynx_pcs_destroy(struct phylink_pcs *pcs)
> > {
> > struct lynx_pcs *lynx = phylink_pcs_to_lynx(pcs);
> > + int i;
> > +
> > + if (lynx->backplane_mode)
> > + for (i = 0; i < lynx->num_lanes; i++)
> > + mtip_backplane_destroy(lynx->anlt[i]);
>
> Won't ->num_lanes only be non-zero when ->backplane_mode is set, so
> isn't the test for ->backplane_mode redundant here?
I think it won't be redundant anymore once the series reaches a less
"WIP" state.
> >
> > mdio_device_put(lynx->mdio);
> > kfree(lynx);
> > --
> > 2.34.1
> >
> >
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 9/23/2023 6:48 AM, Vladimir Oltean wrote:
> Allow driver code to print stuff like the resolved link mode to the
> kernel log, by giving it access to the link_mode_names[] ethtool
> internal array which already holds this info.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian