netdev - RE: [EXT] Re: [PATCH v1 1/7] mdio_bus: Introduce fwnode MDIO helpers

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]

Message-ID: <AM0PR04MB56366808BA5C841E4E7BC86E931C0@AM0PR04MB5636.eurprd04.prod.outlook.com>
Date:   Fri, 7 Feb 2020 09:42:56 +0000
From:   "Calvin Johnson (OSS)" <calvin.johnson@....nxp.com>
To:     Jeremy Linton <jeremy.linton@....com>,
        "linux.cj@...il.com" <linux.cj@...il.com>,
        Jon Nettleton <jon@...id-run.com>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        Makarand Pawagi <makarand.pawagi@....com>,
        Cristi Sovaiala <cristian.sovaiala@....com>,
        Laurentiu Tudor <laurentiu.tudor@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Varun Sethi <V.Sethi@....com>,
        Pankaj Bansal <pankaj.bansal@....com>,
        "Rajesh V. Bikkina" <rajesh.bikkina@....com>,
        Leo Li <leoyang.li@....com>
CC:     Marcin Wojtas <mw@...ihalf.com>,
        "Calvin Johnson (OSS)" <calvin.johnson@....nxp.com>,
        Andrew Lunn <andrew@...n.ch>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: RE: [EXT] Re: [PATCH v1 1/7] mdio_bus: Introduce fwnode MDIO helpers

Hi Jeremy,

> -----Original Message-----
> From: Jeremy Linton <jeremy.linton@....com>
> Sent: Wednesday, February 5, 2020 7:48 PM

<snip>

> > +static int fwnode_mdio_parse_addr(struct device *dev,
> > +                               const struct fwnode_handle *fwnode) {
> > +     u32 addr;
> > +     int ret;
> > +
> > +     ret = fwnode_property_read_u32(fwnode, "reg", &addr);
> > +     if (ret < 0) {
> > +             dev_err(dev, "PHY node has no 'reg' property\n");
> > +             return ret;
> > +     }
> > +
> > +     /* A PHY must have a reg property in the range [0-31] */
> > +     if (addr < 0 || addr >= PHY_MAX_ADDR) {
> > +             dev_err(dev, "PHY address %i is invalid\n", addr);
> > +             return -EINVAL;
> > +     }
> > +
> > +     return addr;
> > +}
> 
> Almost assuredly this is wrong, the _ADR method exists to identify a device
> on its parent bus. The DT reg property shouldn't be used like this in an ACPI
> enviroment.
> 
> Further, there are a number of other dt bindings in this set that seem
> inappropriate in common/shared ACPI code paths. That is because AFAIK the
> _DSD methods are there to provide device implementation specific
> behaviors, not as standardized methods for a generic classes of devices.
> Its vaguly the equivlant of the "vendor,xxxx" properties in DT.
> 
> This has been a discussion point on/off for a while with any attempt to
> publicly specify/standardize for all OS vendors what they might find encoded
> in a DSD property. The few year old "WORK_IN_PROGRESS" link on the uefi
> page has a few suggested ones
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fuefi.
> org%2Fsites%2Fdefault%2Ffiles%2Fresources%2Fnic-request-
> v2.pdf&amp;data=02%7C01%7Ccalvin.johnson%40nxp.com%7Cf16350b8314
> b4992063008d7ab4f6486%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1
> %7C637166229795374486&amp;sdata=zcXu%2Fu%2Fxw5%2Ff7eJd%2FledR%
> 2FgnabvFcCUtOfwTXtMoDBI%3D&amp;reserved=0
> 
> Anyway, the use of phy-handle with a reference to the phy device on a
> shared MDIO is a techically workable solution to the problem brought up in
> the RPI/ACPI thread as well. OTOH, it suffers from the use of DSD and at a
> minimum should probably be added to the document linked above if its
> found to be the best way to handle this. Although, in the common case of a
> mdio bus, matching acpi described devices with their discovered
> counterparts (note the device() defintion in the spec
> 19.6.30) only to define DSD refrences is a bit overkill.
> 
> Put another way, while seemingly not nessiary if a bus can be probed, a
> nic/device->mdio->phy can be described in the normal ACPI heirarchy with
> only appropriatly nested CRS and ADR resources. Its the shared nature of the
> MDIO bus that causes problems.

Thanks! I'll definitely consider your suggestions along with the others received earlier.

While I do more study on this, thought of forwarding DSTD tables used by this patch-set.
https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platforms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Dsdt/Mdio.asl?h=LX2160_UEFI_ACPI_EAR1
https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platforms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Dsdt/Mc.asl?h=LX2160_UEFI_ACPI_EAR1

Regards
Calvin