Dear Markus,

I've made I new patch with git as requested.

regards Christian

Dear Christian,

"Christian Eggers" ceggers@gmx.de writes:

...

I've recognized that a lot of USB code in U-Boot uses the macros swap_16() and swap_32() which are defined in usb.h. The behaviour of the macros is controlled by the define LITTLEENDIAN.

Is there a good reason NOT to use the macros provided in asm/byteorder.h (as in the appropriate code in Linux-2.4 )?

Largely not. But be carefull. Besides big and little endian CPUs we also have controllers that operate in big or little endian (see CFG_OHCI_BE_CONTROLLER), and then there are PCI controllers whose registers need to be accessed as little endian (CFG_OHCI_SWAP_REG_ACCESS).

But your right, there is no real for LITTLEENDIAN.

...

I think that switching to these functions might be useful in order to eliminate the need to use the LITTLEENDIAN define for specifying the byteorder. It seems that LITTLEENDIAN is not used outside the USB code.

Right. Patches cleaning this up are more than welcome! Please note that cleaning up USB drivers under the cpu/ directory is a waste of time though. Boards using these drivers should be converted to use the generic infrastructure in drivers/usb/ instead.

Best regards

Markus Klotzbuecher

--- common/usb.c | 44 ++++++++++++++++++++++---------------------- common/usb_kbd.c | 11 ++++++----- common/usb_storage.c | 28 ++++++++++------------------ 3 files changed, 38 insertions(+), 45 deletions(-)

diff --git a/common/usb.c b/common/usb.c index 2fa5254..7033c2a 100644 --- a/common/usb.c +++ b/common/usb.c @@ -48,6 +48,7 @@ #include <command.h> #include <asm/processor.h> #include <linux/ctype.h> +#include <asm/byteorder.h>

#if defined(CONFIG_CMD_USB)

