linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Calvin Johnson <calvin.johnson@oss.nxp.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Grant Likely <grant.likely@arm.com>,
	Jeremy Linton <jeremy.linton@arm.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Russell King - ARM Linux admin <linux@armlinux.org.uk>,
	Cristi Sovaiala <cristian.sovaiala@nxp.com>,
	Florin Laurentiu Chiculita <florinlaurentiu.chiculita@nxp.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	Madalin Bucur <madalin.bucur@oss.nxp.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux.cj@gmail.com, netdev <netdev@vger.kernel.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Diana Madalina Craciun <diana.craciun@nxp.com>,
	Laurentiu Tudor <laurentiu.tudor@nxp.com>,
	Len Brown <lenb@kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: Re: [net-next PATCH v1 1/7] Documentation: ACPI: DSD: Document MDIO PHY
Date: Thu, 1 Oct 2020 18:56:49 +0530	[thread overview]
Message-ID: <20201001132649.GD9110@lsv03152.swis.in-blr01.nxp.com> (raw)
In-Reply-To: <CAJZ5v0jP8L=bBMYTUkYCSwN=fy8dwTdjqu1JurSxTa2bAHRLew@mail.gmail.com>

Hi Rafael,

On Wed, Sep 30, 2020 at 06:37:09PM +0200, Rafael J. Wysocki wrote:
> On Wed, Sep 30, 2020 at 6:05 PM Calvin Johnson
> <calvin.johnson@oss.nxp.com> wrote:
> >
> > Introduce ACPI mechanism to get PHYs registered on a MDIO bus and
> > provide them to be connected to MAC.
> >
> > Describe properties "phy-handle" and "phy-mode".
> >
> > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
> > ---
> >
> >  Documentation/firmware-guide/acpi/dsd/phy.rst | 78 +++++++++++++++++++
> >  1 file changed, 78 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..f10feb24ec1c
> > --- /dev/null
> > +++ b/Documentation/firmware-guide/acpi/dsd/phy.rst
> > @@ -0,0 +1,78 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +=========================
> > +MDIO bus and PHYs in ACPI
> > +=========================
> > +
> > +The PHYs on an mdiobus are probed and registered using
> > +fwnode_mdiobus_register_phy().
> > +Later, for connecting these PHYs to MAC, the PHYs registered on the
> > +mdiobus have to be referenced.
> > +
> > +phy-handle
> > +-----------
> > +For each MAC node, a property "phy-handle" is used to reference the
> > +PHY that is registered on an MDIO bus.
> 
> It is not clear what "a property" means in this context.
> 
In rev-2, I'll add more info on this.
During the MDIO bus driver initialization, PHYs on this bus are probed
using the _ADR object as shown below and are registered on the mdio bus.

      Scope(\_SB.MDI0)
      {
        Device(PHY1) {
          Name (_ADR, 0x1)
        } // end of PHY1

        Device(PHY2) {
          Name (_ADR, 0x2)
        } // end of PHY2
      }

Later, during the MAC driver initialization, the registered PHY devices
have to be retrieved from the mdio bus. For this, MAC driver needs reference
to the previously registered PHYs which are provided using reference to the
device as {\_SB.MDI0.PHY1}.

> This should refer to the documents introducing the _DSD-based generic
> device properties rules, including the GUID used below.
> 
Sure. I'll refer "Documentation/firmware-guide/acpi/DSD-properties-rules.rst"

> You need to say whether or not the property is mandatory and if it
> isn't mandatory, you need to say what the lack of it means.
> 
I'll do that.

> > +
> > +phy-mode
> > +--------
> > +Property "phy-mode" defines the type of PHY interface.
> 
> This needs to be more detailed too, IMO.  At the very least, please
> list all of the possible values of it and document their meaning.
> 
> > +
> > +An example of this is shown below::
> > +
> > +DSDT entry for MACs where PHY nodes are referenced
> > +--------------------------------------------------
> > +       Scope(\_SB.MCE0.PR17) // 1G
> > +       {
> > +         Name (_DSD, Package () {
> > +            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +                Package () {
> > +                    Package (2) {"phy-mode", "rgmii-id"},
> > +                    Package (2) {"phy-handle", Package (){\_SB.MDI0.PHY1}}
> 
> What is "phy-handle"?
> 
> You haven't introduced it above.
I thought I introduced it earlier in this document as a property. Ofcourse,
more info needs to be added as you mentioned. Other than that am I missing
something?

I've a correction here.
Based on referring some more documents, I'll be using
	Package (2) {"phy-handle",\_SB.MDI0.PHY1}
instead of
        Package (2) {"phy-handle", Package (){\_SB.MDI0.PHY1}}
.

> > +             }
> > +          })
> > +       }
> > +
> > +       Scope(\_SB.MCE0.PR18) // 1G
> > +       {
> > +         Name (_DSD, Package () {
> > +           ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +               Package () {
> > +                   Package (2) {"phy-mode", "rgmii-id"},
> > +                   Package (2) {"phy-handle", Package (){\_SB.MDI0.PHY2}}
> > +           }
> > +         })
> > +       }
> > +
> > +DSDT entry for MDIO node
> > +------------------------
> > +a) Silicon Component
> 
> What is this device, exactly?

