[PATCH] lne390 and Jensen Alphas

[PATCH] lne390 and Jensen Alphas

[Carsten Jacobi]

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

Hello,

I am coming back with an old issue (and I mean 10 years). I have a  
patch for the Mylex LNE390 EISA network adapter that will make it  
work on the Alpha Jensen boards.
The patch has already made public in 1998 (http://lkml.org/lkml/ 
1998/8/9/16) and again in 2003 (http://linux.derkeiler.com/Mailing- 
Lists/Kernel/2003-10/2341.html). I am running the adapter ever since  
in my Jensen Alpha and it had been working just fine during the last  
10 years.
Now, since I posted the patch twice to the linux-kernel mailing list  
without having any snippet of the code getting into the official  
kernel source I try it with this mailing list for a change.
The code itself is supposed for the kernel 2.4.x series but should  
apply to 2.6.x as well. Is there something that speaks against  
pushing it up to the git repository?

With kind regards,

Carsten Jacobi

PS.: I have attached the patch once more

[-- Attachment #2: 998_lne390.c.diff.bz2 --]
[-- Type: application/octet-stream, Size: 2159 bytes --]

Re: [PATCH] lne390 and Jensen Alphas

[Florian Fainelli]

Hi Carsten,

Le samedi 29 mars 2008, Carsten Jacobi a écrit :
> Now, since I posted the patch twice to the linux-kernel mailing list
> without having any snippet of the code getting into the official
> kernel source I try it with this mailing list for a change.
> The code itself is supposed for the kernel 2.4.x series but should
> apply to 2.6.x as well. Is there something that speaks against
> pushing it up to the git repository?

First of all, network drivers maintainers only accept inline patches that can 
be reviewed in the mail, such that they do not have to extract the patch and 
read it, but can comment directly in the body of the email. See 
http://lxr.linux.no/linux/Documentation/SubmittingPatches (also in the linux 
tarball)

Second, your patch does not apply to the the 2.6.25-rc7 tree which is a major 
stopper for its merge into the netdev repository :

patching file drivers/net/lne390.c
Hunk #1 succeeded at 95 (offset 1 line).
Hunk #2 succeeded at 243 (offset 21 lines).
Hunk #3 FAILED at 272.
Hunk #4 FAILED at 379.
Hunk #5 FAILED at 421.
Hunk #6 FAILED at 445.

Should be easy to fix.

Finally, your patch looks good, I have noticed that :

1) There are quite some lines which are more than 80 columns long, fix this 
and use the checkpatch.pl script to make sure there are not any

2) Why add an "activate" module parameter if this case is only for the Jensen 
Alphas series ? Would not it be easier to do this inside an ifdef such that 
this does not confuse people who do not have a Jensen Alpha ?

3) Cannot this activation code be done in the Alpha Jensen architecture code ? 
(Just an idea)

You are very close to getting this merged !
-- 
Cordialement, Florian Fainelli
------------------------------

Re: [PATCH] lne390 and Jensen Alphas

[Carsten Jacobi]

Hello Florian,

thanks for your responds and sorry for my late answer. Now to
your objections:

> First of all, network drivers maintainers only accept inline patches that can 
> be reviewed in the mail, such that they do not have to extract the patch and 
> read it, but can comment directly in the body of the email. See 
> http://lxr.linux.no/linux/Documentation/SubmittingPatches (also in the linux 
> tarball)

Ok, the next one will come within the mail and I tried to keep any line
below 80 culomns.

> Second, your patch does not apply to the the 2.6.25-rc7 tree which is a major 
> stopper for its merge into the netdev repository :
> Should be easy to fix.

Here comes my next try:

--- drivers-net-lne390.c	2008-04-13 22:02:38.000000000 +0200
+++ lne390.c	2008-04-13 22:14:36.000000000 +0200
@@ -95,6 +95,8 @@
 #define LNE390_DEBUG	0
 
 static unsigned char irq_map[] __initdata = {15, 12, 11, 10, 9, 7, 5, 3};
