I still noticed some type cleanups, and possibly a good debug message..

On Mon, May 14, 2007 at 11:14:55PM +0100, Adrian McMenamin wrote:
> +/* spu_memload - write to SPU address space */
> +static void spu_memload(u32 toi, void __iomem * from, int length)
> +{
> +     u32 __iomem *froml = from;
> +     u32 __iomem *to = (u32 __iomem *) (SPU_MEMORY_BASE + toi);
> +     int i, val;

You should use "void *from" because it's not IO memory. Also, use "u32 val"
since that is the native type of "u32 *from".

> +     length = DIV_ROUND_UP(length, 4);
> +     spu_write_wait();
> +     for (i = 0; i < length; i++) {
> +             val = *froml;
> +             writel(val, to);
> +             froml++;
> +             to++;
> +             if (i && !(i % 8))
> +                     spu_write_wait();
> +     }
> +}

It seems a small simplification is possible:

        /* remove first spu_write_wait(); */
        for (i = 0; i < length; i++) {
                if (!(i % 8))
                        spu_write_wait();
                val = *froml;
                ...
        }


> +/* SPU specific functions */
> +/* spu_write_wait - wait for G2-SH FIFO to clear */
> +static void spu_write_wait(void)
> +{
> +     int time_count; 
> +     time_count = 0;
> +     while (1) {
> +             if (!(readl(G2_FIFO) & 0x11))
> +                     break;
> +             /* To ensure hardware failure doesn't wedge kernel */
> +             time_count++;
> +             if (time_count > 0x10000)

Maybe this deserves a printk() message?

> +static struct snd_pcm_hardware snd_pcm_aica_playback_hw = {
> +     .info = (SNDRV_PCM_INFO_NONINTERLEAVED),
> +     .formats =
> +         (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE |
> +          SNDRV_PCM_FMTBIT_IMA_ADPCM),
> +     .rates = SNDRV_PCM_RATE_8000_48000,
> +     .rate_min = 8000,
> +     .rate_max = 48000,
> +     .channels_min = 1,
> +     .channels_max = 2,
> +     .buffer_bytes_max = AICA_BUFFER_SIZE,
> +     .period_bytes_min = AICA_PERIOD_SIZE,
> +     .period_bytes_max = AICA_PERIOD_SIZE,
> +     .periods_min = AICA_PERIOD_NUMBER,
> +     .periods_max = AICA_PERIOD_NUMBER,
> +};
> +
> +static int aica_dma_transfer(int channels, int buffer_size,
> +                          struct snd_pcm_substream *substream)
> +{
> +     int q, err, period_offset;
> +     struct snd_card_aica *dreamcastcard;
> +     struct snd_pcm_runtime *runtime;
> +     err = 0;
> +     dreamcastcard = substream->pcm->private_data;
> +     period_offset = dreamcastcard->clicks;
> +     period_offset %= (AICA_PERIOD_NUMBER / channels);
> +     runtime = substream->runtime;
> +     for (q = 0; q < channels; q++) {
> +             err = dma_xfer(AICA_DMA_CHANNEL,
> +                            (unsigned long)(runtime->dma_area +
> +                                            (AICA_BUFFER_SIZE * q) /
> +                                            channels +
> +                                            AICA_PERIOD_SIZE *
> +                                            period_offset),
> +                            AICA_CHANNEL0_OFFSET + q * CHANNEL_OFFSET +
> +                            AICA_PERIOD_SIZE * period_offset,
> +                            buffer_size / channels, AICA_DMA_MODE);
> +             if (unlikely(err < 0))
> +                     break;
> +             dma_wait_for_completion(AICA_DMA_CHANNEL);
> +     }
> +     return err;
> +}
> +
> +static void startup_aica(struct snd_card_aica *dreamcastcard)
> +{
> +     spu_memload(AICA_CHANNEL0_CONTROL_OFFSET,
> +                 (u8 *) dreamcastcard->channel,

Why cast to u8 *? spu_memload gets a void *, and the void * is casted 
into an u32 *. 

> +                 sizeof(struct aica_channel));
> +     aica_chn_start();
> +}
> +
> +static void run_spu_dma(struct work_struct *work)
> +{
> +     int buffer_size;
> +     struct snd_pcm_substream *substream;
> +     struct snd_pcm_runtime *runtime;
> +     struct snd_card_aica *dreamcastcard;
> +     struct spu_work_holder *holder = container_of(work, struct 
> spu_work_holder, spu_dma_work);
> +     substream = holder-> sspointer;
> +     dreamcastcard = substream->pcm->private_data;
> +     runtime = substream->runtime;
> +     if (unlikely(dreamcastcard->dma_check == 0)) {
> +             buffer_size = frames_to_bytes(runtime, runtime->buffer_size);
> +             if (runtime->channels > 1)
> +                     dreamcastcard->channel->flags |= 0x01;
> +             aica_dma_transfer(runtime->channels, buffer_size, substream);
> +             startup_aica(dreamcastcard);
> +             dreamcastcard->clicks =
> +                 buffer_size / (AICA_PERIOD_SIZE * runtime->channels);
> +             return;
> +     } else {
> +             aica_dma_transfer(runtime->channels,
> +                               AICA_PERIOD_SIZE * runtime->channels,
> +                               substream);
> +             snd_pcm_period_elapsed(dreamcastcard->substream);
> +             dreamcastcard->clicks++;
> +             if (unlikely(dreamcastcard->clicks >= AICA_PERIOD_NUMBER))
> +             {
> +                     dreamcastcard->clicks %= AICA_PERIOD_NUMBER;
> +             }
> +             mod_timer(&dreamcastcard->timer, jiffies + 1);
> +     }
> +}
> +
> +static void aica_period_elapsed(unsigned long timer_var)
> +{
> +     /*timer function - so cannot sleep */
> +     int play_period;
> +     struct snd_pcm_runtime *runtime;
> +     struct snd_pcm_substream *substream;
> +     struct snd_card_aica *dreamcastcard;
> +     substream = (struct snd_pcm_substream *)timer_var;
> +     runtime = substream->runtime;
> +     dreamcastcard = substream->pcm->private_data;
> +     /* Have we played out an additional period? */
> +     play_period =
> +         frames_to_bytes(runtime,
> +                         readl
> +                         (AICA_CONTROL_CHANNEL_SAMPLE_NUMBER)) /
> +         AICA_PERIOD_SIZE;
> +     if (play_period == dreamcastcard->current_period) {
> +             /* reschedule the timer */
> +             mod_timer(&(dreamcastcard->timer), jiffies + 1);
> +             return;
> +     }
> +     if (runtime->channels > 1)
> +             dreamcastcard->current_period = play_period;
> +     if (unlikely(dreamcastcard->dma_check == 0))
> +             dreamcastcard->dma_check = 1;
> +             queue_work(aica_queue, &(spu_working.spu_dma_work));
> +}
> +
> +static void spu_begin_dma(struct snd_pcm_substream *substream)
> +{
> +     /* Must be atomic */
> +     struct snd_card_aica *dreamcastcard;
> +     struct snd_pcm_runtime *runtime;
> +     runtime = substream->runtime;
> +     dreamcastcard = substream->pcm->private_data;
> +     /*  Use queue to do the heavy lifting */
> +     spu_working.sspointer = substream;
> +     INIT_WORK(&(spu_working.spu_dma_work), run_spu_dma);
> +     queue_work(aica_queue, &(spu_working.spu_dma_work));
> +     /* Timer may already be running */
> +     if (unlikely(dreamcastcard->timer.data)) {
> +             mod_timer(&dreamcastcard->timer, jiffies + 4);
> +             return;
> +     }
> +     init_timer(&(dreamcastcard->timer));
> +     dreamcastcard->timer.data = (unsigned long)substream;
> +     dreamcastcard->timer.function = aica_period_elapsed;
> +     dreamcastcard->timer.expires = jiffies + 4;
> +     add_timer(&(dreamcastcard->timer));
> +}
> +
> +static int snd_aicapcm_pcm_open(struct snd_pcm_substream
> +                             *substream)
> +{
> +     struct snd_pcm_runtime *runtime;
> +     struct aica_channel *channel;
> +     struct snd_card_aica *dreamcastcard;
> +     if (!enable)
> +             return -ENOENT;
> +     dreamcastcard = substream->pcm->private_data;
> +     channel = kmalloc(sizeof(struct aica_channel), GFP_KERNEL);
> +     if (!channel)
> +             return -ENOMEM;
> +     /* set defaults for channel */
> +     channel->sfmt = SM_8BIT;
> +     channel->cmd = AICA_CMD_START;
> +     channel->vol = dreamcastcard->master_volume;
> +     channel->pan = 0x80;
> +     channel->pos = 0;
> +     channel->flags = 0;     /* default to mono */
> +     dreamcastcard->channel = channel;
> +     runtime = substream->runtime;
> +     runtime->hw = snd_pcm_aica_playback_hw;
> +     spu_enable();
> +     dreamcastcard->clicks = 0;
> +     dreamcastcard->current_period = 0;
> +     dreamcastcard->dma_check = 0;
> +     return 0;
> +}
> +
> +static int snd_aicapcm_pcm_close(struct snd_pcm_substream
> +                              *substream)
> +{
> +     struct snd_card_aica *dreamcastcard = substream->pcm->private_data;
> +     del_timer(&dreamcastcard->timer);
> +     kfree(dreamcastcard->channel);
> +     spu_disable();
> +     return 0;
> +}
> +
> +static int snd_aicapcm_pcm_hw_free(struct snd_pcm_substream
> +                                *substream)
> +{
> +     /* Free the DMA buffer */
> +     return snd_pcm_lib_free_pages(substream);
> +}
> +
> +static int snd_aicapcm_pcm_hw_params(struct snd_pcm_substream
> +                                  *substream, struct snd_pcm_hw_params
> +                                  *hw_params)
> +{
> +     /* Allocate a DMA buffer using ALSA built-ins */
> +     return
> +         snd_pcm_lib_malloc_pages(substream, params_buffer_bytes(hw_params));
> +}
> +
> +static int snd_aicapcm_pcm_prepare(struct snd_pcm_substream
> +                                *substream)
> +{
> +     struct snd_card_aica *dreamcastcard = substream->pcm->private_data;
> +     if ((substream->runtime)->format == SNDRV_PCM_FORMAT_S16_LE)
> +             dreamcastcard->channel->sfmt = SM_16BIT;
> +     dreamcastcard->channel->freq = substream->runtime->rate;
> +     dreamcastcard->substream = substream;
> +     return 0;
> +}
> +
> +static int snd_aicapcm_pcm_trigger(struct snd_pcm_substream
> +                                *substream, int cmd)
> +{
> +     struct snd_card_aica *dreamcastcard;
> +     switch (cmd) {
> +     case SNDRV_PCM_TRIGGER_START:
> +             spu_begin_dma(substream);
> +             break;
> +     case SNDRV_PCM_TRIGGER_STOP:
> +             dreamcastcard = substream->pcm->private_data;
> +             if (dreamcastcard->timer.data)
> +                     del_timer(&dreamcastcard->timer);
> +             aica_chn_halt();
> +             break;
> +     default:
> +             return -EINVAL;
> +     }
> +     return 0;
> +}
> +
> +static unsigned long snd_aicapcm_pcm_pointer(struct snd_pcm_substream
> +                                          *substream)
> +{
> +     return readl(AICA_CONTROL_CHANNEL_SAMPLE_NUMBER);
> +}
> +
> +static struct snd_pcm_ops snd_aicapcm_playback_ops = {
> +     .open = snd_aicapcm_pcm_open,
> +     .close = snd_aicapcm_pcm_close,
> +     .ioctl = snd_pcm_lib_ioctl,
> +     .hw_params = snd_aicapcm_pcm_hw_params,
> +     .hw_free = snd_aicapcm_pcm_hw_free,
> +     .prepare = snd_aicapcm_pcm_prepare,
> +     .trigger = snd_aicapcm_pcm_trigger,
> +     .pointer = snd_aicapcm_pcm_pointer,
> +};
> +
> +/* TO DO: set up to handle more than one pcm instance */
> +static int __init snd_aicapcmchip(struct snd_card_aica
> +                               *dreamcastcard, int pcm_index)
> +{
> +     struct snd_pcm *pcm;
> +     int err;
> +     /* AICA has no capture ability */
> +     err =
> +         snd_pcm_new(dreamcastcard->card, "AICA PCM", pcm_index, 1, 0, &pcm);
> +     if (unlikely(err < 0))
> +             return err;
> +     pcm->private_data = dreamcastcard;
> +     strcpy(pcm->name, "AICA PCM");
> +     snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK,
> +                     &snd_aicapcm_playback_ops);
> +     /* Allocate the DMA buffers */
> +     err =
> +         snd_pcm_lib_preallocate_pages_for_all(pcm,
> +                                               SNDRV_DMA_TYPE_CONTINUOUS,
> +                                               snd_dma_continuous_data
> +                                               (GFP_KERNEL),
> +                                               AICA_BUFFER_SIZE,
> +                                               AICA_BUFFER_SIZE);
> +     return err;
> +}
> +
> +/* Mixer controls */
> +static int aica_pcmswitch_info(struct snd_kcontrol *kcontrol,
> +                            struct snd_ctl_elem_info *uinfo)
> +{
> +     uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
> +     uinfo->count = 1;
> +     uinfo->value.integer.min = 0;
> +     uinfo->value.integer.max = 1;
> +     return 0;
> +}
> +
> +static int aica_pcmswitch_get(struct snd_kcontrol *kcontrol,
> +                           struct snd_ctl_elem_value *ucontrol)
> +{
> +     ucontrol->value.integer.value[0] = 1;   /* TO DO: Fix me */
> +     return 0;
> +}
> +
> +static int aica_pcmswitch_put(struct snd_kcontrol *kcontrol,
> +                           struct snd_ctl_elem_value *ucontrol)
> +{
> +     if (ucontrol->value.integer.value[0] == 1)
> +             return 0;       /* TO DO: Fix me */
> +     else
> +             aica_chn_halt();
> +     return 0;
> +}
> +
> +static int aica_pcmvolume_info(struct snd_kcontrol *kcontrol,
> +                            struct snd_ctl_elem_info *uinfo)
> +{
> +     uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
> +     uinfo->count = 1;
> +     uinfo->value.integer.min = 0;
> +     uinfo->value.integer.max = 0xFF;
> +     return 0;
> +}
> +
> +static int aica_pcmvolume_get(struct snd_kcontrol *kcontrol,
> +                           struct snd_ctl_elem_value *ucontrol)
> +{
> +     struct snd_card_aica *dreamcastcard;
> +     dreamcastcard = kcontrol->private_data;
> +     if (unlikely(!dreamcastcard->channel))
> +             return -ETXTBSY;        /* we've not yet been set up */
> +     ucontrol->value.integer.value[0] = dreamcastcard->channel->vol;
> +     return 0;
> +}
> +
> +static int aica_pcmvolume_put(struct snd_kcontrol *kcontrol,
> +                           struct snd_ctl_elem_value *ucontrol)
> +{
> +     struct snd_card_aica *dreamcastcard;
> +     dreamcastcard = kcontrol->private_data;
> +     if (unlikely(!dreamcastcard->channel))
> +             return -ETXTBSY;
> +     if (unlikely(dreamcastcard->channel->vol ==
> +                  ucontrol->value.integer.value[0]))
> +             return 0;
> +     dreamcastcard->channel->vol = ucontrol->value.integer.value[0];
> +     dreamcastcard->master_volume = ucontrol->value.integer.value[0];
> +     spu_memload(AICA_CHANNEL0_CONTROL_OFFSET,
> +                 (u8 *) dreamcastcard->channel,

Unuseful cast again.

-- 
Heikki Orsila                   Barbie's law:
[EMAIL PROTECTED]               "Math is hard, let's go shopping!"
http://www.iki.fi/shd
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to