diff mbox

forcedeth: updated phy errata

Message ID 4A9C57F5.10800@nvidia.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Ayaz Abdulla Aug. 31, 2009, 11:08 p.m. UTC 
This patch updates the special programming (and/or errata) needed in 
order to setup the phy for various vendor models.

The new models include:
Marvell E1116
Marvell E1111
Marvell E1011
Marvell E3016
Broadcom 9507
Broadcom AC131
Broadcom 50610

Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>

Comments

David Miller Sept. 3, 2009, 6:22 a.m. UTC | #1 
From: Ayaz Abdulla <aabdulla@nvidia.com>
Date: Mon, 31 Aug 2009 19:08:37 -0400

> This patch updates the special programming (and/or errata) needed in
> order to setup the phy for various vendor models.
> 
> The new models include:
> Marvell E1116
> Marvell E1111
> Marvell E1011
> Marvell E3016
> Broadcom 9507
> Broadcom AC131
> Broadcom 50610
> 
> Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>

Please document what these individual bits mean which you
are clearing, by using an individual define for each register
bit to describe it's purpose, and then define the mask as
a concatenation of these bits.

Having an opaque bitmask is not how to do this.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ayaz Abdulla Sept. 4, 2009, 1:57 p.m. UTC | #2 
David Miller wrote:
> From: Ayaz Abdulla <aabdulla@nvidia.com>
> Date: Mon, 31 Aug 2009 19:08:37 -0400
> 
> 
>>This patch updates the special programming (and/or errata) needed in
>>order to setup the phy for various vendor models.
>>
>>The new models include:
>>Marvell E1116
>>Marvell E1111
>>Marvell E1011
>>Marvell E3016
>>Broadcom 9507
>>Broadcom AC131
>>Broadcom 50610
>>
>>Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>
> 
> 
> Please document what these individual bits mean which you
> are clearing, by using an individual define for each register
> bit to describe it's purpose, and then define the mask as
> a concatenation of these bits.
> 
> Having an opaque bitmask is not how to do this.

Unfortunately, the phy vendors don't want us exposing the meaning of 
their non-standard bits.


> 
> Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ayaz Abdulla Sept. 4, 2009, 5:22 p.m. UTC | #3 
David Miller wrote:
> From: Ayaz Abdulla <aabdulla@nvidia.com>
> Date: Fri, 04 Sep 2009 09:57:59 -0400
> 
> 
>>Unfortunately, the phy vendors don't want us exposing the meaning of
>>their non-standard bits.
> 
> 
> So when you go away and some other developer tries to debug a problem
> in this area, then what happens?  Do they just guess what those bits
> mean?

I understand your concern. However, NVIDIA will always be the maintainer 
for these phy erratas and work with users to solve any issues. We have 
access to the confidential documents provided by the vendors.

Phy vendors have said "No" in the past. I can request for 'opening up' 
the bits again, but not all vendors will comply - resulting in no 
support for forcedeth. This is currently the only way to provide a good 
experience for the end user.

> 
> Sorry, this sort of situation is unacceptable in this modern day and
> age.  You'll need to work this out before I'm willing to take the
> patch.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Sept. 4, 2009, 10 p.m. UTC | #4 
From: Ayaz Abdulla <aabdulla@nvidia.com>
Date: Fri, 04 Sep 2009 09:57:59 -0400

> Unfortunately, the phy vendors don't want us exposing the meaning of
> their non-standard bits.

So when you go away and some other developer tries to debug a problem
in this area, then what happens?  Do they just guess what those bits
mean?

Sorry, this sort of situation is unacceptable in this modern day and
age.  You'll need to work this out before I'm willing to take the
patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Sept. 4, 2009, 10:54 p.m. UTC | #5 
From: Ayaz Abdulla <aabdulla@nvidia.com>
Date: Fri, 04 Sep 2009 13:22:06 -0400

> Phy vendors have said "No" in the past. I can request for 'opening up'
> the bits again, but not all vendors will comply - resulting in no
> support for forcedeth. This is currently the only way to provide a
> good experience for the end user.

If I make it more difficult for you to get you're patch into the tree
than to get permission from the vendor, I'm sure you'll find a way to
make it happen.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- old/drivers/net/forcedeth.c	2009-08-31 19:01:04.000000000 -0400
+++ new/drivers/net/forcedeth.c	2009-08-31 19:05:49.000000000 -0400
@@ -506,6 +506,7 @@ 
 #define PHY_OUI_VITESSE		0x01c1
 #define PHY_OUI_REALTEK		0x0732
 #define PHY_OUI_REALTEK2	0x0020
