On Tuesday 21 August 2012 21:52:43 Lan Tianyu wrote:
> 于 2012/8/21 17:07, Bjørn Mork 写道:
> > Lan Tianyu <tianyu.lan@xxxxxxxxx> writes:
> >
> >> @@ -648,7 +649,14 @@ ehci_hub_descriptor (
> >>
> >> /* two bitmaps: ports removable, and usb 1.0 legacy PortPwrCtrlMask */
> >> memset(&desc->u.hs.DeviceRemovable[0], 0, temp);
> >> - memset(&desc->u.hs.DeviceRemovable[temp], 0xff, temp);
> >> + memset(&desc->u.hs.DeviceRemovable[temp], 0xff, ltemp);
> >> +
> >> + for (i = 1; i <= ports; i++) {
> >> + if (usb_get_hub_port_connect_type(hcd->self.root_hub, i);
> >> + == USB_PORT_CONNECT_TYPE_HARD_WIRED)
> >> + desc->u.hs.DeviceRemovable[ports/8] |= 1 << (i%8);
> >> + }
> >> +
> >>
> >> temp = 0x0008; /* per-port overcurrent reporting */
> >> if (HCS_PPC (ehci->hcs_params))
> >
> >
> > Not that it matters much, but I believe this code would look a lot
> > better if it used linux/bitmap.h and/or asm/bitops.h
> hi Bjørn:
> Great thanks for your review.
> Do you mean something like this?
> + for (i = 1; i <= ports; i++) {
> + if (usb_get_hub_port_connect_type(hcd->self.root_hub, i)
> + == USB_PORT_CONNECT_TYPE_HARD_WIRED)
> + set_bit(i, (unsigned long *)desc->u.hs.DeviceRemovable);
> + }
> Since the parameter type of methods in the bitmap.h and bitops.h are
> are unsigned long and DeviceRemovable is u8, I have a concern about byte order
> problem. Does this make sense?
You must not use setbit on anything less than long.
In addition it is needlessly atomic. It is perfectly OK to do it the way
you originally did.
Regards
Oliver
--
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
