Re: snd-soc-pxa-ssp I2C/FIFO issues

[Daniel Mack <daniel@caiaq.de>]

On Wed, Mar 11, 2009 at 07:10:28PM +0100, pHilipp Zabel wrote:
> > Philipp, could you test that again on your board, please?
> > Applies on top of the other one ("pxa-ssp: don't touch ssp registers
> > ...") I just posted.
> 
> Tested-by me. Of course this still doesn't help me to enable 16-bit
> DMA transfers for stereo, but it doesn't hurt either.

Ok, but good to know this does not break your config.

> >  struct ssp_priv {
> >        struct ssp_dev dev;
> >        unsigned int sysclk;
> > +       unsigned int scr_div;
> 
> Really needed? Or could we just check SSCR0 below?

Well, SSCR0_SerClkDiv() is defined as follows:

#define SSCR0_SCR       (0x000fff00)    /* Serial Clock Rate (mask) */
#define SSCR0_SerClkDiv(x) (((x) - 1) << 8) /* Divisor [1..4096] */

I thought about adding a reverse macro just for that case, but that
seemed a lot more intrusive.

> >        switch (priv->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> >        case SND_SOC_DAIFMT_I2S:
> > -               /* Cleared when the DAI format is set */
> > -               sspsp = ssp_read_reg(ssp, SSPSP) | SSPSP_SFRMWDTH(width);
> > +               sspsp = ssp_read_reg(ssp, SSPSP);
> > +
> > +               if ((priv->scr_div == 4) && (width == 16)) {
> 
> sscr0 & SSCR0_SCR == SSCR0_SerClkDiv(4) instead?

Yes, could do that, but I doubt it is more readable? But we would save
one entry in the priv struct, so I'll do it.

> > -       /* We always use a network mode so we always require TDM slots
> > -        * - complain loudly and fail if they've not been set up yet.
> > -        */
> > -       if (!(ssp_read_reg(ssp, SSTSA) & 0xf)) {
> > -               dev_err(&ssp->pdev->dev, "No TDM timeslot configured\n");
> > -               return -EINVAL;
> > -       }
> > -
> 
> I wonder if this check was useful in some case? If so, we could
> replace it with something like this:

Yep, we didn't kill the network mode entirely, you're right. Will fix
this in a follow-up.

Thanks,
Daniel