On Fri, 12 Jun 2009, Alek Du wrote:
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index d04d018..eb3dc98 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -538,10 +539,18 @@ struct ehci_fstn {
> #define ehci_is_TDI(e) (ehci_to_hcd(e)->has_tt)
>
> /* Returns the speed of a device attached to a port on the root hub. */
> -static inline unsigned int
> +static unsigned int
> ehci_port_speed(struct ehci_hcd *ehci, unsigned int portsc)
> {
Although this will build okay, it's not good form. Non-inline
functions should not be defined in .h files. The function should be
moved into ehci-hub.c.
> if (ehci_is_TDI(ehci)) {
> + if (ehci->has_hostpc) {
> + if (portsc & (1<<25))
> + return 1<<USB_PORT_FEAT_LOWSPEED;
> + if (portsc & (1<<26))
> + return 1<<USB_PORT_FEAT_HIGHSPEED;
> + else
> + return 0; /* Full speed */
> + }
> switch ((portsc>>26)&3) {
Can't you unify these two paths? For example:
unsigned bits;
bits = (portsc >> (ehci->has_hostpc ? 25 : 26)) & 3;
switch (bits) {
...
By the way, why did your team decide to put the speed bits at positions
25 and 26 instead of 26 and 27, like everybody else? Just to make life
more difficult for driver developers?
Alan Stern
--
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
