mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 002/002] de2104x: support for systems lacking cache coherence
@ 2009-02-07 21:30 Risto Suominen
  2009-02-10  1:01 ` Andrew Morton
  0 siblings, 1 reply; 4+ messages in thread
From: Risto Suominen @ 2009-02-07 21:30 UTC (permalink / raw)
  To: Jeff Garzik, lkml

[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]

Add a configurable Descriptor Skip Length for systems that lack cache coherence.

Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
---
The testing is done on kernel version 2.6.18.

--- drivers/net/tulip/Kconfig.org	2006-09-20 06:42:06.000000000 +0300
+++ drivers/net/tulip/Kconfig	2009-02-07 20:48:17.000000000 +0200
@@ -27,6 +27,18 @@ config DE2104X
 	  To compile this driver as a module, choose M here. The module will
 	  be called de2104x.

+config DE2104X_DSL
+	int "Descriptor Skip Length in 32 bit longwords"
+	depends on DE2104X
+	range 0 31
+	default 0
+	help
+	  Setting this value allows to align ring buffer descriptors into their
+	  own cache lines. Value of 4 corresponds to the typical 32 byte line
+	  (the descriptor is 16 bytes). This is necessary on systems that lack
+	  cache coherence, an example is PowerMac 5500. Otherwise 0 is safe.
+	  Default is 0, and range is 0 to 31.
+
 config TULIP
 	tristate "DECchip Tulip (dc2114x) PCI support"
 	depends on PCI
--- drivers/net/tulip/de2104x.c.org	2006-09-20 06:42:06.000000000 +0300
+++ drivers/net/tulip/de2104x.c	2009-02-07 15:04:04.000000000 +0200
@@ -82,6 +82,13 @@ MODULE_PARM_DESC (rx_copybreak, "de2104x
 				 NETIF_MSG_RX_ERR	| \
 				 NETIF_MSG_TX_ERR)

+/* Descriptor skip length in 32 bit longwords. */
+#ifndef CONFIG_DE2104X_DSL
+#define DSL			0
+#else
+#define DSL			CONFIG_DE2104X_DSL
+#endif
+
 #define DE_RX_RING_SIZE		64
 #define DE_TX_RING_SIZE		64
 #define DE_RING_BYTES		\
@@ -153,6 +160,7 @@ enum {
 	CmdReset		= (1 << 0),
 	CacheAlign16		= 0x00008000,
 	BurstLen4		= 0x00000400,
+	DescSkipLen		= (DSL << 2),

 	/* Rx/TxPoll bits */
 	NormalTxPoll		= (1 << 0),
@@ -246,7 +254,7 @@ static const u32 de_intr_mask =
  * Set the programmable burst length to 4 longwords for all:
  * DMA errors result without these values. Cache align 16 long.
  */
-static const u32 de_bus_mode = CacheAlign16 | BurstLen4;
+static const u32 de_bus_mode = CacheAlign16 | BurstLen4 | DescSkipLen;

 struct de_srom_media_block {
 	u8			opts;
@@ -266,6 +274,9 @@ struct de_desc {
 	__le32			opts2;
 	__le32			addr1;
 	__le32			addr2;
+#if DSL
+	__le32			skip[DSL];
+#endif
 };

 struct media_info {

[-- Attachment #2: de2104x-dsl.patch --]
[-- Type: text/x-diff, Size: 2182 bytes --]

Add a configurable Descriptor Skip Length for systems that lack cache coherence.

Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
---
The testing is done on kernel version 2.6.18.

--- drivers/net/tulip/Kconfig.org	2006-09-20 06:42:06.000000000 +0300
+++ drivers/net/tulip/Kconfig	2009-02-07 20:48:17.000000000 +0200
@@ -27,6 +27,18 @@ config DE2104X
 	  To compile this driver as a module, choose M here. The module will
 	  be called de2104x.
 
+config DE2104X_DSL
+	int "Descriptor Skip Length in 32 bit longwords"
+	depends on DE2104X
+	range 0 31
+	default 0
+	help
+	  Setting this value allows to align ring buffer descriptors into their
+	  own cache lines. Value of 4 corresponds to the typical 32 byte line
+	  (the descriptor is 16 bytes). This is necessary on systems that lack
+	  cache coherence, an example is PowerMac 5500. Otherwise 0 is safe.
+	  Default is 0, and range is 0 to 31.
+
 config TULIP
 	tristate "DECchip Tulip (dc2114x) PCI support"
 	depends on PCI
--- drivers/net/tulip/de2104x.c.org	2006-09-20 06:42:06.000000000 +0300
+++ drivers/net/tulip/de2104x.c	2009-02-07 15:04:04.000000000 +0200
@@ -82,6 +82,13 @@ MODULE_PARM_DESC (rx_copybreak, "de2104x
 				 NETIF_MSG_RX_ERR	| \
 				 NETIF_MSG_TX_ERR)
 
+/* Descriptor skip length in 32 bit longwords. */
+#ifndef CONFIG_DE2104X_DSL
+#define DSL			0
+#else
+#define DSL			CONFIG_DE2104X_DSL
+#endif
+
 #define DE_RX_RING_SIZE		64
 #define DE_TX_RING_SIZE		64
 #define DE_RING_BYTES		\
@@ -153,6 +160,7 @@ enum {
 	CmdReset		= (1 << 0),
 	CacheAlign16		= 0x00008000,
 	BurstLen4		= 0x00000400,
+	DescSkipLen		= (DSL << 2),
 
 	/* Rx/TxPoll bits */
 	NormalTxPoll		= (1 << 0),
@@ -246,7 +254,7 @@ static const u32 de_intr_mask =
  * Set the programmable burst length to 4 longwords for all:
  * DMA errors result without these values. Cache align 16 long.
  */
-static const u32 de_bus_mode = CacheAlign16 | BurstLen4;
+static const u32 de_bus_mode = CacheAlign16 | BurstLen4 | DescSkipLen;
 
 struct de_srom_media_block {
 	u8			opts;
@@ -266,6 +274,9 @@ struct de_desc {
 	__le32			opts2;
 	__le32			addr1;
 	__le32			addr2;
+#if DSL
+	__le32			skip[DSL];
+#endif
 };
 
 struct media_info {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 002/002] de2104x: support for systems lacking cache coherence
  2009-02-07 21:30 [PATCH 002/002] de2104x: support for systems lacking cache coherence Risto Suominen
@ 2009-02-10  1:01 ` Andrew Morton
  2009-02-10  1:07   ` David Miller
  2009-02-10  7:16   ` Risto Suominen
  0 siblings, 2 replies; 4+ messages in thread
