Re: [PATCH 2/3] ASoC: DaVinci: Added support based on copy_from_user instead of DMA

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Fri, Jul 16, 2010 at 04:46:58PM +0200, Raffaele Recalcati wrote:
> From: Davide Bonfanti <davide.bonfanti@bticino.it>
> 
>     This driver implements a pcm interface without the use of a DMA but with
>     a copy_from_user.
>     There's a buffer in the driver that is filled with davinci_pcm_copy.
>     When pcm is running, a TIMER interrupt is activated in order to fill
>     HW FIFO.
>     BUG: It happens sometimes that the peripheral stops working so there's a
>     trap.

Looking at this code the main thing that jumps out at me is that it
doesn't look at all DaVinci specific - all the interaction with the
hardware is hidden behind the ops structure you've defined, though the
ops structure doesn't define things like the maximum number of channels
and sample rate which I'd expect it to.  This suggests that either the
patch shouldn't be DaVinci specific or the splitting out of the ops
structure isn't adding anything.

> +int pointer_sub;

These should all be static.

> +int hw_fifo_size;
> +u16 *local_buffer;
> +static struct hrtimer hrtimer;
> +struct snd_pcm_substream *substream_loc;
> +int ns_for_interrupt = 1500000;

Magic number?

> +       gpio_set_value(69, 0);

Magic number again, and nothing requests this GPIO either.  Looks like
another thing for the ops structure.