[RFC PATCH net-next 0/8] Add C72/C73 copper backplane support for LX2160

[RFC PATCH net-next 0/8] Add C72/C73 copper backplane support for LX2160

[Vladimir Oltean]

THIS PATCH SET DOES NOT WORK, AND IT ISN'T INTENDED TO WORK.

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/1587732391-3374-1-git-send-email-florinel.iordache@nxp.com/

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/20200425105210.GZ25745@shell.armlinux.org.uk/

"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 (8):
  phy: introduce the phy_check_cdr_lock() function
  phy: introduce the PHY_MODE_ETHERNET_PHY mode for phy_set_mode_ext()
  phy: xgkr: add configuration interface for copper backplane Ethernet
    PHYs
  net: phy: add C73 base page helpers
  net: phy: balance calls to ->suspend() and ->resume()
  net: phy: initialize phydev->master_slave_set to
    MASTER_SLAVE_CFG_UNKNOWN
  net: phy: mtip_backplane: add driver for MoreThanIP backplane AN/LT
    core
  dt-bindings: net: fsl,backplane-anlt: new binding document

 .../devicetree/bindings/net/ethernet-phy.yaml |    8 +
 .../bindings/net/fsl,backplane-anlt.yaml      |  238 +++
 drivers/net/mii.c                             |   34 +-
 drivers/net/phy/Kconfig                       |    7 +
 drivers/net/phy/Makefile                      |    1 +
 drivers/net/phy/mtip_backplane.c              | 1735 +++++++++++++++++
 drivers/net/phy/phy_device.c                  |    5 +-
 drivers/phy/phy-core.c                        |   18 +
 include/linux/mii.h                           |  105 +
 include/linux/phy/phy-xgkr.h                  |  165 ++
 include/linux/phy/phy.h                       |   27 +
 11 files changed, 2341 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
 create mode 100644 drivers/net/phy/mtip_backplane.c
 create mode 100644 include/linux/phy/phy-xgkr.h

-- 
2.34.1

[RFC PATCH net-next 1/8] phy: introduce the phy_check_cdr_lock() function

[Vladimir Oltean]

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, at 25Gbps, the PCS does not have a MDIO_CTRL1_LPOWER bit
implemented in its MDIO_MMD_PCS:MDIO_CTRL1 register, so a phy_suspend()
procedure will not power down the link, and will not cause a link drop
event on the link partner.

By implementing the networking phy_suspend() as phy_power_off() in the
SerDes and introducing phy_check_cdr_lock() as a link indication, we are
able to detect that condition and signal it to upper layers of the
network stack.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/phy/phy-core.c  | 18 ++++++++++++++++++
 include/linux/phy/phy.h | 22 ++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 96a0b1e111f3..e611ebe993c7 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -553,6 +553,24 @@ int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
 }
 EXPORT_SYMBOL_GPL(phy_validate);
 
+int phy_check_cdr_lock(struct phy *phy, bool *cdr_locked)
+{
+	int ret;
+
+	if (!phy)
+		return -EINVAL;
+
+	if (!phy->ops->check_cdr_lock)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&phy->mutex);
+	ret = phy->ops->check_cdr_lock(phy, cdr_locked);
+	mutex_unlock(&phy->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_check_cdr_lock);
+
 /**
  * _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..456d21c67e4f 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -122,6 +122,19 @@ struct phy_ops {
 			    union phy_configure_opts *opts);
 	int	(*reset)(struct phy *phy);
 	int	(*calibrate)(struct phy *phy);
+
+	/**
+	 * @check_cdr_lock:
+	 *
+	 * Optional.
+	 *
+	 * Check that the CDR (Clock and Data Recovery) logic has locked onto
+	 * bit transitions in the RX stream.
+	 *
+	 * Returns: 0 if the operation was successful, negative error code
+	 * otherwise.
+	 */
+	int	(*check_cdr_lock)(struct phy *phy, bool *cdr_locked);
 	void	(*release)(struct phy *phy);
 	struct module *owner;
 };
@@ -236,6 +249,7 @@ 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_check_cdr_lock(struct phy *phy, bool *cdr_locked);
 
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
@@ -414,6 +428,14 @@ static inline int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
 	return -ENOSYS;
 }
 
+static inline int phy_check_cdr_lock(struct phy *phy, bool *cdr_locked)
+{
+	if (!phy)
+		return 0;
+
+	return -ENOSYS;
+}
+
 static inline int phy_get_bus_width(struct phy *phy)
 {
 	return -ENOSYS;
-- 
2.34.1

[RFC PATCH net-next 2/8] phy: introduce the PHY_MODE_ETHERNET_PHY mode for phy_set_mode_ext()

[Vladimir Oltean]

As opposed to PHY_MODE_ETHERNET which takes a phy_interface_t as is
expected to be used by an Ethernet MAC driver, PHY_MODE_ETHERNET takes
an enum ethtool_link_mode_bit_indices and expects to be used by an
Ethernet PHY driver.

It is true that the phy_interface_t type also contains definitions for
PHY_INTERFACE_MODE_10GKR and PHY_INTERFACE_MODE_1000BASEKX, but those
were deemed to be mistakes, and shouldn't be used going forward, when
10GBase-KR and 1GBase-KX are really link modes. Thus, I believe that the
distinction is necessary, rather than hacking more improper PHY modes.

In particular to the Lynx SerDes, it can be used (as the PMA/PMD layer)
in conjunction with a separate backplane AN/LT block to form a
full-fledged copper backplane Ethernet PHY. The configuration of the
lanes is relatively similar to what is done for a typical MAC-to-PHY
link, except that we allow tuning the electrical equalization parameters
of the link (support for which will come as a separate change).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/linux/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 456d21c67e4f..7e10761303fc 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_ETHERNET_PHY,
 	PHY_MODE_MIPI_DPHY,
 	PHY_MODE_SATA,
 	PHY_MODE_LVDS,
-- 
2.34.1

Re: [RFC PATCH net-next 2/8] phy: introduce the PHY_MODE_ETHERNET_PHY mode for phy_set_mode_ext()

[Sean Anderson]

On 8/17/23 11:06, Vladimir Oltean wrote:
> As opposed to PHY_MODE_ETHERNET which takes a phy_interface_t as is
> expected to be used by an Ethernet MAC driver, PHY_MODE_ETHERNET takes
> an enum ethtool_link_mode_bit_indices and expects to be used by an
> Ethernet PHY driver.
> 
> It is true that the phy_interface_t type also contains definitions for
> PHY_INTERFACE_MODE_10GKR and PHY_INTERFACE_MODE_1000BASEKX, but those
> were deemed to be mistakes, and shouldn't be used going forward, when
> 10GBase-KR and 1GBase-KX are really link modes. Thus, I believe that the
> distinction is necessary, rather than hacking more improper PHY modes.

10GBase-KR and 1000Base-KX are both electrically (e.g. link mode) and
functionally (e.g. phy mode) different from 10GBase-R and 1000Base-X due
to differing autonegotiation. So the phy modes are still relevant, and
should still be used to ensure the correct form of autonegotiation is
selected.

That said, I do agree that from the phy's (serdes's) point of view,
there are only electrical differences between these modes.

However, I'm not sure we need to have a separate mode here. I think this
would only be necessary if there were electrically-incompatible modes
which shared the same signalling. E.g. if 802.3 decided that they wanted
a "long range backplane ethernet" or somesuch with different
drive/equalization requirements from 1000BASE-KX et al. but with the
same signalling. Otherwise, we can infer the link mode from the phy
mode.

--Sean

> In particular to the Lynx SerDes, it can be used (as the PMA/PMD layer)
> in conjunction with a separate backplane AN/LT block to form a
> full-fledged copper backplane Ethernet PHY. The configuration of the
> lanes is relatively similar to what is done for a typical MAC-to-PHY
> link, except that we allow tuning the electrical equalization parameters
> of the link (support for which will come as a separate change).
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  include/linux/phy/phy.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 456d21c67e4f..7e10761303fc 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_ETHERNET_PHY,
>  	PHY_MODE_MIPI_DPHY,
>  	PHY_MODE_SATA,
>  	PHY_MODE_LVDS,

Re: [RFC PATCH net-next 2/8] phy: introduce the PHY_MODE_ETHERNET_PHY mode for phy_set_mode_ext()

[Vladimir Oltean]

Hi Sean,

On Mon, Aug 21, 2023 at 01:30:46PM -0400, Sean Anderson wrote:
> On 8/17/23 11:06, Vladimir Oltean wrote:
> > As opposed to PHY_MODE_ETHERNET which takes a phy_interface_t as is
> > expected to be used by an Ethernet MAC driver, PHY_MODE_ETHERNET takes
> > an enum ethtool_link_mode_bit_indices and expects to be used by an
> > Ethernet PHY driver.
> > 
> > It is true that the phy_interface_t type also contains definitions for
> > PHY_INTERFACE_MODE_10GKR and PHY_INTERFACE_MODE_1000BASEKX, but those
> > were deemed to be mistakes, and shouldn't be used going forward, when
> > 10GBase-KR and 1GBase-KX are really link modes. Thus, I believe that the
> > distinction is necessary, rather than hacking more improper PHY modes.
> 
> 10GBase-KR and 1000Base-KX are both electrically (e.g. link mode) and
> functionally (e.g. phy mode) different from 10GBase-R and 1000Base-X due
> to differing autonegotiation. So the phy modes are still relevant, and
> should still be used to ensure the correct form of autonegotiation is
> selected.
> 
> That said, I do agree that from the phy's (serdes's) point of view,
> there are only electrical differences between these modes.
> 
> However, I'm not sure we need to have a separate mode here. I think this
> would only be necessary if there were electrically-incompatible modes
> which shared the same signalling. E.g. if 802.3 decided that they wanted
> a "long range backplane ethernet" or somesuch with different
> drive/equalization requirements from 1000BASE-KX et al. but with the
> same signalling. Otherwise, we can infer the link mode from the phy
> mode.
> 
> --Sean

Thanks for taking the time to look at this RFC.

I will ask a clarification question. When you say "I'm not sure we need
to have a separate mode here", what do you mean?

The lynx-28g implementation (not shown here) will need to distinguish
between 1000Base-X and 1000Base-KX, and between 10GBase-R and 10GBase-KR
respectively, to configure the number of electrical equalization taps in
the LNmTECR registers, and to allocate memory for the ("K"-specific)
link training algorithm. Also, in the particular case of BaseX vs
BaseKX, we need to modify the PCCR8 register depending on whether the
C22 BaseX PCS or the C45 PCS + AN/LT blocks need to be available over
MDIO.

So, passing PHY_INTERFACE_MODE_1000BASEX when we intend 1000Base-KX is
simply not possible, because the dpaa2-mac consumer already uses
PHY_INTERFACE_MODE_1000BASEX to mean a very different (and legit) thing.

Do you mean instead that we could use the PHY_INTERFACE_MODE_1000BASEKX
that you've added to phy_interface_t? It's not clear that this is what
you're suggesting, so feel free to stop reading here if it isn't.

But mtip_backplane uses linkmode_c73_priority_resolution() (a function
added by me, sure, but nonetheless, it operates in the linkmode namespace,
as a PHY driver helper should) to figure out the proper argument to pass
to phy_set_mode_ext(). That argument has the enum ethtool_link_mode_bit_indices.

So, a translation between enum ethtool_link_mode_bit_indices and
phy_interface_t would be needed. That would be more or less doable for
1000Base-KX and 10GBase-KR, but it needs more phy_interface_t additions
for:

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_25000baseKRS_Full_BIT not supported */
	/* ETHTOOL_LINK_MODE_25000baseCRS_Full_BIT not supported */
	ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
	ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
	ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
};

I guess that network PHY maintainers will need to chime in and say
whether that's the path forward or not.

Re: [RFC PATCH net-next 2/8] phy: introduce the PHY_MODE_ETHERNET_PHY mode for phy_set_mode_ext()

[Sean Anderson]

