Re: Trivial cleanups & 64-bit fixes for donauboe.c

[Jeff Garzik]

Pavel Machek wrote:
> Hi!
>
> donauboe uses __u32; this is kernel code, you are allowed to use u32
> which is less ugly. ASSERT() is pretty ugly. I made it 64-bit clean,
> and if it is outside 32-bit range, it BUG()s. Not ideal, but better
> than not compiling.
[...]
> -#if (BITS_PER_LONG == 64)
> -#error broken on 64-bit:  casts pointer to 32-bit, and then back to pointer.
> -#endif
> -
> -  /*We need to align the taskfile on a taskfile size boundary */
> +  /* We need to align the taskfile on a taskfile size boundary */
>    {
>      unsigned long addr;
>  
> -    addr = (__u32) self->ringbuf;
> +    addr = (unsigned long) self->ringbuf;
>      addr &= ~(OBOE_RING_LEN - 1);
>      addr += OBOE_RING_LEN;
>      self->ring = (struct OboeRing *) addr;
>    }


This driver is awful:

1) it kmalloc's memory for DMA purposes
2) it uses virt_to_bus to map the rx/tx buffers
3) it uses 32-bit hardware field to store a pointer

If you want to clean all that up, and convert it to the PCI DMA API, 
that would be the proper fix for all this.

Making it compile, but leaving the bugs, is worse than the #error I 
added... it just hides the bugs again.  Your patch does not address the 
bugs at all.  (the cleanups are, of course, OK)

	Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/