+#define PHY_OUI_BROADCOM	0x50ef
 #define PHYID1_OUI_MASK	0x03ff
 #define PHYID1_OUI_SHFT	6
 #define PHYID2_OUI_MASK	0xfc00
@@ -517,7 +518,18 @@ 
 #define PHY_REV_REALTEK_8211C		0x0001
 #define PHY_MODEL_REALTEK_8201		0x0200
 #define PHY_MODEL_MARVELL_E3016		0x0220
-#define PHY_MARVELL_E3016_INITMASK	0x0300
+#define PHY_MODEL_MARVELL_E1116		0x0210
+#define PHY_MODEL_MARVELL_E1111		0x00c0
+#define PHY_MODEL_MARVELL_E1011		0x00b0
+#define PHY_MODEL_BROADCOM_9507		0x00a0
+#define PHY_MODEL_BROADCOM_AC131	0x0070
+#define PHY_MODEL_BROADCOM_50610	0x0160
+#define PHY_MARVELL_INIT_REG1	0x16
+#define PHY_MARVELL_INIT_REG2	0x10
+#define PHY_MARVELL_INIT1	0x0300
+#define PHY_MARVELL_INIT2	0x4000
+#define PHY_MARVELL_INIT_MSK1	0x0300
+#define PHY_MARVELL_INIT_MSK2	0x00ff
 #define PHY_CICADA_INIT1	0x0f000
 #define PHY_CICADA_INIT2	0x0e00
 #define PHY_CICADA_INIT3	0x01000
@@ -559,6 +571,19 @@ 
 #define PHY_REALTEK_INIT10	0x0005
 #define PHY_REALTEK_INIT11	0x0200
 #define PHY_REALTEK_INIT_MSK1	0x0003
+#define PHY_BROADCOM_INIT_REG1	0x1c
+#define PHY_BROADCOM_INIT_REG2	0x1f
+#define PHY_BROADCOM_INIT_REG3	0x1b
+#define PHY_BROADCOM_INIT_REG4	0x17
+#define PHY_BROADCOM_INIT_REG5	0x15
+#define PHY_BROADCOM_INIT1	0x2820
+#define PHY_BROADCOM_INIT2	0x0080
+#define PHY_BROADCOM_INIT3	0x0020
+#define PHY_BROADCOM_INIT4	0x8c00
+#define PHY_BROADCOM_INIT5	0x0f08
+#define PHY_BROADCOM_INIT6	0x0001
+#define PHY_BROADCOM_INIT7	0x0f00
+#define PHY_BROADCOM_INIT_MSK1	0x7c20
 
 #define PHY_GIGABIT	0x0100
 
@@ -1198,13 +1223,14 @@ 
 	u8 __iomem *base = get_hwbase(dev);
 	u32 phyinterface, phy_reserved, mii_status, mii_control, mii_control_1000,reg;
 
