netdev - Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Workaround missing PHY ID on mv88e6390

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]

Date:   Wed, 25 Jan 2017 09:51:57 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Gregory CLEMENT <gregory.clement@...e-electrons.com>,
        Andrew Lunn <andrew@...n.ch>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Subject: Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Workaround missing PHY
 ID on mv88e6390

On 01/25/2017 09:27 AM, Gregory CLEMENT wrote:
> Hi Andrew,
>  
>  On mer., janv. 25 2017, Andrew Lunn <andrew@...n.ch> wrote:
> 
>> The internal PHYs of the mv88e6390 do not have a model ID. Trap any
>> calls to the ID register, and if it is zero, return the ID for the
>> mv88e6390. The Marvell PHY driver can then bind to this ID.
>>
>> Signed-off-by: Andrew Lunn <andrew@...n.ch>
>> Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
>> ---
>>  drivers/net/dsa/mv88e6xxx/global2.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
>> index 353e26bea3c3..521a5511bd5f 100644
>> --- a/drivers/net/dsa/mv88e6xxx/global2.c
>> +++ b/drivers/net/dsa/mv88e6xxx/global2.c
>> @@ -520,7 +520,21 @@ int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
>>  	if (err)
>>  		return err;
>>  
>> -	return mv88e6xxx_g2_read(chip, GLOBAL2_SMI_PHY_DATA, val);
>> +	err = mv88e6xxx_g2_read(chip, GLOBAL2_SMI_PHY_DATA, val);
>> +	if (err)
>> +		return err;
>> +
>> +	if (reg == MII_PHYSID2) {
>> +		/* The mv88e6390 internal PHYS don't have a model number.
>> +		 * Use the switch family model number instead.
>> +		 */
>> +		if (!(*val & 0x3ff)) {
> 
> I tested this series on the Topaz switch but it failed because while I
> said we read 0x1410C00 actually we read 0x01410C01. With the
> MARVELL_PHY_ID_MASK we mask the 4 lower bits so that's why in my patch
> "phy: marvell: Add support for the PHY embedded in the topaz switch" I
> used the 0x01410C00 value for MARVELL_PHY_ID_88E6141.
> 
> However with the mask you use it doesn't work.
> 
> So this mask should be changed to 0x3f0 for the Topaz. Actually 0x3fe
> would be enough but it seems more logical to use the same mask that for
> MARVELL_PHY_ID_MASK.
> 
> We could either use the same mask for both family and still use 6390 as
> they seem compatible or we use two different families based on the lower
> bit.

By convention, the lower 4 bits are used to carry revision information,
which is why most drivers use 0xxxxx_fff0, can you try to use that here
for the PHY mask value?
-- 
Florian