On 8/21/23 14:13, Vladimir Oltean wrote:
> Hi Sean,
> 
> On Mon, Aug 21, 2023 at 01:30:46PM -0400, Sean Anderson wrote:
>> On 8/17/23 11:06, Vladimir Oltean wrote:
>> > As opposed to PHY_MODE_ETHERNET which takes a phy_interface_t as is
>> > expected to be used by an Ethernet MAC driver, PHY_MODE_ETHERNET takes
>> > an enum ethtool_link_mode_bit_indices and expects to be used by an
>> > Ethernet PHY driver.
>> > 
>> > It is true that the phy_interface_t type also contains definitions for
>> > PHY_INTERFACE_MODE_10GKR and PHY_INTERFACE_MODE_1000BASEKX, but those
>> > were deemed to be mistakes, and shouldn't be used going forward, when
>> > 10GBase-KR and 1GBase-KX are really link modes. Thus, I believe that the
>> > distinction is necessary, rather than hacking more improper PHY modes.
>> 
>> 10GBase-KR and 1000Base-KX are both electrically (e.g. link mode) and
>> functionally (e.g. phy mode) different from 10GBase-R and 1000Base-X due
>> to differing autonegotiation. So the phy modes are still relevant, and
>> should still be used to ensure the correct form of autonegotiation is
>> selected.
>> 
>> That said, I do agree that from the phy's (serdes's) point of view,
>> there are only electrical differences between these modes.
>> 
>> However, I'm not sure we need to have a separate mode here. I think this
>> would only be necessary if there were electrically-incompatible modes
>> which shared the same signalling. E.g. if 802.3 decided that they wanted
>> a "long range backplane ethernet" or somesuch with different
>> drive/equalization requirements from 1000BASE-KX et al. but with the
>> same signalling. Otherwise, we can infer the link mode from the phy
>> mode.
>> 
>> --Sean
> 
> Thanks for taking the time to look at this RFC.
> 
> I will ask a clarification question. When you say "I'm not sure we need
> to have a separate mode here", what do you mean?
> 
> The lynx-28g implementation (not shown here) will need to distinguish
> between 1000Base-X and 1000Base-KX, and between 10GBase-R and 10GBase-KR
> respectively, to configure the number of electrical equalization taps in
> the LNmTECR registers, and to allocate memory for the ("K"-specific)
> link training algorithm. Also, in the particular case of BaseX vs
> BaseKX, we need to modify the PCCR8 register depending on whether the
> C22 BaseX PCS or the C45 PCS + AN/LT blocks need to be available over
> MDIO.
> 
> So, passing PHY_INTERFACE_MODE_1000BASEX when we intend 1000Base-KX is
> simply not possible, because the dpaa2-mac consumer already uses
> PHY_INTERFACE_MODE_1000BASEX to mean a very different (and legit) thing.
> 
> Do you mean instead that we could use the PHY_INTERFACE_MODE_1000BASEKX
> that you've added to phy_interface_t? It's not clear that this is what
> you're suggesting, so feel free to stop reading here if it isn't.

Yes. The intent for this interface mode is for the PCS to select the
appropriate autonegotiation. So if you use the 1000Base-KX link mode,
you should also use the 1000BASEKX phy mode (unless you have a separate
phy doing the conversion).

> But mtip_backplane uses linkmode_c73_priority_resolution() (a function
> added by me, sure, but nonetheless, it operates in the linkmode namespace,
> as a PHY driver helper should) to figure out the proper argument to pass
> to phy_set_mode_ext(). That argument has the enum ethtool_link_mode_bit_indices.
> 
> So, a translation between enum ethtool_link_mode_bit_indices and
> phy_interface_t would be needed. That would be more or less doable for
> 1000Base-KX and 10GBase-KR, but it needs more phy_interface_t additions
> for:
> 
> 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_25000baseKRS_Full_BIT not supported */
> 	/* ETHTOOL_LINK_MODE_25000baseCRS_Full_BIT not supported */
> 	ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> 	ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
> 	ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
> };

Well, I suppose you really do have "electrically-incompatible modes
which shared the same signalling". So I think it's reasonable to go this
route.

I considered something like this for lynx10g, but that serdes only
supports 10GBase-KR, so there was no need to differentiate that from
10GBase-KX4 (or 10GBase-CR).

> I guess that network PHY maintainers will need to chime in and say
> whether that's the path forward or not.

I think the commit message could better motivate this patch by drawing a
distinction between a serdes which is talking to a phy or sfp which will
convert the signals to the final link mode, and a serdes which is acting
as the final connection to the far end, with nothing intervening.

--Sean

Re: [RFC PATCH net-next 2/8] phy: introduce the PHY_MODE_ETHERNET_PHY mode for phy_set_mode_ext()

[Russell King (Oracle)]

On Thu, Aug 17, 2023 at 06:06:38PM +0300, Vladimir Oltean wrote:
> As opposed to PHY_MODE_ETHERNET which takes a phy_interface_t as is
> expected to be used by an Ethernet MAC driver, PHY_MODE_ETHERNET takes
> an enum ethtool_link_mode_bit_indices and expects to be used by an
> Ethernet PHY driver.

This doesn't seem to be correct. I think one of your PHY_MODE_ETHERNET
above should be PHY_MODE_ETHERNET_PHY - but maybe instead it should
be PHY_MODE_ETHTOOL so that it's clearly different and doesn't get
confused with PHY_MODE_ETHERNET ?

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

Re: [RFC PATCH net-next 2/8] phy: introduce the PHY_MODE_ETHERNET_PHY mode for phy_set_mode_ext()

[Vladimir Oltean]

On Mon, Aug 21, 2023 at 07:14:04PM +0100, Russell King (Oracle) wrote:
> On Thu, Aug 17, 2023 at 06:06:38PM +0300, Vladimir Oltean wrote:
> > As opposed to PHY_MODE_ETHERNET which takes a phy_interface_t as is
> > expected to be used by an Ethernet MAC driver, PHY_MODE_ETHERNET takes
> > an enum ethtool_link_mode_bit_indices and expects to be used by an
> > Ethernet PHY driver.
> 
> This doesn't seem to be correct. I think one of your PHY_MODE_ETHERNET
> above should be PHY_MODE_ETHERNET_PHY - but maybe instead it should
> be PHY_MODE_ETHTOOL so that it's clearly different and doesn't get
> confused with PHY_MODE_ETHERNET ?
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Correct, sorry for the incorrect commit description.

PHY_MODE_ETHTOOL would also work perfectly fine to me, and is more
explicit.

[RFC PATCH net-next 3/8] phy: xgkr: add configuration interface for copper backplane Ethernet PHYs

[Vladimir Oltean]

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_xgkr.

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.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/linux/phy/phy-xgkr.h | 165 +++++++++++++++++++++++++++++++++++
 include/linux/phy/phy.h      |   4 +
 2 files changed, 169 insertions(+)
 create mode 100644 include/linux/phy/phy-xgkr.h

diff --git a/include/linux/phy/phy-xgkr.h b/include/linux/phy/phy-xgkr.h
new file mode 100644
index 000000000000..8accfb1002a0
--- /dev/null
+++ b/include/linux/phy/phy-xgkr.h
@@ -0,0 +1,165 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2023 NXP
+ */
+
+#ifndef __PHY_XGKR_H_
+#define __PHY_XGKR_H_
+
+struct phy;
+
+enum coef_status {
+	COEF_STAT_NOT_UPDATED = 0,
+	COEF_STAT_UPDATED = 1,
+	COEF_STAT_MIN = 2,
+	COEF_STAT_MAX = 3,
+};
+
+enum coef_update {
+	COEF_UPD_HOLD = 0,
+	COEF_UPD_INC = 1,
+	COEF_UPD_DEC = 2,
+};
+
+struct c72_coef_update {
+	enum coef_update com1;
+	enum coef_update coz;
+	enum coef_update cop1;
+	bool preset;
+	bool init;
+};
+
+struct c72_coef_status {
+	enum coef_status com1;
+	enum coef_status coz;
+	enum coef_status cop1;
+};
+
+enum xgkr_phy_configure_type {
+	XGKR_CONFIGURE_LOCAL_TX,
+	XGKR_CONFIGURE_REMOTE_TX,
+	XGKR_CONFIGURE_LT_DONE,
+};
+
+/* Adjust PHY TX equalization in response to a C72 coefficient
+ * update request from the link partner
+ */
+struct xgkr_phy_configure_local_tx {
+	/* input to PHY */
+	struct c72_coef_update update;
+	/* output from PHY */
+	struct c72_coef_status status;
+};
+
+/* Query the PHY RX quality in order to compute a C72 coefficient update
+ * request to the link partner to improve that. 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.
+ */
+struct xgkr_phy_configure_remote_tx {
+	/* output from PHY */
+	bool rx_ready;
+	struct c72_coef_update update;
+	/* input to PHY */
+	void (*cb)(void *cb_priv, int err, struct c72_coef_update update,
+		   struct c72_coef_status status);
+	void *cb_priv;
+};
+
+/**
+ * struct phy_configure_opts_xgkr - 10GBase-KR configuration
+ *
+ * This structure is used to represent the configuration state of a
+ * 10GBase-KR Ethernet Copper Backplane PHY.
+ */
+struct phy_configure_opts_xgkr {
+	enum xgkr_phy_configure_type type;
+	union {
+		struct xgkr_phy_configure_local_tx local_tx;
+		struct xgkr_phy_configure_remote_tx remote_tx;
+	};
+};
+
+/* Some helpers */
+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;
+	}
+}
+
+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;
+}
+
+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_XGKR_H_ */
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 7e10761303fc..8ef7de7d2a90 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -19,6 +19,7 @@
 #include <linux/phy/phy-dp.h>
 #include <linux/phy/phy-lvds.h>
 #include <linux/phy/phy-mipi-dphy.h>
+#include <linux/phy/phy-xgkr.h>
 
 struct phy;
 
@@ -61,11 +62,14 @@ enum phy_media {
  *		the DisplayPort protocol.
  * @lvds:	Configuration set applicable for phys supporting
  *		the LVDS phy mode.
+ * @xgkr:	Configuration set applicable for phys supporting
+ *		the 10GBase-KR 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_xgkr		xgkr;
 };
 
 /**
-- 
2.34.1

[RFC PATCH net-next 4/8] net: phy: add C73 base page helpers

[Vladimir Oltean]

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.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 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..c8a1df5d52a9 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_25000baseKRS_Full_BIT not supported */
+	/* ETHTOOL_LINK_MODE_25000baseCRS_Full_BIT not supported */
+	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 <jgarzik@pobox.com>");
 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

[RFC PATCH net-next 5/8] net: phy: balance calls to ->suspend() and ->resume()

[Vladimir Oltean]

Some future PHY drivers will need to do balanced stuff in these methods,
like phy_power_off() and phy_power_on(). Currently ->resume() is called
twice during initialization, leading to phy_power_off() not actually
doing anything during ->suspend() because the refcount in the generic
PHY core remains elevated.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/phy_device.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 17cb3e07216a..9cb5aa04b2b5 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1882,7 +1882,7 @@ int __phy_resume(struct phy_device *phydev)
 
 	lockdep_assert_held(&phydev->lock);
 
-	if (!phydrv || !phydrv->resume)
+	if (!phydrv || !phydrv->resume || !phydev->suspended)
 		return 0;
 
 	ret = phydrv->resume(phydev);
@@ -3275,6 +3275,8 @@ static int phy_probe(struct device *dev)
 	if (phydrv->flags & PHY_IS_INTERNAL)
 		phydev->is_internal = true;
 
+	phydev->suspended = true;
+
 	/* Deassert the reset signal */
 	phy_device_reset(phydev, 0);
 
-- 
2.34.1

[RFC PATCH net-next 6/8] net: phy: initialize phydev->master_slave_set to MASTER_SLAVE_CFG_UNKNOWN

[Vladimir Oltean]

It doesn't really make sense for phydev->master_slave_set (the user
requested AN master/slave configuration) to take the value
MASTER_SLAVE_CFG_UNSUPPORTED. That only exists, AFAICS, to detect
implementations which don't report phydev->master_slave_get and
phydev->master_slave_state.

Permit the most trivial of drivers to exist, that where .config_aneg()
accepts any user request, and is implemented as:

	/* We support anything */
	phydev->master_slave_get = phydev->master_slave_set;

This is currently rejected by ethnl_update_linkmodes() with the message
"master/slave configuration not supported by device", precisely because
lsettings->master_slave_cfg (which came from phydev->master_slave_get
through phy_ethtool_ksettings_get()) is MASTER_SLAVE_CFG_UNSUPPORTED
(coming from phydev->master_slave_get, see implementation above).

By making phydev->master_slave_set never hold UNSUPPORTED, we avoid the
above confusion (driver does implement master/slave but still gets
detected as unsupported by core) without special casing in the driver.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/phy_device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9cb5aa04b2b5..e374d1a57030 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3276,6 +3276,7 @@ static int phy_probe(struct device *dev)
 		phydev->is_internal = true;
 
 	phydev->suspended = true;
+	phydev->master_slave_set = MASTER_SLAVE_CFG_UNKNOWN;
 
 	/* Deassert the reset signal */
 	phy_device_reset(phydev, 0);
-- 
2.34.1

[RFC PATCH net-next 7/8] net: phy: mtip_backplane: add driver for MoreThanIP backplane AN/LT core

[Vladimir Oltean]

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 networking PHY driver implementation is as generic as
possible.

Initial support is present only for the LX2160A SoC and the single-lane
link modes (10G, 25G). For this SoC, the register map of the IP block
was a bit mangled, and we don't have any PHY ID. I didn't want to invent
one, so this driver probes on a specific compatible string, plus the
secondary "ethernet-phy-ieee802.3-c45" which makes the PHY library
recognize it as a clause 45 PHY and not an mdio_device.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/Kconfig          |    7 +
 drivers/net/phy/Makefile         |    1 +
 drivers/net/phy/mtip_backplane.c | 1735 ++++++++++++++++++++++++++++++
 3 files changed, 1743 insertions(+)
 create mode 100644 drivers/net/phy/mtip_backplane.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 107880d13d21..536ce9f118c5 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -290,6 +290,13 @@ config MOTORCOMM_PHY
 	  Enables support for Motorcomm network PHYs.
 	  Currently supports YT85xx Gigabit Ethernet PHYs.
 