+static unsigned char irq_reverse_map[] __initdata = {0xff, 0xff, 0xff, 7,
+0xff, 6, 0xff, 5, 0xff, 4, 3, 2, 1, 0xff, 0xff, 0};
 static unsigned int shmem_mapA[] __initdata = {0xff, 0xfe, 0xfd, 0xfff, 0xffe, 0xffc, 0x0d, 0x0};
 static unsigned int shmem_mapB[] __initdata = {0xff, 0xfe, 0x0e, 0xfff, 0xffe, 0xffc, 0x0d, 0x0};
 
@@ -242,6 +244,10 @@
 	printk("%dkB memory at physical address %#lx\n",
 			LNE390_STOP_PG/4, dev->mem_start);
 
+#ifdef CONFIG_ALPHA_JENSEN
+	/* On the Jensen board EISA cards will see their own address
+	 * space */
+#else
 	/*
 	   BEWARE!! Some dain-bramaged EISA SCUs will allow you to put
 	   the card mem within the region covered by `normal' RAM  !!!
@@ -260,6 +266,7 @@
 			LNE390_STOP_PG/4, ei_status.mem);
 
 	dev->mem_start = (unsigned long)ei_status.mem;
+#endif /* CONFIG_ALPHA_JENSEN */
 	dev->mem_end = dev->mem_start + (LNE390_STOP_PG - LNE390_START_PG)*256;
 
 	/* The 8390 offset is zero for the LNE390 */
@@ -372,7 +379,25 @@
 	void __iomem *shmem = ei_status.mem + ((start_page - LNE390_START_PG)<<8);
 
 	count = (count + 3) & ~3;     /* Round up to doubleword */
+#ifndef CONFIG_ALPHA_JENSEN
 	memcpy_toio(shmem, buf, count);
+#else
+	/* The mylex lne390 adapter requires 32bit access (see above) for every
+	 * operation to the shared mem buffer. Since the block buffer is hardly
+	 * aligned to a 32bit boundary memcpy_toio() will use 16bit
+	 * operations to access the buffer ... we must use something else here. */
+
+	const void *from = buf;
+	count -= 4;
+	do {
+		__raw_writel(*(const u16 *)from | (*(const u16 *)(from+2))<<16,
+		(unsigned long) shmem);
+		count -= 4;
+		(unsigned long) shmem += 4;
+		from += 4;
+	} while (count >= 0);
+#endif
+
 }
 
 static int lne390_open(struct net_device *dev)

> Finally, your patch looks good, I have noticed that :
> 
> 2) Why add an "activate" module parameter if this case is only for the Jensen 
> Alphas series ? Would not it be easier to do this inside an ifdef such that 
> this does not confuse people who do not have a Jensen Alpha ?

Right, the activation part is not Jensen-Alpha specific but is a kind of
a goodie for 2.4.x kernels to force the configuration of an lne390
adapter despite of different settings with the EISA configuration
utility. On the SRM console of the Jensen Alpha this is even obligatory
since that console does not initialize any EISA cards but the Adaptec
1740 for system start.

> 3) Cannot this activation code be done in the Alpha Jensen architecture code ? 
I think in kernel 2.6 you would do it more cleanly with the /sys
filesystem.
A problem is also, that the patch above should apply well on the lne390.c
source of the 2.6 kernel but since I run kernel 2.4.27 I have no way to
actually test that code.

Is it possible to merge fixes for 2.4.36 first?

With kind regards,

Carsten Jacobi

Re: [PATCH] lne390 and Jensen Alphas

[Al Viro]

