2021-06-21 17:32:01

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v3 1/6] Documentation: ACPI: DSD: describe additional MAC configuration

Document additional MAC configuration modes which can be processed
by the existing fwnode_ phylink helpers:

* "managed" standard ACPI _DSD property [1]
* "fixed-link" data-only subnode linked in the _DSD package via
generic mechanism of the hierarchical data extension [2]

[1] https://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
[2] https://github.com/UEFI/DSD-Guide/blob/main/dsd-guide.pdf

Signed-off-by: Marcin Wojtas <[email protected]>
---
Documentation/firmware-guide/acpi/dsd/phy.rst | 59 ++++++++++++++++++++
1 file changed, 59 insertions(+)

diff --git a/Documentation/firmware-guide/acpi/dsd/phy.rst b/Documentation/firmware-guide/acpi/dsd/phy.rst
index 0d49bad2ea9c..680ad179e5f9 100644
--- a/Documentation/firmware-guide/acpi/dsd/phy.rst
+++ b/Documentation/firmware-guide/acpi/dsd/phy.rst
@@ -50,6 +50,21 @@ phy-mode
The "phy-mode" _DSD property is used to describe the connection to
the PHY. The valid values for "phy-mode" are defined in [4].

+managed
+-------
+Optional property, which specifies the PHY management type.
+The valid values for "managed" are defined in [4].
+
+fixed-link
+----------
+The "fixed-link" is described by a data-only subnode of the
+MAC port, which is linked in the _DSD package via
+hierarchical data extension (UUID dbb8e3e6-5886-4ba6-8795-1319f52a966b
+in accordance with [5] "_DSD Implementation Guide" document).
+The subnode should comprise a required property ("speed") and
+possibly the optional ones - complete list of parameters and
+their values are specified in [4].
+
The following ASL example illustrates the usage of these properties.

DSDT entry for MDIO node
@@ -128,6 +143,48 @@ phy-mode and phy-handle are used as explained earlier.
})
}

+MAC node example where "managed" property is specified.
+-------------------------------------------------------
+
+.. code-block:: none
+
+ Scope(\_SB.PP21.ETH0)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"phy-mode", "sgmii"},
+ Package () {"managed", "in-band-status"}
+ }
+ })
+ }
+
+MAC node example with a "fixed-link" subnode.
+---------------------------------------------
+
+.. code-block:: none
+
+ Scope(\_SB.PP21.ETH1)
+ {
+ Name (_DSD, Package () {
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"phy-mode", "sgmii"},
+ },
+ ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+ Package () {
+ Package () {"fixed-link", "LNK0"}
+ }
+ })
+ Name (LNK0, Package(){ // Data-only subnode of port
+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ Package () {
+ Package () {"speed", 1000},
+ Package () {"full-duplex", 1}
+ }
+ })
+ }
+
References
==========

@@ -138,3 +195,5 @@ References
[3] Documentation/firmware-guide/acpi/DSD-properties-rules.rst

[4] Documentation/devicetree/bindings/net/ethernet-controller.yaml
+
+[5] https://github.com/UEFI/DSD-Guide/blob/main/dsd-guide.pdf
--
2.29.0


2021-06-23 20:20:40

by Andrew Lunn

[permalink] [raw]
Subject: Re: [net-next: PATCH v3 1/6] Documentation: ACPI: DSD: describe additional MAC configuration

> +MAC node example with a "fixed-link" subnode.
> +---------------------------------------------
> +
> +.. code-block:: none
> +
> + Scope(\_SB.PP21.ETH1)
> + {
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package () {"phy-mode", "sgmii"},
> + },
> + ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
> + Package () {
> + Package () {"fixed-link", "LNK0"}
> + }
> + })

At least in the DT world, it is pretty unusual to see both fixed-link
and phy-mode. You might have one of the four RGMII modes, in order to
set the delays when connecting to a switch. But sgmii and fixed link
seems very unlikely, how is sgmii autoneg going to work?

> + Name (LNK0, Package(){ // Data-only subnode of port
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package () {"speed", 1000},
> + Package () {"full-duplex", 1}
> + }
> + })
> + }
> +

Andrew

2021-06-23 21:02:59

by Marcin Wojtas

[permalink] [raw]
Subject: Re: [net-next: PATCH v3 1/6] Documentation: ACPI: DSD: describe additional MAC configuration

Hi Andrew,

