[alsa-devel] [PATCH] ad1848 ac loop
rene.herman at gmail.com
Wed Sep 19 09:15:34 CEST 2007
On 09/19/2007 07:22 AM, Trent Piepho wrote:
> I'm not an expert on this chip by any means, but from looking at the code
> I think there are few things that could be fixed.
I'd agree, although I'd suggest to at this point keep anything (in ad1848,
cs4231 wants a similar setup in its mce_down) not completely trivial for
> The reg_lock spinlock isn't acquired from the irq handler, as the handler
> doesn't use any of the indirect registers. I think this means that is
> isn't necessary to use the irq disabling versions of the spin_lock
> functions with reg_lock.
This would appear to be the case yes.
> It does look like there is a different SMP race condition wrt the irq handler.
> From snd_ad1848_interrupt():
> 578 if ((chip->mode & AD1848_MODE_PLAY) && chip->playback_substream &&
> 579 (chip->mode & AD1848_MODE_RUNNING))
> 580 snd_pcm_period_elapsed(chip->playback_substream);
> Another CPU could close the substream between the check and the call to
> snd_pcm_period_elapsed(). snd_pcm_period_elapsed() could be called with
> either NULL or a stale substream pointer depending on how the code compiled.
> I'd think creating an irq spinlock would be an easy way to fix this. The irq
> handler would grab it, and the same with the open() and close() functions
> around the code that sets the substream pointers. Alternatively, one could
> disabled the irq handler during open and close.
Also sounds sensible, and the irq spinlock the expected method.
> I think there might also be problem with setting mixers controls during
> auto-calibration . While waiting for auto-calibration to finish, the chip
> isn't locked. If another thread tries to set the volume via the mixer
> interface, it won't be blocked and will try to talk to the chip during
> AC. The datasheet just says to wait for auto-calibration to finish, it
> doesn't say what happens if you don't. So maybe there isn't any problem
Guess we should be testing this, but generallt, "hand-off" would be the best
More information about the Alsa-devel