On Sun, Apr 13, 2008 at 11:06:46PM +0200, Carsten Jacobi wrote:
> +	/* The mylex lne390 adapter requires 32bit access (see above) for every
> +	 * operation to the shared mem buffer. Since the block buffer is hardly
> +	 * aligned to a 32bit boundary memcpy_toio() will use 16bit
> +	 * operations to access the buffer ... we must use something else here. */
> +
> +	const void *from = buf;
> +	count -= 4;
> +	do {
> +		__raw_writel(*(const u16 *)from | (*(const u16 *)(from+2))<<16,
> +		(unsigned long) shmem);

The last argument of __raw_writel() is void __iomem *, not unsigned long.
WTF is that cast doing that?  Besides, it looks like misspelled
		const __le32 *from = buf;
		...
		__raw_writel(get_unaligned(from), shmem);

> +		(unsigned long) shmem += 4;

Write in C, please.  This isn't - you might as well write something like
	shmem * 2 -= 4;
since result of cast is not an l-value.

Finally, how much of that actually depends on Jensen and how much is
actually "oh, on x86 we use rep movsl in memcpy_toio(), so it ends up
with 32bit accesses"?

Re: [PATCH] lne390 and Jensen Alphas

[Carsten Jacobi]

Hello,

first thanks for the feedback, I am still willing to deliver
a "decent" patch.

Al Viro wrote:
> The last argument of __raw_writel() is void __iomem *, not unsigned long.
> WTF is that cast doing that?  Besides, it looks like misspelled
> 		const __le32 *from = buf;
> 		...
> 		__raw_writel(get_unaligned(from), shmem);
>
>   
Tststs ... such strong words. But I see your point. As I look at it 
myself it doesn't appear to be clean style.
>> +		(unsigned long) shmem += 4;
>>     
>
> Write in C, please.  This isn't - you might as well write something like
> 	shmem * 2 -= 4;
> since result of cast is not an l-value.
>   
I admit this must be done using appropriate types. The patch was 
originally intended for kernel version 2.1.115 in 1998 and I just come 
back with it every 5 years because it haven't made it into the official 
source so far. Actually, I just took the memcpy_toio() function from 
arch/alpha/lib/io.c did some modifications to let it read from unaligned 
addresses and then just pasted it into the lne390.c source.
But for an official patch it should look better. Hmm, the 
get_unaligned(from) function seems indeed to be exactly what I need 
here. I guess that wasn't available 10 years before when I writed the 
original patch.
> Finally, how much of that actually depends on Jensen and how much is
> actually "oh, on x86 we use rep movsl in memcpy_toio(), so it ends up
> with 32bit accesses"?
>   
Only the lne390_block_output() routine has to be adjusted for all 
systems that have EISA slots and which use a memcpy_toio() function that 
copies 16bit aligned addresses with 16bit operations. I can't tell at 
once which other systems than CONFIG_ALPHA_JENSEN would match here. I 
guess HPPA and Silicon-Graphics (are there MIPS systems with EISA 
slots?) would be candidates.
The other #ifdef allows choosing arbitrary memory spaces for the shared 
mem buffer of the adapter. Also here I don't know how other non-x86 
architectures map the EISA memory space into their CPU address space.
Here is my next patch. I have removed the misleading casts. I have taken 
the memcpy_toio() function from arch/alpha/kernel/io.c (as I see the 
source file has moved from lib/ to kernel/) and did my little 
modification again:
--- lne390.c.orig       2008-04-20 14:25:32.000000000 +0200
+++ lne390.c    2008-04-20 14:49:21.000000000 +0200
@@ -242,6 +242,10 @@
        printk("%dkB memory at physical address %#lx\n",
                        LNE390_STOP_PG/4, dev->mem_start);

+#ifdef CONFIG_ALPHA_JENSEN
+       /* On the Jensen board EISA cards will see their own address
+        * space */
+#else
        /*
           BEWARE!! Some dain-bramaged EISA SCUs will allow you to put
           the card mem within the region covered by `normal' RAM  !!!
@@ -260,6 +264,7 @@
                        LNE390_STOP_PG/4, ei_status.mem);

        dev->mem_start = (unsigned long)ei_status.mem;
+#endif /* CONFIG_ALPHA_JENSEN */
        dev->mem_end = dev->mem_start + (LNE390_STOP_PG - 
LNE390_START_PG)*256;

        /* The 8390 offset is zero for the LNE390 */
@@ -372,7 +377,25 @@
        void __iomem *shmem = ei_status.mem + ((start_page - 
LNE390_START_PG)<<8);

        count = (count + 3) & ~3;     /* Round up to doubleword */
+#ifndef CONFIG_ALPHA_JENSEN
        memcpy_toio(shmem, buf, count);
+#else
+       /* The mylex lne390 adapter requires 32bit access (see above) 
for every
+        * operation to the shared mem buffer. Since the block buffer is 
hardly
+        * aligned to a 32bit boundary memcpy_toio() will use 16bit
+        * operations to access the buffer on alpha architectures (maybe 
also
+        * for others ... we must use something else here. */
+
+       const void *from = buf;
+       count -= 4;
+       do {
+               __raw_writel(*(const u32 *)get_unaligned(from), shmem);
+               count -= 4;
+               shmem += 4;
+               from += 4;
+       } while (count >= 0);
+#endif
+
 }

 static int lne390_open(struct net_device *dev)

