netdev - Re: [PATCH] Revert "net/phy: Add Vitesse 8641 phy ID"

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: <20150919062243.GA11699@pek-khao-d1.corp.ad.wrs.com>
Date:	Sat, 19 Sep 2015 14:22:43 +0800
From:	Kevin Hao <haokexin@...il.com>
To:	Shaohui Xie <Shaohui.Xie@...escale.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH] Revert "net/phy: Add Vitesse 8641 phy ID"

On Fri, Sep 18, 2015 at 09:36:42AM +0000, Shaohui Xie wrote:
> > -----Original Message-----
> > From: Kevin Hao [mailto:haokexin@...il.com]
> > Sent: Friday, September 18, 2015 3:43 PM
> > To: netdev@...r.kernel.org
> > Cc: Florian Fainelli; Xie Shaohui-B21989
> > Subject: [PATCH] Revert "net/phy: Add Vitesse 8641 phy ID"
> > 
> > This reverts commit 1298267b548a78840bd4b3e030993ff8747ca5e6.
> > 
> > That commit claim that the Vitesse VSC8641 is compatible with Vitesse
> > 82xx. But this is not true. It seems that all the registers used in
> > Vitesse phy driver are not compatible between 8641 and 82xx.
> [S.H] There are differences between some register's bit define. 
> But some are not used by driver. 
> 
> > It does cause malfunction of the Ethernet on p1010rdb-pa board.
> [S.H] Which exact register's setting caused problem?
> If it needs different setting,
> It can be handled by distinguishing phy_id, or replacing relative API for VSC8641.

In my case, the malfunction of the Ethernet is caused by writing the wrong
value for the skew timing. The Ethernet can work if I skip the setting of
skew in phy driver. But as I said in the commit log, all the registers used
in the current Vitesse phy driver are not compatible between 8641 and 82xx.
The following are the main differences between these registers:


Auxiliary Control & Status Register (0x1c):
              8641                                  8244
             6: reserved                        6: ActiPHY Mode Enable
             1: Sticky Reset Enable             1-0: ActiPHYTM Sleep Timer

Extended PHY Control Set 1 (0x17):
              8641                                  8244
            8: RGMII skew timing                11-10: RGMII TX_CLK Skew Selection
                                                9 - 8: RGMII RX_CLK Skew Selection
            5: ActiPHY mode enable              5: RX Idle Clock Enable
            3: reserved                         3: Far End Loopback Mode Enable
            1: GMII transmit pin reversal       2 - 1: MAC/Media Interface Mode Select
            0: reserved                         0: EEPROM Status

MII_VSC82X4_EXT_PAGE_16E (0x10):
              8641                                  8244
           Enhanced LED Method Select register     Reserved register 

MII_VSC82X4_EXT_PAGE_17E (0x11):
              8641                                  8244
           Enhanced LED Behavior register          CLK125 micro Clock Enable

MII_VSC82X4_EXT_PAGE_18E (0x12):
              8641                                  8244
           CRC Good Counter register               Reserved register

As you can see, I don't think it is a better option to sprinkle the checking
phy id for each writing of these register. Of course we can add the specific
API for 8641 to fix this problem. But since the generic phy driver works well
with the 8641 phy, this commit does seem a regression for me.  So I prefer a
simple revert to this commit and merge the revert into the stable kernel tree
to fix the regression first. We can add the corresponding 8641 phy API in the
following patches.

Thanks,
Kevin

Content of type "application/pgp-signature" skipped