public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: kernel list <linux-kernel@vger.kernel.org>,
	irda-users@lists.sourceforge.net, Andrew Morton <akpm@osdl.org>
Subject: Re: Trivial cleanups & 64-bit fixes for donauboe.c
Date: Tue, 13 Jul 2004 18:07:24 -0400	[thread overview]
Message-ID: <40F45D1C.8060005@pobox.com> (raw)
In-Reply-To: <20040713212156.GA2971@elf.ucw.cz>

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



      reply	other threads:[~2004-07-13 22:07 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-07-13 21:21 Trivial cleanups & 64-bit fixes for donauboe.c Pavel Machek
2004-07-13 22:07 ` Jeff Garzik [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=40F45D1C.8060005@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=akpm@osdl.org \
    --cc=irda-users@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pavel@ucw.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox