On Thu, Feb 13, 2014 at 03:49:55PM +0000, Ludovic wrote:

> I've created a little libusb perl script to debug the device, and found that
> I could simulate the MIPS bug if I would send a read_download_mem address in
> LE format instead of BE, e.g.:
> 
>   $dev->control_msg((64 + 0 + 128), 0x92 , 0x82, 0x200, $bytes, 4, 1000);
>   => OK returns 01 09 00 b0
>   
>   $dev->control_msg((64 + 0 + 128), 0x92 , 0x82, 0x0002, $bytes, 4, 1000);
>   => BAD returns 06 e4 c9 fb
> 
> In the io_ti driver, in read_download_mem(), we have:
> 
>       be_start_address = cpu_to_be16(start_address);
>       status = ti_vread_sync(dev, UMPC_MEMORY_READ,
>                                (__u16)address_type,
>                                (__force __u16)be_start_address,
>                          buffer, read_length);
>       
>       
> Of course cpu_to_be16() has no effect on MIPS BE, and that's the cause of
> the bug.
> I don't know if it should be replaced with a swab16() or something else, I'm
> not a kernel expert :) and there are a few other lines to fix.
> 
> Would could fix this bug, or would give me some advice on how to fix it?

I believe you are right. We cannot use cpu_to_be16 as the wIndex
parameter holding the address is always sent in little-endian byte
order.

Care to test the patch below (against usb-next) on both your LE and BE
machines?

Thanks,
Johan


>From c0a8ce2d0565569a6d8849340111f237eafc1cc2 Mon Sep 17 00:00:00 2001
From: Johan Hovold <jhov...@gmail.com>
Date: Mon, 17 Feb 2014 10:38:43 +0100
Subject: [PATCH] USB: io_ti: fix firmware download on big-endian machines

During firmware download the device expects memory addresses in
big-endian byte order. As the wIndex parameter which hold the address is
sent in little-endian byte order regardless of host byte order, we need
to use swab16 rather than cpu_to_be16.

Reported-by: Ludovic <ldro...@debian.org>
Cc: sta...@vger.kernel.org
---
 drivers/usb/serial/io_ti.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index a2db5be9c305..e9d200400550 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -28,6 +28,7 @@
 #include <linux/spinlock.h>
 #include <linux/mutex.h>
 #include <linux/serial.h>
+#include <linux/swab.h>
 #include <linux/kfifo.h>
 #include <linux/ioctl.h>
 #include <linux/firmware.h>
@@ -280,7 +281,7 @@ static int read_download_mem(struct usb_device *dev, int 
start_address,
 {
        int status = 0;
        __u8 read_length;
-       __be16 be_start_address;
+       u16 be_start_address;
 
        dev_dbg(&dev->dev, "%s - @ %x for %d\n", __func__, start_address, 
length);
 
@@ -296,10 +297,14 @@ static int read_download_mem(struct usb_device *dev, int 
start_address,
                if (read_length > 1) {
                        dev_dbg(&dev->dev, "%s - @ %x for %d\n", __func__, 
start_address, read_length);
                }
-               be_start_address = cpu_to_be16(start_address);
+               /*
+                * NOTE: Must use swab as wIndex is sent in little-endian
+                *       byte order regardless of host byte order.
+                */
+               be_start_address = swab16((u16)start_address);
                status = ti_vread_sync(dev, UMPC_MEMORY_READ,
                                        (__u16)address_type,
-                                       (__force __u16)be_start_address,
+                                       be_start_address,
                                        buffer, read_length);
 
                if (status) {
@@ -394,7 +399,7 @@ static int write_i2c_mem(struct edgeport_serial *serial,
        struct device *dev = &serial->serial->dev->dev;
        int status = 0;
        int write_length;
-       __be16 be_start_address;
+       u16 be_start_address;
 
        /* We can only send a maximum of 1 aligned byte page at a time */
 
@@ -409,11 +414,16 @@ static int write_i2c_mem(struct edgeport_serial *serial,
                __func__, start_address, write_length);
        usb_serial_debug_data(dev, __func__, write_length, buffer);
 
-       /* Write first page */
-       be_start_address = cpu_to_be16(start_address);
+       /*
+        * Write first page.
+        *
+        * NOTE: Must use swab as wIndex is sent in little-endian byte order
+        *       regardless of host byte order.
+        */
+       be_start_address = swab16((u16)start_address);
        status = ti_vsend_sync(serial->serial->dev,
                                UMPC_MEMORY_WRITE, (__u16)address_type,
-                               (__force __u16)be_start_address,
+                               be_start_address,
                                buffer, write_length);
        if (status) {
                dev_dbg(dev, "%s - ERROR %d\n", __func__, status);
@@ -436,11 +446,16 @@ static int write_i2c_mem(struct edgeport_serial *serial,
                        __func__, start_address, write_length);
                usb_serial_debug_data(dev, __func__, write_length, buffer);
 
-               /* Write next page */
-               be_start_address = cpu_to_be16(start_address);
+               /*
+                * Write next page.
+                *
+                * NOTE: Must use swab as wIndex is sent in little-endian byte
+                *       order regardless of host byte order.
+                */
+               be_start_address = swab16((u16)start_address);
                status = ti_vsend_sync(serial->serial->dev, UMPC_MEMORY_WRITE,
                                (__u16)address_type,
-                               (__force __u16)be_start_address,
+                               be_start_address,
                                buffer, write_length);
                if (status) {
                        dev_err(dev, "%s - ERROR %d\n", __func__, status);
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to