Messages in this thread | | | From | Mike Frysinger <> | Date | Tue, 24 May 2011 13:20:24 -0400 | Subject | Re: [PATCH v13 5/8] usb:gadget: Add SuperSpeed support to the Gadget Framework |
| |
On Tue, May 24, 2011 at 06:02, Tatyana Brokhman wrote: > +config USB_GADGET_SUPERSPEED > + boolean "Gadget operating in Super Speed"
perhaps better phrasing would be: Enable Super Speed support
maybe i didnt look too closely, but it seems like very little even depends upon this Kconfig option in the actual source. only the "gadget_is_superspeed()", and that is used lightly. so i wonder how useful this is even having ...
> + value = min(w_length, (u16) value);
i know you're just following existing style, but i wonder if these all shouldnt just be min_t(u16, ...)
> + ERROR(cdev, "func_suspend() returned " > + "error %d\n", value);
please dont split string literals over multiple lines
> + default: > + break; > + }
isnt that the default behavior already ? so these two lines are pointless ?
> --- a/include/linux/usb/ch9.h > +++ b/include/linux/usb/ch9.h > @@ -142,8 +142,6 @@ > #define USB_DEVICE_LTM_ENABLE 50 /* dev may send LTM */ > #define USB_INTRF_FUNC_SUSPEND 0 /* function suspend */ > > -#define USB_INTR_FUNC_SUSPEND_OPT_MASK 0xFF00 > - > #define USB_ENDPOINT_HALT 0 /* IN/OUT will STALL */ > > /* Bit array elements as returned by the USB_REQ_GET_STATUS request. */
erm, seems unrelated to this patchset ? did this sneak in by accident ?
> +static inline int gadget_is_superspeed(struct usb_gadget *g) > +{ > +#ifdef CONFIG_USB_GADGET_SUPERSPEED > + /* runtime test would check "g->is_superspeed" ... that might be > + * useful to work around hardware bugs, but is mostly pointless > + */
multiline comments should read: /* * foo ..... */ -mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |