Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Daniel Mack <zonque@gmail.com>]

Hi Takashi,

On 11/01/2011 05:19 PM, Takashi Iwai wrote:
 > The overall design looks good to me.  Nice work!
 > A few nitpicking:

Thanks for your review!

 > - No need for check of activated flags at starting streams?

Such a condition would be a bug in the driver. But I'll add a check.

 > - Better to clear subs->data_endpoint&  co at closing.

Jup.

 > - There might be unbalances when deactivate_endpoints() isn't called
 >    in prepare callback before activate_endpoints().  The apps may call
 >    like
 > 	open ->  hw_params ->  prepare ->  hw_params ->  prepare ->  trigger
 >    and
 > 	trigger (stop) ->  prepare ->  trigger (start)
 >    So, be careful about refcounting and active flags.

Hmm, I don't think I follow here. snd_usb_endpoint_{de,}activate() don't 
actually touch the refcounts but act upon the "activated" flag, and 
snd_usb_endpoint_{start,stop}() will touch the refcounts only. Hence, an 
unbalanced call to activate() would basically be a noop. Do I miss your 
point?

 > And, yes, the protection for snd_usb_add_endpoint() would be really
 > needed for concurrent access.  The card-global mutex would be enough.

Ok. However, there is actually no card-global mutex for that purpose, 
right? I'll add one :)


Thanks,
Daniel