-	/* phy errata for E3016 phy */
-	if (np->phy_model == PHY_MODEL_MARVELL_E3016) {
-		reg = mii_rw(dev, np->phyaddr, MII_NCONFIG, MII_READ);
-		reg &= ~PHY_MARVELL_E3016_INITMASK;
-		if (mii_rw(dev, np->phyaddr, MII_NCONFIG, reg)) {
-			printk(KERN_INFO "%s: phy write to errata reg failed.\n", pci_name(np->pci_dev));
-			return PHY_ERROR;
+	if (np->phy_oui == PHY_OUI_MARVELL) {
+		if (np->phy_model == PHY_MODEL_MARVELL_E3016) {
+			reg = mii_rw(dev, np->phyaddr, MII_NCONFIG, MII_READ);
+			reg &= ~PHY_MARVELL_INIT_MSK1;
+			if (mii_rw(dev, np->phyaddr, MII_NCONFIG, reg)) {
+				printk(KERN_INFO "%s: phy write to errata reg failed.\n", pci_name(np->pci_dev));
+				return PHY_ERROR;
+			}
 		}
 	}
 	if (np->phy_oui == PHY_OUI_REALTEK) {
@@ -1340,6 +1366,89 @@ 
 	}
 
 	/* phy vendor specific configuration */
+	if (np->phy_oui == PHY_OUI_MARVELL) {
+		if (np->driver_data & DEV_NEED_PHY_INIT_FIX) {
+			if (np->phy_model == PHY_MODEL_MARVELL_E1116) {
+				phy_reserved = mii_rw(dev, np->phyaddr, PHY_MARVELL_INIT_REG1, MII_READ);
+				phy_reserved &= ~PHY_MARVELL_INIT_MSK2;
+				if (mii_rw(dev, np->phyaddr, PHY_MARVELL_INIT_REG1,phy_reserved)) {
+					printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+					return PHY_ERROR;
+				}
+
+				phy_reserved = mii_rw(dev, np->phyaddr, PHY_MARVELL_INIT_REG2, MII_READ);
+				phy_reserved |= PHY_MARVELL_INIT1;
+				if (mii_rw(dev, np->phyaddr, PHY_MARVELL_INIT_REG2 ,phy_reserved)) {
+					printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+					return PHY_ERROR;
+				}
+			}
+			if (np->phy_model == PHY_MODEL_MARVELL_E1111) {
+				phy_reserved = mii_rw(dev, np->phyaddr, PHY_MARVELL_INIT_REG2, MII_READ);
+				phy_reserved |= PHY_MARVELL_INIT1;
+				if (mii_rw(dev, np->phyaddr, PHY_MARVELL_INIT_REG2 ,phy_reserved)) {
+					printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+					return PHY_ERROR;
+				}
+			}
+			if (np->phy_model == PHY_MODEL_MARVELL_E3016) {
+				phy_reserved = mii_rw(dev, np->phyaddr, PHY_MARVELL_INIT_REG2, MII_READ);
+				phy_reserved |= PHY_MARVELL_INIT2;
+				if (mii_rw(dev, np->phyaddr, PHY_MARVELL_INIT_REG2 ,phy_reserved)) {
+					printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+					return PHY_ERROR;
+				}
+			}
+		}
+	}
+
+	if (np->phy_oui == PHY_OUI_BROADCOM) {
+		if (np->driver_data & DEV_NEED_PHY_INIT_FIX) {
+			if (np->phy_model == PHY_MODEL_BROADCOM_9507) {
+				phy_reserved = mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG1, MII_READ);
+				phy_reserved &= ~PHY_BROADCOM_INIT_MSK1;
+				phy_reserved |= PHY_BROADCOM_INIT1;
+				if (mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG1 ,phy_reserved)) {
+					printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+					return PHY_ERROR;
+				}
+			}
+			if (np->phy_model == PHY_MODEL_BROADCOM_AC131) {
+				phy_reserved = mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG2, MII_READ);
+				phy_reserved |= PHY_BROADCOM_INIT2;
+				if (mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG2 ,phy_reserved)) {
+					printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+					return PHY_ERROR;
+				}
+
+				phy_reserved = mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG3, MII_READ);
+				phy_reserved |= PHY_BROADCOM_INIT3;
+				if (mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG3 ,phy_reserved)) {
+					printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+					return PHY_ERROR;
+				}
+			}
+		}
+		if (np->phy_model == PHY_MODEL_BROADCOM_50610) {
+			if (mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG1, PHY_BROADCOM_INIT4)) {
+ 				printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+ 				return PHY_ERROR;
+ 			}
+			if (mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG4, PHY_BROADCOM_INIT5)) {
+ 				printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+ 				return PHY_ERROR;
+   			}
+			if (mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG5, PHY_BROADCOM_INIT6)) {
+ 				printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+ 				return PHY_ERROR;
+ 			}
+			if (mii_rw(dev, np->phyaddr, PHY_BROADCOM_INIT_REG4, PHY_BROADCOM_INIT7)) {
+ 				printk(KERN_INFO "%s: phy init failed.\n", pci_name(np->pci_dev));
+ 				return PHY_ERROR;
+ 			}
+		}
+	}
+
 	if ((np->phy_oui == PHY_OUI_CICADA) && (phyinterface & PHY_RGMII) ) {
 		phy_reserved = mii_rw(dev, np->phyaddr, MII_RESV1, MII_READ);
 		phy_reserved &= ~(PHY_CICADA_INIT1 | PHY_CICADA_INIT2);