+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 NATIONAL_PHY
 	tristate "National Semiconductor PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index c945ed9bd14b..06ef4b59e223 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_MICROCHIP_T1_PHY)	+= microchip_t1.o
 obj-$(CONFIG_MICROCHIP_T1S_PHY) += microchip_t1s.o
 obj-$(CONFIG_MICROSEMI_PHY)	+= mscc/
 obj-$(CONFIG_MOTORCOMM_PHY)	+= motorcomm.o
+obj-$(CONFIG_MTIP_BACKPLANE_PHY) += mtip_backplane.o
 obj-$(CONFIG_NATIONAL_PHY)	+= national.o
 obj-$(CONFIG_NCN26000_PHY)	+= ncn26000.o
 obj-$(CONFIG_NXP_C45_TJA11XX_PHY)	+= nxp-c45-tja11xx.o
diff --git a/drivers/net/phy/mtip_backplane.c b/drivers/net/phy/mtip_backplane.c
new file mode 100644
index 000000000000..09eeaff653aa
--- /dev/null
+++ b/drivers/net/phy/mtip_backplane.c
@@ -0,0 +1,1735 @@
+// 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/phy/phy.h>
+#include <linux/clk.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_25GKRS		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_IN_PROGRESS		BIT(2)
+#define LT_STAT_FAIL			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		100000
+
+#define MTIP_LT_RESTART_SLEEP_US	10
+#define MTIP_LT_RESTART_TIMEOUT_US	100
+
+#define MTIP_FRAME_LOCK_SLEEP_US	10
+#define MTIP_FRAME_LOCK_TIMEOUT_US	100
+
+#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_AN_TIMEOUT_MS		10000
+
+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 phy_device *phydev;
+	struct mutex lock;
+	struct delayed_work work;
+	u16 old_an_stat;
+	u16 old_pcs_stat;
+	bool link;
+	bool link_ack;
+	bool cdr_locked;
+};
+
+struct mtip_lt_work {
+	struct phy_device *phydev;
+	struct kthread_work work;
+};
+
+struct mtip_backplane {
+	struct phy *serdes;
+	struct mdio_device *pcs;
+	const u16 *an_regs;
+	const u16 *lt_regs;
+	int lt_mmd;
+	enum ethtool_link_mode_bit_indices link_mode;
+	bool link_mode_resolved;
+	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_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;
+};
+
+#define phydev_to_irqpoll(phydev)	\
+	(&((struct mtip_backplane *)(phydev)->priv)->irqpoll)
+
+/* 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,
+};
+
+static const enum ethtool_link_mode_bit_indices mtip_backplane_link_modes[] = {
+	ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+	ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
+};
+
+static const char *
+ethtool_link_mode_str(enum ethtool_link_mode_bit_indices link_mode)
+{
+	switch (link_mode) {
+	case ETHTOOL_LINK_MODE_1000baseKX_Full_BIT:
+		return "1000base-KX";
+	case ETHTOOL_LINK_MODE_10000baseKR_Full_BIT:
+		return "10Gbase-KR";
+	case ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT:
+		return "40Gbase-KR4";
+	case ETHTOOL_LINK_MODE_25000baseKR_Full_BIT:
+		return "25Gbase-KR";
+	case ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT:
+		return "100Gbase-KR4";
+	default:
+		return "unknown";
+	}
+}
+
+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 mtip_read_an(struct phy_device *phydev, enum mtip_an_reg reg)
+{
+	struct mtip_backplane *priv = phydev->priv;
+
+	return phy_read_mmd(phydev, MDIO_MMD_AN, priv->an_regs[reg]);
+}
+
+static int mtip_write_an(struct phy_device *phydev, enum mtip_an_reg reg,
+			 u16 val)
+{
+	struct mtip_backplane *priv = phydev->priv;
+
+	return phy_write_mmd(phydev, MDIO_MMD_AN, priv->an_regs[reg], val);
+}
+
+static int mtip_modify_an(struct phy_device *phydev, enum mtip_an_reg reg,
+			  u16 mask, u16 set)
+{
+	struct mtip_backplane *priv = phydev->priv;
+
+	return phy_modify_mmd(phydev, MDIO_MMD_AN, priv->an_regs[reg],
+			      mask, set);
+}
+
+static int mtip_read_lt(struct phy_device *phydev, enum mtip_lt_reg reg)
+{
+	struct mtip_backplane *priv = phydev->priv;
+
+	return phy_read_mmd(phydev, priv->lt_mmd, priv->lt_regs[reg]);
+}
+
+static int mtip_write_lt(struct phy_device *phydev, enum mtip_lt_reg reg,
+			 u16 val)
+{
+	struct mtip_backplane *priv = phydev->priv;
+
+	return phy_write_mmd(phydev, priv->lt_mmd, priv->lt_regs[reg], val);
+}
+
+static int mtip_modify_lt(struct phy_device *phydev, enum mtip_lt_reg reg,
+			  u16 mask, u16 set)
+{
+	struct mtip_backplane *priv = phydev->priv;
+
+	return phy_modify_mmd(phydev, priv->lt_mmd, priv->lt_regs[reg],
+			      mask, set);
+}
+
+static int mtip_read_pcs(struct phy_device *phydev, int reg)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	struct mdio_device *pcs = priv->pcs;
+
+	return mdiodev_c45_read(pcs, MDIO_MMD_PCS, reg);
+}
+
+static int mtip_modify_pcs(struct phy_device *phydev, int reg, u16 mask,
+			   u16 set)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	struct mdio_device *pcs = priv->pcs;
+
+	return mdiodev_c45_modify(pcs, MDIO_MMD_PCS, reg, mask, set);
+}
+
+static int mtip_reset_an(struct phy_device *phydev)
+{
+	int err, val;
+
+	err = mtip_modify_an(phydev, 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, phydev, AN_CTRL);
+
+	return (val < 0) ? val : err;
+}
+
+static int mtip_reset_pcs(struct phy_device *phydev)
+{
+	int err, val;
+
+	err = mtip_modify_pcs(phydev, MDIO_CTRL1, MDIO_CTRL1_RESET,
+			      MDIO_CTRL1_RESET);
+	if (err < 0)
+		return err;
+
+	err = read_poll_timeout(mtip_read_pcs, val,
+				val < 0 || !(val & MDIO_CTRL1_RESET),
+				MTIP_RESET_SLEEP_US, MTIP_RESET_TIMEOUT_US,
+				false, phydev, MDIO_CTRL1);
+
+	return (val < 0) ? val : err;
+}
+
+static int mtip_wait_for_cdr_lock(struct phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	bool cdr_locked;
+	int err, val;
+
+	err = read_poll_timeout(phy_check_cdr_lock, val,
+				val < 0 || cdr_locked,
+				MTIP_CDR_SLEEP_US, MTIP_CDR_TIMEOUT_US,
+				false, priv->serdes, &cdr_locked);
+
+	return (val < 0) ? val : err;
+}
+
+static int mtip_lx2160a_get_features(struct phy_device *phydev)
+{
+	const enum ethtool_link_mode_bit_indices *link_modes;
+	struct mtip_backplane *priv = phydev->priv;
+	int i, err;
+
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Backplane_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, phydev->supported);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, phydev->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_ETHERNET_PHY,
+				   link_modes[i], NULL);
+		if (err)
+			continue;
+
+		linkmode_set_bit(link_modes[i], phydev->supported);
+	}
+
+	return 0;
+}
+
+static int mtip_lt_frame_lock(struct phy_device *phydev)
+{
+	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, phydev, LT_CTRL);
+
+	return (val < 0) ? val : err;
+}
+
+static int mtip_restart_lt(struct phy_device *phydev, 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(phydev, 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, phydev, 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 phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	err = mtip_restart_lt(phydev, false);
+	if (err) {
+		phydev_err(phydev, "Failed to disable link training: %pe\n",
+			   ERR_PTR(err));
+		return err;
+	}
+
+	/* Subsequent attempts to disable LT will time out, so stop them */
+	WRITE_ONCE(priv->lt_enabled, false);
+
+	return 0;
+}
+
+static int mtip_stop_lt(struct phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+	struct mtip_backplane *priv = phydev->priv;
+	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(phydev);
+	if (err)
+		return err;
+
+	phydev_dbg(phydev, "Link training disabled\n");
+
+	return 0;
+}
+
+static int mtip_reset_lt(struct phy_device *phydev)
+{
+	int err;
+
+	/* Don't allow AN to complete without training */
+	err = mtip_modify_lt(phydev, LT_STAT, LT_STAT_RX_STATUS, 0);
+	if (err < 0)
+		return err;
+
+	err = mtip_write_lt(phydev, LT_LD_COEF, 0);
+	if (err < 0)
+		return err;
+
+	err = mtip_write_lt(phydev, LT_LD_STAT, 0);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+/* Reset state when detecting that the previously determined link mode
+ * is no longer valid
+ */
+static int mtip_state_reset(struct phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	priv->link_mode_resolved = false;
+
+	if (READ_ONCE(priv->lt_enabled)) {
+		err = mtip_stop_lt(phydev);
+		if (err)
+			return err;
+	}
+
+	err = mtip_reset_lt(phydev);
+	if (err < 0)
+		return err;
+
+	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 phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+	int val;
+
+	lockdep_assert_held(&irqpoll->lock);
+
+	val = mtip_read_an(phydev, 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 phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+
+	irqpoll->old_pcs_stat = 0;
+
+	return 0;
+}
+
+static int mtip_read_adv(struct phy_device *phydev, u64 *base_page)
+{
+	int val;
+
+	val = mtip_read_an(phydev, AN_ADV_0);
+	if (val < 0)
+		return val;
+
+	*base_page = (u64)val;
+
+	val = mtip_read_an(phydev, AN_ADV_1);
+	if (val < 0)
+		return val;
+
+	*base_page |= (u64)val << 16;
+
+	val = mtip_read_an(phydev, AN_ADV_2);
+	if (val < 0)
+		return val;
+
+	*base_page |= (u64)val << 32;
+
+	return 0;
+}
+
+static int mtip_write_adv(struct phy_device *phydev, u64 base_page)
+{
+	int val;
+
+	val = mtip_write_an(phydev, AN_ADV_0, C73_ADV_0(base_page));
+	if (val < 0)
+		return val;
+
+	val = mtip_write_an(phydev, AN_ADV_1, C73_ADV_1(base_page));
+	if (val < 0)
+		return val;
+
+	val = mtip_write_an(phydev, AN_ADV_2, C73_ADV_2(base_page));
+	if (val < 0)
+		return val;
+
+	return 0;
+}
+
+static int mtip_read_lpa(struct phy_device *phydev, u64 *base_page)
+{
+	int val;
+
+	val = mtip_read_an(phydev, AN_LPA_0);
+	if (val < 0)
+		return val;
+
+	*base_page = (u64)val;
+
+	val = mtip_read_an(phydev, AN_LPA_1);
+	if (val < 0)
+		return val;
+
+	*base_page |= (u64)val << 16;
+
+	val = mtip_read_an(phydev, AN_LPA_2);
+	if (val < 0)
+		return val;
+
+	*base_page |= (u64)val << 32;
+
+	return 0;
+}
+
+static int mtip_config_an_adv(struct phy_device *phydev)
+{
+	u64 base_page = linkmode_adv_to_c73_base_page(phydev->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).
+	 * With purely randomly generated nonces, that is always a chance
+	 * though. 98.2.1.2.3 Transmitted Nonce Field gives a way for
+	 * management to avoid that.
+	 */
+	get_random_bytes(&nonce, sizeof(nonce));
+
+	switch (phydev->master_slave_set) {
+	case MASTER_SLAVE_CFG_MASTER_PREFERRED:
+	case MASTER_SLAVE_CFG_MASTER_FORCE:
+		nonce |= BIT(4);
+		break;
+	case MASTER_SLAVE_CFG_SLAVE_PREFERRED:
+	case MASTER_SLAVE_CFG_SLAVE_FORCE:
+		nonce &= ~BIT(4);
+		break;
+	}
+
+	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(phydev, base_page);
+}
+
+static int mtip_an_restart(struct phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	err = mtip_state_reset(phydev);
+	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(phydev);
+	if (err < 0)
+		return err;
+
+	err = mtip_unlatch_an_stat(phydev);
+	if (err)
+		return err;
+
+	err = mtip_unlatch_pcs_stat(phydev);
+	if (err)
+		return err;
+
+	err = mtip_config_an_adv(phydev);
+	if (err < 0)
+		return err;
+
+	err = mtip_modify_an(phydev, 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;
+}
+
+/* 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 phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+
+	mutex_lock(&priv->an_restart_lock);
+	priv->an_restart_pending = true;
+	mutex_unlock(&priv->an_restart_lock);
+}
+
+static int mtip_finalize_lt(struct phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	union phy_configure_opts opts = {
+		.xgkr = {
+			.type = XGKR_CONFIGURE_LT_DONE,
+		},
+	};
+	int err, val;
+
+	lockdep_assert_held(&priv->lt_lock);
+
+	if (!priv->local_tx_lt_done || !priv->remote_tx_lt_done)
+		return 0;
+
+	/* Let the local state machine know we're done */
+	err = mtip_modify_lt(phydev, LT_STAT, LT_STAT_RX_STATUS,
+			     LT_STAT_RX_STATUS);
+	if (err < 0) {
+		phydev_err(phydev, "Failed to update LT_STAT: %pe\n",
+			   ERR_PTR(err));
+		return err;
+	}
+
+	err = mtip_finish_lt(phydev);
+	if (err)
+		return err;
+
+	val = mtip_read_lt(phydev, LT_STAT);
+	if (val < 0)
+		return val;
+
+	if (!(val & LT_STAT_SIGNAL_DETECT) || (val & LT_STAT_FAIL)) {
+		phydev_err(phydev,
+			   "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 phy_device *phydev,
+				   const struct c72_coef_update *upd,
+				   struct c72_coef_status *stat)
+{
+	char upd_buf[C72_COEF_UPDATE_BUFSIZ], stat_buf[C72_COEF_STATUS_BUFSIZ];
+	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, phydev, LT_LP_STAT);
+	if (val < 0)
+		return val;
+	if (err) {
+		phydev_err(phydev,
+			   "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(phydev, 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, phydev, LT_LP_STAT);
+	if (val < 0)
+		return val;
+	if (err) {
+		phydev_err(phydev,
+			   "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(phydev, LT_LD_COEF, ld_coef);
+	if (err < 0)
+		return err;
+
+	phydev_dbg(phydev, "Sent update: %s, got status: %s\n",
+		   upd_buf, stat_buf);
+
+	return 0;
+}
+
+static int mtip_c72_process_request(struct phy_device *phydev,
+				    const struct c72_coef_update *upd,
+				    struct c72_coef_status *stat)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	union phy_configure_opts opts = {
+		.xgkr = {
+			.type = XGKR_CONFIGURE_LOCAL_TX,
+			.local_tx = {
+				.update = *upd,
+			},
+		},
+	};
+	int err;
+
+	err = phy_configure(priv->serdes, &opts);
+	if (err)
+		return err;
+
+	*stat = opts.xgkr.local_tx.status;
+
+	return 0;
+}
+
+static int mtip_read_lt_lp_coef_if_not_ready(struct phy_device *phydev,
+					     bool *rx_ready)
+{
+	int val;
+
+	val = mtip_read_lt(phydev, 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(phydev, LT_LP_COEF);
+}
+
+static int mtip_rx_c72_coef_update(struct phy_device *phydev,
+				   struct c72_coef_update *upd,
+				   bool *rx_ready)
+{
+	char upd_buf[C72_COEF_UPDATE_BUFSIZ], stat_buf[C72_COEF_STATUS_BUFSIZ];
+	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, phydev, rx_ready);
+	if (val < 0)
+		return val;
+	if (*rx_ready) {
+		phydev_dbg(phydev, "LP says its RX is ready\n");
+		return 0;
+	}
+	if (err) {
+		phydev_err(phydev,
+			   "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) {
+		phydev_warn(phydev, "Ignoring illegal request: %s (LP_COEF = 0x%x)\n",
+			    upd_buf, val);
+		return 0;
+	}
+
+	err = mtip_c72_process_request(phydev, upd, &stat);
+	if (err)
+		return err;
+
+	c72_coef_status_print(&stat, stat_buf);
+	phydev_dbg(phydev, "Received update: %s, responded with status: %s\n",
+		   upd_buf, stat_buf);
+
+	err = mtip_modify_lt(phydev, 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, phydev, LT_LP_COEF);
+	if (val < 0)
+		return val;
+	if (err) {
+		phydev_err(phydev, "LP did not revert to HOLD; LP_COEF = 0x%x\n",
+			   val);
+		return err;
+	}
+
+	err = mtip_modify_lt(phydev, 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 phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	mutex_lock(&priv->lt_lock);
+
+	priv->local_tx_lt_done = true;
+
+	err = mtip_finalize_lt(phydev);
+
+	mutex_unlock(&priv->lt_lock);
+
+	return err;
+}
+
+static int mtip_remote_tx_lt_done(struct phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	mutex_lock(&priv->lt_lock);
+
+	priv->remote_tx_lt_done = true;
+
+	err = mtip_finalize_lt(phydev);
+
+	mutex_unlock(&priv->lt_lock);
+
+	return err;
+}
+
+/* This is our hardware-based 500 ms timer for the link training */
+static bool mtip_lt_in_progress(struct phy_device *phydev)
+{
+	int val;
+
+	val = mtip_read_lt(phydev, LT_STAT);
+	if (val < 0)
+		return false;
+
+	return !!(val & LT_STAT_IN_PROGRESS);
+}
+
+/* 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 phy_device *phydev = lt_work->phydev;
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	err = mtip_lt_frame_lock(phydev);
+	if (err) {
+		phydev_err(phydev,
+			   "Failed to acquire training frame delineation: %pe\n",
+			   ERR_PTR(err));
+		goto out;
+	}
+
+	while (READ_ONCE(priv->lt_enabled)) {
+		struct c72_coef_update upd = {};
+		bool rx_ready;
+
+		if (!mtip_lt_in_progress(phydev)) {
+			phydev_err(phydev, "Local TX LT timed out\n");
+			break;
+		}
+
+		err = mtip_rx_c72_coef_update(phydev, &upd, &rx_ready);
+		if (err)
+			goto out;
+
+		if (rx_ready) {
+			err = mtip_local_tx_lt_done(phydev);
+			if (err) {
+				phydev_err(phydev, "Failed to finalize local LT: %pe\n",
+					   ERR_PTR(err));
+				goto out;
+			}
+			break;
+		}
+	}
+
+out:
+	if (err && READ_ONCE(priv->lt_enabled))
+		mtip_an_restart_from_lt(phydev);
+
+	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 phy_device *phydev = lt_work->phydev;
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	err = mtip_lt_frame_lock(phydev);
+	if (err) {
+		phydev_err(phydev,
+			   "Failed to acquire training frame delineation: %pe\n",
+			   ERR_PTR(err));
+		goto out;
+	}
+
+	while (true) {
+		struct c72_coef_status status = {};
+		union phy_configure_opts opts = {
+			.xgkr = {
+				.type = XGKR_CONFIGURE_REMOTE_TX,
+			},
+		};
+
+		if (!READ_ONCE(priv->lt_enabled))
+			goto out;
+
+		if (!mtip_lt_in_progress(phydev)) {
+			phydev_err(phydev, "Remote TX LT timed out\n");
+			goto out;
+		}
+
+		err = phy_configure(priv->serdes, &opts);
+		if (err) {
+			phydev_err(phydev,
+				   "Failed to get remote TX training request from SerDes: %pe\n",
+				   ERR_PTR(err));
+			goto out;
+		}
+
+		if (opts.xgkr.remote_tx.rx_ready)
+			break;
+
+		err = mtip_tx_c72_coef_update(phydev, &opts.xgkr.remote_tx.update,
+					      &status);
+		if (opts.xgkr.remote_tx.cb)
+			opts.xgkr.remote_tx.cb(opts.xgkr.remote_tx.cb_priv, err,
+					       opts.xgkr.remote_tx.update,
+					       status);
+		if (err)
+			goto out;
+	}
+
+	/* Let the link partner know we're done */
+	err = mtip_modify_lt(phydev, LT_LD_STAT, LT_COEF_STAT_RX_READY,
+			     LT_COEF_STAT_RX_READY);
+	if (err < 0) {
+		phydev_err(phydev, "Failed to update LT_LD_STAT: %pe\n",
+			   ERR_PTR(err));
+		goto out;
+	}
+
+	err = mtip_remote_tx_lt_done(phydev);
+	if (err) {
+		phydev_err(phydev, "Failed to finalize remote LT: %pe\n",
+			   ERR_PTR(err));
+		goto out;
+	}
+
+out:
+	if (err && READ_ONCE(priv->lt_enabled))
+		mtip_an_restart_from_lt(phydev);
+
+	kfree(lt_work);
+}
+
+static int mtip_start_lt(struct phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+	struct mtip_backplane *priv = phydev->priv;
+	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(phydev);
+	if (err)
+		goto out_free_remote_tx_lt;
+
+	err = mtip_restart_lt(phydev, true);
+	if (err)
+		goto out_free_remote_tx_lt;
+
+	priv->local_tx_lt_done = false;
+	priv->remote_tx_lt_done = false;
+	WRITE_ONCE(priv->lt_enabled, true);
+
+	local_tx_lt_work->phydev = phydev;
+	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->phydev = phydev;
+	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);
+
+	phydev_dbg(phydev, "Link training enabled\n");
+
+	return 0;
+
+out_free_remote_tx_lt:
+	kfree(remote_tx_lt_work);
+out_free_local_tx_lt:
+	kfree(local_tx_lt_work);
+out:
+	return err;
+}
+
+static void mtip_update_link_latch(struct phy_device *phydev,
+				   bool cdr_locked, bool phy_los,
+				   bool an_complete, bool pcs_lstat)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+	struct mtip_backplane *priv = phydev->priv;
+
+	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;
+	}
+
+	phydev_dbg(phydev, "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 phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+
+	lockdep_assert_held(&irqpoll->lock);
+
+	return !!(irqpoll->old_an_stat & MDIO_AN_STAT1_COMPLETE);
+}
+
+static bool mtip_read_link_unlatch(struct phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+	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_100000baseKR4_Full_BIT:
+		return BP_ETH_STAT_ALWAYS_1 | BP_ETH_STAT_100GKR4;
+	default:
+		return 0;
+	}
+}
+
+static int mtip_wait_bp_eth_stat(struct phy_device *phydev,
+				 enum ethtool_link_mode_bit_indices link_mode)
+{
+	u16 expected = mtip_expected_bp_eth_stat(link_mode);
+	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, phydev, AN_BP_ETH_STAT);
+	if (val < 0)
+		return val;
+
+	if (err) {
+		phydev_warn(phydev,
+			    "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)" : "");
+		/* It seems less consequential to ignore the error
+		 * than to restart autoneg...
+		 */
+	}
+
+	return 0;
+}
+
+static int mtip_c73_page_received(struct phy_device *phydev, bool *restart_an)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
+	enum ethtool_link_mode_bit_indices resolved;
+	struct mtip_backplane *priv = phydev->priv;
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	u64 base_page, lpa;
+	int err;
+
+	err = mtip_state_reset(phydev);
+	if (err)
+		return err;
+
+	err = mtip_read_adv(phydev, &base_page);
+	if (err < 0)
+		return err;
+
+	err = mtip_read_lpa(phydev, &lpa);
+	if (err < 0)
+		return err;
+
+	if (lpa & C73_BASE_PAGE_NP)
+		phydev_warn(phydev, "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) {
+		phydev_dbg(phydev, "C73 page received, no common link mode\n");
+		return 0;
+	}
+
+	err = mtip_wait_bp_eth_stat(phydev, resolved);
+	if (err) {
+		*restart_an = true;
+		return 0;
+	}
+
+	phydev_dbg(phydev,
+		   "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 = phy_set_mode_ext(priv->serdes, PHY_MODE_ETHERNET_PHY, resolved);
+	if (err) {
+		phydev_err(phydev, "phy_set_mode_ext(%s) returned %pe\n",
+			   ethtool_link_mode_str(resolved), ERR_PTR(err));
+		return err;
+	}
+
+	err = mtip_wait_for_cdr_lock(phydev);
+	if (err)
+		phydev_warn(phydev, "Failed to reacquire CDR lock after protocol change: %pe\n",
+			    ERR_PTR(err));
+
+	if (link_mode_needs_training(resolved)) {
+		err = mtip_start_lt(phydev);
+		if (err)
+			return err;
+	} else {
+		/* Allow the datapath to come up without link training */
+		err = mtip_modify_lt(phydev, LT_STAT, LT_STAT_RX_STATUS,
+				     LT_STAT_RX_STATUS);
+		if (err < 0)
+			return err;
+	}
+
+	priv->link_mode = resolved;
+	priv->link_mode_resolved = true;
+
+	return 0;
+}
+
+static void mtip_c73_remote_fault(struct phy_device *phydev, bool fault)
+{
+	phydev_err(phydev, "Remote fault: %d\n", fault);
+}
+
+static void mtip_irqpoll_work(struct work_struct *work)
+{
+	struct mtip_irqpoll *irqpoll = container_of(work, struct mtip_irqpoll, work.work);
+	struct phy_device *phydev = irqpoll->phydev;
+	struct mtip_backplane *priv = phydev->priv;
+	bool restart_an = false;
+	int val, err = 0;
+	int pcs_stat = 0;
+	bool cdr_locked;
+
+	/* 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 = phy_check_cdr_lock(priv->serdes, &cdr_locked);
+	if (err)
+		goto out_unlock;
+
+	if (priv->link_mode_resolved) {
+		pcs_stat = mtip_read_pcs(phydev, MDIO_STAT1);
+		if (pcs_stat < 0) {
+			err = pcs_stat;
+			goto out_unlock;
+		}
+	}
+
+	val = mtip_read_an(phydev, AN_STAT);
+	if (val < 0) {
+		err = val;
+		goto out_unlock;
+	}
+
+	if ((irqpoll->cdr_locked != cdr_locked) ||
+	    ((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(phydev, cdr_locked,
+				       !!(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)) {
+		err = mtip_c73_page_received(phydev, &restart_an);
+		if (err)
+			goto out_unlock;
+	}
+
+	if ((irqpoll->old_an_stat ^ val) & MDIO_AN_STAT1_RFAULT)
+		mtip_c73_remote_fault(phydev, 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 &&
+	    (irqpoll->old_pcs_stat & MDIO_STAT1_LSTATUS) &&
+	    !(pcs_stat & MDIO_STAT1_LSTATUS)) {
+		phydev_dbg(phydev, "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))) {
+		phydev_err(phydev,
+			   "Hardware says AN has completed, but we never saw a base page, and that's bogus\n");
+		restart_an = true;
+	}
+
+	if (restart_an) {
+		err = mtip_an_restart(phydev);
+		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 = cdr_locked;
+
+out_unlock:
+	mutex_unlock(&irqpoll->lock);
+
+	if (err) {
+		phy_error(phydev);
+		return;
+	}
+
+	schedule_delayed_work(&irqpoll->work, IRQPOLL_INTERVAL);
+}
+
+static int mtip_parse_dt(struct phy_device *phydev)
+{
+	struct device_node *dn = phydev->mdio.dev.of_node;
+	struct mtip_backplane *priv = phydev->priv;
+	struct device_node *pcs_node;
+
+	priv->serdes = of_phy_get(dn, NULL);
+	if (IS_ERR(priv->serdes))
+		return PTR_ERR(priv->serdes);
+
+	pcs_node = of_parse_phandle(dn, "pcs-handle", 0);
+	if (pcs_node) {
+		if (!of_device_is_available(pcs_node)) {
+			phydev_err(phydev, "pcs-handle node not available\n");
+			of_node_put(pcs_node);
+			return -ENODEV;
+		}
+
+		priv->pcs = of_mdio_find_device(pcs_node);
+		of_node_put(pcs_node);
+		if (!priv->pcs) {
+			phydev_err(phydev, "missing PCS device\n");
+			return -EPROBE_DEFER;
+		}
+	} else {
+		priv->pcs = &phydev->mdio;
+	}
+
+	return 0;
+}
+
+static bool mtip_is_lx2160a(struct phy_device *phydev)
+{
+	return of_device_is_compatible(phydev->mdio.dev.of_node,
+				       "fsl,lx2160a-backplane-anlt");
+}
+
+static int mtip_lx2160a_match_phy_device(struct phy_device *phydev)
+{
+	return mtip_is_lx2160a(phydev);
+}
+
+static void mtip_irqpoll_init(struct phy_device *phydev,
+			      struct mtip_irqpoll *irqpoll)
+{
+	mutex_init(&irqpoll->lock);
+	INIT_DELAYED_WORK(&irqpoll->work, mtip_irqpoll_work);
+	irqpoll->phydev = phydev;
+}
+
+static void mtip_start_irqpoll(struct phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+
+	schedule_delayed_work(&irqpoll->work, 0);
+}
+
+static void mtip_stop_irqpoll(struct phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+
+	cancel_delayed_work_sync(&irqpoll->work);
+}
+
+static int mtip_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct mtip_backplane *priv;
+	int err;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	phydev->port = PORT_DA;
+
+	if (mtip_is_lx2160a(phydev)) {
+		priv->an_regs = mtip_lx2160a_an_regs;
+		priv->lt_regs = mtip_lx2160a_lt_regs;
+		priv->lt_mmd = MDIO_MMD_AN;
+	} /* else TODO */
+
+	mtip_irqpoll_init(phydev, &priv->irqpoll);
+	mutex_init(&priv->an_restart_lock);
+	mutex_init(&priv->lt_lock);
+	phydev->priv = priv;
+
+	priv->local_tx_lt_worker = kthread_create_worker(0, "%s_local_tx_lt",
+							 dev_name(dev));
+	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, "%s_remote_tx_lt",
+							  dev_name(dev));
+	if (IS_ERR(priv->remote_tx_lt_worker)) {
+		err = PTR_ERR(priv->remote_tx_lt_worker);
+		goto out_destroy_local_tx_lt;
+	}
+
+	err = mtip_parse_dt(phydev);
+	if (err)
+		goto out_destroy_remote_tx_lt;
+
+	err = phy_init(priv->serdes);
+	if (err) {
+		phydev_err(phydev, "Failed to initialize SerDes: %pe\n",
+			   ERR_PTR(err));
+		goto out_put_phy;
+	}
+
+	mtip_start_irqpoll(phydev);
+
+	return 0;
+
+out_put_phy:
+	of_phy_put(priv->serdes);
+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_priv:
+	kfree(priv);
+out:
+	return err;
+}
+
+static void mtip_remove(struct phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+
+	mtip_stop_irqpoll(phydev);
+	phy_exit(priv->serdes);
+	of_phy_put(priv->serdes);
+	kthread_destroy_worker(priv->remote_tx_lt_worker);
+	kthread_destroy_worker(priv->local_tx_lt_worker);
+	kfree(priv);
+}
+
+static int mtip_config_aneg(struct phy_device *phydev)
+{
+	u16 mask = MDIO_AN_CTRL1_ENABLE | MDIO_AN_CTRL1_RESTART;
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	mutex_lock(&irqpoll->lock);
+
+	/* We support anything */
+	phydev->master_slave_get = phydev->master_slave_set;
+
+	/* Only allow advertising what this PHY supports. The device tree
+	 * property "max-speed" may further limit the speed and thus the
+	 * link modes. Similar to genphy_config_advert().
+	 */
+	linkmode_and(phydev->advertising, phydev->advertising,
+		     phydev->supported);
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		err = mtip_an_restart(phydev);
+		if (err)
+			goto out_unlock;
+
+		priv->an_enabled = true;
+	} else {
+		err = mtip_modify_an(phydev, AN_CTRL, mask, 0);
+		if (err < 0)
+			goto out_unlock;
+
+		priv->an_enabled = false;
+	}
+
+out_unlock:
+	mutex_unlock(&irqpoll->lock);
+
+	return err;
+}
+
+static int mtip_resolve_aneg_linkmode(struct phy_device *phydev)
+{
+	u64 base_page;
+	int err;
+
+	linkmode_zero(phydev->lp_advertising);
+
+	err = mtip_read_lpa(phydev, &base_page);
+	if (err)
+		return err;
+
+	mii_c73_mod_linkmode_lpa_t(phydev->lp_advertising, base_page);
+	phy_resolve_aneg_linkmode(phydev);
+
+	return 0;
+}
+
+static int mtip_read_status(struct phy_device *phydev)
+{
+	struct mtip_irqpoll *irqpoll = phydev_to_irqpoll(phydev);
+	u64 base_page;
+	int err = 0;
+
+	mutex_lock(&irqpoll->lock);
+
+	err = mtip_read_adv(phydev, &base_page);
+	if (err)
+		goto out_unlock;
+
+	if (C73_BASE_PAGE_TRANSMITTED_NONCE_X(base_page) & BIT(4))
+		phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
+	else
+		phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
+
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	phydev->link = mtip_read_link_unlatch(phydev);
+	if (!phydev->link)
+		goto out_unlock;
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		phydev->autoneg_complete = mtip_cached_an_complete(phydev);
+
+		if (phydev->autoneg_complete)
+			err = mtip_resolve_aneg_linkmode(phydev);
+	}
+
+out_unlock:
+	mutex_unlock(&irqpoll->lock);
+
+	return err;
+}
+
+static int mtip_suspend(struct phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	err = phy_power_off(priv->serdes);
+	if (err) {
+		phydev_err(phydev, "Failed to power off SerDes: %pe\n",
+			   ERR_PTR(err));
+		return err;
+	}
+
+	return 0;
+}
+
+static int mtip_resume(struct phy_device *phydev)
+{
+	struct mtip_backplane *priv = phydev->priv;
+	int err;
+
+	err = phy_power_on(priv->serdes);
+	if (err) {
+		phydev_err(phydev, "Failed to power on SerDes: %pe\n",
+			   ERR_PTR(err));
+		return err;
+	}
+
+	return 0;
+}
+
+static int mtip_config_init(struct phy_device *phydev)
+{
+	int err;
+
+	err = mtip_reset_an(phydev);
+	if (err < 0)
+		return err;
+
+	err = mtip_reset_pcs(phydev);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static struct phy_driver mtip_backplane_driver[] = {
+	{
+		.match_phy_device	= mtip_lx2160a_match_phy_device,
+		.flags			= PHY_IS_INTERNAL,
+		.name			= "MTIP AN/LT",
+		.probe			= mtip_probe,
+		.remove			= mtip_remove,
+		.get_features		= mtip_lx2160a_get_features,
+		.suspend		= mtip_suspend,
+		.resume			= mtip_resume,
+		.config_aneg		= mtip_config_aneg,
+		.read_status		= mtip_read_status,
+		.config_init		= mtip_config_init,
+	},
+};
+
+static const struct of_device_id __maybe_unused mtip_backplane_of_match[] = {
+	{ .compatible = "fsl,lx2160a-backplane-anlt" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mtip_backplane_of_match);
+
+module_phy_driver(mtip_backplane_driver);
+
+MODULE_AUTHOR("Vladimir Oltean <vladimir.oltean@nxp.com>");
+MODULE_DESCRIPTION("MTIP Backplane PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

[RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Vladimir Oltean]

Illustrate how the backplane AN/LT blocks can be instantiated on the
LX2160A SoC with SerDes protocol 19.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../devicetree/bindings/net/ethernet-phy.yaml |   8 +
 .../bindings/net/fsl,backplane-anlt.yaml      | 238 ++++++++++++++++++
 2 files changed, 246 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index c1241c8a3b77..96fa672e4786 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -49,6 +49,14 @@ properties:
       - items:
           - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$"
           - const: ethernet-phy-ieee802.3-c45
+      - items:
+          - const: fsl,lx2160a-backplane-anlt
+          - const: ethernet-phy-ieee802.3-c45
+        description:
+          Some C45 PHYs have no PHY ID in the standard location, and there is
+          also no PHY ID allocated for them to fake. They are identified by the
+          primary compatible string, plus the secondary one to distinguish them
+          from a raw MDIO device.
 
   reg:
     minimum: 0
diff --git a/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
new file mode 100644
index 000000000000..7282e93b1dd4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
@@ -0,0 +1,238 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/fsl,backplane-anlt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Backplane Ethernet PHY
+
+maintainers:
+  - Vladimir Oltean <vladimir.oltean@nxp.com>
+
+description: |
+  Some QorIQ and Layerscape SoCs have an additional block on each SerDes
+  networking lane, based on an IP core from MoreThanIP, which performs IEEE
+  802.3 clause 73 base page exchanges (for auto-negotiation) and clause 72
+  training frame exchanges (for link training).
+
+  By default, this AN/LT block comes up with auto-negotiation disabled, and
+  in that case it allows itself to be quickly bypassed from the data path and
+  for the PCS link to come up without its involvement.
+
+  Software can optionally make use of it, to turn the PCS, AN/LT block and lane
+  (PMA/PMD) into a full copper backplane internal PHY.
+
+  As a warning, the binding for the multi-lane link modes (40GBase-KR4) is not
+  currently backed up by a driver implementation.
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          - items:
+              - const: fsl,lx2160a-backplane-anlt
+              - const: ethernet-phy-ieee802.3-c45
+    then:
+      $ref: ethernet-phy.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: fsl,lx2160a-backplane-anlt
+          - const: ethernet-phy-ieee802.3-c45
+      - const: fsl,lx2160a-secondary-anlt
+
+  reg:
+    minimum: 0
+    maximum: 31
+    description: |
+      The address of the AN/LT block within the internal MDIO bus of the MAC it
+      is attached to.
+
+      In the 1000Base-KX and 10GBase-KR link modes, the AN/LT block responds at
+      the same MDIO address as the PCS (determined by the SGMIInCR1[MDEV_PORT]
+      or SXGMIInCR1[MDEV_PORT] registers of the SerDes block, by default 0).
+      The PCS and AN/LT block respond to different MMDs, though.
+
+      In the 25GBase-KR and higher link modes, the AN/LT block responds at a
+      different MDIO address than the PCS, determined by the
+      ANLTnCR1[MDEV_PORT] register of the SerDes block. By default this is 4
+      for lanes A and E, 5 for lanes B and F, 6 for lanes C and G, 7 for lanes
+      D and H.
+
+      The PCS responds in all cases at the address determined by the MDEV_PORT
+      field of the SGMIInCR1, SXGMIIaCR1, E25GaCR1, E40GaCR1, E50GaCR1 or
+      E100GaCR1 registers of the SerDes block.
+
+  phys:
+    maxItems: 1
+    description:
+      phandle for the generic PHY (SerDes lane) that acts as PMA/PMD layer
+
+  pcs-handle:
+    maxItems: 1
+    description:
+      phandle for the technology-dependent PCS block corresponding to the
+      initial (RCW-based) configuration of the port. Must be omitted for the
+      link modes where the PCS and AN/LT block respond at the same MDIO
+      address. Must be specified otherwise.
+
+  secondary-anlt-handle:
+    maxItems: 1
+    description:
+      In case this is the primary (first) lane of a multi-lane link mode, this
+      property holds an array of phandles for the other AN/LT blocks, that are
+      involved in link training but not in auto-negotiation. These have the
+      "fsl,lx2160a-secondary-anlt" compatible string.
+
+required:
+  - compatible
+  - reg
+  - phys
+
+unevaluatedProperties: false
+
+examples:
+
+  # LX2160A lanes A, B, C, D with SerDes 1 protocol 19: dpmac2 uses 40GBase-KR4
+  - |
+    dpmac2 {
+        phy-handle = <&mac2_backplane_anlt>;
+        phy-connection-type = "internal";
+    };
+
+    pcs_mdio2 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        status = "okay";
+
+        pcs2: ethernet-phy@0 {
+            reg = <0>;
+        };
+
+        mac2_backplane_anlt: ethernet-phy@7 {
+            compatible = "fsl,lx2160a-backplane-anlt",
+                          "ethernet-phy-ieee802.3-c45";
+            reg = <7>; /* according to ANLTDCR1[MDEV_PORT] */
+            phys = <&serdes_1 3>; /* lane D */
+            max-speed = <40000>;
+            pcs-handle = <&pcs2>;
+            secondary-anlt-handle = <&mac2_lane2_anlt>, <&mac2_lane3_anlt>,
+                                    <&mac2_lane4_anlt>;
+        };
+
+        mac2_lane2_anlt: ethernet-backplane-anlt@6 {
+            compatible = "fsl,lx2160a-secondary-anlt";
+            reg = <6>; /* according to ANLTCCR1[MDEV_PORT] */
+            phys = <&serdes_1 2>; /* lane C */
+        };
+
+        mac2_lane3_anlt: ethernet-backplane-anlt@5 {
+            compatible = "fsl,lx2160a-secondary-anlt";
+            reg = <5>; /* according to ANLTBCR1[MDEV_PORT] */
+            phys = <&serdes_1 1>; /* lane B */
+        };
+
+        mac2_lane4_anlt: ethernet-backplane-anlt@4 {
+            compatible = "fsl,lx2160a-secondary-anlt";
+            reg = <4>; /* according to ANLTACR1[MDEV_PORT] */
+            phys = <&serdes_1 0>; /* lane A */
+        };
+    };
+
+  # LX2160A lane E with SerDes 1 protocol 19: dpmac6 uses 25GBase-KR
+  - |
+    dpmac6 {
+        phy-handle = <&mac6_backplane_anlt>;
+        phy-connection-type = "internal";
+    };
+
+    pcs_mdio6 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        status = "okay";
+
+        pcs6: ethernet-phy@0 {
+            reg = <0>;
+        };
+
+        mac6_backplane_anlt: ethernet-phy@4 {
+            compatible = "fsl,lx2160a-backplane-anlt",
+                          "ethernet-phy-ieee802.3-c45";
+            reg = <4>; /* according to ANLTFCR1[MDEV_PORT] */
+            phys = <&serdes_1 4>; /* lane E */
+            max-speed = <25000>;
+            pcs-handle = <&pcs6>;
+        };
+    };
+
+  # LX2160A lane F with SerDes 1 protocol 19: dpmac5 uses 25GBase-KR
+  - |
+    dpmac5 {
+        phy-handle = <&mac5_backplane_anlt>;
+        phy-connection-type = "internal";
+    };
+
+    pcs_mdio5 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        status = "okay";
+
+        pcs5: ethernet-phy@0 {
+            reg = <0>;
+        };
+
+        mac5_backplane_anlt: ethernet-phy@5 {
+            compatible = "fsl,lx2160a-backplane-anlt",
+                         "ethernet-phy-ieee802.3-c45";
+            reg = <5>; /* according to ANLTFCR1[MDEV_PORT] */
+            phys = <&serdes_1 5>; /* lane F */
+            max-speed = <25000>;
+            pcs-handle = <&pcs5>;
+        };
+    };
+
+  # LX2160A lane G with SerDes 1 protocol 19: dpmac4 uses 10GBase-KR
+  - |
+    dpmac4 {
+        phy-handle = <&mac4_backplane_anlt>;
+        phy-connection-type = "internal";
+    };
+
+    pcs_mdio4 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        status = "okay";
+
+        mac4_backplane_anlt: ethernet-phy@0 {
+            compatible = "fsl,lx2160a-backplane-anlt",
+                         "ethernet-phy-ieee802.3-c45";
+            reg = <0>; /* merged with PCS SXGMIIGCR1[MDEV_PORT] */
+            phys = <&serdes_1 7>; /* lane G */
+            max-speed = <10000>;
+            /* no pcs-handle to &pcs4 */
+        };
+    };
+
+  # LX2160A lane H with SerDes 1 protocol 19: dpmac3 uses 10GBase-KR
+  - |
+    dpmac3 {
+        phy-handle = <&mac3_backplane_anlt>;
+        phy-connection-type = "internal";
+    };
+
+    pcs_mdio3 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        status = "okay";
+
+        mac3_backplane_anlt: ethernet-phy@0 {
+            compatible = "fsl,lx2160a-backplane-anlt",
+                         "ethernet-phy-ieee802.3-c45";
+            reg = <0>; /* merged with PCS SXGMIIHCR1[MDEV_PORT] */
+            phys = <&serdes_1 7>; /* lane H */
+            max-speed = <10000>;
+            /* no pcs-handle to &pcs3 */
+        };
+    };
-- 
2.34.1

