linux-kernel - Re: [net-next PATCH v1 1/7] Documentation: ACPI: DSD: Document MDIO PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 2 Oct 2020 16:13:54 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Grant Likely <grant.likely@....com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Calvin Johnson <calvin.johnson@....nxp.com>,
        Jeremy Linton <jeremy.linton@....com>,
        Andrew Lunn <andrew@...n.ch>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Cristi Sovaiala <cristian.sovaiala@....com>,
        Florin Laurentiu Chiculita <florinlaurentiu.chiculita@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Madalin Bucur <madalin.bucur@....nxp.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux.cj@...il.com, netdev <netdev@...r.kernel.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Diana Madalina Craciun <diana.craciun@....com>,
        Laurentiu Tudor <laurentiu.tudor@....com>,
        Len Brown <lenb@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>, nd <nd@....com>
Subject: Re: [net-next PATCH v1 1/7] Documentation: ACPI: DSD: Document MDIO PHY

On Fri, Oct 2, 2020 at 1:09 PM Grant Likely <grant.likely@....com> wrote:
>
>
>
> On 30/09/2020 17:37, Rafael J. Wysocki wrote:
> > On Wed, Sep 30, 2020 at 6:05 PM Calvin Johnson
> > <calvin.johnson@....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@....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.
> >
> > This should refer to the documents introducing the _DSD-based generic
> > device properties rules, including the GUID used below.
> >
> > 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.
> >
> >> +
> >> +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.
>
> If the goal is to align with DT, it would be appropriate to point to
> where those properties are defined for DT rather than to have a separate
> description here. I suggest something along the lines of:
>
>     The "phy-mode" _DSD property is used to describe the connection to
>     the PHY. The valid values for "phy-mode" are defined in
>     Documentation/devicetree/bindings/ethernet-controller.yaml
>
> >
> >> +
> >> +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.
>
> Can you elaborate? "phy-handle" has a section to itself in this
> document.

Yes, it does.

I overlooked it, sorry.

> Agree that it needs to be defined more, but it does read to me
> as having been defined.

Yup.

Cheers!