[alsa-devel] [PATCH] ad1848 ac loop

[Takashi Iwai]

At Tue, 18 Sep 2007 22:22:42 -0700 (PDT),
Trent Piepho wrote:
> 
> New thread for the the final version of the patches for my fixes to the ad1848
> auto-calibration loop.  I made a couple suggested changes.  These should be ok
> to apply, and should be independent of the other as yet un-applied patches
> that have been posted by Rene and Krzysztof.

OK, I merged them to HG tree now.  Thanks.

> 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.
> 
> 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.
> 
> 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.

Yes, it'd be an easy solution.

> 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 here.

Hm, this may happen, too.

Anyway, let's do more cleanups after 1.0.15 release.


Takashi