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  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:   Mon, 11 May 2020 11:22:31 +0530
From:   Calvin Johnson <>
To:     Andrew Lunn <>
Cc:     Jeremy Linton <>,
        Andy Shevchenko <>,
        "Rafael J . Wysocki" <>,
        Russell King - ARM Linux admin <>,, Florian Fainelli <>,
        Cristi Sovaiala <>,
        Florin Laurentiu Chiculita <>,
        Ioana Ciornei <>,
        Madalin Bucur <>,
        Greg Kroah-Hartman <>,
        Heikki Krogerus <>,
        Varun Sethi <>,
        "Rajesh V . Bikkina" <>,
        ACPI Devel Maling List <>,
        Linux Kernel Mailing List <>,
        Diana Madalina Craciun <>,
        netdev <>, Marcin Wojtas <>,
        Laurentiu Tudor <>,
        Makarand Pawagi <>,
        linux-arm Mailing List <>,
        Pankaj Bansal <>,
        "David S. Miller" <>,
        Heiner Kallweit <>
Subject: Re: [net-next PATCH v3 4/5] net: phy: Introduce fwnode_get_phy_id()

Thanks Andrew and Jeremy for the detailed discussion!

On Fri, May 08, 2020 at 08:13:01PM +0200, Andrew Lunn wrote:
> > > It does have a numeric version defined for EISA types. OTOH I suspect that
> > > your right. If there were a "PHY\VEN_IDvvvv&ID_DDDD" definition, it may not
> > > be ideal to parse it. Instead the normal ACPI model of exactly matching the
> > > complete string in the phy driver might be more appropriate.
> > 
> > IMO, it should be fine to parse the string to extract the phy_id. Is there any
> > reason why we cannot do this?
> Some background here, about what the PHY core does.
> PHYs have two ID registers. This contains vendor, device, and often
> revision of the PHY. Only the vendor part is standardised, vendors can
> decide how to use the device part, but it is common for the lowest
> nibble to be revision. The core will read these ID registers, and then
> go through all the PHY drivers registered and ask them if they support
> this ID. The drivers provide a table of IDs and masks. The mask is
> applied, and then if the ID matches, the driver is used. The mask
> allows the revision to be ignored, etc.
> There is a very small number of devices where the vendor messed up,
> and did not put valid contents in the ID registers. In such cases, we
> can read the IDs from device tree. These are then used in exactly the
> same way as if they were read from the device.
> If you want the ACPI model to be used, an exact match on the string,
> you are going to have to modify the core and the drivers. They
> currently don't have any string, and have no idea about different
> revisions which are out in the wild.

I don't think ACPI mandates that OS driver use exact string match and not parse
the string.

First of all, I would suggest that we use "compatible" property instead of _CID.
Not sure of a reason why we cannot. This will simplify implementation of fwnode

Already I've pointed out couple of ASL files in tianocore where they are already
one eg:

Even if we use _CID, I'm not sure we are prohibited from extracting characters
(phy_id) from it.
If we decide to use _CID, then we need to define somewhere and standardize
exactly how we are going to use it. I'm not sure where we can do this.

> > > Similarly to how I suspect the next patch's use of "compatible" isn't ideal
> > > either, because whether a device is c45 or not, should tend to be fixed to a
> > > particular vendor/device implementation and not a firmware provided
> > > property.
> Not exactly true. It is the combination of can the bus master do C45
> and can the device do C45. Unfortunately, we have no knowledge of the
> bus masters capabilities, if it can do C45. And many MDIO drivers will
> do a C22 transaction when asked to perform a C45 transaction. All new
> submissions for MDIO drivers i ask for EOPNOTSUPP to be returned if
> C45 is not supported. But we cannot rely on that. Too much history.

Makes sense to me.

> > 
> > I tend to agree with you on this. Even for DT, ideal case, IMO should be:
> > 
> > 1) mdiobus_scan scans the mdiobus for c22 devices by reading phy id from
> > registers 2 and 3
> > 2) if not found scan for c45 devices <= looks like this is missing in Linux
> > 3) look for phy_id from compatible string.
> It is somewhat more complex, in that there are a small number of
> devices which will respond to both C22 and C45. Generally, you want to
> use C45 if supported. So you would want to do the C45 scan first. But
> then the earlier problem comes to play, you have no idea if the bus
> master actually correctly supports C45.
> Given the issues, we assume all bus masters and PHY devices are C22
> unless DT says the bus master and PHY combination is compatible with
> C45.

Makes sense to me.

Powered by blists - more mailing lists