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.
--
Greetings, Michael.
--
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
