netdev - Re: net: micrel: confusion about phyids used in driver

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: <da599967-c423-80dd-945d-5b993c041e90@gmail.com>
Date:   Thu, 9 May 2019 22:55:29 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Yuiko Oshino <yuiko.oshino@...rochip.com>
Cc:     netdev@...r.kernel.org, kernel@...gutronix.de
Subject: Re: net: micrel: confusion about phyids used in driver

On 09.05.2019 22:29, Uwe Kleine-König wrote:
> Hello,
> 
> I have a board here that has a KSZ8051MLL (datasheet:
> http://ww1.microchip.com/downloads/en/DeviceDoc/ksz8051mll.pdf, phyid:
> 0x0022155x) assembled. The actual phyid is 0x00221556.
> 
I think the datasheets are the source of the confusion. If the
datasheets for different chips list 0x0022155x as PHYID each, and
authors of support for additional chips don't check the existing code,
then happens what happened.
However it's not a rare exception and not Microchip-specific that
sometimes vendors use the same PHYID for different chips.

And it seems you even missed one: KSZ8795
It's a switch and the internal PHY's have id 0x00221550.

If the drivers for the respective chips are actually different then we
may change the driver to match the exact model number only.
However, if there should be a PHY with e.g. id 0x00221554 out there,
it wouldn't be supported any longer and the generic PHY driver would
be used (what may work or not).

Obviously best would be get some input from Microchip.

> When enabling the micrel driver it successfully binds and claims to have
> detected a "Micrel KSZ8031" because phyid & 0x00ffffff ==
> PHY_ID_KSZ8031 (with PHY_ID_KSZ8031 = 0x00221556). I found a datasheet
> for KSZ8021RNL and KSZ8031RNL in our collection, there the phyid is
> documented as 0x0022155x, too. (So there is a deviation between driver
> and data sheet.)
> 
> A difference between these two parts are register bits 0x16.9 and
> 0x1f.7. (I didn't check systematically and there are definitely more
> differences, for example 0x16.7 which isn't handled at all in the
> driver.)
> 
> The driver does the right thing with KSZ8051MLL for bit 0x16.9 (which is
> setting a reserved bit on KSZ8021RNL/KSZ8031RNL) and for 0x1f.7 it's the
> other way round.
> 
> To make the situation still more interesting there is another phy entry
> "Micrel KSZ8051" that has .phy_id = 0x00221550 and .phy_id_mask =
> 0x00fffff0, which just judging from the name would be the better match.
> (This isn't used however because even though it matches "Micrel KSZ8031"
> is listed before and so is preferred.) With this the handling of the
> register bit 0x16.9 isn't right for the KSZ8051MLL. (I think it would if
> ksz8051_type had .has_broadcast_disable = true.)
> 
> I'm unclear what the right approach is to fix this muddle, maybe someone
> with more insight in the driver and maybe also in possession of more
> data sheets and hardware can tell?
> 
> My impression is that it is not possible to determine all features by
> just using the phyid. Would it be sensible to not difference between
> KSZ8031 and KSZ8051 and assume that writing to a reserved register bit
> does nothing?
> 
> Best regards
> Uwe
> 
Heiner