Re: [PATCH] lne390 and Jensen Alphas

Previous thread: [PATCH] [NETFILTER] xt_TCPMSS: combine nearly identical reverse_mtu4/6 by Ilpo Järvinen on Saturday, March 29, 2008 - 9:25 am. (3 messages)

Next thread: Re: linux-next: Tree for March 29 by Haavard Skinnemoen on Saturday, March 29, 2008 - 2:12 pm. (3 messages)
To: <netdev@...>
Date: Saturday, March 29, 2008 - 10:48 am

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

To: Carsten Jacobi <carsten@...>
Cc: <netdev@...>
Date: Saturday, March 29, 2008 - 2:32 pm

Hi Carsten,

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
------------------------------
--

To: Florian Fainelli <florian.fainelli@...>
Cc: <netdev@...>
Date: Sunday, April 13, 2008 - 5:06 pm

Hello Florian,

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

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

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 ...

To: Carsten Jacobi <carsten@...>
Cc: Florian Fainelli <florian.fainelli@...>, <netdev@...>
Date: Sunday, April 13, 2008 - 6:11 pm

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;
...

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"?
--

To: Al Viro <viro@...>
Cc: Florian Fainelli <florian.fainelli@...>, <netdev@...>
Date: Sunday, April 20, 2008 - 8:54 am

Hello,

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

Tststs ... such strong words. But I see your point. As I look at it
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
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-br...

Previous thread: [PATCH] [NETFILTER] xt_TCPMSS: combine nearly identical reverse_mtu4/6 by Ilpo Järvinen on Saturday, March 29, 2008 - 9:25 am. (3 messages)

Next thread: Re: linux-next: Tree for March 29 by Haavard Skinnemoen on Saturday, March 29, 2008 - 2:12 pm. (3 messages)