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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87o9yvyqwr.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
Date:   Wed, 25 Jan 2017 12:45:40 -0500
From:   Vivien Didelot <vivien.didelot@...oirfairelinux.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>
Subject: Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Workaround missing PHY ID on mv88e6390

Hi Gregory, Andrew,

Gregory CLEMENT <gregory.clement@...e-electrons.com> writes:

>> +	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.

Since several chips have this issue, we can introduce a u16 physid2_mask
member in the mv88e6xxx_info structure and move the check in
mv88e6xxx_phy_read() so that the logic of device (as in Global2) helpers
are not affected by such (necessary) hack. Something like:

    static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
                                  int reg, u16 *val)
    {
        ...

        err = chip->info->ops->phy_read(chip, bus, addr, reg, val);
        if (err)
            return err;

        if (reg == MII_PHYSID2 && chip->info->physid2_mask) {
            /* Some internal PHYs don't have a model number,
             * so return the switch family model number directly.
             */
            if (!(*val & chip->info->physid2_mask))
                *val |= chip->info->prod_num;
        }

        return 0;
    }


Thanks,

        Vivien