On Wed, 2006-08-16 at 20:31 +0900, Mitsuhiro KOGA wrote:
> Hi,
>
> 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().
> +
> 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
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
