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