[alsa-devel] [PATCH] ASoC: DaVinci: Added support for cpu clocking I2S

[Raffaele Recalcati]

2010/6/28 Mark Brown <broonie at opensource.wolfsonmicro.com>

> On Mon, Jun 28, 2010 at 08:44:46AM +0200, Raffaele Recalcati wrote:
> > From: Raffaele Recalcati <raffaele.recalcati at bticino.it>
>
> It's still very hard to understand what this patch is supposed to do.
> As previously mentioned this would probably be a lot clearer if you
> split this into multiple patches, for example one adding support for the
> fast clock mode, one adding support for selecting the pin used for any
> external clock and then further patches with any other changes.
>

Looking at other paches, they are simpler than mine.
I'll try to split it, hoping to obtain the final result.


>
> I strongly suggest looking at the commit messages for other patches in
> the kernel and trying to follow a similar style.
>
> >     Added audio playback support with [frame sync master - clock master]
> mode
> >     and with [frame sync master - clock slave].
>
> What are these modes - which clock are you talking about?
>

McBSP i2s interface to external codec.


>
> >        i2s_fast_clock switch can be used to have better approximate or
> >        symmetric waveforms.
>
> Why would someone choose not to use this?
>

I was not sure if symmetric waveform can be a must.
In general I think it is better a non symmetric, but better approximate,
waveform.
Anyway, it is better to have the possibility to choose in my opinion,
because I have not so much experience in i2s communication.


>
> >        clk_input_pin board info can be used to select it depending on hw
> >        connections
>
> >     3. We haven't changed the evmdm365 support (due also to CPLD that
> doesn't
> >                                                 help to understand)
> >         We don't know in this mode if audio stereo works on evmdm365.
> >         Probably it does.
>
> This is what makes me unsure if you're trying to add new modes or not -
> if you're adding new modes then I'd expect that existing boards would be
> unaffected with any changes to use the new feature being easily
> seperable.
>

Some tests are needed, but it requires time.
I'll try try to make some tests on evmdm365, but I'm not sure to have time
to do it.


> > +     /*
> > +      * This define works when both clock and FS are output for the cpu
> > +      * and makes clock very fast (FS is not simmetrical, but sampling
>
> symmetrical.
>

thx


>
> > +      * frequency is better approximated
> > +      */
> > +     int i2s_fast_clock;
>
> Is this a bool?
>

yes, I'll change it.


>
> > +     /* To be used when cpu gets clock from extenal pin */
> > +     int clk_input_pin;
> > +
>
> How would one use this?
>

looking at 2.5 Clock, Frames, and Data in
you can select MCBSP_CLKS or other input clock pins.