[alsa-devel] [PATCH] 1/2 ALSA: ca0106 support 44100Hz playback to spdif

[Takashi Iwai]

At Wed, 17 Dec 2008 01:37:07 +1100,
Ben Stanley wrote:
> 
> >From 241939bdf80de14e204d6fcb43d850d33bdeda20 Mon Sep 17 00:00:00 2001
> From: Ben Stanley <Ben.Stanley at exemail.com.au>
> Date: Tue, 16 Dec 2008 23:47:36 +1100
> Subject: [PATCH] ALSA: ca0106 support 44100Hz playback to spdif
> 
> This patch provides support for playback of 44100Hz sampled material in
> 16 and 32 bit sample depths through the digital spdif/iec958 connection/s
> available on the ca0106 card.
> 
> I have re-worked the patch to address comments from a previous review 
> by Takashi [1]. In particular, I have incorporated spin-locking to address 
> a race condition that Takashi was concerned about.
> 
> This code has been running on my MythTV box for the last fortnight without
> apparent ills. I have re-based the code a bit for submission.
> 
> This patch is against alsa-kmirror.

Could you rebase against sound git tree?  There have been changes
wrt ca0106 recently there, and your patch conflicts.
   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git

The corresponding alsa-driver build stub is found in
   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/alsa-driver-build.git

   
> diff --git a/pci/ca0106/ca0106_main.c b/pci/ca0106/ca0106_main.c
> index 6ac1936..bc78312 100644
> --- a/pci/ca0106/ca0106_main.c
> +++ b/pci/ca0106/ca0106_main.c
(snip)
> +void snd_ca0106_rebuild_playback_channel_counters(struct snd_ca0106 *chip,
> +						  int skip_channel)

Missing static.

> @@ -463,7 +573,20 @@ static void snd_ca0106_intr_disable(struct snd_ca0106 *emu, unsigned int intrenb
>  
>  static void snd_ca0106_pcm_free_substream(struct snd_pcm_runtime *runtime)
>  {
> +	struct snd_ca0106_pcm *epcm = runtime->private_data;
> +	struct snd_ca0106 *chip = epcm->emu;
> +	/* FIXME how to tell which case to use? */
> +	/* struct snd_ca0106_channel *channel =
> +		&(chip->playback_channels[epcm->channel_id]); */
> +	/* struct snd_ca0106_channel *channel =
> +		&(chip->capture_channels[epcm->channel_id]); */

You can check substream->stream.

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&chip->pcm_lock, flags);

This function is always non-atomic and safe to call without
saving/restoring flags (i.e. spin_lock_irq() and spin_unlock_irq())
But, as far as I see, all places you use the lock are non-atomic.
That is, it'd be better to use a mutex instead of a spinlock.


thanks,

Takashi