Re: [RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Rob Herring]

On Thu, Aug 17, 2023 at 06:06:44PM +0300, Vladimir Oltean wrote:
> Illustrate how the backplane AN/LT blocks can be instantiated on the
> LX2160A SoC with SerDes protocol 19.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  .../devicetree/bindings/net/ethernet-phy.yaml |   8 +
>  .../bindings/net/fsl,backplane-anlt.yaml      | 238 ++++++++++++++++++
>  2 files changed, 246 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index c1241c8a3b77..96fa672e4786 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -49,6 +49,14 @@ properties:
>        - items:
>            - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$"
>            - const: ethernet-phy-ieee802.3-c45
> +      - items:
> +          - const: fsl,lx2160a-backplane-anlt
> +          - const: ethernet-phy-ieee802.3-c45

What's the benefit of having ethernet-phy-ieee802.3-c45? Will it work if 
the OS only understands that and not fsl,lx2160a-backplane-anlt?

> +        description:
> +          Some C45 PHYs have no PHY ID in the standard location, and there is
> +          also no PHY ID allocated for them to fake. They are identified by the
> +          primary compatible string, plus the secondary one to distinguish them
> +          from a raw MDIO device.
>  
>    reg:
>      minimum: 0
> diff --git a/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
> new file mode 100644
> index 000000000000..7282e93b1dd4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
> @@ -0,0 +1,238 @@
> +# SPDX-License-Identifier: GPL-2.0+

