Re: [net-next PATCH v7 1/6] Documentation: ACPI: DSD: Document MDIO PHY

[Jeremy Linton <jeremy.linton@arm.com>]

Hi,

On 7/15/20 4:03 AM, Calvin Johnson wrote:
> Introduce ACPI mechanism to get PHYs registered on a MDIO bus and
> provide them to be connected to MAC.
> 
> An ACPI node property "mdio-handle" is introduced to reference the
> MDIO bus on which PHYs are registered with autoprobing method used
> by mdiobus_register().
> 
> Describe properties "phy-channel" and "phy-mode"
> 
> Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
> 
> ---
> 
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3:
> - cleanup based on v2 comments
> - Added description for more properties
> - Added MDIO node DSDT entry
> 
> Changes in v2: None
> 
>   Documentation/firmware-guide/acpi/dsd/phy.rst | 90 +++++++++++++++++++
>   1 file changed, 90 insertions(+)
>   create mode 100644 Documentation/firmware-guide/acpi/dsd/phy.rst
> 
> diff --git a/Documentation/firmware-guide/acpi/dsd/phy.rst b/Documentation/firmware-guide/acpi/dsd/phy.rst
> new file mode 100644
> index 000000000000..0132fee10b45
> --- /dev/null
> +++ b/Documentation/firmware-guide/acpi/dsd/phy.rst
> @@ -0,0 +1,90 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=========================
> +MDIO bus and PHYs in ACPI
> +=========================
> +
> +The PHYs on an mdiobus are probed and registered using mdiobus_register().
> +Later, for connecting these PHYs to MAC, the PHYs registered on the
> +mdiobus have to be referenced.

First, this is all perfectly compatible with my literal interpretation 
and understanding of the ACPI spec. The use of _DSD is there to provide 
a way to "extend" if you will the specification for device specific edge 
cases that aren't directly covered by the spec.

OTOH, it may be my lack of knowledge here, but IMHO this is a bit of a 
difficult pill for all arm/sbsa systems though. Primary because I don't 
see how one is expected to use the generic ACPI power states on the 
parent device here. I also have some questions about how one might 
import such a device into a VM. Further AFAIK arm's current 
recommendations for SBSA/ACPI systems point in the direction of RCiEP's.

IMHO what should be clarified in this document is something to the 
effect that the "mdio-handle" is used for systems which have multiple 
nic/mac's sharing a single MDIO bus. Otherwise the MDIO bus and its phy 
should be a child of the nic/mac using it, with standardized 
behaviors/etc left up to the OSPM when it comes to MDIO bus 
enumeration/etc.

Thanks,


> +
> +mdio-handle
> +-----------
> +For each MAC node, a property "mdio-handle" is used to reference the
> +MDIO bus on which the PHYs are registered. On getting hold of the MDIO
> +bus, use find_phy_device() to get the PHY connected to the MAC.
> +
> +phy-channel
> +-----------
> +Property "phy-channel" defines the address of the PHY on the mdiobus.
> +
> +phy-mode
> +--------
> +Property "phy-mode" defines the type of PHY interface.
> +
> +An example of this is shown below::
> +
> +DSDT entry for MAC where MDIO node is referenced
> +------------------------------------------------
> +	Scope(\_SB.MCE0.PR17) // 1G
> +	{
> +	  Name (_DSD, Package () {
> +	     ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +		 Package () {
> +		     Package () {"phy-channel", 1},
> +		     Package () {"phy-mode", "rgmii-id"},
> +		     Package () {"mdio-handle", Package (){\_SB.MDI0}}
> +	      }
> +	   })
> +	}
> +
> +	Scope(\_SB.MCE0.PR18) // 1G
> +	{
> +	  Name (_DSD, Package () {
> +	    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +		Package () {
> +		    Package () {"phy-channel", 2},
> +		    Package () {"phy-mode", "rgmii-id"},
> +		    Package () {"mdio-handle", Package (){\_SB.MDI0}}
> +	    }
> +	  })
> +	}
> +
> +DSDT entry for MDIO node
> +------------------------
> +a) Silicon Component
> +--------------------
> +	Scope(_SB)
> +	{
> +	  Device(MDI0) {
> +	    Name(_HID, "NXP0006")
> +	    Name(_CCA, 1)
> +	    Name(_UID, 0)
> +	    Name(_CRS, ResourceTemplate() {
> +	      Memory32Fixed(ReadWrite, MDI0_BASE, MDI_LEN)
> +	      Interrupt(ResourceConsumer, Level, ActiveHigh, Shared)
> +	       {
> +		 MDI0_IT
> +	       }
> +	    }) // end of _CRS for MDI0
> +	    Name (_DSD, Package () {
> +	      ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +	      Package () {
> +		 Package () {"little-endian", 1},
> +	      }
> +	    })
> +	  } // end of MDI0
> +	}
> +
> +b) Platform Component
> +---------------------
> +	Scope(\_SB.MDI0)
> +	{
> +	  Device(PHY1) {
> +	    Name (_ADR, 0x1)
> +	  } // end of PHY1
> +
> +	  Device(PHY2) {
> +	    Name (_ADR, 0x2)
> +	  } // end of PHY2
> +	}
>