Re: [PATCH] Revert "net/phy: Add Vitesse 8641 phy ID"
From: Kevin Hao Date: Fri, 18 Sep 2015 15:42:30 +0800 > 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. > It does cause malfunction of the Ethernet on p1010rdb-pa board. > So we definitely need a rework in order to support the 8641 phy > in this driver. > > Signed-off-by: Kevin Hao Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert "net/phy: Add Vitesse 8641 phy ID"
On Fri, Sep 18, 2015 at 09:36:42AM +, Shaohui Xie wrote: > > -Original Message- > > From: Kevin Hao [mailto:haoke...@gmail.com] > > Sent: Friday, September 18, 2015 3:43 PM > > To: netdev@vger.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: reserved6: ActiPHY Mode Enable 1: Sticky Reset Enable 1-0: ActiPHYTM Sleep Timer Extended PHY Control Set 1 (0x17): 8641 8244 8: RGMII skew timing11-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 pgpansPo0zADy.pgp Description: PGP signature
RE: [PATCH] Revert "net/phy: Add Vitesse 8641 phy ID"
> -Original Message- > From: Kevin Hao [mailto:haoke...@gmail.com] > Sent: Friday, September 18, 2015 3:43 PM > To: netdev@vger.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. Thanks! Shaohui -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[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.
It does cause malfunction of the Ethernet on p1010rdb-pa board.
So we definitely need a rework in order to support the 8641 phy
in this driver.
Signed-off-by: Kevin Hao
---
drivers/net/phy/vitesse.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index 17cad185169d..76cad712ddb2 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -66,7 +66,6 @@
#define PHY_ID_VSC8244 0x000fc6c0
#define PHY_ID_VSC8514 0x00070670
#define PHY_ID_VSC8574 0x000704a0
-#define PHY_ID_VSC8641 0x00070431
#define PHY_ID_VSC8662 0x00070660
#define PHY_ID_VSC8221 0x000fc550
#define PHY_ID_VSC8211 0x000fc4b0
@@ -273,18 +272,6 @@ static struct phy_driver vsc82xx_driver[] = {
.config_intr= &vsc82xx_config_intr,
.driver = { .owner = THIS_MODULE,},
}, {
- .phy_id = PHY_ID_VSC8641,
- .name = "Vitesse VSC8641",
- .phy_id_mask= 0x0000,
- .features = PHY_GBIT_FEATURES,
- .flags = PHY_HAS_INTERRUPT,
- .config_init= &vsc824x_config_init,
- .config_aneg= &vsc82x4_config_aneg,
- .read_status= &genphy_read_status,
- .ack_interrupt = &vsc824x_ack_interrupt,
- .config_intr= &vsc82xx_config_intr,
- .driver = { .owner = THIS_MODULE,},
-}, {
.phy_id = PHY_ID_VSC8662,
.name = "Vitesse VSC8662",
.phy_id_mask= 0x0000,
@@ -331,7 +318,6 @@ static struct mdio_device_id __maybe_unused vitesse_tbl[] =
{
{ PHY_ID_VSC8244, 0x000fffc0 },
{ PHY_ID_VSC8514, 0x0000 },
{ PHY_ID_VSC8574, 0x0000 },
- { PHY_ID_VSC8641, 0x0000 },
{ PHY_ID_VSC8662, 0x0000 },
{ PHY_ID_VSC8221, 0x0000 },
{ PHY_ID_VSC8211, 0x0000 },
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
