[alsa-devel] [PATCH] snd-usb-us122l v0.4 for US-122L

[Takashi Iwai]

At Tue, 15 Apr 2008 16:48:34 +0200,
Karsten Wiese wrote:
> 
> > diff --git a/sound/usb/usx2y/Makefile b/sound/usb/usx2y/Makefile
> > index 9ac22bc..7489330 100644
> > --- a/sound/usb/usx2y/Makefile
> > +++ b/sound/usb/usx2y/Makefile
> > @@ -1,3 +1,5 @@
> >  snd-usb-usx2y-objs := usbusx2y.o usX2Yhwdep.o usx2yhwdeppcm.o
> > +snd-usb-us122l-objs := us122l.o
> >  
> >  obj-$(CONFIG_SND_USB_USX2Y) += snd-usb-usx2y.o
> > +obj-$(CONFIG_SND_USB_US122L) += snd-usb-us122l.o
> > 
> > Missing dependency to snd-usb-audio?
> > Maybe we need to split a helper module...
> 
> don't understand. please explain.

snd-usb-us122l module calls functions that are present in
snd-usb-audio module.  This dependency should be written either in
Kconfig or Makefile properly.

> > +struct usb_iso_packet_descriptor {
> > +	unsigned int offset;
> > +	unsigned int length;		/* expected length */
> > +	unsigned int actual_length;
> > +	int status;
> > +};
> > 
> > Why this is needed for !__KERNEL__ ?
> > I see only struct usb_stream_config is required.
> 
> see the plugin. Userspace directly reads urbs.

Hmm, but this is basically defined in usb stack?
Not a big business to redefine here, though...


> > +#endif
> > +
> > +#define USB_STREAM_NURBS 4
> > +struct usb_stream_config {
> > +	unsigned version;
> > +	unsigned sample_rate;
> > +	unsigned period_frames;
> > +	unsigned frame_size;
> > +};
> > 
> > The codes below here should be hidden to user-space.
> > If it's exported, then prepare more explicit struct.  For example,
> > pointers may have different sizes on user-space.
> 
> Needed in user-space. Currently only 64Bit userspace works on 64Bit kernel.
> Also 32bit user-space on 32bit kernel.
> 32bit user-space on 64bit kernel returns an error to the caller, please see
> the plugin.
> IMO 32bit user-space working on 64bit kernel can wait for "INTERFACE_VERSION 2"
Exporting this kind of particular internal struct to the user-space is
very dangerous.  This shouldn't be done.  If a part of the data struct
is needed to be accessible via mmap etc, define explicitly the data
struct instead.


Takashi