Skip to contentSkip to conversation

Trivial cleanups & 64-bit fixes for donauboe.c

PATCH Pavel Machek 2 participants 2 messages Jul 13, 2004 Networking

lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

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.

Index: drivers/net/irda/donauboe.c
===================================================================
RCS file: /home/pavel/sf/bitbucket/bkcvs/linux-2.5/drivers/net/irda/donauboe.c,v retrieving revision 1.19

net/irda/donauboe.c +8 -13
diff -u -r1.19 donauboe.c
@@ -28,6 +28,7 @@
  * Modified: 2.17 jeu sep 12 08:50:20 2002 (save_flags();cli(); replaced by spinlocks)
  * Modified: 2.18 Christian Gennerat <[email protected]>
  * Modified: 2.18 ven jan 10 03:14:16 2003 Change probe default options
+ * Modified: 2.19 Pavel Machek <[email protected]>
  *
  *     Copyright (c) 1999 James McKenzie, All Rights Reserved.
  *

@@ -247,7 +248,7 @@
 static void
 toshoboe_dumpregs (struct toshoboe_cb *self)
 {
-  __u32 ringbase;
+  u32 ringbase;
 
   IRDA_DEBUG (4, "%s()\n", __FUNCTION__);
 

@@ -552,7 +553,7 @@
 static void
 toshoboe_startchip (struct toshoboe_cb *self)
 {
-  __u32 physaddr;
+  unsigned long physaddr;
 
   IRDA_DEBUG (4, "%s()\n", __FUNCTION__);
 

@@ -587,9 +588,7 @@
   /*Find out where the rings live */
   physaddr = virt_to_bus (self->ring);
 
-  ASSERT ((physaddr & 0x3ff) == 0,
-          printk (KERN_ERR DRIVER_NAME "ring not correctly aligned\n");
-          return;);
+  BUG_ON(physaddr & 0xffffffff000003ff);
 
   OUTB ((physaddr >> 10) & 0xff, OBOE_RING_BASE0);
   OUTB ((physaddr >> 18) & 0xff, OBOE_RING_BASE1);

@@ -601,7 +600,7 @@
   /* Start up the clocks */
   OUTB (OBOE_ENABLEH_PHYANDCLOCK, OBOE_ENABLEH);
 
-  /*set to sensible speed */
+  /* Set to sensible speed */
   self->speed = 9600;
   toshoboe_setbaud (self);
   toshoboe_initptrs (self);

@@ -1622,22 +1621,18 @@
       goto freeregion;
     }
 
-#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;
   }
 
   memset (self->ring, 0, OBOE_RING_LEN);
-  self->io.mem_base = (__u32) self->ring;
+  self->io.mem_base = (unsigned long) self->ring;
 
   ok = 1;
   for (i = 0; i < TX_SLOTS; ++i)

Index: drivers/net/irda/donauboe.h
===================================================================
RCS file: /home/pavel/sf/bitbucket/bkcvs/linux-2.5/drivers/net/irda/donauboe.h,v retrieving revision 1.2

net/irda/donauboe.h +6 -7
diff -u -r1.2 donauboe.h
@@ -268,12 +268,11 @@
 
 struct OboeSlot
 {
-  __u16 len;                    /*Tweleve bits of packet length */
-  __u8 unused;
-  __u8 control;                 /*Slot control/status see below */
-  __u32 address;                /*Slot buffer address */
-}
-__attribute__ ((packed));
+  u16 len;                    /*Tweleve bits of packet length */
+  u8 unused;
+  u8 control;                 /*Slot control/status see below */
+  u32 address;                /*Slot buffer address */
+} __attribute__ ((packed));
 
 #define OBOE_NTASKS OBOE_TXRING_OFFSET_IN_SLOTS
 

@@ -316,7 +315,7 @@
   chipio_t io;                  /* IrDA controller information */
   struct qos_info qos;          /* QoS capabilities for this device */
 
-  __u32 flags;                  /* Interface flags */
+  u32 flags;                  /* Interface flags */
 
   struct pci_dev *pdev;         /*PCI device */
   int base;                     /*IO base */
Signature · 2 lines
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
#1 lore raw
Quoted 6 lines · Pavel Machek

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.

[...]

Quoted 5 lines
net/irda/donauboe.c +0 -5
-#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 */

net/irda/donauboe.c line 1632

Quoted 10 lines
net/irda/donauboe.c +2 -1
+  /* 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