[PATCH] e1000: Work around 82571 completion timout on Pseries HW

[PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Auke Kok]

Our 82571 (first PCI-E hardware) causes P-Series hardware to throw
issues. Disabling PCI-E completion timeouts in our NIC resolves
the issue.

Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
Cc: Wen Xiong <wenxiong@us.ibm.com>
---

 drivers/net/e1000/e1000_main.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 49be393..830d851 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -819,6 +819,16 @@ e1000_reset(struct e1000_adapter *adapter)
 		E1000_WRITE_REG(&adapter->hw, CTRL, ctrl);
 	}
 
+#if defined(CONFIG_PPC64) || defined(CONFIG_PPC)
+#define E1000_GCR_DISABLE_TIMEOUT_MECHANISM 0x80000000
+	if (adapter->hw.mac.type == e1000_82571) {
+		/* work around pSeries hardware by disabling timeouts */
+		u32 gcr = E1000_READ_REG(&adapter->hw, E1000_GCR);
+		gcr |= E1000_GCR_DISABLE_TIMEOUT_MECHANISM;
+		E1000_WRITE_REG(&adapter->hw, E1000_GCR, gcr);
+	}
+#endif
+
 	/* Enable h/w to recognize an 802.1Q VLAN Ethernet packet */
 	E1000_WRITE_REG(&adapter->hw, VET, ETHERNET_IEEE_VLAN_TYPE);
 

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Jeff Garzik]

Auke Kok wrote:
> Our 82571 (first PCI-E hardware) causes P-Series hardware to throw
> issues. Disabling PCI-E completion timeouts in our NIC resolves
> the issue.
> 
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Cc: Wen Xiong <wenxiong@us.ibm.com>
> ---
> 
>  drivers/net/e1000/e1000_main.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index 49be393..830d851 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -819,6 +819,16 @@ e1000_reset(struct e1000_adapter *adapter)
>  		E1000_WRITE_REG(&adapter->hw, CTRL, ctrl);
>  	}
>  
> +#if defined(CONFIG_PPC64) || defined(CONFIG_PPC)
> +#define E1000_GCR_DISABLE_TIMEOUT_MECHANISM 0x80000000
> +	if (adapter->hw.mac.type == e1000_82571) {
> +		/* work around pSeries hardware by disabling timeouts */
> +		u32 gcr = E1000_READ_REG(&adapter->hw, E1000_GCR);
> +		gcr |= E1000_GCR_DISABLE_TIMEOUT_MECHANISM;
> +		E1000_WRITE_REG(&adapter->hw, E1000_GCR, gcr);

Why is this needed?

Is pSeries PCI-E implementation problematic?  Or is this a e1000 PCI-E 
hardware bug?  If the former, I wonder if this is the best way to work 
around the issue, especially since it might crop up in other PCI-E 
boards besides e1000.  If the latter, it seems like we should disable 
the feature on all platforms.

	Jeff

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Wen Xiong]