From: Andrew Morton @ 2009-02-10  1:01 UTC (permalink / raw)
  To: Risto Suominen; +Cc: jgarzik, linux-kernel, netdev, Grant Grundler


(cc netdev, and maintainer)

On Sat, 7 Feb 2009 23:30:40 +0200
Risto Suominen <risto.suominen@gmail.com> wrote:

> Add a configurable Descriptor Skip Length for systems that lack cache coherence.
> 
> Signed-off-by: Risto Suominen <Risto.Suominen@gmail.com>
> ---
> The testing is done on kernel version 2.6.18.
> 
> --- drivers/net/tulip/Kconfig.org	2006-09-20 06:42:06.000000000 +0300
> +++ drivers/net/tulip/Kconfig	2009-02-07 20:48:17.000000000 +0200

Please prefer to prepare patches in `patch -p1' form:

--- a/drivers/net/tulip/Kconfig
+++ a/drivers/net/tulip/Kconfig

> @@ -27,6 +27,18 @@ config DE2104X
>  	  To compile this driver as a module, choose M here. The module will
>  	  be called de2104x.
> 
> +config DE2104X_DSL
> +	int "Descriptor Skip Length in 32 bit longwords"
> +	depends on DE2104X
> +	range 0 31
> +	default 0
> +	help
> +	  Setting this value allows to align ring buffer descriptors into their
> +	  own cache lines. Value of 4 corresponds to the typical 32 byte line
> +	  (the descriptor is 16 bytes). This is necessary on systems that lack
> +	  cache coherence, an example is PowerMac 5500. Otherwise 0 is safe.
> +	  Default is 0, and range is 0 to 31.
> +
>  config TULIP
>  	tristate "DECchip Tulip (dc2114x) PCI support"
>  	depends on PCI
> --- drivers/net/tulip/de2104x.c.org	2006-09-20 06:42:06.000000000 +0300
> +++ drivers/net/tulip/de2104x.c	2009-02-07 15:04:04.000000000 +0200
> @@ -82,6 +82,13 @@ MODULE_PARM_DESC (rx_copybreak, "de2104x
>  				 NETIF_MSG_RX_ERR	| \
>  				 NETIF_MSG_TX_ERR)
> 
> +/* Descriptor skip length in 32 bit longwords. */
> +#ifndef CONFIG_DE2104X_DSL
> +#define DSL			0
> +#else
> +#define DSL			CONFIG_DE2104X_DSL
> +#endif

I think CONFIG_DE2104X_DSL is always defined if this driver is being
compiled.  So the Kconfig `default' should suffice here?  In which case
we can do