śr., 23 cze 2021 o 22:18 Andrew Lunn <[email protected]> napisał(a):
>
> > +MAC node example with a "fixed-link" subnode.
> > +---------------------------------------------
> > +
> > +.. code-block:: none
> > +
> > + Scope(\_SB.PP21.ETH1)
> > + {
> > + Name (_DSD, Package () {
> > + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > + Package () {
> > + Package () {"phy-mode", "sgmii"},
> > + },
> > + ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
> > + Package () {
> > + Package () {"fixed-link", "LNK0"}
> > + }
> > + })
>
> At least in the DT world, it is pretty unusual to see both fixed-link
> and phy-mode.

I did a quick experiment:
git grep -C 8 fixed-link arch/arm64/boot/dts/
git grep -C 8 fixed-link arch/arm/boot/dts/
almost all MAC nodes (i.e. not switch ports) containing 'fixed-link'
have an adjacent 'phy-mode' property defined. How else would the
drivers know how to configure the HW connection type in its registers?

> You might have one of the four RGMII modes, in order to
> set the delays when connecting to a switch. But sgmii and fixed link
> seems very unlikely, how is sgmii autoneg going to work?
>

Indeed most cases in the tree are "rgmii*", but we can also see e.g.
10gbase-r, sgmii and 2500base-x. You can find sgmii + fixed-link on
the eth1 of the armada-388-clearfog. Regarding the autoneg - I'm
mostly familiar with the mvneta/mvpp2, but in this mode the
autonegotiation is disabled and the link is forcibly set up/down in
MAC registers during the netedev_open/close accordingly. FYI, along
with the 10G ports on CN913x-DB, I tested fixed-link on Macchiatobin
sgmii port.

Anyway - all above is a bit side discussion to the actual DSDT
description and how the fixed-link subnode looks like. I think
phy-mode set to "sgmii" is not incorrect, but we can change it to
whatever other type of your preference, as well.

Best regards,
Marcin

> > + Name (LNK0, Package(){ // Data-only subnode of port
> > + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > + Package () {
> > + Package () {"speed", 1000},
> > + Package () {"full-duplex", 1}
> > + }
> > + })
> > + }
> > +
>
> Andrew

2021-06-23 21:37:31

by Andrew Lunn

[permalink] [raw]
Subject: Re: [net-next: PATCH v3 1/6] Documentation: ACPI: DSD: describe additional MAC configuration

> Anyway - all above is a bit side discussion to the actual DSDT
> description and how the fixed-link subnode looks like. I think
> phy-mode set to "sgmii" is not incorrect, but we can change it to
> whatever other type of your preference, as well.

rgmii would be better, so we side step the whole auto-neg discussion.

Andrew

2021-06-23 21:43:13

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [net-next: PATCH v3 1/6] Documentation: ACPI: DSD: describe additional MAC configuration

Hi Andrew,

On Wed, Jun 23, 2021 at 10:18:02PM +0200, Andrew Lunn wrote:
> > +MAC node example with a "fixed-link" subnode.
> > +---------------------------------------------
> > +
> > +.. code-block:: none
> > +
> > + Scope(\_SB.PP21.ETH1)
> > + {
> > + Name (_DSD, Package () {
> > + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > + Package () {
> > + Package () {"phy-mode", "sgmii"},
> > + },
> > + ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
> > + Package () {
> > + Package () {"fixed-link", "LNK0"}
> > + }
> > + })
>
> At least in the DT world, it is pretty unusual to see both fixed-link
> and phy-mode. You might have one of the four RGMII modes, in order to
> set the delays when connecting to a switch. But sgmii and fixed link
> seems very unlikely, how is sgmii autoneg going to work?

SGMII autoneg is supposed to be disabled if you have a fixed-link, and
there is nothing unusual in that kind of setup.
There are 3 types of phylink setups:

MLO_AN_INBAND: there might or might not be a phy-handle, but SGMII
autoneg should be enabled and the MAC should be
reconfigured automatically (in hardware) to the right
speed/duplex based on that
MLO_AN_PHY: there is a phy-handle but SGMII autoneg should be disabled*
and the MAC should be reconfigured (forced) in software to
the speed/duplex determined by reading the PHY MDIO
registers
MLO_AN_FIXED: there is no phy-handle or phy_device, but the driver
should do the same thing, the speed/duplex is configured
by management (in this case DT/ACPI)

*there appears to be some debate here, since the "managed" property is
phylink-specific and therefore a phylib driver will not necessarily
disable its in-band autoneg, but this is what the existing phylink_pcs
drivers in drivers/net/pcs/ do and I think there's nothing wrong with
settling on that if phylink is being used. It does create some interesting
questions though when a driver is being converted from phylib to phylink,
since the meaning of the existing firmware bindings suddenly changes.

An SGMII link with MLO_AN_FIXED is nothing unusual, it is in fact very
widespread as a way to reduce pin count compared to the parallel RGMII.
I suspect there are more DSA setups in the field with an SGMII fixed-link
than with RGMII fixed-link, due to practicality.

The 2 characteristic features of SGMII compared to 1000base-X are:
- customization of the 16-bit configuration word communicated via the
clause 37 state machines. Those are bypassed in MLO_AN_PHY and
MLO_AN_FIXED modes, true
- symbol replication at 10/100 speeds.

So since it is equally valid to have an SGMII fixed-link at 100Mbps or
10Mbps, it is just as valid to have an SGMII fixed-link at 1Gbps with
in-band autoneg disabled.