On Thu, Jul 15, 2010 at 08:09:04AM +0200, davide.bonfa...@bticino.it wrote:
> >Per: Raffaele Recalcati <lamiapost...@gmail.com>
> >Da: Mark Brown <broo...@opensource.wolfsonmicro.com>
> >Data: 14/07/2010 16.44

You might want to look at your MUA configuration here - it's sending
stuff with broken line endings which make things hard to read.

> >>     This driver implements Voicecodec without the use of a DMA but with
> >>     a copy_from_user.

> >Why is this specific to the voice CODEC?  Shouldn't this be generally
> >usable, and wouldn't it be better if the DMA driver were able to do some
> >automatic fallback here so that in cases where DMA can be used it will
> >be?

> The DMA can be dynamically allocated and I need to keep it free for I2S
> because it needs more throughput. I don't think it can be a good idea to
> copy 44KHz stereo data using a copy_from_user. What's your opinion about?

It's not ideal, no, but it can work and obviously people might choose to
play 8kHz out of either interface.  It could also come in handy if the
CPUs get a new DMA controller while waiting for support for it.

> >> +/*
> >> + *      ppcm = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
> >> + *                &pcm_hardware_playback : &pcm_hardware_capture;
> >> + */

> >Remove this if it's not used.

> I've implemented only voice output using copy_from_user since I don't need
> input. This is an anchor to attach also input. Can I put a TODO or shall I
> remove everything?

Remove it until it's implemented.

> >> +struct snd_soc_platform davinci_soc_platform_copy = {
> >> +          .name =                 "davinci-audio-copy",
> >> +          .pcm_ops =      &davinci_pcm_ops,
> >> +          .pcm_new =      davinci_pcm_new,
> >> +}; EXPORT_SYMBOL_GPL(davinci_soc_platform_copy);
> >
> >Fix your formatting here.

> checkpatch didn't help me.

The export should be on a line by itself.

> >Um....?  I'm not entirely sure what this and the rest of the changes in
> >the file are supposed to do but they weren't mentioned at all in the
> >changelog.  If this is needed it should be a separate change with a
> >proper changelog explaining what's going on.

> Of course I have to split the commit. here the problem is that soc_pcm_ops
> was statically instantiated so if one call more than one time soc_new_pcm
> the operations functions are overwritten.

You're looking for the multi-CODEC work, due to be integrated very soon.
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to