Setting processor endianess for USB modules

Hello,

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 )?

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.

regards

Christian Eggers

Dear Christian,

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.

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

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de

Dear Markus,

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

regards
Christian

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

index 2fa5254..7033c2a 100644
@@ -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-

- swap_16(dev-

+ le16_to_cpus(&(dev-

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-

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);

index 1703b23..f54c527 100644
@@ -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;

index 7c08f95..2ca4721 100644
@@ -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];

1.4.3.4

Unfortunately it's unusable as your mailer wrapped long lines:

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Here.

And so on. Please fix your mailer (or even better use git-send-email)

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Q: How do you spell "onomatopoeia"?
A: The way it sounds.