[ALSA] au1x00 - Code clean up

Modules: MIPS AU1x00 driver

Clean up snd-au1x00 driver code:

- Remove global variables
- Remove old compatibility codes
- Fix DMA-link allocation/release functions in hw_params and hw_free
  callbacks (they may be called multiple times)
- Fix spinlocks

Signed-off-by: Takashi Iwai <tiwai@suse.de>

1 files changed, 150 insertions, 150 deletions

diff --git a/sound/mips/au1x00.c b/sound/mips/au1x00.c
index d08a42b24b1f..a8f9a3b6e73d 100644
--- a/sound/mips/au1x00.c
+++ b/sound/mips/au1x00.c
@@ -50,12 +50,7 @@
 MODULE_AUTHOR("Charles Eidsness <charles@cooper-street.com>");
 MODULE_DESCRIPTION("Au1000 AC'97 ALSA Driver");
 MODULE_LICENSE("GPL");
-#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,8)
 MODULE_SUPPORTED_DEVICE("{{AMD,Au1000 AC'97}}");
-#else
-MODULE_CLASSES("{sound}");
-MODULE_DEVICES("{{AMD,Au1000 AC'97}}");
-#endif
 
 #define PLAYBACK 0
 #define CAPTURE 1
@@ -68,8 +63,6 @@ MODULE_DEVICES("{{AMD,Au1000 AC'97}}");
 #define READ_WAIT 2
 #define RW_DONE 3
 