Not the right license.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/fsl,backplane-anlt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Backplane Ethernet PHY
> +
> +maintainers:
> +  - Vladimir Oltean <vladimir.oltean@nxp.com>
> +
> +description: |
> +  Some QorIQ and Layerscape SoCs have an additional block on each SerDes
> +  networking lane, based on an IP core from MoreThanIP, which performs IEEE
> +  802.3 clause 73 base page exchanges (for auto-negotiation) and clause 72
> +  training frame exchanges (for link training).
> +
> +  By default, this AN/LT block comes up with auto-negotiation disabled, and
> +  in that case it allows itself to be quickly bypassed from the data path and
> +  for the PCS link to come up without its involvement.
> +
> +  Software can optionally make use of it, to turn the PCS, AN/LT block and lane
> +  (PMA/PMD) into a full copper backplane internal PHY.
> +
> +  As a warning, the binding for the multi-lane link modes (40GBase-KR4) is not
> +  currently backed up by a driver implementation.
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          - items:
> +              - const: fsl,lx2160a-backplane-anlt
> +              - const: ethernet-phy-ieee802.3-c45

Use "contains" and drop ethernet-phy-ieee802.3-c45.

> +    then:
> +      $ref: ethernet-phy.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: fsl,lx2160a-backplane-anlt
> +          - const: ethernet-phy-ieee802.3-c45
> +      - const: fsl,lx2160a-secondary-anlt
> +
> +  reg:
> +    minimum: 0
> +    maximum: 31
> +    description: |
> +      The address of the AN/LT block within the internal MDIO bus of the MAC it
> +      is attached to.
> +
> +      In the 1000Base-KX and 10GBase-KR link modes, the AN/LT block responds at
> +      the same MDIO address as the PCS (determined by the SGMIInCR1[MDEV_PORT]
> +      or SXGMIInCR1[MDEV_PORT] registers of the SerDes block, by default 0).
> +      The PCS and AN/LT block respond to different MMDs, though.
> +
> +      In the 25GBase-KR and higher link modes, the AN/LT block responds at a
> +      different MDIO address than the PCS, determined by the
> +      ANLTnCR1[MDEV_PORT] register of the SerDes block. By default this is 4
> +      for lanes A and E, 5 for lanes B and F, 6 for lanes C and G, 7 for lanes
> +      D and H.
> +
> +      The PCS responds in all cases at the address determined by the MDEV_PORT
> +      field of the SGMIInCR1, SXGMIIaCR1, E25GaCR1, E40GaCR1, E50GaCR1 or
> +      E100GaCR1 registers of the SerDes block.
> +
> +  phys:
> +    maxItems: 1
> +    description:
> +      phandle for the generic PHY (SerDes lane) that acts as PMA/PMD layer
> +
> +  pcs-handle:
> +    maxItems: 1
> +    description:
> +      phandle for the technology-dependent PCS block corresponding to the
> +      initial (RCW-based) configuration of the port. Must be omitted for the
> +      link modes where the PCS and AN/LT block respond at the same MDIO
> +      address. Must be specified otherwise.
> +
> +  secondary-anlt-handle:

