On Tue, Jun 16, 2009 at 03:29:13PM +0200, Laurent Pinchart wrote:
> Hi,
>
> On Tuesday 16 June 2009 07:23:30 Greg Kroah-Hartman wrote:
> > From: Bryan Wu <cooloney@xxxxxxxxxx>
> >
> > Signed-off-by: Bryan Wu <cooloney@xxxxxxxxxx>
> > Signed-off-by: Mike Frysinger <vapier@xxxxxxxxxx>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
> > ---
> > include/linux/usb/audio.h | 265
> > ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 263
> > insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/usb/audio.h b/include/linux/usb/audio.h
> > index 8cb025f..b5744bc 100644
> > --- a/include/linux/usb/audio.h
> > +++ b/include/linux/usb/audio.h
> > @@ -24,10 +24,75 @@
> > #define USB_SUBCLASS_AUDIOCONTROL 0x01
> > #define USB_SUBCLASS_AUDIOSTREAMING 0x02
> > #define USB_SUBCLASS_MIDISTREAMING 0x03
> > +#define USB_SUBCLASS_VENDOR_SPEC 0xff
>
> Doesn't this belong to include/linux/usb/ch9.h instead ?
Probably, send a patch to move it :)
> > +#define GET_STAT 0xff
>
> A lot of those identifiers are quite generic for a public header file. They
> should be prefixed with USB_AUDIO_, USB_AC_ or UAC_ (UAC_ is shorter and thus
> has my preference).
Yeah, good point, patches gladly accepted.
> > +/* As above, but more useful for defining your own descriptors: */
> > +#define DECLARE_USB_AC_FEATURE_UNIT_DESCRIPTOR(ch) \
> > +struct usb_ac_feature_unit_descriptor_##ch { \
> > + __u8 bLength; \
> > + __u8 bDescriptorType; \
> > + __u8 bDescriptorSubtype; \
> > + __u8 bUnitID; \
> > + __u8 bSourceID; \
> > + __u8 bControlSize; \
> > + __le16 bmaControls[ch + 1]; \
> > + __u8 iFeature; \
> > +} __attribute__ ((packed))
>
> Using the structure defined by this macro requires knowledge of how the macro
> creates the structure name. It would be better to create another macro to get
> the structure name:
>
> #define USB_AC_FEATURE_UNIT_DESCRIPTOR(ch) usb_ac_feature_unit_descriptor_##ch
> #define DECLARE_USB_AC_FEATURE_UNIT_DESCRIPTOR(ch) \
> struct USB_AC_FEATURE_UNIT_DESCRIPTOR(ch) { \
> ...
If you feel that is easier to work with, sure.
> > +static inline int generic_set_cmd(struct usb_audio_control *con, u8 cmd,
> > int value) +{
> > + con->data[cmd] = value;
> > +
> > + return 0;
> > +}
> > +
> > +static inline int generic_get_cmd(struct usb_audio_control *con, u8 cmd)
> > +{
> > + return con->data[cmd];
> > +}
>
> Those functions are used only used to fill the set/get members of
> usb_audio_control. It doesn't make much sense to inline them.
Ok. Patches please.
thanks,
greg k-h
--
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
