Re: [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
From: "Antonino A. Daplas" <adaplas@gmail.com>
Date: 2006-08-16 13:00:54
On Wed, 2006-08-16 at 20:31 +0900, Mitsuhiro KOGA wrote:
Hi,
quoted hunk ↗ jump to hunk
static void sisusb_free_buffers(struct sisusb_usb_data *sisusb) {@@ -408,7 +465,11 @@ static int sisusb_send_bulk_msg(struct s } else if (fromkern) { - memcpy(buffer, kernbuffer, passsize); + if ((len & 3) == 0) + sisusb_memcpy((u16 *)buffer, (u16 *)kernbuffer, passsize / 2); + else + memcpy(buffer, kernbuffer, passsize);
What happens if (len & 3) != 0? It ends up doing a regular memcpy()
without the reordering. Better if you move the if (len & 3) == 0 check
to sisusb_memcpy(), so instead of
if ((len & 3) == 0)
sisusb_memcpy((u16 *)buffer, (u16 )kernbuffer, passsize / 2);
else
memcpy(buffer, kernbuffer, passsize);
it simply condenses into
sisusb_memcpy().
quoted hunk ↗ jump to hunk
+ kernbuffer += passsize; }@@ -872,25 +933,9 @@ static int sisusb_write_mem_bulk(struct if (userbuffer) { if (copy_from_user(&buf, userbuffer, 3)) return -EFAULT; -#ifdef __BIG_ENDIAN - swap32 = (buf[0] << 16) | - (buf[1] << 8) | - buf[2]; -#else - swap32 = (buf[2] << 16) | - (buf[1] << 8) | - buf[0]; -#endif + sisusb_order_wmem_24bit(&buf, &swap32); } else -#ifdef __BIG_ENDIAN - swap32 = (kernbuffer[0] << 16) | - (kernbuffer[1] << 8) | - kernbuffer[2]; -#else - swap32 = (kernbuffer[2] << 16) | - (kernbuffer[1] << 8) | - kernbuffer[0]; -#endif + sisusb_order_wmem_24bit(kernbuffer, &swap32); ret = sisusb_write_memio_24bit(sisusb, SISUSB_TYPE_MEM,@@ -907,7 +952,7 @@ static int sisusb_write_mem_bulk(struct if (get_user(swap32, (u32 __user *)userbuffer)) return -EFAULT; } else - swap32 = *((u32 *)kernbuffer); + sisusb_order_mem_32bit((u32 *)kernbuffer, &swap32); ret = sisusb_write_memio_long(sisusb, SISUSB_TYPE_MEM,@@ -1227,15 +1272,7 @@ static int sisusb_read_mem_bulk(struct s addr, &swap32); if (!ret) { (*bytes_read) += 3; -#ifdef __BIG_ENDIAN - buf[0] = (swap32 >> 16) & 0xff; - buf[1] = (swap32 >> 8) & 0xff; - buf[2] = swap32 & 0xff; -#else - buf[2] = (swap32 >> 16) & 0xff; - buf[1] = (swap32 >> 8) & 0xff; - buf[0] = swap32 & 0xff; -#endif + sisusb_order_rmem_24bit(&swap32, &buf[0]); if (userbuffer) { if (copy_to_user(userbuffer, &buf[0], 3)) return -EFAULT;@@ -1259,7 +1296,7 @@ static int sisusb_read_mem_bulk(struct s userbuffer += 4; } else { - *((u32 *)kernbuffer) = swap32; + sisusb_order_mem_32bit((u32 *)kernbuffer, &swap32); kernbuffer += 4; } addr += 4;@@ -3435,6 +3472,9 @@ static void sisusb_disconnect(struct usb
I would agree with Geert. Combine all into a single #ifdef/#else/#endif. Since we are still in the reviewing phase, do not submit incremental patches. Tony ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642