2006/8/16, Antonino A. Daplas <[EMAIL PROTECTED]>:
> 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);
>
(a) if (len & 3) != 0
When former data length is 4 or less, struct sisusb_packet is used.
It becomes 10 in length for writing.
Because data is not used, it becomes 6 in length for reading.
struct sisusb_packet {
unsigned short header;
u32 address;
u32 data;
} __attribute__((__packed__));
(b) if (len & 3) == 0
When former data length is larger than 4,
it corresponds to this case because the length of (data & ~3) is passed.
As for the data of the remainder, the logic of (a) is used.
> it simply condenses into
>
> sisusb_memcpy().
>
>
yes.
I also think that it is good.
> > +
> > 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.
Thank you for your advice. I understand.
--
Mitsuhiro KOGA
<[EMAIL PROTECTED]>
-------------------------------------------------------------------------
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