Needs a vendor prefix and type.

> +    maxItems: 1
> +    description:
> +      In case this is the primary (first) lane of a multi-lane link mode, this
> +      property holds an array of phandles for the other AN/LT blocks, that are
> +      involved in link training but not in auto-negotiation. These have the
> +      "fsl,lx2160a-secondary-anlt" compatible string.
> +
> +required:
> +  - compatible
> +  - reg
> +  - phys
> +
> +unevaluatedProperties: false
> +
> +examples:
> +
> +  # LX2160A lanes A, B, C, D with SerDes 1 protocol 19: dpmac2 uses 40GBase-KR4
> +  - |
> +    dpmac2 {
> +        phy-handle = <&mac2_backplane_anlt>;
> +        phy-connection-type = "internal";
> +    };
> +
> +    pcs_mdio2 {

mdio {

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        status = "okay";

Don't need status in examples.

> +
> +        pcs2: ethernet-phy@0 {
> +            reg = <0>;
> +        };
> +
> +        mac2_backplane_anlt: ethernet-phy@7 {
> +            compatible = "fsl,lx2160a-backplane-anlt",
> +                          "ethernet-phy-ieee802.3-c45";
> +            reg = <7>; /* according to ANLTDCR1[MDEV_PORT] */
> +            phys = <&serdes_1 3>; /* lane D */
> +            max-speed = <40000>;
> +            pcs-handle = <&pcs2>;
> +            secondary-anlt-handle = <&mac2_lane2_anlt>, <&mac2_lane3_anlt>,
> +                                    <&mac2_lane4_anlt>;
> +        };
> +
> +        mac2_lane2_anlt: ethernet-backplane-anlt@6 {
> +            compatible = "fsl,lx2160a-secondary-anlt";
> +            reg = <6>; /* according to ANLTCCR1[MDEV_PORT] */
> +            phys = <&serdes_1 2>; /* lane C */
> +        };
> +
> +        mac2_lane3_anlt: ethernet-backplane-anlt@5 {
> +            compatible = "fsl,lx2160a-secondary-anlt";
> +            reg = <5>; /* according to ANLTBCR1[MDEV_PORT] */
> +            phys = <&serdes_1 1>; /* lane B */
> +        };
> +
> +        mac2_lane4_anlt: ethernet-backplane-anlt@4 {
> +            compatible = "fsl,lx2160a-secondary-anlt";
> +            reg = <4>; /* according to ANLTACR1[MDEV_PORT] */
> +            phys = <&serdes_1 0>; /* lane A */
> +        };
> +    };
> +
> +  # LX2160A lane E with SerDes 1 protocol 19: dpmac6 uses 25GBase-KR
> +  - |
> +    dpmac6 {
> +        phy-handle = <&mac6_backplane_anlt>;
> +        phy-connection-type = "internal";
> +    };
> +
> +    pcs_mdio6 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        status = "okay";
> +
> +        pcs6: ethernet-phy@0 {
> +            reg = <0>;
> +        };
> +
> +        mac6_backplane_anlt: ethernet-phy@4 {
> +            compatible = "fsl,lx2160a-backplane-anlt",
> +                          "ethernet-phy-ieee802.3-c45";
> +            reg = <4>; /* according to ANLTFCR1[MDEV_PORT] */
> +            phys = <&serdes_1 4>; /* lane E */
> +            max-speed = <25000>;
> +            pcs-handle = <&pcs6>;
> +        };
> +    };
> +
> +  # LX2160A lane F with SerDes 1 protocol 19: dpmac5 uses 25GBase-KR
> +  - |
> +    dpmac5 {
> +        phy-handle = <&mac5_backplane_anlt>;
> +        phy-connection-type = "internal";
> +    };
> +
> +    pcs_mdio5 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        status = "okay";
> +
> +        pcs5: ethernet-phy@0 {
> +            reg = <0>;
> +        };
> +
> +        mac5_backplane_anlt: ethernet-phy@5 {
> +            compatible = "fsl,lx2160a-backplane-anlt",
> +                         "ethernet-phy-ieee802.3-c45";
> +            reg = <5>; /* according to ANLTFCR1[MDEV_PORT] */
> +            phys = <&serdes_1 5>; /* lane F */
> +            max-speed = <25000>;
> +            pcs-handle = <&pcs5>;
> +        };
> +    };
> +
> +  # LX2160A lane G with SerDes 1 protocol 19: dpmac4 uses 10GBase-KR
> +  - |
> +    dpmac4 {
> +        phy-handle = <&mac4_backplane_anlt>;
> +        phy-connection-type = "internal";
> +    };
> +
> +    pcs_mdio4 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        status = "okay";
> +
> +        mac4_backplane_anlt: ethernet-phy@0 {
> +            compatible = "fsl,lx2160a-backplane-anlt",
> +                         "ethernet-phy-ieee802.3-c45";
> +            reg = <0>; /* merged with PCS SXGMIIGCR1[MDEV_PORT] */
> +            phys = <&serdes_1 7>; /* lane G */
> +            max-speed = <10000>;
> +            /* no pcs-handle to &pcs4 */
> +        };
> +    };
> +
> +  # LX2160A lane H with SerDes 1 protocol 19: dpmac3 uses 10GBase-KR
> +  - |
> +    dpmac3 {
> +        phy-handle = <&mac3_backplane_anlt>;
> +        phy-connection-type = "internal";
> +    };
> +
> +    pcs_mdio3 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        status = "okay";
> +
> +        mac3_backplane_anlt: ethernet-phy@0 {
> +            compatible = "fsl,lx2160a-backplane-anlt",
> +                         "ethernet-phy-ieee802.3-c45";
> +            reg = <0>; /* merged with PCS SXGMIIHCR1[MDEV_PORT] */
> +            phys = <&serdes_1 7>; /* lane H */
> +            max-speed = <10000>;
> +            /* no pcs-handle to &pcs3 */
> +        };
> +    };

