* [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
@ 2006-08-13 20:47 Mitsuhiro KOGA
2006-08-15 0:09 ` Antonino A. Daplas
0 siblings, 1 reply; 9+ messages in thread
From: Mitsuhiro KOGA @ 2006-08-13 20:47 UTC (permalink / raw)
To: akpm; +Cc: linux-fbdev-devel, linux-usb-devel
I had a problem with usb2vga and ppc linux.
The background of the console greens, and the font is not displayed.
This patch can be normally displayed by the Big endian machine.
Moreover, because KAIREN's usb2vga has two or more devices id, it adds it.
Signed-off-by: Mitsuhiro Koga <shiena.jp@gmail.com>
Cc: linux-usb-devel@lists.sourceforge.net
Cc: linux-fbdev-devel@lists.sourceforge.net
---
drivers/usb/misc/sisusbvga/sisusb.c | 27 ++++++++++++++++++++++++++-
1 files changed, 26 insertions(+), 1 deletions(-)
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -76,6 +76,16 @@
DEFINE_MUTEX(disconnect_mutex);
+#ifdef __BIG_ENDIAN
+static void
+correct_endianness_buffer(u16* data, const int length)
+{
+ int i;
+ for (i = 0; i < length; i++)
+ *(data+i) = cpu_to_le16(*(data+i));
+}
+#endif
+
static void
sisusb_free_buffers(struct sisusb_usb_data *sisusb)
{
@@ -411,6 +421,10 @@
memcpy(buffer, kernbuffer, passsize);
kernbuffer += passsize;
+#ifdef __BIG_ENDIAN
+ if ((len & 3) == 0)
+ correct_endianness_buffer((u16 *)buffer, thispass/2);
+#endif
}
retry = 5;
@@ -906,9 +920,13 @@
if (userbuffer) {
if (get_user(swap32, (u32 __user *)userbuffer))
return -EFAULT;
- } else
+ } else {
swap32 = *((u32 *)kernbuffer);
+#ifdef __BIG_ENDIAN
+ swap32 = ((swap32 << 16) & 0xffff0000) | ((swap32 >>
16) & 0xffff);
+#endif
+ }
ret = sisusb_write_memio_long(sisusb,
SISUSB_TYPE_MEM,
addr,
@@ -1259,7 +1277,11 @@
userbuffer += 4;
} else {
+#ifdef __BIG_ENDIAN
+ *((u32 *)kernbuffer) = ((swap32 << 16) &
0xffff0000) | ((swap32 >> 16) & 0xffff);
+#else
*((u32 *)kernbuffer) = swap32;
+#endif
kernbuffer += 4;
}
addr += 4;
@@ -3435,6 +3457,9 @@
static struct usb_device_id sisusb_table [] = {
{ USB_DEVICE(0x0711, 0x0900) },
+ { USB_DEVICE(0x0711, 0x0901) },
+ { USB_DEVICE(0x0711, 0x0902) },
+ { USB_DEVICE(0x0711, 0x0920) },
{ USB_DEVICE(0x182d, 0x021c) },
{ USB_DEVICE(0x182d, 0x0269) },
{ }
-------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
@ 2006-08-13 21:13 Mitsuhiro KOGA
0 siblings, 0 replies; 9+ messages in thread
From: Mitsuhiro KOGA @ 2006-08-13 21:13 UTC (permalink / raw)
To: linux-usb-devel, linux-fbdev-devel
I had a problem with usb2vga and ppc linux.
The background of the console greens, and the font is not displayed.
This patch can be normally displayed by the Big endian machine.
Moreover, because KAIREN's usb2vga has two or more devices id, it adds it.
Signed-off-by: Mitsuhiro Koga <shiena.jp@gmail.com>
Cc: linux-usb-devel@lists.sourceforge.net
Cc: linux-fbdev-devel@lists.sourceforge.net
---
drivers/usb/misc/sisusbvga/sisusb.c | 27 ++++++++++++++++++++++++++-
1 files changed, 26 insertions(+), 1 deletions(-)
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -76,6 +76,16 @@
DEFINE_MUTEX(disconnect_mutex);
+#ifdef __BIG_ENDIAN
+static void
+correct_endianness_buffer(u16* data, const int length)
+{
+ int i;
+ for (i = 0; i < length; i++)
+ *(data+i) = cpu_to_le16(*(data+i));
+}
+#endif
+
static void
sisusb_free_buffers(struct sisusb_usb_data *sisusb)
{
@@ -411,6 +421,10 @@
memcpy(buffer, kernbuffer, passsize);
kernbuffer += passsize;
+#ifdef __BIG_ENDIAN
+ if ((len & 3) == 0)
+ correct_endianness_buffer((u16 *)buffer, thispass/2);
+#endif
}
retry = 5;
@@ -906,9 +920,13 @@
if (userbuffer) {
if (get_user(swap32, (u32 __user *)userbuffer))
return -EFAULT;
- } else
+ } else {
swap32 = *((u32 *)kernbuffer);
+#ifdef __BIG_ENDIAN
+ swap32 = ((swap32 << 16) & 0xffff0000) | ((swap32 >>
16) & 0xffff);
+#endif
+ }
ret = sisusb_write_memio_long(sisusb,
SISUSB_TYPE_MEM,
addr,
@@ -1259,7 +1277,11 @@
userbuffer += 4;
} else {
+#ifdef __BIG_ENDIAN
+ *((u32 *)kernbuffer) = ((swap32 << 16) &
0xffff0000) | ((swap32 >> 16) & 0xffff);
+#else
*((u32 *)kernbuffer) = swap32;
+#endif
kernbuffer += 4;
}
addr += 4;
@@ -3435,6 +3457,9 @@
static struct usb_device_id sisusb_table [] = {
{ USB_DEVICE(0x0711, 0x0900) },
+ { USB_DEVICE(0x0711, 0x0901) },
+ { USB_DEVICE(0x0711, 0x0902) },
+ { USB_DEVICE(0x0711, 0x0920) },
{ USB_DEVICE(0x182d, 0x021c) },
{ USB_DEVICE(0x182d, 0x0269) },
{ }
-------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
2006-08-13 20:47 [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c Mitsuhiro KOGA
@ 2006-08-15 0:09 ` Antonino A. Daplas
2006-08-16 11:31 ` Mitsuhiro KOGA
0 siblings, 1 reply; 9+ messages in thread
From: Antonino A. Daplas @ 2006-08-15 0:09 UTC (permalink / raw)
To: linux-fbdev-devel; +Cc: akpm, thomas, linux-usb-devel
On Mon, 2006-08-14 at 05:47 +0900, Mitsuhiro KOGA wrote:
Added the author to the CC list.
It would be better if you remove the #ifdef within the functions.
So something like...
> I had a problem with usb2vga and ppc linux.
> The background of the console greens, and the font is not displayed.
>
> This patch can be normally displayed by the Big endian machine.
> Moreover, because KAIREN's usb2vga has two or more devices id, it adds it.
>
> Signed-off-by: Mitsuhiro Koga <shiena.jp@gmail.com>
> Cc: linux-usb-devel@lists.sourceforge.net
> Cc: linux-fbdev-devel@lists.sourceforge.net
> ---
> drivers/usb/misc/sisusbvga/sisusb.c | 27 ++++++++++++++++++++++++++-
> 1 files changed, 26 insertions(+), 1 deletions(-)
> --- a/drivers/usb/misc/sisusbvga/sisusb.c
> +++ b/drivers/usb/misc/sisusbvga/sisusb.c
> @@ -76,6 +76,16 @@
>
> DEFINE_MUTEX(disconnect_mutex);
>
> +#ifdef __BIG_ENDIAN
> +static void
> +correct_endianness_buffer(u16* data, const int length)
> +{
> + int i;
> + for (i = 0; i < length; i++)
> + *(data+i) = cpu_to_le16(*(data+i));
> +}
> +#endif
> +
> static void
> sisusb_free_buffers(struct sisusb_usb_data *sisusb)
> {
> @@ -411,6 +421,10 @@
> memcpy(buffer, kernbuffer, passsize);
> kernbuffer += passsize;
>
> +#ifdef __BIG_ENDIAN
> + if ((len & 3) == 0)
> + correct_endianness_buffer((u16 *)buffer, thispass/2);
> +#endif
> }
>
#ifdef __LITTLE_ENDIAN
static inline void sisusb_memcpy(void *d, void *s, int count)
{
memcpy(d, s, count);
}
#else
static inline void sisusb_memcpy(void *d, void *s, int count)
{
memcpy_and_reverse(d, s, count);
}
#endif
> retry = 5;
> @@ -906,9 +920,13 @@
> if (userbuffer) {
> if (get_user(swap32, (u32 __user *)userbuffer))
> return -EFAULT;
> - } else
> + } else {
> swap32 = *((u32 *)kernbuffer);
>
> +#ifdef __BIG_ENDIAN
> + swap32 = ((swap32 << 16) & 0xffff0000) | ((swap32 >>
> 16) & 0xffff);
> +#endif
> + }
> ret = sisusb_write_memio_long(sisusb,
> SISUSB_TYPE_MEM,
> addr,
> @@ -1259,7 +1277,11 @@
>
> userbuffer += 4;
> } else {
> +#ifdef __BIG_ENDIAN
> + *((u32 *)kernbuffer) = ((swap32 << 16) &
> 0xffff0000) | ((swap32 >> 16) & 0xffff);
> +#else
> *((u32 *)kernbuffer) = swap32;
> +#endif
> kernbuffer += 4;
> }
And for the above 2, something like...
#ifdef __BIG_ENDIAN
static inline void sisusb_reverse(u32 *s, u32 *d)
{
*d = ((*s << 16) & 0xffff0000) | ((*s >> 16) & 0xffff);
}
#else
static inline void sisiusb_reverse(u32 *s, u32 *d)
{
*d = *s;
}
#endif
Tony
> addr += 4;
> @@ -3435,6 +3457,9 @@
>
> static struct usb_device_id sisusb_table [] = {
> { USB_DEVICE(0x0711, 0x0900) },
> + { USB_DEVICE(0x0711, 0x0901) },
> + { USB_DEVICE(0x0711, 0x0902) },
> + { USB_DEVICE(0x0711, 0x0920) },
> { USB_DEVICE(0x182d, 0x021c) },
> { USB_DEVICE(0x182d, 0x0269) },
> { }
>
> -------------------------------------------------------------------------
> 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-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
-------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
2006-08-15 0:09 ` Antonino A. Daplas
@ 2006-08-16 11:31 ` Mitsuhiro KOGA
2006-08-16 11:41 ` [Linux-fbdev-devel] " Geert Uytterhoeven
2006-08-16 12:56 ` Antonino A. Daplas
0 siblings, 2 replies; 9+ messages in thread
From: Mitsuhiro KOGA @ 2006-08-16 11:31 UTC (permalink / raw)
To: Antonino A. Daplas; +Cc: akpm, thomas, linux-fbdev-devel, linux-usb-devel
Hi,
> It would be better if you remove the #ifdef within the functions.
> So something like...
>
<snip>
> #ifdef __LITTLE_ENDIAN
> static inline void sisusb_memcpy(void *d, void *s, int count)
> {
> memcpy(d, s, count);
> }
> #else
> static inline void sisusb_memcpy(void *d, void *s, int count)
> {
> memcpy_and_reverse(d, s, count);
> }
> #endif
>
<snip>
>
> And for the above 2, something like...
>
> #ifdef __BIG_ENDIAN
> static inline void sisusb_reverse(u32 *s, u32 *d)
> {
> *d = ((*s << 16) & 0xffff0000) | ((*s >> 16) & 0xffff);
> }
> #else
> static inline void sisiusb_reverse(u32 *s, u32 *d)
> {
> *d = *s;
> }
> #endif
Thanks.
I re-fixed the patch according to your advice.
And, because it's used #ifdef in other part, I also fixed it.
Would you please check it?
Signed-off-by: Mitsuhiro Koga <shiena.jp@gmail.com>
---
diff -urp a/drivers/usb/misc/sisusbvga/sisusb.c
b/drivers/usb/misc/sisusbvga/sisusb.c
--- a/drivers/usb/misc/sisusbvga/sisusb.c 2006-08-07 03:20:11.000000000 +0900
+++ b/drivers/usb/misc/sisusbvga/sisusb.c 2006-08-16 15:53:59.000000000 +0900
@@ -76,6 +76,63 @@ static struct usb_driver sisusb_driver;
DEFINE_MUTEX(disconnect_mutex);
+#ifdef __BIG_ENDIAN
+static inline void sisusb_memcpy(u16 *dst, u16 *src, int count)
+{
+ while (count--)
+ *(dst++) = cpu_to_le16(*(src++));
+}
+#else
+static inline void sisusb_memcpy(u16 *dst, u16 *src, int count)
+{
+ memcpy(dst, src, count);
+}
+#endif
+
+#ifdef __BIG_ENDIAN
+static inline void sisusb_order_wmem_24bit(char *src, u32 *dst)
+{
+ *dst = (*src << 16) |
+ (*(src + 1) << 8) |
+ *(src + 2);
+}
+#else
+static inline void sisusb_order_wmem_24bit(char *src, u32 *dst)
+{
+ *dst = (*(src + 2) << 16) |
+ (*(src + 1) << 8) |
+ *src;
+}
+#endif
+
+#ifdef __BIG_ENDIAN
+static inline void sisusb_order_rmem_24bit(u32 *src, char *dst)
+{
+ *dst = (*src >> 16) & 0xff;
+ *(dst + 1) = (*src >> 8) & 0xff;
+ *(dst + 2) = *src & 0xff;
+}
+#else
+static inline void sisusb_order_rmem_24bit(u32 *src, char *dst)
+{
+ *(dst + 2) = (*src >> 16) & 0xff;
+ *(dst + 1) = (*src >> 8) & 0xff;
+ *dst = *src & 0xff;
+}
+#endif
+
+#ifdef __BIG_ENDIAN
+static inline void sisusb_order_mem_32bit(u32 *src, u32 *dst)
+{
+ *dst = ((*src << 16) & 0xffff0000) | ((*src >> 16) & 0xffff);
+}
+#else
+static inline void sisusb_order_mem_32bit(u32 *src, u32 *dst)
+{
+ *dst = *src;
+}
+#endif
+
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);
+
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
static struct usb_device_id sisusb_table [] = {
{ USB_DEVICE(0x0711, 0x0900) },
+ { USB_DEVICE(0x0711, 0x0901) },
+ { USB_DEVICE(0x0711, 0x0902) },
+ { USB_DEVICE(0x0711, 0x0920) },
{ USB_DEVICE(0x182d, 0x021c) },
{ USB_DEVICE(0x182d, 0x0269) },
{ }
-------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Linux-fbdev-devel] [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
2006-08-16 11:31 ` Mitsuhiro KOGA
@ 2006-08-16 11:41 ` Geert Uytterhoeven
2006-08-16 12:06 ` Mitsuhiro KOGA
2006-08-16 12:56 ` Antonino A. Daplas
1 sibling, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2006-08-16 11:41 UTC (permalink / raw)
To: Linux Frame Buffer Device Development
Cc: Andrew Morton, thomas, linux-usb-devel, Antonino A. Daplas
On Wed, 16 Aug 2006, Mitsuhiro KOGA wrote:
> @@ -76,6 +76,63 @@ static struct usb_driver sisusb_driver;
>
> DEFINE_MUTEX(disconnect_mutex);
>
> +#ifdef __BIG_ENDIAN
> +static inline void sisusb_memcpy(u16 *dst, u16 *src, int count)
> +{
> + while (count--)
> + *(dst++) = cpu_to_le16(*(src++));
> +}
> +#else
> +static inline void sisusb_memcpy(u16 *dst, u16 *src, int count)
> +{
> + memcpy(dst, src, count);
> +}
> +#endif
> +
> +#ifdef __BIG_ENDIAN
> +static inline void sisusb_order_wmem_24bit(char *src, u32 *dst)
> +{
> + *dst = (*src << 16) |
> + (*(src + 1) << 8) |
> + *(src + 2);
> +}
> +#else
> +static inline void sisusb_order_wmem_24bit(char *src, u32 *dst)
> +{
> + *dst = (*(src + 2) << 16) |
> + (*(src + 1) << 8) |
> + *src;
> +}
> +#endif
> +
> +#ifdef __BIG_ENDIAN
> +static inline void sisusb_order_rmem_24bit(u32 *src, char *dst)
> +{
> + *dst = (*src >> 16) & 0xff;
> + *(dst + 1) = (*src >> 8) & 0xff;
> + *(dst + 2) = *src & 0xff;
> +}
> +#else
> +static inline void sisusb_order_rmem_24bit(u32 *src, char *dst)
> +{
> + *(dst + 2) = (*src >> 16) & 0xff;
> + *(dst + 1) = (*src >> 8) & 0xff;
> + *dst = *src & 0xff;
> +}
> +#endif
> +
> +#ifdef __BIG_ENDIAN
> +static inline void sisusb_order_mem_32bit(u32 *src, u32 *dst)
> +{
> + *dst = ((*src << 16) & 0xffff0000) | ((*src >> 16) & 0xffff);
> +}
> +#else
> +static inline void sisusb_order_mem_32bit(u32 *src, u32 *dst)
> +{
> + *dst = *src;
> +}
> +#endif
> +
> static void
> sisusb_free_buffers(struct sisusb_usb_data *sisusb)
> {
Why not merge the #ifdefs, so you need only one?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
-------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Linux-fbdev-devel] [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
2006-08-16 11:41 ` [Linux-fbdev-devel] " Geert Uytterhoeven
@ 2006-08-16 12:06 ` Mitsuhiro KOGA
0 siblings, 0 replies; 9+ messages in thread
From: Mitsuhiro KOGA @ 2006-08-16 12:06 UTC (permalink / raw)
To: linux-fbdev-devel
Cc: Andrew Morton, thomas, linux-usb-devel, Antonino A. Daplas
2006/8/16, Geert Uytterhoeven <geert@linux-m68k.org>:
> On Wed, 16 Aug 2006, Mitsuhiro KOGA wrote:
> > @@ -76,6 +76,63 @@ static struct usb_driver sisusb_driver;
> >
> > DEFINE_MUTEX(disconnect_mutex);
> >
> > +#ifdef __BIG_ENDIAN
> > +static inline void sisusb_memcpy(u16 *dst, u16 *src, int count)
> > +{
> > + while (count--)
> > + *(dst++) = cpu_to_le16(*(src++));
> > +}
> > +#else
> > +static inline void sisusb_memcpy(u16 *dst, u16 *src, int count)
> > +{
> > + memcpy(dst, src, count);
> > +}
> > +#endif
> > +
> > +#ifdef __BIG_ENDIAN
> > +static inline void sisusb_order_wmem_24bit(char *src, u32 *dst)
> > +{
> > + *dst = (*src << 16) |
> > + (*(src + 1) << 8) |
> > + *(src + 2);
> > +}
> > +#else
> > +static inline void sisusb_order_wmem_24bit(char *src, u32 *dst)
> > +{
> > + *dst = (*(src + 2) << 16) |
> > + (*(src + 1) << 8) |
> > + *src;
> > +}
> > +#endif
> > +
> > +#ifdef __BIG_ENDIAN
> > +static inline void sisusb_order_rmem_24bit(u32 *src, char *dst)
> > +{
> > + *dst = (*src >> 16) & 0xff;
> > + *(dst + 1) = (*src >> 8) & 0xff;
> > + *(dst + 2) = *src & 0xff;
> > +}
> > +#else
> > +static inline void sisusb_order_rmem_24bit(u32 *src, char *dst)
> > +{
> > + *(dst + 2) = (*src >> 16) & 0xff;
> > + *(dst + 1) = (*src >> 8) & 0xff;
> > + *dst = *src & 0xff;
> > +}
> > +#endif
> > +
> > +#ifdef __BIG_ENDIAN
> > +static inline void sisusb_order_mem_32bit(u32 *src, u32 *dst)
> > +{
> > + *dst = ((*src << 16) & 0xffff0000) | ((*src >> 16) & 0xffff);
> > +}
> > +#else
> > +static inline void sisusb_order_mem_32bit(u32 *src, u32 *dst)
> > +{
> > + *dst = *src;
> > +}
> > +#endif
> > +
> > static void
> > sisusb_free_buffers(struct sisusb_usb_data *sisusb)
> > {
>
> Why not merge the #ifdefs, so you need only one?
>
I think it is because legibly two functions of the same name queues up.
Of course, both are not cared about either.
--
Mitsuhiro KOGA
<shiena.jp@gmail.com>
-------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
2006-08-16 11:31 ` Mitsuhiro KOGA
2006-08-16 11:41 ` [Linux-fbdev-devel] " Geert Uytterhoeven
@ 2006-08-16 12:56 ` Antonino A. Daplas
2006-08-16 15:33 ` Mitsuhiro KOGA
1 sibling, 1 reply; 9+ messages in thread
From: Antonino A. Daplas @ 2006-08-16 12:56 UTC (permalink / raw)
To: Mitsuhiro KOGA; +Cc: akpm, thomas, linux-fbdev-devel, linux-usb-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
2006-08-16 12:56 ` Antonino A. Daplas
@ 2006-08-16 15:33 ` Mitsuhiro KOGA
2006-08-24 6:34 ` Mitsuhiro KOGA
0 siblings, 1 reply; 9+ messages in thread
From: Mitsuhiro KOGA @ 2006-08-16 15:33 UTC (permalink / raw)
To: Antonino A. Daplas; +Cc: akpm, thomas, linux-fbdev-devel, linux-usb-devel
2006/8/16, Antonino A. Daplas <adaplas@gmail.com>:
> 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
<shiena.jp@gmail.com>
-------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c
2006-08-16 15:33 ` Mitsuhiro KOGA
@ 2006-08-24 6:34 ` Mitsuhiro KOGA
0 siblings, 0 replies; 9+ messages in thread
From: Mitsuhiro KOGA @ 2006-08-24 6:34 UTC (permalink / raw)
To: Antonino A. Daplas; +Cc: akpm, thomas, linux-fbdev-devel, linux-usb-devel
I'm sorry. Because the mistake and lack were found in the explanation,
a postscript is added to it.
2006/8/17, Mitsuhiro KOGA <shiena.jp@gmail.com>:
> 2006/8/16, Antonino A. Daplas <adaplas@gmail.com>:
> > 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.
The length passed here is not (data & ~3) but (former data length & ~3).
And, variable buffer becomes a character string displayed in the monitor
a couple of byte sequence by two bytes (the character-code and the delimiter).
Because this order reverses for the Big endian, variable buffer is
being replaced.
In the case of (a), sisusb_pack->data is being replaced with
cpu_to_le32 in existing processing,
and because the order of the character reverses for four bytes, data
is being replaced
with sisusb_order_mem_32 further.
Regards,
--
Mitsuhiro KOGA
<shiena.jp@gmail.com>
-------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2006-08-24 6:34 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-13 20:47 [PATCH 2.6.18-rc4] sisusbvga: fix sisusb.c Mitsuhiro KOGA
2006-08-15 0:09 ` Antonino A. Daplas
2006-08-16 11:31 ` Mitsuhiro KOGA
2006-08-16 11:41 ` [Linux-fbdev-devel] " Geert Uytterhoeven
2006-08-16 12:06 ` Mitsuhiro KOGA
2006-08-16 12:56 ` Antonino A. Daplas
2006-08-16 15:33 ` Mitsuhiro KOGA
2006-08-24 6:34 ` Mitsuhiro KOGA
-- strict thread matches above, loose matches on Subject: below --
2006-08-13 21:13 Mitsuhiro KOGA
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).