[ Please do not drop recipients from your replies, that is, in this case
respond directly to me while keeping the list on CC to avoid messages
being overlooked.
Keeping some context and responding inline is also recommended. ]
On Thu, Feb 20, 2014 at 11:09:55AM +0000, Ludovic wrote:
> Hi !
>
> I've tried your patch but that was not ok.
>
> Here are the logs:
>
> Feb 19 20:44:24 10.0.0.254 kernel: [517090.880000]
> drivers/usb/serial/io_ti.c: read_download_mem - @ 0 for 1
> Feb 19 20:44:24 10.0.0.254 kernel: [517090.900000]
> drivers/usb/serial/io_ti.c: read_download_mem - @ 2 for 4
> Feb 19 20:44:24 10.0.0.254 kernel: [517090.900000]
> drivers/usb/serial/io_ti.c: read_download_mem - @ 2 for 4
> Feb 19 20:44:24 10.0.0.254 kernel: [517090.910000] usb 1-1:
> read_download_mem - length = 4, data = 01 09 00 b0
Well, now at least we can read the header correctly.
> Feb 19 20:44:24 10.0.0.254 kernel: [517090.920000]
> drivers/usb/serial/io_ti.c: check_i2c_image Type = 0x1
> Feb 19 20:44:24 10.0.0.254 kernel: [517090.930000]
> drivers/usb/serial/io_ti.c: read_download_mem - @ 6 for 2304
> Feb 19 20:44:24 10.0.0.254 kernel: [517090.930000]
> drivers/usb/serial/io_ti.c: read_download_mem - @ 6 for 64
> Feb 19 20:44:24 10.0.0.254 kernel: [517090.960000] usb 1-1:
> read_download_mem - length = 64, data = 00 08 16 88 02 05 03 00 00 07 00 3e
> 03 02 2b 69 02 1c e0 00 00 00 00 00 02 00 1e 00 00 00 00 00 00 00 00 00 00
> 00 00 00 02 00 c9 85 4b 8c 85 4c 8a c0 e0 c0 d0 c0 f
> ...
> ...
> ... lots of other lines before the driver gives up
> ...
> ...
>
> A logs extract when it works (x86):
> Feb 11 20:13:35 red kernel: [801958.038652]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c: choose_config -
> Number of Interfaces = 1
> Feb 11 20:13:35 red kernel: [801958.038656]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c: choose_config -
> MAX Power = 80
> Feb 11 20:13:35 red kernel: [801958.038659]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c: download_fw -
> RUNNING IN DOWNLOAD MODE
> Feb 11 20:13:35 red kernel: [801958.038664]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c:
> read_download_mem - @ 0 for 1
> Feb 11 20:13:35 red kernel: [801958.043284]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c:
> read_download_mem - @ 2 for 4
> Feb 11 20:13:35 red kernel: [801958.043292]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c:
> read_download_mem - @ 2 for 4
> Feb 11 20:13:35 red kernel: [801958.047275] usb 3-1: read_download_mem -
> length = 4, data = 01 09 00 b0
> Feb 11 20:13:35 red kernel: [801958.047288]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c: check_i2c_image
> Type = 0x1
> Feb 11 20:13:35 red kernel: [801958.047294]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c:
> read_download_mem - @ 6 for 9
> Feb 11 20:13:35 red kernel: [801958.047300]
> /build/linux-vnts0_/linux-3.2.54/drivers/usb/serial/io_ti.c:
> read_download_mem - @ 6 for 9
> Feb 11 20:13:35 red kernel: [801958.053278] usb 3-1: read_download_mem -
> length = 9, data = 00 08 16 88 02 05 03 00 00
>
>
> So we have a 'read_download_mem - @ 6 for 2304' instead of a
> 'read_download_mem - @ 6 for 9'.
Ok, this a another endian bug as I suspected there would be (2304 =
0x900).
Care to try the patch below?
This driver could use some cleaning up so I might rework the patch
somewhat, but let's find the bugs first.
Thanks,
Johan
>From 4c7b6e84a08918e2cd7402f9863e359529f0f8c3 Mon Sep 17 00:00:00 2001
From: Johan Hovold <jhovold@xxxxxxxxx>
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.
Also make sure to handle the struct ti_i2c_desc size parameter which is
returned in little-endian byte order.
---
drivers/usb/serial/io_ti.c | 50 ++++++++++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index a2db5be9c305..df90dae53eb9 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);
@@ -585,8 +600,8 @@ static int get_descriptor_addr(struct edgeport_serial *serial,
if (rom_desc->Type == desc_type)
return start_address;
- start_address = start_address + sizeof(struct ti_i2c_desc)
- + rom_desc->Size;
+ start_address = start_address + sizeof(struct ti_i2c_desc) +
+ le16_to_cpu(rom_desc->Size);
} while ((start_address < TI_MAX_I2C_SIZE) && rom_desc->Type);
@@ -599,7 +614,7 @@ static int valid_csum(struct ti_i2c_desc *rom_desc, __u8 *buffer)
__u16 i;
__u8 cs = 0;
- for (i = 0; i < rom_desc->Size; i++)
+ for (i = 0; i < le16_to_cpu(rom_desc->Size); i++)
cs = (__u8)(cs + buffer[i]);
if (cs != rom_desc->CheckSum) {
@@ -650,7 +665,7 @@ static int check_i2c_image(struct edgeport_serial *serial)
break;
if ((start_address + sizeof(struct ti_i2c_desc) +
- rom_desc->Size) > TI_MAX_I2C_SIZE) {
+ le16_to_cpu(rom_desc->Size)) > TI_MAX_I2C_SIZE) {
status = -ENODEV;
dev_dbg(dev, "%s - structure too big, erroring out.\n", __func__);
break;
@@ -665,7 +680,8 @@ static int check_i2c_image(struct edgeport_serial *serial)
/* Read the descriptor data */
status = read_rom(serial, start_address +
sizeof(struct ti_i2c_desc),
- rom_desc->Size, buffer);
+ le16_to_cpu(rom_desc->Size),
+ buffer);
if (status)
break;
@@ -674,7 +690,7 @@ static int check_i2c_image(struct edgeport_serial *serial)
break;
}
start_address = start_address + sizeof(struct ti_i2c_desc) +
- rom_desc->Size;
+ le16_to_cpu(rom_desc->Size);
} while ((rom_desc->Type != I2C_DESC_TYPE_ION) &&
(start_address < TI_MAX_I2C_SIZE));
@@ -712,7 +728,7 @@ static int get_manuf_info(struct edgeport_serial *serial, __u8 *buffer)
/* Read the descriptor data */
status = read_rom(serial, start_address+sizeof(struct ti_i2c_desc),
- rom_desc->Size, buffer);
+ le16_to_cpu(rom_desc->Size), buffer);
if (status)
goto exit;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