[-- Attachment #1.1: Type: text/plain, Size: 2452 bytes --]

Hi Jeff,

 little history, on PCI-X adapters we did not time dma reads being 
returned to the adapter. On PCIe unfortunately the spec says that adapters 
need to time the dma reads the spec is not specific on the range but the 
timers can range from worst case 50 usecs to 10 ms with some adapters 
supporting upto 50ms. The PCIe spec has approved an ECR to the PCIe 1.0a 
spec that will support the ability to disable the timer.  To work aroung 
the adapter doesn't support ECR.  We need the driver to disable the timer.

Thanks,
wendy 



Jeff Garzik <jeff@garzik.org> 
05/16/2007 11:00 AM

To
Auke Kok <auke-jan.h.kok@intel.com>
cc
Wen Xiong/Austin/IBM@IBMUS, e1000-devel@lists.sourceforge.net, 
netdev@vger.kernel.org, Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject
Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW






Auke Kok wrote:
> Our 82571 (first PCI-E hardware) causes P-Series hardware to throw
> issues. Disabling PCI-E completion timeouts in our NIC resolves
> the issue.
> 
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Cc: Wen Xiong <wenxiong@us.ibm.com>
> ---
> 
>  drivers/net/e1000/e1000_main.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/e1000/e1000_main.c 
b/drivers/net/e1000/e1000_main.c
> index 49be393..830d851 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -819,6 +819,16 @@ e1000_reset(struct e1000_adapter *adapter)
>                                E1000_WRITE_REG(&adapter->hw, CTRL, 
ctrl);
>                }
> 
> +#if defined(CONFIG_PPC64) || defined(CONFIG_PPC)
> +#define E1000_GCR_DISABLE_TIMEOUT_MECHANISM 0x80000000
> +              if (adapter->hw.mac.type == e1000_82571) {
> +                              /* work around pSeries hardware by 
disabling timeouts */
> +                              u32 gcr = E1000_READ_REG(&adapter->hw, 
E1000_GCR);
> +                              gcr |= 
E1000_GCR_DISABLE_TIMEOUT_MECHANISM;
> +                              E1000_WRITE_REG(&adapter->hw, E1000_GCR, 
gcr);

Why is this needed?

Is pSeries PCI-E implementation problematic?  Or is this a e1000 PCI-E 
hardware bug?  If the former, I wonder if this is the best way to work 
around the issue, especially since it might crop up in other PCI-E 
boards besides e1000.  If the latter, it seems like we should disable 
the feature on all platforms.

                 Jeff




[-- Attachment #2: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #3: Type: text/plain, Size: 164 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Benjamin Herrenschmidt]

On Wed, 2007-05-16 at 17:11 -0500, Wen Xiong wrote:
>  little history, on PCI-X adapters we did not time dma reads being
> returned to the adapter. On PCIe unfortunately the spec says that
> adapters need to time the dma reads the spec is not specific on the
> range but the timers can range from worst case 50 usecs to 10 ms with
> some adapters supporting upto 50ms. The PCIe spec has approved an ECR
> to the PCIe 1.0a spec that will support the ability to disable the
> timer.  To work aroung the adapter doesn't support ECR.  We need the
> driver to disable the timer. 

You do get more than 10ms delay for DMA replies on pSeries ? Ouch...

Ben.



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Kumar Gala]

On May 16, 2007, at 3:53 AM, Auke Kok wrote:

> Our 82571 (first PCI-E hardware) causes P-Series hardware to throw
> issues. Disabling PCI-E completion timeouts in our NIC resolves
> the issue.
>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Cc: Wen Xiong <wenxiong@us.ibm.com>
> ---
>
>  drivers/net/e1000/e1000_main.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/ 
> e1000_main.c
> index 49be393..830d851 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -819,6 +819,16 @@ e1000_reset(struct e1000_adapter *adapter)
>  		E1000_WRITE_REG(&adapter->hw, CTRL, ctrl);
>  	}
>
> +#if defined(CONFIG_PPC64) || defined(CONFIG_PPC)

If this is really on need on pseries HW this should be #ifdef  
CONFIG_PPC_PSERIES.  No reason to force this on all PPC.

> +#define E1000_GCR_DISABLE_TIMEOUT_MECHANISM 0x80000000
> +	if (adapter->hw.mac.type == e1000_82571) {
> +		/* work around pSeries hardware by disabling timeouts */
> +		u32 gcr = E1000_READ_REG(&adapter->hw, E1000_GCR);
> +		gcr |= E1000_GCR_DISABLE_TIMEOUT_MECHANISM;
> +		E1000_WRITE_REG(&adapter->hw, E1000_GCR, gcr);
> +	}
> +#endif
> +
>  	/* Enable h/w to recognize an 802.1Q VLAN Ethernet packet */
>  	E1000_WRITE_REG(&adapter->hw, VET, ETHERNET_IEEE_VLAN_TYPE);
>
> -
> 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

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Christoph Hellwig]