#define DSL CONFIG_DE2104X_DSL

and leave it at that.

>  #define DE_RX_RING_SIZE		64
>  #define DE_TX_RING_SIZE		64
>  #define DE_RING_BYTES		\
> @@ -153,6 +160,7 @@ enum {
>  	CmdReset		= (1 << 0),
>  	CacheAlign16		= 0x00008000,
>  	BurstLen4		= 0x00000400,
> +	DescSkipLen		= (DSL << 2),

In which case we can do away with DSL everywhere and just use
CONFIG_DE2104X_DSL here.


But really, we shouldn't do this at configuration time at all.  It
would be much better to do it at runtime, via a module parameter.

And it would be much^2 better to do it automatically, based upon the
chip probing information or whatever.  That's probably hard.


>  	/* Rx/TxPoll bits */
>  	NormalTxPoll		= (1 << 0),
> @@ -246,7 +254,7 @@ static const u32 de_intr_mask =
>   * Set the programmable burst length to 4 longwords for all:
>   * DMA errors result without these values. Cache align 16 long.
>   */
> -static const u32 de_bus_mode = CacheAlign16 | BurstLen4;
> +static const u32 de_bus_mode = CacheAlign16 | BurstLen4 | DescSkipLen;
> 
>  struct de_srom_media_block {
>  	u8			opts;
> @@ -266,6 +274,9 @@ struct de_desc {
>  	__le32			opts2;
>  	__le32			addr1;
>  	__le32			addr2;
> +#if DSL
> +	__le32			skip[DSL];
> +#endif
>  };

Can remove the ifdefs here.  A zero-length array is OK, and will
consume zero space.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 002/002] de2104x: support for systems lacking cache coherence
  2009-02-10  1:01 ` Andrew Morton
@ 2009-02-10  1:07   ` David Miller
  2009-02-10  7:16   ` Risto Suominen
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2009-02-10  1:07 UTC (permalink / raw)
  To: akpm; +Cc: risto.suominen, jgarzik, linux-kernel, netdev, grundler

From: Andrew Morton <akpm@linux-foundation.org>
Date: Mon, 9 Feb 2009 17:01:20 -0800

> 
> (cc netdev, and maintainer)

We're already discussing this patch on netdev, he resent it
there.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 002/002] de2104x: support for systems lacking cache  coherence
  2009-02-10  1:01 ` Andrew Morton
  2009-02-10  1:07   ` David Miller
@ 2009-02-10  7:16   ` Risto Suominen
  1 sibling, 0 replies; 4+ messages in thread
From: Risto Suominen @ 2009-02-10  7:16 UTC (permalink / raw)
  To: Andrew Morton; +Cc: jgarzik, linux-kernel, netdev, Grant Grundler

2009/2/10 Andrew Morton <akpm@linux-foundation.org>:
>
> Please prefer to prepare patches in `patch -p1' form:
>
> --- a/drivers/net/tulip/Kconfig
> +++ a/drivers/net/tulip/Kconfig
>
I'll try to remember that.

> I think CONFIG_DE2104X_DSL is always defined if this driver is being
> compiled.  So the Kconfig `default' should suffice here?  In which case
> we can do
>
> #define DSL CONFIG_DE2104X_DSL
>
> and leave it at that.
>
> In which case we can do away with DSL everywhere and just use
> CONFIG_DE2104X_DSL here.
>
I was thinking of the case where someone just compiles without reconfiguring.

> But really, we shouldn't do this at configuration time at all.  It
> would be much better to do it at runtime, via a module parameter.
>
Well, yeah, but that would probably cost cpu cycles. At least it would
mean lots of changes.

> And it would be much^2 better to do it automatically, based upon the
> chip probing information or whatever.  That's probably hard.
>
I agree.

> Can remove the ifdefs here.  A zero-length array is OK, and will
> consume zero space.
>
I was uncertain whether this applies to all compilers. I kind of
remember opposite cases in the past.

Risto

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-02-10  7:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-07 21:30 [PATCH 002/002] de2104x: support for systems lacking cache coherence Risto Suominen
2009-02-10  1:01 ` Andrew Morton
2009-02-10  1:07   ` David Miller
2009-02-10  7:16   ` Risto Suominen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®