-DECLARE_WAIT_QUEUE_HEAD(ac97_command_wq);
-
 typedef struct au1000_period au1000_period_t;
 struct au1000_period
 {
@@ -94,7 +87,8 @@ struct audio_stream {
         int dma;
         spinlock_t dma_lock;
         au1000_period_t * buffer;
-        unsigned long period_size;
+        unsigned int period_size;
+        unsigned int periods;
 };
 
 typedef struct snd_card_au1000 {
@@ -109,11 +103,9 @@ typedef struct snd_card_au1000 {
         audio_stream_t *stream[2];      /* playback & capture */
 } au1000_t;
 
-static au1000_t *au1000 = NULL;
-
 /*--------------------------- Local Functions --------------------------------*/
 static void
-au1000_set_ac97_xmit_slots(long xmit_slots)
+au1000_set_ac97_xmit_slots(au1000_t *au1000, long xmit_slots)
 {
         u32 volatile ac97_config;
 
@@ -126,7 +118,7 @@ au1000_set_ac97_xmit_slots(long xmit_slots)
 }
 
 static void
-au1000_set_ac97_recv_slots(long recv_slots)
+au1000_set_ac97_recv_slots(au1000_t *au1000, long recv_slots)
 {
         u32 volatile ac97_config;
 
@@ -140,79 +132,92 @@ au1000_set_ac97_recv_slots(long recv_slots)
 
 
 static void
-au1000_dma_stop(audio_stream_t *stream)
+au1000_release_dma_link(audio_stream_t *stream)
 {
-        unsigned long   flags;
         au1000_period_t * pointer;
         au1000_period_t * pointer_next;
 
-        if (stream->buffer != NULL) {
-                spin_lock_irqsave(&stream->dma_lock, flags);
-                disable_dma(stream->dma);
-                spin_unlock_irqrestore(&stream->dma_lock, flags);
+        stream->period_size = 0;
+        stream->periods = 0;
+        pointer = stream->buffer;
+        if (! pointer)
+                return;
+        do {
+                pointer_next = pointer->next;
+                kfree(pointer);
+                pointer = pointer_next;
+        } while (pointer != stream->buffer);
+        stream->buffer = NULL;
+}
+
+static int
+au1000_setup_dma_link(audio_stream_t *stream, unsigned int period_bytes,
+                      unsigned int periods)
+{
+        snd_pcm_substream_t *substream = stream->substream;
+        snd_pcm_runtime_t *runtime = substream->runtime;
+        unsigned long dma_start;
+        int i;
+
+        dma_start = virt_to_phys(runtime->dma_area);
 
-                pointer = stream->buffer;
-                pointer_next = stream->buffer->next;
+        if (stream->period_size == period_bytes &&
+            stream->periods == periods)
+                return 0; /* not changed */
 
-                do {
-                        kfree(pointer);
-                        pointer = pointer_next;
-                        pointer_next = pointer->next;
-                } while (pointer != stream->buffer);
+        au1000_release_dma_link(stream);
 
-                stream->buffer = NULL;
+        stream->period_size = period_bytes;
+        stream->periods = periods;
+
+        stream->buffer = kmalloc(sizeof(au1000_period_t), GFP_KERNEL);
+        if (! stream->buffer)
+                return -ENOMEM;
+        pointer = stream->buffer;
+        for (i = 0; i < periods; i++) {
+                pointer->start = (u32)(dma_start + (i * period_bytes));
+                pointer->relative_end = (u32) (((i+1) * period_bytes) - 0x1);
+                if (i < periods - 1) {
+                        pointer->next = kmalloc(sizeof(struct au1000_period), GFP_KERNEL);
+                        if (! pointer->next) {
+                                au1000_release_dma_link(stream);
+                                return -ENOMEM;
+                        }
+                        pointer = pointer->next;
+                }
         }
+        pointer->next = stream->buffer;
+        return 0;
 }
 
 static void
-au1000_dma_start(audio_stream_t *stream)
+au1000_dma_stop(audio_stream_t *stream)
 {
-        snd_pcm_substream_t *substream = stream->substream;
-        snd_pcm_runtime_t *runtime = substream->runtime;
+        snd_assert(stream->buffer, return);
+        disable_dma(stream->dma);
+}
 
-        unsigned long flags, dma_start;
-        int i;
-        au1000_period_t * pointer;
+static void
+au1000_dma_start(audio_stream_t *stream)
+{
+        snd_assert(stream->buffer, return);
 
-        if (stream->buffer == NULL) {
-                dma_start = virt_to_phys(runtime->dma_area);
-
-                stream->period_size = frames_to_bytes(runtime,
-                        runtime->period_size);
-                stream->buffer = kmalloc(sizeof(au1000_period_t), GFP_KERNEL);
-                pointer = stream->buffer;
-                for (i = 0 ; i < runtime->periods ; i++) {
-                        pointer->start = (u32)(dma_start +
-                                (i * stream->period_size));
-                        pointer->relative_end = (u32)
-                                (((i+1) * stream->period_size) - 0x1);
-                        if ( i < runtime->periods - 1) {
-                                pointer->next = kmalloc(sizeof(au1000_period_t)
-                                        , GFP_KERNEL);
-                                pointer = pointer->next;
-                        }
-                }
-                pointer->next = stream->buffer;
-
-                spin_lock_irqsave(&stream->dma_lock, flags);
-                init_dma(stream->dma);
-                if (get_dma_active_buffer(stream->dma) == 0) {
-                        clear_dma_done0(stream->dma);
-                        set_dma_addr0(stream->dma, stream->buffer->start);
-                        set_dma_count0(stream->dma, stream->period_size >> 1);
-                        set_dma_addr1(stream->dma, stream->buffer->next->start);
-                        set_dma_count1(stream->dma, stream->period_size >> 1);
-                } else {
-                        clear_dma_done1(stream->dma);
-                        set_dma_addr1(stream->dma, stream->buffer->start);
-                        set_dma_count1(stream->dma, stream->period_size >> 1);
-                        set_dma_addr0(stream->dma, stream->buffer->next->start);
-                        set_dma_count0(stream->dma, stream->period_size >> 1);
-                }
-                enable_dma_buffers(stream->dma);
-                start_dma(stream->dma);
-                spin_unlock_irqrestore(&stream->dma_lock, flags);
+        init_dma(stream->dma);
+        if (get_dma_active_buffer(stream->dma) == 0) {
+                clear_dma_done0(stream->dma);
+                set_dma_addr0(stream->dma, stream->buffer->start);
+                set_dma_count0(stream->dma, stream->period_size >> 1);
+                set_dma_addr1(stream->dma, stream->buffer->next->start);
+                set_dma_count1(stream->dma, stream->period_size >> 1);
+        } else {
+                clear_dma_done1(stream->dma);
+                set_dma_addr1(stream->dma, stream->buffer->start);
+                set_dma_count1(stream->dma, stream->period_size >> 1);
+                set_dma_addr0(stream->dma, stream->buffer->next->start);
+                set_dma_count0(stream->dma, stream->period_size >> 1);
         }
+        enable_dma_buffers(stream->dma);
+        start_dma(stream->dma);
 }
 
 static irqreturn_t
@@ -238,11 +243,9 @@ au1000_dma_interrupt(int irq, void *dev_id, struct pt_regs *regs)
                 enable_dma_buffer1(stream->dma);
                 break;
         case (DMA_D0 | DMA_D1):
-                spin_unlock(&stream->dma_lock);
                 printk(KERN_ERR "DMA %d missed interrupt.\n",stream->dma);
                 au1000_dma_stop(stream);
                 au1000_dma_start(stream);
-                spin_lock(&stream->dma_lock);
                 break;
         case (~DMA_D0 & ~DMA_D1):
                 printk(KERN_ERR "DMA %d empty irq.\n",stream->dma);
@@ -261,7 +264,7 @@ static snd_pcm_hw_constraint_list_t hw_constraints_rates = {
         .mask   = 0,
 };
 
-static snd_pcm_hardware_t snd_au1000 =
+static snd_pcm_hardware_t snd_au1000_hw =
 {
         .info                   = (SNDRV_PCM_INFO_INTERLEAVED | \
                                 SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID),
@@ -283,10 +286,12 @@ static snd_pcm_hardware_t snd_au1000 =
 static int
 snd_au1000_playback_open(snd_pcm_substream_t * substream)
 {
+        au1000_t *au1000 = substream->pcm->private_data;
+
         au1000->stream[PLAYBACK]->substream = substream;
         au1000->stream[PLAYBACK]->buffer = NULL;
         substream->private_data = au1000->stream[PLAYBACK];
-        substream->runtime->hw = snd_au1000;
+        substream->runtime->hw = snd_au1000_hw;
         return (snd_pcm_hw_constraint_list(substream->runtime, 0,
                 SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates) < 0);
 }
@@ -294,10 +299,12 @@ snd_au1000_playback_open(snd_pcm_substream_t * substream)
 static int
 snd_au1000_capture_open(snd_pcm_substream_t * substream)
 {
+        au1000_t *au1000 = substream->pcm->private_data;
+
         au1000->stream[CAPTURE]->substream = substream;
         au1000->stream[CAPTURE]->buffer = NULL;
         substream->private_data = au1000->stream[CAPTURE];
-        substream->runtime->hw = snd_au1000;
+        substream->runtime->hw = snd_au1000_hw;
         return (snd_pcm_hw_constraint_list(substream->runtime, 0,
                 SNDRV_PCM_HW_PARAM_RATE, &hw_constraints_rates) < 0);
 
@@ -306,6 +313,8 @@ snd_au1000_capture_open(snd_pcm_substream_t * substream)
 static int
 snd_au1000_playback_close(snd_pcm_substream_t * substream)
 {
+        au1000_t *au1000 = substream->pcm->private_data;
+
         au1000->stream[PLAYBACK]->substream = NULL;
         return 0;
 }
@@ -313,6 +322,8 @@ snd_au1000_playback_close(snd_pcm_substream_t * substream)
 static int
 snd_au1000_capture_close(snd_pcm_substream_t * substream)
 {
+        au1000_t *au1000 = substream->pcm->private_data;
+
         au1000->stream[CAPTURE]->substream = NULL;
         return 0;
 }
@@ -321,25 +332,36 @@ static int
 snd_au1000_hw_params(snd_pcm_substream_t * substream,
                                         snd_pcm_hw_params_t * hw_params)
 {
-        return snd_pcm_lib_malloc_pages(substream,
-                                        params_buffer_bytes(hw_params));
+        audio_stream_t *stream = substream->private_data;
+        int err;
+
+        err = snd_pcm_lib_malloc_pages(substream,
+                                       params_buffer_bytes(hw_params));
+        if (err < 0)
+                return err;
+        return au1000_setup_dma_link(stream,
+                                     params_period_bytes(hw_params),
+                                     params_periods(hw_params));
 }
 
 static int
 snd_au1000_hw_free(snd_pcm_substream_t * substream)
 {
+        audio_stream_t *stream = substream->private_data;
+        au1000_release_dma_link(stream);
         return snd_pcm_lib_free_pages(substream);
 }
 
 static int
 snd_au1000_playback_prepare(snd_pcm_substream_t * substream)
 {
+        au1000_t *au1000 = substream->pcm->private_data;
         snd_pcm_runtime_t *runtime = substream->runtime;
 
-        if (runtime->channels == 1 )
-                au1000_set_ac97_xmit_slots(AC97_SLOT_4);
+        if (runtime->channels == 1)
+                au1000_set_ac97_xmit_slots(au1000, AC97_SLOT_4);
         else
-                au1000_set_ac97_xmit_slots(AC97_SLOT_3 | AC97_SLOT_4);
+                au1000_set_ac97_xmit_slots(au1000, AC97_SLOT_3 | AC97_SLOT_4);
         snd_ac97_set_rate(au1000->ac97, AC97_PCM_FRONT_DAC_RATE, runtime->rate);
         return 0;
 }
@@ -347,12 +369,13 @@ snd_au1000_playback_prepare(snd_pcm_substream_t * substream)
 static int
 snd_au1000_capture_prepare(snd_pcm_substream_t * substream)
 {
+        au1000_t *au1000 = substream->pcm->private_data;
         snd_pcm_runtime_t *runtime = substream->runtime;
 
-        if (runtime->channels == 1 )
-                au1000_set_ac97_recv_slots(AC97_SLOT_4);
+        if (runtime->channels == 1)
+                au1000_set_ac97_recv_slots(au1000, AC97_SLOT_4);
         else
-                au1000_set_ac97_recv_slots(AC97_SLOT_3 | AC97_SLOT_4);
+                au1000_set_ac97_recv_slots(au1000, AC97_SLOT_3 | AC97_SLOT_4);
         snd_ac97_set_rate(au1000->ac97, AC97_PCM_LR_ADC_RATE, runtime->rate);
         return 0;
 }
@@ -363,6 +386,7 @@ snd_au1000_trigger(snd_pcm_substream_t * substream, int cmd)
         audio_stream_t *stream = substream->private_data;
         int err = 0;
 
+        spin_lock(&stream->dma_lock);
         switch (cmd) {
         case SNDRV_PCM_TRIGGER_START:
                 au1000_dma_start(stream);
@@ -374,6 +398,7 @@ snd_au1000_trigger(snd_pcm_substream_t * substream, int cmd)
                 err = -EINVAL;
                 break;
         }
+        spin_unlock(&stream->dma_lock);
         return err;
 }
 
@@ -382,12 +407,11 @@ snd_au1000_pointer(snd_pcm_substream_t * substream)
 {
         audio_stream_t *stream = substream->private_data;
         snd_pcm_runtime_t *runtime = substream->runtime;
-        unsigned long flags;
         long location;
 
-        spin_lock_irqsave(&stream->dma_lock, flags);
+        spin_lock(&stream->dma_lock);
         location = get_dma_residue(stream->dma);
-        spin_unlock_irqrestore(&stream->dma_lock, flags);
+        spin_unlock(&stream->dma_lock);
         location = stream->buffer->relative_end - location;
         if (location == -1)
                 location = 0;
@@ -438,6 +462,9 @@ snd_au1000_pcm_new(void)
         pcm->info_flags = 0;
         strcpy(pcm->name, "Au1000 AC97 PCM");
 
+        spin_lock_init(&au1000->stream[PLAYBACK]->dma_lock);
+        spin_lock_init(&au1000->stream[CAPTURE]->dma_lock);
+
         flags = claim_dma_lock();
         if ((au1000->stream[PLAYBACK]->dma = request_au1000_dma(DMA_ID_AC97C_TX,
                         "AC97 TX", au1000_dma_interrupt, SA_INTERRUPT,
@@ -457,8 +484,6 @@ snd_au1000_pcm_new(void)
         set_dma_mode(au1000->stream[CAPTURE]->dma,
                      get_dma_mode(au1000->stream[CAPTURE]->dma) & ~DMA_NC);
         release_dma_lock(flags);
-        spin_lock_init(&au1000->stream[PLAYBACK]->dma_lock);
-        spin_lock_init(&au1000->stream[CAPTURE]->dma_lock);
         au1000->pcm = pcm;
         return 0;
 }
@@ -469,9 +494,11 @@ snd_au1000_pcm_new(void)
 static unsigned short
 snd_au1000_ac97_read(ac97_t *ac97, unsigned short reg)
 {
+        au1000_t *au1000 = ac97->private_data;
         u32 volatile cmd;
         u16 volatile data;
         int             i;
+
         spin_lock(&au1000->ac97_lock);
 /* would rather use the interupt than this polling but it works and I can't
 get the interupt driven case to work efficiently */
@@ -505,8 +532,10 @@ get the interupt driven case to work efficiently */
 static void
 snd_au1000_ac97_write(ac97_t *ac97, unsigned short reg, unsigned short val)
 {
+        au1000_t *au1000 = ac97->private_data;
         u32 cmd;
         int i;
+
         spin_lock(&au1000->ac97_lock);
 /* would rather use the interupt than this polling but it works and I can't
 get the interupt driven case to work efficiently */
@@ -522,28 +551,13 @@ get the interupt driven case to work efficiently */
         au1000->ac97_ioport->cmd = cmd;
         spin_unlock(&au1000->ac97_lock);
 }
-static void
-snd_au1000_ac97_free(ac97_t *ac97)
-{
-        au1000->ac97 = NULL;
-}
 
 static int __devinit
-snd_au1000_ac97_new(void)
+snd_au1000_ac97_new(au1000_t *au1000)
 {
         int err;
-
-#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,8)
-        ac97_bus_t *pbus;
-        ac97_template_t ac97;
-        static ac97_bus_ops_t ops = {
-                .write = snd_au1000_ac97_write,
-                .read = snd_au1000_ac97_read,
-        };
-#else
         ac97_bus_t bus, *pbus;
         ac97_t ac97;
-#endif
 
         if ((au1000->ac97_res_port = request_region(AC97C_CONFIG,
                         sizeof(au1000_ac97_reg_t), "Au1x00 AC97")) == NULL) {
@@ -554,8 +568,6 @@ snd_au1000_ac97_new(void)
 
         spin_lock_init(&au1000->ac97_lock);
 
-        spin_lock(&au1000->ac97_lock);
-
         /* configure pins for AC'97
         TODO: move to board_setup.c */
         au_writel(au_readl(SYS_PINFUNC) & ~0x02, SYS_PINFUNC);
@@ -572,26 +584,16 @@ snd_au1000_ac97_new(void)
         au1000->ac97_ioport->config = 0x0;
         mdelay(5);
 
-        spin_unlock(&au1000->ac97_lock);
-
         /* Initialise AC97 middle-layer */
-#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,8)
         if ((err = snd_ac97_bus(au1000->card, 0, &ops, au1000, &pbus)) < 0)
                 return err;
-#else
-        memset(&bus, 0, sizeof(bus));
-        bus.write = snd_au1000_ac97_write;
-        bus.read = snd_au1000_ac97_read;
-        if ((err = snd_ac97_bus(au1000->card, &bus, &pbus)) < 0)
-                return err;
-#endif
+
         memset(&ac97, 0, sizeof(ac97));
         ac97.private_data = au1000;
-        ac97.private_free = snd_au1000_ac97_free;
         if ((err = snd_ac97_mixer(pbus, &ac97, &au1000->ac97)) < 0)
                 return err;
-        return 0;
 
+        return 0;
 }
 
 /*------------------------------ Setup / Destroy ----------------------------*/
@@ -599,6 +601,7 @@ snd_au1000_ac97_new(void)
 void
 snd_au1000_free(snd_card_t *card)
 {
+        au1000_t *au1000 = card->private_data;
 
         if (au1000->ac97_res_port) {
                 /* put internal AC97 block into reset */
@@ -614,73 +617,70 @@ snd_au1000_free(snd_card_t *card)
                 free_au1000_dma(au1000->stream[CAPTURE]->dma);
 
         kfree(au1000->stream[PLAYBACK]);
-        au1000->stream[PLAYBACK] = NULL;
         kfree(au1000->stream[CAPTURE]);
-        au1000->stream[CAPTURE] = NULL;
-        kfree(au1000);
-        au1000 = NULL;
-
 }
 
+
+static snd_card_t *au1000_card;
+
 static int __init
 au1000_init(void)
 {
         int err;
+        snd_card_t *card;
+        au1000_t *au1000;
 
-        au1000 = kmalloc(sizeof(au1000_t), GFP_KERNEL);
-        if (au1000 == NULL)
-                return -ENOMEM;
-        au1000->stream[PLAYBACK] = kmalloc(sizeof(audio_stream_t), GFP_KERNEL);
-        if (au1000->stream[PLAYBACK] == NULL)
-                return -ENOMEM;
-        au1000->stream[CAPTURE] = kmalloc(sizeof(audio_stream_t), GFP_KERNEL);
-        if (au1000->stream[CAPTURE] == NULL)
+        card = snd_card_new(-1, "AC97", THIS_MODULE, sizeof(au1000_t));
+        if (card == NULL)
                 return -ENOMEM;
+
+        card->private_free = snd_au1000_free;
+        au1000 = card->private_data;
         /* so that snd_au1000_free will work as intended */
+        au1000->card = card;
         au1000->stream[PLAYBACK]->dma = -1;
         au1000->stream[CAPTURE]->dma = -1;
         au1000->ac97_res_port = NULL;
-
-        au1000->card = snd_card_new(-1, "AC97", THIS_MODULE, sizeof(au1000_t));
-        if (au1000->card == NULL) {
-                snd_au1000_free(au1000->card);
+        au1000->stream[PLAYBACK] = kmalloc(sizeof(audio_stream_t), GFP_KERNEL);
+        au1000->stream[CAPTURE] = kmalloc(sizeof(audio_stream_t), GFP_KERNEL);
+        if (au1000->stream[PLAYBACK] == NULL ||
+            au1000->stream[CAPTURE] == NULL) {
+                snd_card_free(card);
                 return -ENOMEM;
         }
 
-        au1000->card->private_data = (au1000_t *)au1000;
-        au1000->card->private_free = snd_au1000_free;
-
-        if ((err = snd_au1000_ac97_new()) < 0 ) {
-                snd_card_free(au1000->card);
+        if ((err = snd_au1000_ac97_new(au1000)) < 0 ) {
+                snd_card_free(card);
                 return err;
         }
 
-        if ((err = snd_au1000_pcm_new()) < 0) {
-                snd_card_free(au1000->card);
+        if ((err = snd_au1000_pcm_new(au1000)) < 0) {
+                snd_card_free(card);
                 return err;
         }
 
-        strcpy(au1000->card->driver, "AMD-Au1000-AC97");
-        strcpy(au1000->card->shortname, "Au1000-AC97");
-        sprintf(au1000->card->longname, "AMD Au1000--AC97 ALSA Driver");
+        strcpy(card->driver, "Au1000-AC97");
+        strcpy(card->shortname, "AMD Au1000-AC97");
+        sprintf(card->longname, "AMD Au1000--AC97 ALSA Driver");
 
-        if ((err = snd_card_set_generic_dev(au1000->card)) < 0) {
-                snd_card_free(au1000->card);
+        if ((err = snd_card_set_generic_dev(card)) < 0) {
+                snd_card_free(card);
                 return err;
         }
 
-        if ((err = snd_card_register(au1000->card)) < 0) {
-                snd_card_free(au1000->card);
+        if ((err = snd_card_register(card)) < 0) {
+                snd_card_free(card);
                 return err;
         }
 
         printk( KERN_INFO "ALSA AC97: Driver Initialized\n" );
+        au1000_card = card;
         return 0;
 }
 
 static void __exit au1000_exit(void)
 {
-        snd_card_free(au1000->card);
+        snd_card_free(au1000_card);
 }
 
 module_init(au1000_init);