5 examples yet not one for "fsl,lx2160a-secondary-anlt"

Re: [RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Vladimir Oltean]

Hi Rob,

On Mon, Aug 21, 2023 at 02:58:40PM -0500, Rob Herring wrote:
> On Thu, Aug 17, 2023 at 06:06:44PM +0300, Vladimir Oltean wrote:
> > Illustrate how the backplane AN/LT blocks can be instantiated on the
> > LX2160A SoC with SerDes protocol 19.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> >  .../devicetree/bindings/net/ethernet-phy.yaml |   8 +
> >  .../bindings/net/fsl,backplane-anlt.yaml      | 238 ++++++++++++++++++
> >  2 files changed, 246 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index c1241c8a3b77..96fa672e4786 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -49,6 +49,14 @@ properties:
> >        - items:
> >            - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$"
> >            - const: ethernet-phy-ieee802.3-c45
> > +      - items:
> > +          - const: fsl,lx2160a-backplane-anlt
> > +          - const: ethernet-phy-ieee802.3-c45
> 
> What's the benefit of having ethernet-phy-ieee802.3-c45? Will it work if 
> the OS only understands that and not fsl,lx2160a-backplane-anlt?

No. The "is_c45" bool won't get set correctly in fwnode_mdiobus_register_phy().

	is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");

With that bool set incorrectly, the MDIO protocol cannot access the device's
registers.

> > +        description:
> > +          Some C45 PHYs have no PHY ID in the standard location, and there is
> > +          also no PHY ID allocated for them to fake. They are identified by the
> > +          primary compatible string, plus the secondary one to distinguish them
> > +          from a raw MDIO device.
> >  
> >    reg:
> >      minimum: 0
> > diff --git a/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
> > new file mode 100644
> > index 000000000000..7282e93b1dd4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml
> > @@ -0,0 +1,238 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> 
> Not the right license.

What's wrong with it?

> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/fsl,backplane-anlt.yaml
> > +$schema: http://devicetree.org/meta-schemas/core.yaml
> > +
> > +title: Freescale Backplane Ethernet PHY
> > +
> > +maintainers:
> > +  - Vladimir Oltean <vladimir.oltean@nxp.com>
> > +
> > +description: |
> > +  Some QorIQ and Layerscape SoCs have an additional block on each SerDes
> > +  networking lane, based on an IP core from MoreThanIP, which performs IEEE
> > +  802.3 clause 73 base page exchanges (for auto-negotiation) and clause 72
> > +  training frame exchanges (for link training).
> > +
> > +  By default, this AN/LT block comes up with auto-negotiation disabled, and
> > +  in that case it allows itself to be quickly bypassed from the data path and
> > +  for the PCS link to come up without its involvement.
> > +
> > +  Software can optionally make use of it, to turn the PCS, AN/LT block and lane
> > +  (PMA/PMD) into a full copper backplane internal PHY.
> > +
> > +  As a warning, the binding for the multi-lane link modes (40GBase-KR4) is not
> > +  currently backed up by a driver implementation.
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          - items:
> > +              - const: fsl,lx2160a-backplane-anlt
> > +              - const: ethernet-phy-ieee802.3-c45
> 
> Use "contains" and drop ethernet-phy-ieee802.3-c45.

Thanks, I'll try it.

> > +    then:
> > +      $ref: ethernet-phy.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - const: fsl,lx2160a-backplane-anlt
> > +          - const: ethernet-phy-ieee802.3-c45
> > +      - const: fsl,lx2160a-secondary-anlt
> > +
> > +  reg:
> > +    minimum: 0
> > +    maximum: 31
> > +    description: |
> > +      The address of the AN/LT block within the internal MDIO bus of the MAC it
> > +      is attached to.
> > +
> > +      In the 1000Base-KX and 10GBase-KR link modes, the AN/LT block responds at
> > +      the same MDIO address as the PCS (determined by the SGMIInCR1[MDEV_PORT]
> > +      or SXGMIInCR1[MDEV_PORT] registers of the SerDes block, by default 0).
> > +      The PCS and AN/LT block respond to different MMDs, though.
> > +
> > +      In the 25GBase-KR and higher link modes, the AN/LT block responds at a
> > +      different MDIO address than the PCS, determined by the
> > +      ANLTnCR1[MDEV_PORT] register of the SerDes block. By default this is 4
> > +      for lanes A and E, 5 for lanes B and F, 6 for lanes C and G, 7 for lanes
> > +      D and H.
> > +
> > +      The PCS responds in all cases at the address determined by the MDEV_PORT
> > +      field of the SGMIInCR1, SXGMIIaCR1, E25GaCR1, E40GaCR1, E50GaCR1 or
> > +      E100GaCR1 registers of the SerDes block.
> > +
> > +  phys:
> > +    maxItems: 1
> > +    description:
> > +      phandle for the generic PHY (SerDes lane) that acts as PMA/PMD layer
> > +
> > +  pcs-handle:
> > +    maxItems: 1
> > +    description:
> > +      phandle for the technology-dependent PCS block corresponding to the
> > +      initial (RCW-based) configuration of the port. Must be omitted for the
> > +      link modes where the PCS and AN/LT block respond at the same MDIO
> > +      address. Must be specified otherwise.
> > +
> > +  secondary-anlt-handle:
> 
> Needs a vendor prefix and type.

Ok (assuming this will remain in the final solution, after PHY maintainers' review)

> > +    maxItems: 1
> > +    description:
> > +      In case this is the primary (first) lane of a multi-lane link mode, this
> > +      property holds an array of phandles for the other AN/LT blocks, that are
> > +      involved in link training but not in auto-negotiation. These have the
> > +      "fsl,lx2160a-secondary-anlt" compatible string.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - phys
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +
> > +  # LX2160A lanes A, B, C, D with SerDes 1 protocol 19: dpmac2 uses 40GBase-KR4
> > +  - |
> > +    dpmac2 {
> > +        phy-handle = <&mac2_backplane_anlt>;
> > +        phy-connection-type = "internal";
> > +    };
> > +
> > +    pcs_mdio2 {
> 
> mdio {

Hmm, is it bad if I can keep the name? (it's actually important for documentary purposes)
Actually I wanted to put a label like "pcs_mdio2: mdio {", but I couldn't,
and this is what led me to the compromise.

> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        status = "okay";
> 
> Don't need status in examples.

Ok.

> > +
> > +        pcs2: ethernet-phy@0 {
> > +            reg = <0>;
> > +        };
> > +
> > +        mac2_backplane_anlt: ethernet-phy@7 {
> > +            compatible = "fsl,lx2160a-backplane-anlt",
> > +                          "ethernet-phy-ieee802.3-c45";
> > +            reg = <7>; /* according to ANLTDCR1[MDEV_PORT] */
> > +            phys = <&serdes_1 3>; /* lane D */
> > +            max-speed = <40000>;
> > +            pcs-handle = <&pcs2>;
> > +            secondary-anlt-handle = <&mac2_lane2_anlt>, <&mac2_lane3_anlt>,
> > +                                    <&mac2_lane4_anlt>;
> > +        };
> > +
> > +        mac2_lane2_anlt: ethernet-backplane-anlt@6 {
> > +            compatible = "fsl,lx2160a-secondary-anlt";
> > +            reg = <6>; /* according to ANLTCCR1[MDEV_PORT] */
> > +            phys = <&serdes_1 2>; /* lane C */
> > +        };
> > +
> > +        mac2_lane3_anlt: ethernet-backplane-anlt@5 {
> > +            compatible = "fsl,lx2160a-secondary-anlt";
> > +            reg = <5>; /* according to ANLTBCR1[MDEV_PORT] */
> > +            phys = <&serdes_1 1>; /* lane B */
> > +        };
> > +
> > +        mac2_lane4_anlt: ethernet-backplane-anlt@4 {
> > +            compatible = "fsl,lx2160a-secondary-anlt";
> > +            reg = <4>; /* according to ANLTACR1[MDEV_PORT] */
> > +            phys = <&serdes_1 0>; /* lane A */
> > +        };
> > +    };
> > +
> > +  # LX2160A lane E with SerDes 1 protocol 19: dpmac6 uses 25GBase-KR
> > +  - |
> > +    dpmac6 {
> > +        phy-handle = <&mac6_backplane_anlt>;
> > +        phy-connection-type = "internal";
> > +    };
> > +
> > +    pcs_mdio6 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        status = "okay";
> > +
> > +        pcs6: ethernet-phy@0 {
> > +            reg = <0>;
> > +        };
> > +
> > +        mac6_backplane_anlt: ethernet-phy@4 {
> > +            compatible = "fsl,lx2160a-backplane-anlt",
> > +                          "ethernet-phy-ieee802.3-c45";
> > +            reg = <4>; /* according to ANLTFCR1[MDEV_PORT] */
> > +            phys = <&serdes_1 4>; /* lane E */
> > +            max-speed = <25000>;
> > +            pcs-handle = <&pcs6>;
> > +        };
> > +    };
> > +
> > +  # LX2160A lane F with SerDes 1 protocol 19: dpmac5 uses 25GBase-KR
> > +  - |
> > +    dpmac5 {
> > +        phy-handle = <&mac5_backplane_anlt>;
> > +        phy-connection-type = "internal";
> > +    };
> > +
> > +    pcs_mdio5 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        status = "okay";
> > +
> > +        pcs5: ethernet-phy@0 {
> > +            reg = <0>;
> > +        };
> > +
> > +        mac5_backplane_anlt: ethernet-phy@5 {
> > +            compatible = "fsl,lx2160a-backplane-anlt",
> > +                         "ethernet-phy-ieee802.3-c45";
> > +            reg = <5>; /* according to ANLTFCR1[MDEV_PORT] */
> > +            phys = <&serdes_1 5>; /* lane F */
> > +            max-speed = <25000>;
> > +            pcs-handle = <&pcs5>;
> > +        };
> > +    };
> > +
> > +  # LX2160A lane G with SerDes 1 protocol 19: dpmac4 uses 10GBase-KR
> > +  - |
> > +    dpmac4 {
> > +        phy-handle = <&mac4_backplane_anlt>;
> > +        phy-connection-type = "internal";
> > +    };
> > +
> > +    pcs_mdio4 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        status = "okay";
> > +
> > +        mac4_backplane_anlt: ethernet-phy@0 {
> > +            compatible = "fsl,lx2160a-backplane-anlt",
> > +                         "ethernet-phy-ieee802.3-c45";
> > +            reg = <0>; /* merged with PCS SXGMIIGCR1[MDEV_PORT] */
> > +            phys = <&serdes_1 7>; /* lane G */
> > +            max-speed = <10000>;
> > +            /* no pcs-handle to &pcs4 */
> > +        };
> > +    };
> > +
> > +  # LX2160A lane H with SerDes 1 protocol 19: dpmac3 uses 10GBase-KR
> > +  - |
> > +    dpmac3 {
> > +        phy-handle = <&mac3_backplane_anlt>;
> > +        phy-connection-type = "internal";
> > +    };
> > +
> > +    pcs_mdio3 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        status = "okay";
> > +
> > +        mac3_backplane_anlt: ethernet-phy@0 {
> > +            compatible = "fsl,lx2160a-backplane-anlt",
> > +                         "ethernet-phy-ieee802.3-c45";
> > +            reg = <0>; /* merged with PCS SXGMIIHCR1[MDEV_PORT] */
> > +            phys = <&serdes_1 7>; /* lane H */
> > +            max-speed = <10000>;
> > +            /* no pcs-handle to &pcs3 */
> > +        };
> > +    };
> 
> 5 examples yet not one for "fsl,lx2160a-secondary-anlt"

