netdev - Re: confusing inconsistency between "PHY ID" and "PHY type" in comments

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] [day] [month] [year] [list]

Date:   Wed, 12 Dec 2018 17:18:46 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     rpjday@...shcourse.ca
Cc:     netdev@...r.kernel.org
Subject: Re: confusing inconsistency between "PHY ID" and "PHY type" in
 comments

On Wed, Dec 12, 2018 at 09:35:27AM -0500, rpjday@...shcourse.ca wrote:
>   (totally willing to embarrass myself further, this time with respect to
> inconsistency in basic terminology in the kernel files.)
> 
>   from include/linux/mod_devicetable.h:
> 
> /**
>  * struct mdio_device_id - identifies PHY devices on an MDIO/MII bus
>  * @phy_id: The result of
>  *     (mdio_read(&MII_PHYSID1) << 16 | mdio_read(&PHYSID2)) & @phy_id_mask
>  *     for this PHY type
>  * @phy_id_mask: Defines the significant bits of @phy_id.  A value of 0
>  *     is used to terminate an array of struct mdio_device_id.
>  */
> struct mdio_device_id {
>         __u32 phy_id;
>         __u32 phy_id_mask;
> };
> 
>   stop right there ... ignoring the obvious typo in "&PHYSID2" (should
> of course be "&MII_PHYSID2"),

Yes, patch welcome.

 the definition of "phy_id" is puzzling, as
> it's defined as the full 32-bit value with the mask applied to it.
> is that right? is the technical definition of phy_id the value *after*
> the phy_id_mask is applied to the register contents? because that
> certainly seems inconsistent with most explanations i've seen.

What might be the source of this confusion is that the lower nibble
mostly means the PHY silicon revision. So:

#define PHY_ID_ASIX_AX88796B            0x003b1841 

This is the second revision of the silicon, but the mask
         .phy_id_mask    = 0xfffffff0,

indicates this driver is happy to work with any revision of the
silicon. The driver writer probably had the second revision of the
silicon, which is why they set the #define to that value.

But some vendors don't use the lower nibble as silicon revision,
e.g. ACME Corp.

     Andrew