Re: [net-next PATCH v3 4/5] net: phy: Introduce fwnode_get_phy_id()

[Andrew Lunn]

On Mon, May 11, 2020 at 11:22:31AM +0530, Calvin Johnson wrote:
> 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
> APIs.
> 
> Already I've pointed out couple of ASL files in tianocore where they are already
> used.
> one eg:https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl#L280
> 
> 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.

Hi Calvin

Whatever is decided needs to be documented as it becomes a defacto
standard. Once this is in the Linux PHY core, that is how it is done
for all boards using ACPI.

Maybe sometime in the future when the ACPI standards committee
definitively defines how this should be done, we can add a second
implementation which is standards conformant.

       Andrew