On Wed, May 16, 2007 at 07:43:27PM -0500, Kumar Gala wrote:
> >index 49be393..830d851 100644
> >--- a/drivers/net/e1000/e1000_main.c
> >+++ b/drivers/net/e1000/e1000_main.c
> >@@ -819,6 +819,16 @@ e1000_reset(struct e1000_adapter *adapter)
> > 		E1000_WRITE_REG(&adapter->hw, CTRL, ctrl);
> > 	}
> >
> >+#if defined(CONFIG_PPC64) || defined(CONFIG_PPC)
> 
> If this is really on need on pseries HW this should be #ifdef  
> CONFIG_PPC_PSERIES.  No reason to force this on all PPC.

No, it shouldn't.  powerpc kernels are multi-platform.

It really shouldn't be there at all because something in either the intel
or pseries hardware is totally buggy and we should disable features in
the buggy one completely.


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Kumar Gala]

On May 17, 2007, at 4:01 AM, Christoph Hellwig wrote:

> On Wed, May 16, 2007 at 07:43:27PM -0500, Kumar Gala wrote:
>>> index 49be393..830d851 100644
>>> --- a/drivers/net/e1000/e1000_main.c
>>> +++ b/drivers/net/e1000/e1000_main.c
>>> @@ -819,6 +819,16 @@ e1000_reset(struct e1000_adapter *adapter)
>>> 		E1000_WRITE_REG(&adapter->hw, CTRL, ctrl);
>>> 	}
>>>
>>> +#if defined(CONFIG_PPC64) || defined(CONFIG_PPC)
>>
>> If this is really on need on pseries HW this should be #ifdef
>> CONFIG_PPC_PSERIES.  No reason to force this on all PPC.
>
> No, it shouldn't.  powerpc kernels are multi-platform.

Right, but if you build a multi-platform kernel with pseries support  
you'll get CONFIG_PPC_PSERIES set.

> It really shouldn't be there at all because something in either the  
> intel
> or pseries hardware is totally buggy and we should disable features in
> the buggy one completely.

Agreed.

- k

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Wen Xiong]

[-- Attachment #1.1: Type: text/plain, Size: 1543 bytes --]

>>It really shouldn't be there at all because something in either the 
intel
>>or pseries hardware is totally buggy and we should disable features in
>>the buggy one completely.

Hi,

Here there are not hardware issue on both Intel or PPC.  The patch is to 
work around a loop hold on early version of PCI SGI spec. 
The later PCI Sgi have spec have corrected it.  We can just implement it 
for PPC only.  Other vendor may have the same issue.

Thanks,
wendy




Christoph Hellwig <hch@infradead.org> 
05/17/2007 04:01 AM

To
Kumar Gala <galak@kernel.crashing.org>
cc
Auke Kok <auke-jan.h.kok@intel.com>, jeff@garzik.org, Wen 
Xiong/Austin/IBM@IBMUS, e1000-devel@lists.sourceforge.net, 
netdev@vger.kernel.org
Subject
Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW






On Wed, May 16, 2007 at 07:43:27PM -0500, Kumar Gala wrote:
> >index 49be393..830d851 100644
> >--- a/drivers/net/e1000/e1000_main.c
> >+++ b/drivers/net/e1000/e1000_main.c
> >@@ -819,6 +819,16 @@ e1000_reset(struct e1000_adapter *adapter)
> >                              E1000_WRITE_REG(&adapter->hw, CTRL, 
ctrl);
> >              }
> >
> >+#if defined(CONFIG_PPC64) || defined(CONFIG_PPC)
> 
> If this is really on need on pseries HW this should be #ifdef 
> CONFIG_PPC_PSERIES.  No reason to force this on all PPC.

No, it shouldn't.  powerpc kernels are multi-platform.

It really shouldn't be there at all because something in either the intel
or pseries hardware is totally buggy and we should disable features in
the buggy one completely.