I'll explain it more clearly.

> 
> > +--------------------
> > +       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
> > +         } // 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
> > +       }
> > --
> 
> What is the connection between the last two pieces of ASL and the _DSD
> definitions above?

In rev-2, I'll explain the relation between these pieces. What I tried to show
is that the MDIO bus has an SoC component(mdio controller) and a platform
component(PHYs on the mdiobus).

In the MAC nodes, the PHYs are referenced and that is done using the "phy-handle"
property.

Thanks
Calvin

  reply	other threads:[~2020-10-01 13:27 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-30 16:04 [net-next PATCH v1 0/7] ACPI support for dpaa2 driver Calvin Johnson
2020-09-30 16:04 ` [net-next PATCH v1 1/7] Documentation: ACPI: DSD: Document MDIO PHY Calvin Johnson
2020-09-30 16:37   ` Rafael J. Wysocki
2020-10-01 13:26     ` Calvin Johnson [this message]
2020-10-02 11:08     ` Grant Likely
2020-10-02 14:13       ` Rafael J. Wysocki
2020-09-30 16:04 ` [net-next PATCH v1 2/7] net: phy: Introduce phy related fwnode functions Calvin Johnson
2020-09-30 22:03   ` David Miller
2020-10-01  3:58     ` Calvin Johnson
2020-09-30 16:04 ` [net-next PATCH v1 3/7] net: phy: Introduce fwnode_get_phy_id() Calvin Johnson
2020-09-30 16:34   ` Andrew Lunn
2020-09-30 18:07     ` Russell King - ARM Linux admin
2020-09-30 18:19       ` Andrew Lunn
2020-10-01  4:00         ` Calvin Johnson
2020-10-02 10:48           ` Grant Likely
2020-10-02 11:05   ` Grant Likely
2020-10-02 15:14     ` Florian Fainelli
2020-10-02 15:50       ` Russell King - ARM Linux admin
2020-10-03 18:03         ` Calvin Johnson
2020-10-03 18:00     ` Calvin Johnson
2020-09-30 16:04 ` [net-next PATCH v1 4/7] net: mdiobus: Introduce fwnode_mdiobus_register_phy() Calvin Johnson
2020-09-30 22:04   ` David Miller
2020-09-30 16:04 ` [net-next PATCH v1 5/7] phylink: introduce phylink_fwnode_phy_connect() Calvin Johnson
2020-09-30 16:04 ` [net-next PATCH v1 6/7] net: dpaa2-mac: Add ACPI support for DPAA2 MAC driver Calvin Johnson
2020-10-01 15:36   ` Andy Shevchenko
2020-10-03 16:30     ` Calvin Johnson
2020-10-02 11:22   ` Grant Likely
2020-10-03 17:39     ` Calvin Johnson
2020-10-07 15:50       ` Calvin Johnson
2020-09-30 16:04 ` [net-next PATCH v1 7/7] net/fsl: Use _ADR ACPI object to register PHYs Calvin Johnson
2020-09-30 16:27   ` Andrew Lunn
2020-09-30 22:04   ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201001132649.GD9110@lsv03152.swis.in-blr01.nxp.com \
    --to=calvin.johnson@oss.nxp.com \
    --cc=andrew@lunn.ch \
    --cc=andy.shevchenko@gmail.com \
    --cc=cristian.sovaiala@nxp.com \
    --cc=diana.craciun@nxp.com \
    --cc=f.fainelli@gmail.com \
    --cc=florinlaurentiu.chiculita@nxp.com \
    --cc=grant.likely@arm.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=jeremy.linton@arm.com \
    --cc=laurentiu.tudor@nxp.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux.cj@gmail.com \
    --cc=linux@armlinux.org.uk \
    --cc=madalin.bucur@oss.nxp.com \
    --cc=netdev@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).