Skip to contentSkip to conversation

Linux 2.5.40-ac1

DISCUSSION Manfred Spraul 3 participants 8 messages Oct 3, 2002

lore
Share & tools
lore
#0 lore raw
Quoted 1 line

o Fix abuse of set_bit in winbond-840 (me)

Were there bug reports, Have you tested the change?

I think the cpu_to_le32 is wrong.

On big endian computers, the nic is set into a big-endian mode, and it did work with set_bit on my power mac. Unfortunately, I don't have access to it right now.

--
Manfred

#1 lore raw

From: Manfred Spraul <[email protected]>
Date: Thu, 03 Oct 2002 16:45:59 +0200

On big endian computers, the nic is set into a big-endian mode, and it did work with set_bit on my power mac. Unfortunately, I don't have access to it right now.

How about a 64-bit system where set_bit works on 64-bit longs and not 32-bit ones? That is why the current code there is broken.

#2 lore raw
Quoted 4 lines · David S. Miller

How about a 64-bit system where set_bit works on 64-bit longs and not 32-bit ones? That is why the current code there is broken.

There should bit nonatomic bit ops for every byte width.

http://marc.theaimsgroup.com/?l=linux-kernel&m=99167415926343&w=2

I even sent you the patch proposal, but never got a reply.

Patch again attached, but untested.
--
Manfred

include/linux/bitops.h +22 -0
@@ -68,5 +68,27 @@
 
 #include <asm/bitops.h>
 
+#ifdef __KERNEL__
+#include <linux/types.h>
+#define BUILD_SET_BIT(n) \
+static inline void __set_bit_##n(int offset, u##n *data) \
+{ \
+	data[offset/n] |= (1 << (offset%n)); \
+}
+
+#ifndef _HAVE_ARCH_SET_BIT_8
+BUILD_SET_BIT(8)
+#endif
+#ifndef _HAVE_ARCH_SET_BIT_16
+BUILD_SET_BIT(16)
+#endif
+#ifndef _HAVE_ARCH_SET_BIT_32
+BUILD_SET_BIT(32)
+#endif
+#ifndef _HAVE_ARCH_SET_BIT_64
+BUILD_SET_BIT(64)
+#endif
+#undef BUILD_SET_BIT
+#endif
 
 #endif
#3 lore raw

From: Manfred Spraul <[email protected]>
Date: Thu, 03 Oct 2002 17:18:06 +0200

Patch again attached, but untested.

I'm fine with it, just make sure it doesn't give Linus heart burn :-)

#5 lore raw
Quoted 4 lines · Alan Cox
Quoted 2 lines · Manfred Spraul

There should bit nonatomic bit ops for every byte width.

Quoted 8 lines
Quoted 5 lines

http://marc.theaimsgroup.com/?l=linux-kernel&m=99167415926343&w=2

I even sent you the patch proposal, but never got a reply.

Patch again attached, but untested.

What about reverse endianness ?

AFAIK, writeX macros should swap as needed, i.e.

	writel(0x100,ioaddr);

should arrive as bit 8 set on the hardware. [please correct me if I'm wrong] Thus the input into write{b,w,l} should be in host byte order.

	u{8,16,32}	array[];

	__set_bit_{8,16,32}(,array);

	write{b,w,l}(array[],ioaddr);

would achieve that.

#6 lore raw
In reply to Manfred Spraul
Quoted 3 lines · Manfred Spraul
Quoted 1 line

o Fix abuse of set_bit in winbond-840 (me)

Were there bug reports, Have you tested the change?

Yes and no - I don't have an 840

Quoted 5 lines

I think the cpu_to_le32 is wrong.

On big endian computers, the nic is set into a big-endian mode, and it did work with set_bit on my power mac. Unfortunately, I don't have access to it right now.

The checks I did were that it seemed to produce the same data in both cases. So if it was wrong before (using set_bit) its probably wrong now.

#7 lore raw
Quoted 4 lines · Alan Cox

The checks I did were that it seemed to produce the same data in both cases. So if it was wrong before (using set_bit) its probably wrong now.

Which arch?
On ppc, __set_bit() stores in big endian format, i.e. the cpu_to_le32 would be wrong I try to figure out what set_bit() does.

--
Manfred