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