Regards,

Carsten Jacobi

[PATCH] lne390 and Jensen Alphas

[Carsten Jacobi]

Sorry, I forgot to add that my patch applies to kernel 2.4.22 and is
meant for the 2.4.x series.

Regards,
	Carsten Jacobi

[PATCH] lne390 and Jensen Alphas

[Carsten Jacobi]

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

Hi there!

I am running a Jensen board with a lne390 network adapter for two years
with some private patches that I would like to have in the official
kernel releases.
About five years ago, I sent some patches to Alan Cox and to this mailing
list (http://lkml.org/lkml/1998/8/9/16) that didn't make it into the
kernel. This here is my second approach!
The patch is attached and provides activation support for the EISA
adapter. This is very helpful for the Jensen board since the EISA config
utility won't help for operating systems started on the SRM console (as
i.e. Linux). When you load the module with the argument "activation=1"
it will not only request interrupt number and iospace in the kernel API
but also configure the adapter accordingly.

A CNET 900E EISA Ethernet adapter is also here that I would like to use.
Can anyone give me some information or better datasheets about that
adapter. I would also write the driver. Last time I asked (also about
five years ago) they gave me Lan-Manager drivers and two useless
EISA-Config files (all configurations shared the same bit pattern) ...
still better than nothing. Is it easy to disassemble Lan-Manager drivers
and retrieve appropriate info from them to write a driver? The CNET 900E
also seems to use a National Semiconductor 8390-compatible Chip set and
it seems to have 16K of shared mem on board.

Anyways, I would be glad if I saw my patches in one of the next kernel
releases.

Regards,
	Carsten Jacobi

[-- Attachment #2: lne390.c.patch --]
[-- Type: text/plain, Size: 5162 bytes --]

--- lne390.c.orig	2003-09-21 18:49:55.000000000 +0200
+++ lne390.c	2003-10-10 23:38:48.000000000 +0200
@@ -94,6 +94,7 @@
 #define LNE390_DEBUG	0
 
 static unsigned char irq_map[] __initdata = {15, 12, 11, 10, 9, 7, 5, 3};
+static unsigned char irq_reverse_map[] __initdata = {0xff, 0xff, 0xff, 7, 0xff, 6, 0xff, 5, 0xff, 4, 3, 2, 1, 0xff, 0xff, 0};
 static unsigned int shmem_mapA[] __initdata = {0xff, 0xfe, 0xfd, 0xfff, 0xffe, 0xffc, 0x0d, 0x0};
 static unsigned int shmem_mapB[] __initdata = {0xff, 0xfe, 0x0e, 0xfff, 0xffe, 0xffc, 0x0d, 0x0};
 
@@ -221,6 +222,10 @@
 	printk("%dkB memory at physical address %#lx\n",
 			LNE390_STOP_PG/4, dev->mem_start);
 
+#ifdef CONFIG_ALPHA_JENSEN
+	/* On the Jensen board EISA cards will see their own address
+	 * space */
+#else
 	/*
 	   BEWARE!! Some dain-bramaged EISA SCUs will allow you to put
 	   the card mem within the region covered by `normal' RAM  !!!
@@ -246,6 +251,7 @@
 		printk("lne390.c: remapped %dkB card memory to virtual address %#lx\n",
 				LNE390_STOP_PG/4, dev->mem_start);
 	}
+#endif /* CONFIG_ALPHA_JENSEN */
 
 	dev->mem_end = dev->rmem_end = dev->mem_start
 		+ (LNE390_STOP_PG - LNE390_START_PG)*256;
@@ -352,7 +358,24 @@
 	unsigned long shmem = dev->mem_start + ((start_page - LNE390_START_PG)<<8);
 
 	count = (count + 3) & ~3;     /* Round up to doubleword */
+#ifndef CONFIG_ALPHA_JENSEN
 	isa_memcpy_toio(shmem, buf, count);
+#else
+	/* The mylex lne390 adapter requires 32bit access (see above) for every
+	 * operation to the shared mem buffer. Since the block buffer is hardly
+	 * aligned to a 32bit boundary isa_memcpy_toio() will use 16bit
+	 * operations to access the buffer ... we must use something else here. */
+
+	const void *from = buf;
+	count -= 4;
+	do {
+		__raw_writel(*(const u16 *)from | (*(const u16 *)(from+2))<<16, (unsigned long) shmem);
+		count -= 4;
+		(unsigned long) shmem += 4;
+		from += 4;
+	} while (count >= 0);
+#endif
+
 }
 
 static int lne390_open(struct net_device *dev)