Are you sure? They're in the first one:
"LX2160A lanes A, B, C, D with SerDes 1 protocol 19: dpmac2 uses 40GBase-KR4"
(and still not trimmed from this email)

Re: [RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Andrew Lunn]

> > >        - items:
> > >            - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$"
> > >            - const: ethernet-phy-ieee802.3-c45
> > > +      - items:
> > > +          - const: fsl,lx2160a-backplane-anlt
> > > +          - const: ethernet-phy-ieee802.3-c45
> > 
> > What's the benefit of having ethernet-phy-ieee802.3-c45? Will it work if 
> > the OS only understands that and not fsl,lx2160a-backplane-anlt?
> 
> No. The "is_c45" bool won't get set correctly in fwnode_mdiobus_register_phy().
> 
> 	is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");
> 
> With that bool set incorrectly, the MDIO protocol cannot access the device's
> registers.
> 
> > > +        description:
> > > +          Some C45 PHYs have no PHY ID in the standard location, and there is
> > > +          also no PHY ID allocated for them to fake. They are identified by the
> > > +          primary compatible string, plus the secondary one to distinguish them
> > > +          from a raw MDIO device.

Could you fake ID registers? Is this on any arbitrary MDIO bus, or an
internal bus with its own MDIO driver which could trap reads to the ID
registers and return well known values?

	Andrew

Re: [RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Vladimir Oltean]

On Mon, Aug 21, 2023 at 10:20:43PM +0200, Andrew Lunn wrote:
> > > >        - items:
> > > >            - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$"
> > > >            - const: ethernet-phy-ieee802.3-c45
> > > > +      - items:
> > > > +          - const: fsl,lx2160a-backplane-anlt
> > > > +          - const: ethernet-phy-ieee802.3-c45
> > > 
> > > What's the benefit of having ethernet-phy-ieee802.3-c45? Will it work if 
> > > the OS only understands that and not fsl,lx2160a-backplane-anlt?
> > 
> > No. The "is_c45" bool won't get set correctly in fwnode_mdiobus_register_phy().
> > 
> > 	is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");
> > 
> > With that bool set incorrectly, the MDIO protocol cannot access the device's
> > registers.
> > 
> > > > +        description:
> > > > +          Some C45 PHYs have no PHY ID in the standard location, and there is
> > > > +          also no PHY ID allocated for them to fake. They are identified by the
> > > > +          primary compatible string, plus the secondary one to distinguish them
> > > > +          from a raw MDIO device.
> 
> Could you fake ID registers? Is this on any arbitrary MDIO bus, or an
> internal bus with its own MDIO driver which could trap reads to the ID
> registers and return well known values?
> 
> 	Andrew

The MDIO bus is not arbitrary, the integration choice with this register
layout is specific to the LX2160A SoC, and it's an internal PHY there.

But, there's already something else at those MDIO registers (where the
standard PHY ID location is), in the MMD that the AN/LT block responds to.
And that would be:

/* 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, // overlaps with MII_PHYSID1
	[AN_ADV_1] = 3, // overlaps with MII_PHYSID2
	[AN_ADV_2] = 4,
	[AN_LPA_0] = 5, // overlaps with MDIO_DEVS1
	[AN_LPA_1] = 6, // overlaps with MDIO_DEVS2
	[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,
};

The AN advertisement registers are kinda important to the operation of
the driver, so I wouldn't want to mask them with fake PHY ID values
reported by the MDIO controller.

The other option would be to somehow make the mtip_backplane driver
remap (and thus, standardize) its own register space as phy_read_mmd()
and phy_write_mmd() see it, but it's not clear at all how that would
be done, or if it was done before / would be useful generally.

Re: [RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Andrew Lunn]

> But, there's already something else at those MDIO registers (where the
> standard PHY ID location is), in the MMD that the AN/LT block responds to.
> And that would be:
> 
> /* 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, // overlaps with MII_PHYSID1
> 	[AN_ADV_1] = 3, // overlaps with MII_PHYSID2
> 	[AN_ADV_2] = 4,
> 	[AN_LPA_0] = 5, // overlaps with MDIO_DEVS1
> 	[AN_LPA_1] = 6, // overlaps with MDIO_DEVS2
> 	[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,
> };
> 
> The AN advertisement registers are kinda important to the operation of
> the driver, so I wouldn't want to mask them with fake PHY ID values
> reported by the MDIO controller.

O.K, not ideal. For C22, you could just put the ID values in the
compatible, which is enough to get a driver loaded which supports that
ID. But somebody recently commented that that does not work for C45. I
assume NXP has an OUI, and could allocate an ID to this device in
retrospect. So maybe it makes sense to make C45 work with an ID in the
compatible? And get the driver loaded that way?

	Andrew

Re: [RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Vladimir Oltean]

On Mon, Aug 21, 2023 at 11:10:27PM +0200, Andrew Lunn wrote:
> > But, there's already something else at those MDIO registers (where the
> > standard PHY ID location is), in the MMD that the AN/LT block responds to.
> > And that would be:
> > 
> > /* 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, // overlaps with MII_PHYSID1
> > 	[AN_ADV_1] = 3, // overlaps with MII_PHYSID2
> > 	[AN_ADV_2] = 4,
> > 	[AN_LPA_0] = 5, // overlaps with MDIO_DEVS1
> > 	[AN_LPA_1] = 6, // overlaps with MDIO_DEVS2
> > 	[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,
> > };
> > 
> > The AN advertisement registers are kinda important to the operation of
> > the driver, so I wouldn't want to mask them with fake PHY ID values
> > reported by the MDIO controller.
> 
> O.K, not ideal. For C22, you could just put the ID values in the
> compatible, which is enough to get a driver loaded which supports that
> ID. But somebody recently commented that that does not work for C45. I
> assume NXP has an OUI, and could allocate an ID to this device in
> retrospect. So maybe it makes sense to make C45 work with an ID in the
> compatible? And get the driver loaded that way?
> 
> 	Andrew

There are 2 clarification questions that I can think of right now.
Maybe more later.

First: Compatible strings per C45 MMD? Drivers per C45 MMD? Is there
supposed to be an interest in that? I might end up needing it (see the
problem description in the cover letter, with PCS and AN/LT block merged
into the same MDIO address, but responding to separate MMDs)

Second: Suppose I add something like "ethernet-phy-ieee802.3-c45-idXXXX.XXXX".
Do I replace just this with that:

compatible = "fsl,lx2160a-backplane-anlt", "ethernet-phy-ieee802.3-c45";

or also this?

compatible = "fsl,lx2160a-secondary-anlt";


I suppose it would be just the first one, but going that route would IMO
just increase the dissonance between the description of primary and
secondary AN/LT blocks. They're the same IP blocks, don't they also have
the same fake PHY ID?

Re: [RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Andrew Lunn]

> > O.K, not ideal. For C22, you could just put the ID values in the
> > compatible, which is enough to get a driver loaded which supports that
> > ID. But somebody recently commented that that does not work for C45. I
> > assume NXP has an OUI, and could allocate an ID to this device in
> > retrospect. So maybe it makes sense to make C45 work with an ID in the
> > compatible? And get the driver loaded that way?
> > 
> > 	Andrew
> 
> There are 2 clarification questions that I can think of right now.
> Maybe more later.
> 
> First: Compatible strings per C45 MMD? Drivers per C45 MMD? Is there
> supposed to be an interest in that? I might end up needing it (see the
> problem description in the cover letter, with PCS and AN/LT block merged
> into the same MDIO address, but responding to separate MMDs)
> 
> Second: Suppose I add something like "ethernet-phy-ieee802.3-c45-idXXXX.XXXX".
> Do I replace just this with that:
> 
> compatible = "fsl,lx2160a-backplane-anlt", "ethernet-phy-ieee802.3-c45";
> 
> or also this?
> 
> compatible = "fsl,lx2160a-secondary-anlt";
> 
> 
> I suppose it would be just the first one, but going that route would IMO
> just increase the dissonance between the description of primary and
> secondary AN/LT blocks. They're the same IP blocks, don't they also have
> the same fake PHY ID?

For C22 PHYs, the ID registers are only used to ask user space to load
a driver which supports that ID, and then used to match a device to a
driver. We often say that if the ID registers don't actually contain
an ID, or the wrong ID, use ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$
to let the subsystem know the correct ID.

The device you are trying to support has the same problem, invalid
IDs, but its C45.

C45 IDs however work slightly differently. An C45 package can have
multiple devices in it, up to 32. Each device has its own ID
registers. So there can be up to 32 different IDs for one package. The
core will try to determine which of the 32 devices are actually in the
package, and if they are, what the ID is. It then asks user space to
load a driver for all the IDs it finds. And when matching devices to
drivers, it sees if any of the ID of the package matches the IDs the
driver says it supports. If a match is found, that one driver is
expected to drive all the devices in that one package.

I don't see a need for ethernet-phy-ieee802.3-c45-idXXXX.XXXX,
ethernet-phy-ieee802.3-idXXXX.XXXX should be sufficient, since all you
are doing it matching the ID against the driver. That matching does
not differ between C22 and C45. 

Saying "ethernet-phy-ieee802.3-c45" might be useful, because at the
moment we have a mixup between C45 register space and C45 bus
transactions. The drive is free to access C22 and/or C45 registers,
since it should know what the device actually has. But some of the
core might get the wrong idea without "ethernet-phy-ieee802.3-c45".

O.K, that the background now:

> First: Compatible strings per C45 MMD? Drivers per C45 MMD

So far, nobody has needed that. All current drivers are package
drivers, they drive all the devices in the package. At least for a
PHY, there is close integration between devices in a package. Russell
has commented that the Marvell 10G PHY does appear to contain a number
of licensed devices, but so far, we have not noticed the same licensed
device used by multiple vendors. So there has not been any need to
reuse code.

However, it sounds like the package you are trying to support does
contain multiple independent devices. So from an architecture
perspective, having multiple drivers would make sense, even if there
is no reuse. But are the devices PHY? Everything i've said so far
applies to PHYs. It does not apply to a PCS, etc.

	Andrew

Re: [RFC PATCH net-next 8/8] dt-bindings: net: fsl,backplane-anlt: new binding document

[Vladimir Oltean]

Hi Andrew and phylib/phylink maintainers in general,

On Tue, Aug 22, 2023 at 04:10:45PM +0200, Andrew Lunn wrote:
> For C22 PHYs, the ID registers are only used to ask user space to load
> a driver which supports that ID, and then used to match a device to a
> driver. We often say that if the ID registers don't actually contain
> an ID, or the wrong ID, use ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$
> to let the subsystem know the correct ID.
> 
> The device you are trying to support has the same problem, invalid
> IDs, but its C45.
> 
> C45 IDs however work slightly differently. An C45 package can have
> multiple devices in it, up to 32. Each device has its own ID
> registers. So there can be up to 32 different IDs for one package. The
> core will try to determine which of the 32 devices are actually in the
> package, and if they are, what the ID is. It then asks user space to
> load a driver for all the IDs it finds. And when matching devices to
> drivers, it sees if any of the ID of the package matches the IDs the
> driver says it supports. If a match is found, that one driver is
> expected to drive all the devices in that one package.
> 
> I don't see a need for ethernet-phy-ieee802.3-c45-idXXXX.XXXX,
> ethernet-phy-ieee802.3-idXXXX.XXXX should be sufficient, since all you
> are doing it matching the ID against the driver. That matching does
> not differ between C22 and C45. 
> 
> Saying "ethernet-phy-ieee802.3-c45" might be useful, because at the
> moment we have a mixup between C45 register space and C45 bus
> transactions. The drive is free to access C22 and/or C45 registers,
> since it should know what the device actually has. But some of the
> core might get the wrong idea without "ethernet-phy-ieee802.3-c45".
> 
> O.K, that the background now:
> 
> > First: Compatible strings per C45 MMD? Drivers per C45 MMD
> 
> So far, nobody has needed that. All current drivers are package
> drivers, they drive all the devices in the package. At least for a
> PHY, there is close integration between devices in a package. Russell
> has commented that the Marvell 10G PHY does appear to contain a number
> of licensed devices, but so far, we have not noticed the same licensed
> device used by multiple vendors. So there has not been any need to
> reuse code.
> 
> However, it sounds like the package you are trying to support does
> contain multiple independent devices. So from an architecture
> perspective, having multiple drivers would make sense, even if there
> is no reuse. But are the devices PHY? Everything i've said so far
> applies to PHYs. It does not apply to a PCS, etc.
> 
> 	Andrew

I don't know if the devices qualify as phy_device structures according
to the opinion of the maintainers, and that is one of the fundamental
aspects I would like this RFC to clarify, before I proceed to request
NXP to allocate a new PHY ID that I can put in the compatible string.

If the backplane AN/LT block is not a phy_device structure, my
imagination will need a bit of help on how to integrate it, as a raw
mdio_device, with phylink or with the consumer MAC drivers directly.