[-- Attachment #2: Type: text/plain, Size: 286 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

[-- Attachment #3: Type: text/plain, Size: 164 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Jeff Garzik]

Wen Xiong wrote:
> Here there are not hardware issue on both Intel or PPC.  The patch is to 
> work around a loop hold on early version of PCI SGI spec.  
> The later PCI Sgi have spec have corrected it.  We can just implement it 
> for PPC only.  Other vendor may have the same issue.


If it is not a PPC-specific issue, then we should not limit it to PPC.

	Jeff

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[wendy xiong]

On Thu, 2007-05-17 at 17:04 -0400, Jeff Garzik wrote:
> Wen Xiong wrote:
> > Here there are not hardware issue on both Intel or PPC.  The patch is to 
> > work around a loop hold on early version of PCI SGI spec.  
> > The later PCI Sgi have spec have corrected it.  We can just implement it 
> > for PPC only.  Other vendor may have the same issue.
> 
> 
> If it is not a PPC-specific issue, then we should not limit it to PPC.
> 
> 	Jeff
> 
> 

Hi Jeff,

We have tested the patch on PowerPC CEC and IO drawers. We really want
to fix this for PPC. If you can accept for vendors, we are ok.

Thanks for your help!
wendy

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Jeff Garzik]

Auke Kok wrote:
> Our 82571 (first PCI-E hardware) causes P-Series hardware to throw
> issues. Disabling PCI-E completion timeouts in our NIC resolves
> the issue.
> 
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Cc: Wen Xiong <wenxiong@us.ibm.com>
> ---
> 
>  drivers/net/e1000/e1000_main.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index 49be393..830d851 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -819,6 +819,16 @@ e1000_reset(struct e1000_adapter *adapter)
>  		E1000_WRITE_REG(&adapter->hw, CTRL, ctrl);
>  	}
>  
> +#if defined(CONFIG_PPC64) || defined(CONFIG_PPC)
> +#define E1000_GCR_DISABLE_TIMEOUT_MECHANISM 0x80000000
> +	if (adapter->hw.mac.type == e1000_82571) {
> +		/* work around pSeries hardware by disabling timeouts */
> +		u32 gcr = E1000_READ_REG(&adapter->hw, E1000_GCR);
> +		gcr |= E1000_GCR_DISABLE_TIMEOUT_MECHANISM;
> +		E1000_WRITE_REG(&adapter->hw, E1000_GCR, gcr);
> +	}
> +#endif

not applied.  based on the conversation, it sounds like this is not 
specific to PPC

	Jeff

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Christoph Hellwig]

On Thu, May 17, 2007 at 09:58:03AM -0500, Wen Xiong wrote:
> >>It really shouldn't be there at all because something in either the 
> intel
> >>or pseries hardware is totally buggy and we should disable features in
> >>the buggy one completely.
> 
> Hi,
> 
> Here there are not hardware issue on both Intel or PPC.  The patch is to 
> work around a loop hold on early version of PCI SGI spec. 
> The later PCI Sgi have spec have corrected it.  We can just implement it 
> for PPC only.  Other vendor may have the same issue.

In this case we should add a blacklist for implementations of the old
spec.  There should be a way to find specific bridges in the OF firmware
tree on powerpc and similar things on other platforms aswell.


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH] e1000: Work around 82571 completion timout on Pseries HW

[Kok, Auke]

Christoph Hellwig wrote:
> On Thu, May 17, 2007 at 09:58:03AM -0500, Wen Xiong wrote:
>>>> It really shouldn't be there at all because something in either the 
>> intel
>>>> or pseries hardware is totally buggy and we should disable features in
>>>> the buggy one completely.
>> Hi,
>>
>> Here there are not hardware issue on both Intel or PPC.  The patch is to 
>> work around a loop hold on early version of PCI SGI spec. 
>> The later PCI Sgi have spec have corrected it.  We can just implement it 
>> for PPC only.  Other vendor may have the same issue.
> 
> In this case we should add a blacklist for implementations of the old
> spec.  There should be a way to find specific bridges in the OF firmware
> tree on powerpc and similar things on other platforms aswell.

Yes, this is almost what we did.

IBM is currently testing my patches that implement a generic pci quirk that will 
be enabled for only selected root complex ID's that require the (1.0a spec) 
device to disable the completion timeouts.

They are currently validating this test on the affected hardware. I expect to 
get the results within a week and then I will post the patch.

Since this is one of the few holes in between the two specs (where manual 
intervention is needed) I think that a single quirk is a fairly sane approach.

Cheers,

Auke

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/