@@ -377,19 +400,23 @@
 static int io[MAX_LNE_CARDS];
 static int irq[MAX_LNE_CARDS];
 static int mem[MAX_LNE_CARDS];
+static int activate[MAX_LNE_CARDS];
 
 MODULE_PARM(io, "1-" __MODULE_STRING(MAX_LNE_CARDS) "i");
 MODULE_PARM(irq, "1-" __MODULE_STRING(MAX_LNE_CARDS) "i");
 MODULE_PARM(mem, "1-" __MODULE_STRING(MAX_LNE_CARDS) "i");
+MODULE_PARM(activate, "1-" __MODULE_STRING(MAX_LNE_CARDS) "i");
 MODULE_PARM_DESC(io, "I/O base address(es)");
 MODULE_PARM_DESC(irq, "IRQ number(s)");
 MODULE_PARM_DESC(mem, "memory base address(es)");
+MODULE_PARM_DESC(activate, "activate the board(s) to use indicated configuration(s)\n\tAttention: This overwrites the settings of your EISA Configuration!");
 MODULE_DESCRIPTION("Mylex LNE390A/B EISA Ethernet driver");
 MODULE_LICENSE("GPL");
 
 int init_module(void)
 {
-	int this_dev, found = 0;
+	int this_dev, index, found = 0;
+	unsigned char lne390_config;
 
 	for (this_dev = 0; this_dev < MAX_LNE_CARDS; this_dev++) {
 		struct net_device *dev = &dev_lne[this_dev];
@@ -397,6 +424,52 @@
 		dev->base_addr = io[this_dev];
 		dev->mem_start = mem[this_dev];
 		dev->init = lne390_probe;
+
+		/* Activate the adapter, this may overwrite the settings
+		 * done in the EISA Configuration Utility! */
+
+		if (activate[this_dev]) {
+
+			if (inb_p(dev->base_addr + LNE390_ID_PORT) == 0xff) return -ENODEV; 
+			if (dev->mem_start & 0xffff) {
+				printk (KERN_ERR "The lne390 shared mem buffer must be on a 16-bit boundary,\nwill try to initialize 0x%lx instead of 0x%lx\n", dev->mem_start & 0xffff0000, dev->mem_start);
+				dev->mem_start = dev->mem_start & 0xffff0000;
+			}
+			printk("Trying to activate LNE390 card in slot %d with irq %d and shared mem at 0x%lx\n", (int)dev->base_addr>>12, dev->irq, dev->mem_start);
+
+			/* Verify the irq number */
+			if ((dev->irq < 16) && (irq_reverse_map[dev->irq]!=0xff)) lne390_config = irq_reverse_map[dev->irq]<<3;
+			else {
+				printk(KERN_ERR "Sorry, the irq of lne390 cards can not be mapped to %d\n", dev->irq);
+				return -ENXIO;
+			}
+
+			/* Verify shared memory */
+			int revision = (inl(dev->base_addr + LNE390_ID_PORT) >> 24) & 0x01;
+			if (revision) {
+				for (index = 0; (index < 8) && (dev->mem_start >> 16 != shmem_mapB[index]); index++);
+				if (index > 7) {
+					printk (KERN_ERR "The lne390B adapter can not map shared mem at 0x%lx\n", dev->mem_start);
+					return -ENXIO;
+				}
+				else lne390_config = lne390_config | index;
+			}
+			else {
+				for (index = 0; (index < 8) && (dev->mem_start >> 16 != shmem_mapA[index]); index++);
+				if (index > 7) {
+					printk (KERN_ERR "The lne390A adapter can not map shared mem at 0x%lx\n", dev->mem_start);
+					return -ENXIO;
+				}
+				else lne390_config = lne390_config | index;
+			}
+
+			/* Now activate the adapter */
+			outb (0x0, dev->base_addr + LNE390_RESET_PORT);
+			outb (0x1, dev->base_addr + LNE390_RESET_PORT);
+			outb (lne390_config, dev->base_addr + LNE390_CFG2);
+			printk ("lne390 in slot %d has been activated\n", dev->base_addr>>12);
+		}
+
 		/* Default is to only install one card. */
 		if (io[this_dev] == 0 && this_dev != 0) break;
 		if (register_netdev(dev) != 0) {

Re: [PATCH] lne390 and Jensen Alphas

[Marc Zyngier]

Carsten,

+#ifdef CONFIG_ALPHA_JENSEN
+	/* On the Jensen board EISA cards will see their own address
+	 * space */
+#else

[...]

+#ifndef CONFIG_ALPHA_JENSEN
 	isa_memcpy_toio(shmem, buf, count);
+#else
+	/* The mylex lne390 adapter requires 32bit access (see above) for every
+	 * operation to the shared mem buffer. Since the block buffer is hardly
+	 * aligned to a 32bit boundary isa_memcpy_toio() will use 16bit
+	 * operations to access the buffer ... we must use something else here. */
+
+	const void *from = buf;
+	count -= 4;
+	do {
+		__raw_writel(*(const u16 *)from | (*(const u16 *)(from+2))<<16, (unsigned long) shmem);
+		count -= 4;
+		(unsigned long) shmem += 4;
+		from += 4;
+	} while (count >= 0);
+#endif
+

This doesn't look to me like a proper fix. You should unconditionally
use ioremap and memcpy_{from,to}io (not the isa_* version). No need to
have a special Jensen case here.

Please look at the way ne3210.c was fixed in 2.6.0-test7.

	M.
-- 
Places change, faces change. Life is so very strange.

Re: [PATCH] lne390 and Jensen Alphas

[Carsten Jacobi]

On Sun, Oct 12, 2003 at 01:24:30PM +0200, Marc Zyngier wrote:

> This doesn't look to me like a proper fix. You should unconditionally
> use ioremap and memcpy_{from,to}io (not the isa_* version). No need to
> have a special Jensen case here.

Marc,

I wish I could use the memcpy_{from,to}io-functions because I know by
myself that they are well optimized and tested. But in this case they
don't behave the way the lne390 adapter needs them.
If you take a look into arch/alpha/lib/io.c you see that the function
_memcpy_toio() is checking the count of bytes to copy and the address to
copy to against their alignment. In case they are on a 16bit boundary
(and unfortunately they are always on such a boundary because the MAC
header is 14 bytes and the alignment of skb->data is honoring the start
of IP data for alignment to increase the performance of the stack)
_memcpy_toio() will use __raw_writew() to actually copy the data. And
this is exactly what doesn't work: Filling the shared mem of the lne390
adapter using 16bit write operations. I think this problem does not arise
on x86 architectures.
About five years ago I addressed that issue 
(http://www.uwsg.iu.edu/hypermail/linux/kernel/9807.3/1132.html) and got
an answer from Alan Cox
(http://www.uwsg.iu.edu/hypermail/linux/kernel/9807.3/1212.html)

> Please look at the way ne3210.c was fixed in 2.6.0-test7.

Hmm, I see you are using memcpy_{from,to}io-functions only. Five years
ago I sent another patch that established a new function I called
"memcpy_toio_g32a"()
(http://www.ussg.iu.edu/hypermail/linux/kernel/9808.1/0275.html), but that was
the one that never did it into an official release. Indeed, I am also
unwilling to write a dirty hack and use well defined io-functions
instead. This is clean and it is even easier to develop and debug.
Unfortunately .... I start to repeat. Or, is there another way to copy
something from mem to iomem that is definitely using 32bit write
operations? Any help is welcome ...

Regards,
	Carsten Jacobi