No subject

static struct snd_platform_data dm365_bmx_snd_data[] =3D {
        {
        },
        {
                .i2s_fast_clock =3D 0,
                .clk_input_pin =3D MCBSP_CLKS,
        },
};

If not set, it works as evm works, that is the default mode.


> >       case SND_SOC_DAIFMT_CBM_CFS:
> > -             /* McBSP CLKR pin is the input for the Sample Rate
> Generator.
> > -              * McBSP FSR and FSX are driven by the Sample Rate
> Generator. */
> > -             pcr =3D DAVINCI_MCBSP_PCR_SCLKME |
> > -                     DAVINCI_MCBSP_PCR_FSXM |
> > -                     DAVINCI_MCBSP_PCR_FSRM;
> > +             pcr =3D DAVINCI_MCBSP_PCR_FSRM | DAVINCI_MCBSP_PCR_FSXM;
> > +             if (dev->clk_input_pin =3D=3D MCBSP_CLKS)
> > +                     pcr |=3D DAVINCI_MCBSP_PCR_CLKXM |
> > +                             DAVINCI_MCBSP_PCR_CLKRM;
> > +             else
> > +                     /*
> > +                      * McBSP CLKR pin is the input for the Sample Rat=
e
> > +                      * Generator.
> > +                      * McBSP FSR and FSX are driven by the Sample Rat=
e
> > +                      * Generator.
> > +                      */
> > +                     pcr |=3D DAVINCI_MCBSP_PCR_SCLKME;
>
> This looks like you need a switch statement for the clock selection.
>


ok.



> > +static int davinci_i2s_dai_set_clkdiv(struct snd_soc_dai *cpu_dai,
> > +                             int div_id, int div)
> > +{
> > +     struct davinci_mcbsp_dev *dev =3D cpu_dai->private_data;
> > +     int srgr;
> > +
> > +     dev->clk_div =3D div;
> > +     return 0;
> > +}
> > +
>
> I would add a clock ID here in case someone wants to configure another
> clock in the patch.
>

Can you explain better,
please?


>
> > +     if (master =3D=3D SND_SOC_DAIFMT_CBS_CFS) {
>
> Use a switch staetment for this too.
>
> > +             clk =3D clk_get(NULL, "pll1_sysclk6");
>
> You're doing a clk_get() every time you go through here but never
> freeing it - it would seem better to just do the clk_get() at startup.
>

Thx for this.
I think it could create problems, right?
I can do it in the probe, I think, similarly to other drivers.


> I'd also expect this to be doing a clk_get() using the struct device for
> the DAI so that the driver can function even if the clock tree for a new.=
.
> SoC is different.
>

Sorry, I don't understand, can you explain me better?



>
> > +             if (clk)
> > +                     freq =3D clk_get_rate(clk);
>
> clk_get() returns an error pointer on error, not NULL, and...
>


gueSsing that I have to use this example code:
        aemif_clk =3D clk_get(NULL, "aemif");
        if (IS_ERR(aemif_clk))
                return;

I'll do it.


>
> > +             freq =3D 122000000; /* FIXME ask to Texas */
>
> ...this presumably ought to be in an else clause (or perhaps just not
> using this clocking at all if you can't find the clock?).
>

freq is used to obtain clk_div.
The real problem is that, as explained in the manual, it is not clear its
value.
I hope someone can help!!


>
> > +     } else if (master =3D=3D SND_SOC_DAIFMT_CBM_CFS) {
> > +             /* Clock given on CLKS */
>
> What if the user selected a different clock source?
>

I need another check,
right!


>
> > +             if (master =3D=3D SND_SOC_DAIFMT_CBS_CFS ||
> > +                             master =3D=3D SND_SOC_DAIFMT_CBS_CFM) {
>
> Again, switch statement.
>

ok


>
> > +     if (master =3D=3D SND_SOC_DAIFMT_CBS_CFS ||
> > +                     master =3D=3D SND_SOC_DAIFMT_CBS_CFM) {
>
> Here too.
>

again.


Conclusion
I split patches and I do fixes above.
Than I'll submit again and wait for new info.

The freq problem is describe here below, but it is not clear to me: