Message ID | 200906181903.47541.mb@bu3sch.de |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Thu, Jun 18, 2009 at 07:03:47PM CEST, mb@bu3sch.de wrote: >This fixes various endianness bugs. Some harmless and some real ones. >This is tested on a PowerPC-64 machine. > >Signed-off-by: Michael Buesch <mb@bu3sch.de> >Cc: Stable <stable@kernel.org> > >--- > drivers/net/usb/pegasus.c | 29 +++++++++++++++++------------ > 1 file changed, 17 insertions(+), 12 deletions(-) > >--- linux-2.6.30.orig/drivers/net/usb/pegasus.c >+++ linux-2.6.30/drivers/net/usb/pegasus.c >@@ -297,7 +297,7 @@ static int update_eth_regs_async(pegasus > > pegasus->dr.bRequestType = PEGASUS_REQT_WRITE; > pegasus->dr.bRequest = PEGASUS_REQ_SET_REGS; >- pegasus->dr.wValue = 0; >+ pegasus->dr.wValue = cpu_to_le16(0); Is this necessary? I mean zero is still zero :-) > pegasus->dr.wIndex = cpu_to_le16(EthCtrl0); > pegasus->dr.wLength = cpu_to_le16(3); > pegasus->ctrl_urb->transfer_buffer_length = 3; >@@ -446,11 +446,12 @@ static int write_eprom_word(pegasus_t * > int i; > __u8 tmp, d[4] = { 0x3f, 0, 0, EPROM_WRITE }; > int ret; >+ __le16 le_data = cpu_to_le16(data); > > set_registers(pegasus, EpromOffset, 4, d); > enable_eprom_write(pegasus); > set_register(pegasus, EpromOffset, index); >- set_registers(pegasus, EpromData, 2, &data); >+ set_registers(pegasus, EpromData, 2, &le_data); > set_register(pegasus, EpromCtrl, EPROM_WRITE); > > for (i = 0; i < REG_TIMEOUT; i++) { >@@ -923,29 +924,32 @@ static struct net_device_stats *pegasus_ > > static inline void disable_net_traffic(pegasus_t * pegasus) > { >- int tmp = 0; >+ __le16 tmp = cpu_to_le16(0); > >- set_registers(pegasus, EthCtrl0, 2, &tmp); >+ set_registers(pegasus, EthCtrl0, sizeof(tmp), &tmp); > } > > static inline void get_interrupt_interval(pegasus_t * pegasus) > { >- __u8 data[2]; >+ u16 data; >+ u8 interval; > >- read_eprom_word(pegasus, 4, (__u16 *) data); >+ read_eprom_word(pegasus, 4, &data); >+ interval = data >> 8; > if (pegasus->usb->speed != USB_SPEED_HIGH) { >- if (data[1] < 0x80) { >+ if (interval < 0x80) { > if (netif_msg_timer(pegasus)) > dev_info(&pegasus->intf->dev, "intr interval " > "changed from %ums to %ums\n", >- data[1], 0x80); >- data[1] = 0x80; >+ interval, 0x80); >+ interval = 0x80; >+ data = (data & 0x00FF) | ((u16)interval << 8); ^^^^^ you do not need this typecast > #ifdef PEGASUS_WRITE_EEPROM >- write_eprom_word(pegasus, 4, *(__u16 *) data); >+ write_eprom_word(pegasus, 4, data); > #endif > } > } >- pegasus->intr_interval = data[1]; >+ pegasus->intr_interval = interval; > } > > static void set_carrier(struct net_device *net) >@@ -1299,7 +1303,8 @@ static int pegasus_blacklisted(struct us > /* Special quirk to keep the driver from handling the Belkin Bluetooth > * dongle which happens to have the same ID. > */ >- if ((udd->idVendor == VENDOR_BELKIN && udd->idProduct == 0x0121) && >+ if ((udd->idVendor == cpu_to_le16(VENDOR_BELKIN)) && >+ (udd->idProduct == cpu_to_le16(0x0121)) && > (udd->bDeviceClass == USB_CLASS_WIRELESS_CONTROLLER) && > (udd->bDeviceProtocol == 1)) > return 1; > > >-- >Greetings, Michael. >-- >To unsubscribe from this list: send the line "unsubscribe netdev" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 18 June 2009 19:46:22 Jiri Pirko wrote: > >--- linux-2.6.30.orig/drivers/net/usb/pegasus.c > >+++ linux-2.6.30/drivers/net/usb/pegasus.c > >@@ -297,7 +297,7 @@ static int update_eth_regs_async(pegasus > > > > pegasus->dr.bRequestType = PEGASUS_REQT_WRITE; > > pegasus->dr.bRequest = PEGASUS_REQ_SET_REGS; > >- pegasus->dr.wValue = 0; > >+ pegasus->dr.wValue = cpu_to_le16(0); > Is this necessary? I mean zero is still zero :-) Well, yes. However wValue is a little endian variable and 0 is CPU endian. The fact that 0 is represented with the same bits in LE and BE doesn't really matter. It documents the fact that we really require this to be a LE value. GCC will recognize it and optimize it away, so it's not a runtime issue. And wValue = cpu_to_le16(0) is used in the rest of the driver, too. This is the only place that doesn't use it. So let's fix it. If only for consistency. > >+ u8 interval; > > > >- read_eprom_word(pegasus, 4, (__u16 *) data); > >+ read_eprom_word(pegasus, 4, &data); > >+ interval = data >> 8; > > if (pegasus->usb->speed != USB_SPEED_HIGH) { > >- if (data[1] < 0x80) { > >+ if (interval < 0x80) { > > if (netif_msg_timer(pegasus)) > > dev_info(&pegasus->intf->dev, "intr interval " > > "changed from %ums to %ums\n", > >- data[1], 0x80); > >- data[1] = 0x80; > >+ interval, 0x80); > >+ interval = 0x80; > >+ data = (data & 0x00FF) | ((u16)interval << 8); > ^^^^^ you do not need this > typecast Hm, well. The cast is there because "interval" is an 8bit variable. I think the behavior of C is machine dependent in such a situation. But as Linux doesn't run on anything with less than 32bit registers, it probably doesn't matter.
From: Michael Buesch <mb@bu3sch.de> Date: Thu, 18 Jun 2009 19:03:47 +0200 > This fixes various endianness bugs. Some harmless and some real ones. > This is tested on a PowerPC-64 machine. > > Signed-off-by: Michael Buesch <mb@bu3sch.de> > Cc: Stable <stable@kernel.org> Applied and queued up for -stable, thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- linux-2.6.30.orig/drivers/net/usb/pegasus.c +++ linux-2.6.30/drivers/net/usb/pegasus.c @@ -297,7 +297,7 @@ static int update_eth_regs_async(pegasus pegasus->dr.bRequestType = PEGASUS_REQT_WRITE; pegasus->dr.bRequest = PEGASUS_REQ_SET_REGS; - pegasus->dr.wValue = 0; + pegasus->dr.wValue = cpu_to_le16(0); pegasus->dr.wIndex = cpu_to_le16(EthCtrl0); pegasus->dr.wLength = cpu_to_le16(3); pegasus->ctrl_urb->transfer_buffer_length = 3; @@ -446,11 +446,12 @@ static int write_eprom_word(pegasus_t * int i; __u8 tmp, d[4] = { 0x3f, 0, 0, EPROM_WRITE }; int ret; + __le16 le_data = cpu_to_le16(data); set_registers(pegasus, EpromOffset, 4, d); enable_eprom_write(pegasus); set_register(pegasus, EpromOffset, index); - set_registers(pegasus, EpromData, 2, &data); + set_registers(pegasus, EpromData, 2, &le_data); set_register(pegasus, EpromCtrl, EPROM_WRITE); for (i = 0; i < REG_TIMEOUT; i++) { @@ -923,29 +924,32 @@ static struct net_device_stats *pegasus_ static inline void disable_net_traffic(pegasus_t * pegasus) { - int tmp = 0; + __le16 tmp = cpu_to_le16(0); - set_registers(pegasus, EthCtrl0, 2, &tmp); + set_registers(pegasus, EthCtrl0, sizeof(tmp), &tmp); } static inline void get_interrupt_interval(pegasus_t * pegasus) { - __u8 data[2]; + u16 data; + u8 interval; - read_eprom_word(pegasus, 4, (__u16 *) data); + read_eprom_word(pegasus, 4, &data); + interval = data >> 8; if (pegasus->usb->speed != USB_SPEED_HIGH) { - if (data[1] < 0x80) { + if (interval < 0x80) { if (netif_msg_timer(pegasus)) dev_info(&pegasus->intf->dev, "intr interval " "changed from %ums to %ums\n", - data[1], 0x80); - data[1] = 0x80; + interval, 0x80); + interval = 0x80; + data = (data & 0x00FF) | ((u16)interval << 8); #ifdef PEGASUS_WRITE_EEPROM - write_eprom_word(pegasus, 4, *(__u16 *) data); + write_eprom_word(pegasus, 4, data); #endif } } - pegasus->intr_interval = data[1]; + pegasus->intr_interval = interval; } static void set_carrier(struct net_device *net) @@ -1299,7 +1303,8 @@ static int pegasus_blacklisted(struct us /* Special quirk to keep the driver from handling the Belkin Bluetooth * dongle which happens to have the same ID. */ - if ((udd->idVendor == VENDOR_BELKIN && udd->idProduct == 0x0121) && + if ((udd->idVendor == cpu_to_le16(VENDOR_BELKIN)) && + (udd->idProduct == cpu_to_le16(0x0121)) && (udd->bDeviceClass == USB_CLASS_WIRELESS_CONTROLLER) && (udd->bDeviceProtocol == 1)) return 1;
This fixes various endianness bugs. Some harmless and some real ones. This is tested on a PowerPC-64 machine. Signed-off-by: Michael Buesch <mb@bu3sch.de> Cc: Stable <stable@kernel.org> --- drivers/net/usb/pegasus.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-)