@@ -177,10 +178,10 @@ int usb_control_msg(struct usb_device *d /* set setup command */ setup_packet.requesttype = requesttype; setup_packet.request = request; - setup_packet.value = swap_16(value); - setup_packet.index = swap_16(index); - setup_packet.length = swap_16(size); - USB_PRINTF("usb_control_msg: request: 0x%X, requesttype: 0x%X\nvalue 0x%X index 0x%X length 0x%X\n", + setup_packet.value = cpu_to_le16(value); + setup_packet.index = cpu_to_le16(index); + setup_packet.length = cpu_to_le16(size); + USB_PRINTF("usb_control_msg: request: 0x%X, requesttype: 0x%X, value 0x%X index 0x%X length 0x%X\n", request,requesttype,value,index,size); dev->status=USB_ST_NOT_PROC; /*not yet processed */

@@ -300,7 +301,7 @@ int usb_parse_config(struct usb_device * return -1; } memcpy(&dev->config, buffer, buffer[0]); - dev->config.wTotalLength = swap_16(dev->config.wTotalLength); + le16_to_cpus(&(dev->config.wTotalLength)); dev->config.no_of_if = 0;

index = dev->config.bLength; @@ -329,8 +330,7 @@ int usb_parse_config(struct usb_device * dev->config.if_desc[ifno].no_of_ep++; /* found an endpoint */ memcpy(&dev->config.if_desc[ifno].ep_desc[epno], &buffer[index], buffer[index]); - dev-

config.if_desc[ifno].ep_desc[epno].wMaxPacketSize =

- swap_16(dev-

config.if_desc[ifno].ep_desc[epno].wMaxPacketSize);

+ le16_to_cpus(&(dev-

config.if_desc[ifno].ep_desc[epno].wMaxPacketSize));

USB_PRINTF("if %d, ep %d\n", ifno, epno); break; default: @@ -413,7 +413,7 @@ int usb_get_configuration_no(struct usb_ printf("config descriptor too short (expected %i, got %i)\n",8,result); return -1; } - tmp=swap_16(config->wTotalLength); + tmp = le16_to_cpu(config->wTotalLength);

if (tmp > USB_BUFSIZ) { USB_PRINTF("usb_get_configuration_no: failed to get descriptor - too long: %d\n", @@ -816,10 +816,10 @@ int usb_new_device(struct usb_device *de return 1; } /* correct le values */ - dev->descriptor.bcdUSB=swap_16(dev->descriptor.bcdUSB); - dev->descriptor.idVendor=swap_16(dev->descriptor.idVendor); - dev->descriptor.idProduct=swap_16(dev->descriptor.idProduct); - dev->descriptor.bcdDevice=swap_16(dev->descriptor.bcdDevice); + le16_to_cpus(&dev->descriptor.bcdUSB); + le16_to_cpus(&dev->descriptor.idVendor); + le16_to_cpus(&dev->descriptor.idProduct); + le16_to_cpus(&dev->descriptor.bcdDevice); /* only support for one config for now */ usb_get_configuration_no(dev,&tmpbuf[0],0); usb_parse_config(dev,&tmpbuf[0],0); @@ -979,8 +979,8 @@ static int hub_port_reset(struct usb_dev USB_HUB_PRINTF("get_port_status failed status %lX\n",dev-

status);

return -1; } - portstatus = swap_16(portsts.wPortStatus); - portchange = swap_16(portsts.wPortChange); + portstatus = le16_to_cpu(portsts.wPortStatus); + portchange = le16_to_cpu(portsts.wPortChange); USB_HUB_PRINTF("portstatus %x, change %x, %s\n", portstatus ,portchange, portstatus&(1<<USB_PORT_FEAT_LOWSPEED) ? "Low Speed" : "High Speed"); USB_HUB_PRINTF("STAT_C_CONNECTION = %d STAT_CONNECTION = %d USB_PORT_STAT_ENABLE %d\n", @@ -1024,8 +1024,8 @@ void usb_hub_port_connect_change(struct return; }

- portstatus = swap_16(portsts.wPortStatus); - portchange = swap_16(portsts.wPortChange); + portstatus = le16_to_cpu(portsts.wPortStatus); + portchange = le16_to_cpu(portsts.wPortChange); USB_HUB_PRINTF("portstatus %x, change %x, %s\n", portstatus, portchange, portstatus&(1<<USB_PORT_FEAT_LOWSPEED) ? "Low Speed" : "High Speed");

@@ -1099,7 +1099,7 @@ int usb_hub_configure(struct usb_device } memcpy((unsigned char *)&hub->desc,buffer,descriptor->bLength); /* adjust 16bit values */ - hub->desc.wHubCharacteristics=swap_16(descriptor->wHubCharacteristics); + hub->desc.wHubCharacteristics = le16_to_cpu(descriptor->wHubCharacteristics); /* set the bitmap */ bitmap=(unsigned char *)&hub->desc.DeviceRemovable[0]; memset(bitmap,0xff,(USB_MAXCHILDREN+1+7)/8); /* devices not removable by default */ @@ -1161,11 +1161,11 @@ int usb_hub_configure(struct usb_device } hubsts = (struct usb_hub_status *)buffer; USB_HUB_PRINTF("get_hub_status returned status %X, change %X\n", - swap_16(hubsts->wHubStatus),swap_16(hubsts->wHubChange)); + le16_to_cpu(hubsts->wHubStatus),le16_to_cpu(hubsts->wHubChange)); USB_HUB_PRINTF("local power source is %s\n", - (swap_16(hubsts->wHubStatus) & HUB_STATUS_LOCAL_POWER) ? "lost (inactive)" : "good"); + (le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_LOCAL_POWER) ? "lost (inactive)" : "good"); USB_HUB_PRINTF("%sover-current condition exists\n", - (swap_16(hubsts->wHubStatus) & HUB_STATUS_OVERCURRENT) ? "" : "no "); + (le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_OVERCURRENT) ? "" : "no "); usb_hub_power_on(hub); for (i = 0; i < dev->maxchild; i++) { struct usb_port_status portsts; @@ -1175,8 +1175,8 @@ int usb_hub_configure(struct usb_device USB_HUB_PRINTF("get_port_status failed\n"); continue; } - portstatus = swap_16(portsts.wPortStatus); - portchange = swap_16(portsts.wPortChange); + portstatus = le16_to_cpu(portsts.wPortStatus); + portchange = le16_to_cpu(portsts.wPortChange); USB_HUB_PRINTF("Port %d Status %X Change %X\n",i+1,portstatus,portchange); if (portchange & USB_PORT_STAT_C_CONNECTION) { USB_HUB_PRINTF("port %d connection change\n", i + 1); diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 1703b23..f54c527 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -26,6 +26,7 @@ */ #include <common.h> #include <devices.h> +#include <asm/byteorder.h>

#ifdef CONFIG_USB_KEYBOARD

@@ -475,14 +476,14 @@ static int fetch_item(unsigned char *sta break; case 2: if ((end - start) >= 2) { - item->data.u16 = swap_16((unsigned short *)start); + item->data.u16 = le16_to_cpu((unsigned short *)start); start+=2; return item->size; } case 3: item->size++; if ((end - start) >= 4) { - item->data.u32 = swap_32((unsigned long *)start); + item->data.u32 = le32_to_cpu((unsigned long *)start); start+=4; return item->size; } @@ -705,15 +706,15 @@ static int usb_kbd_get_hid_desc(struct u } index=head->bLength; config=(struct usb_config_descriptor *)&buffer[0]; - len=swap_16(config->wTotalLength); + len=le16_to_cpu(config->wTotalLength); /* Ok the first entry must be a configuration entry, now process the others */ head=(struct usb_descriptor_header *)&buffer[index]; while(index+1 < len) { if(head->bDescriptorType==USB_DT_HID) { printf("HID desc found\n"); memcpy(&usb_kbd_hid_desc,&buffer[index],buffer[index]); - usb_kbd_hid_desc.bcdHID=swap_16(usb_kbd_hid_desc.bcdHID); - usb_kbd_hid_desc.wDescriptorLength=swap_16(usb_kbd_hid_desc.wDescriptorLength); + le16_to_cpus(&usb_kbd_hid_desc.bcdHID); + le16_to_cpus(&usb_kbd_hid_desc.wDescriptorLength); usb_kbd_display_hid(&usb_kbd_hid_desc); len=0; break; diff --git a/common/usb_storage.c b/common/usb_storage.c index 7c08f95..2ca4721 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -52,6 +52,7 @@

#include <common.h> #include <command.h> +#include <asm/byteorder.h> #include <asm/processor.h>

@@ -474,9 +475,9 @@ int usb_stor_BBB_comdat(ccb *srb, struct /* always OUT to the ep */ pipe = usb_sndbulkpipe(us->pusb_dev, us->ep_out);

- cbw.dCBWSignature = swap_32(CBWSIGNATURE); - cbw.dCBWTag = swap_32(CBWTag++); - cbw.dCBWDataTransferLength = swap_32(srb->datalen); + cbw.dCBWSignature = cpu_to_le32(CBWSIGNATURE); + cbw.dCBWTag = cpu_to_le32(CBWTag++); + cbw.dCBWDataTransferLength = cpu_to_le32(srb->datalen); cbw.bCBWFlags = (dir_in? CBWFLAGS_IN : CBWFLAGS_OUT); cbw.bCBWLUN = srb->lun; cbw.bCDBLength = srb->cmdlen; @@ -692,14 +693,14 @@ int usb_stor_BBB_transport(ccb *srb, str printf("\n"); #endif /* misuse pipe to get the residue */ - pipe = swap_32(csw.dCSWDataResidue); + pipe = le32_to_cpu(csw.dCSWDataResidue); if (pipe == 0 && srb->datalen != 0 && srb->datalen - data_actlen != 0) pipe = srb->datalen - data_actlen; - if (CSWSIGNATURE != swap_32(csw.dCSWSignature)) { + if (CSWSIGNATURE != le32_to_cpu(csw.dCSWSignature)) { USB_STOR_PRINTF("!CSWSIGNATURE\n"); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; - } else if ((CBWTag - 1) != swap_32(csw.dCSWTag)) { + } else if ((CBWTag - 1) != le32_to_cpu(csw.dCSWTag)) { USB_STOR_PRINTF("!Tag\n"); usb_stor_BBB_reset(us); return USB_STOR_TRANSPORT_FAILED; @@ -1222,18 +1223,9 @@ int usb_stor_get_info(struct usb_device if(cap[0]>(0x200000 * 10)) /* greater than 10 GByte */ cap[0]>>=16; #endif -#ifdef LITTLEENDIAN - cap[0] = ((unsigned long)( - (((unsigned long)(cap[0]) & (unsigned long)0x000000ffUL) << 24) | - (((unsigned long)(cap[0]) & (unsigned long)0x0000ff00UL) << 8) | - (((unsigned long)(cap[0]) & (unsigned long)0x00ff0000UL) >> 8) | - (((unsigned long)(cap[0]) & (unsigned long)0xff000000UL) >> 24) )); - cap[1] = ((unsigned long)( - (((unsigned long)(cap[1]) & (unsigned long)0x000000ffUL) << 24) | - (((unsigned long)(cap[1]) & (unsigned long)0x0000ff00UL) << 8) | - (((unsigned long)(cap[1]) & (unsigned long)0x00ff0000UL) >> 8) | - (((unsigned long)(cap[1]) & (unsigned long)0xff000000UL) >> 24) )); -#endif + cap[0] = cpu_to_be32(cap[0]); + cap[1] = cpu_to_be32(cap[1]); + /* this assumes bigendian! */ cap[0] += 1; capacity = &cap[0];