[alsa-devel] [PATCH 4/4] pxa-ssp: switch from network mode to psp

[pHilipp Zabel]

On Thu, Mar 5, 2009 at 12:03 AM, Daniel Mack <daniel at caiaq.de> wrote:
> Hi Philipp,
>
> On Wed, Mar 04, 2009 at 09:56:01PM +0100, pHilipp Zabel wrote:
>> > diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c
>> > index 45fb600..97f11d6 100644
>> > --- a/sound/soc/pxa/pxa-ssp.c
>> > +++ b/sound/soc/pxa/pxa-ssp.c
>> > @@ -319,6 +319,7 @@ static int pxa_ssp_set_dai_sysclk(struct snd_soc_dai *cpu_dai,
>> >                break;
>> >        case PXA_SSP_CLK_EXT:
>> >                priv->sysclk = freq;
>> > +               ssp_set_scr(&priv->dev, 4);
>>
>> Shouldn't this be somehow set by set_dai_clkdiv instead?
>
> This is actually a very common case - MCLK is BCLK*4, so I put it here.
> We could still move it to some more configurable place if it turns out
> we need to configure it per board.
>
>> >                switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>> >                case SND_SOC_DAIFMT_NB_NF:
>> > -                       sspsp |= SSPSP_FSRT;
>> >                        break;
>> >                case SND_SOC_DAIFMT_NB_IF:
>> > -                       sspsp |= SSPSP_SFRMP | SSPSP_FSRT;
>> > -                       break;
>> > -               case SND_SOC_DAIFMT_IB_IF:
>> >                        sspsp |= SSPSP_SFRMP;
>> >                        break;
>>
>> Removal of SSPSP_FSRT from NB/IB selection seems to be correct from the docs.
>
> SSPSP_FSRT has a totally different meaning according to the PXA3xx docs,
> but I'll have a look at the PXA2x specs - maybe we need a special case
> here. Unfortunately, I'm not able to quote from PXA3x specs here due to
> NDA restrictions.
>
>> Can you check if IB could be properly handled by setting SCMODE(1)?
>
> Can't follow that - what are you referring to here?
>
>> >                default:
>> > @@ -652,33 +649,39 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream,
>> >                break;
>> >        case SNDRV_PCM_FORMAT_S24_LE:
>> >                sscr0 |= (SSCR0_EDSS | SSCR0_DataSize(8));
>> > -               /* we must be in network mode (2 slots) for 24 bit stereo */
>>
>> This is still dubious ...
>> S24_LE is 24-bit sound LSB-aligned in 32-bit frames, so DataSize
>> should be 32 here.
>
> I didn't test that, and I didn't change it either - I just removed the
> comment, as we're not running in network mode anymore.
>
>> >                break;
>> >        case SNDRV_PCM_FORMAT_S32_LE:
>> >                sscr0 |= (SSCR0_EDSS | SSCR0_DataSize(16));
>> > -               /* we must be in network mode (2 slots) for 32 bit stereo */
>>
>> How is it possible to send 64bit in one frame otherwise?
>
> We're not running in network mode anymore - things are different now :)
>
>> > +               switch (priv->dai_fmt & SND_SOC_DAIFMT_FRAME_FORMAT_MASK) {
>> > +               case SND_SOC_DAIFMT_FF_I2S_32:
>> > +                       /* These values are all found out by trying and
>> > +                        * failing a lot. PXA's SSP is all black magic and
>> > +                        * does not work like described in any datasheet.
>> > +                        */
>> > +                       sspsp |= SSPSP_SFRMWDTH(32);
>> > +                       sspsp |= SSPSP_SFRMDLY(32 * 2);
>> > +                       sspsp |= SSPSP_EDMYSTOP(3);
>> > +                       sspsp |= SSPSP_DMYSTOP(3);
>> > +                       sspsp |= SSPSP_DMYSTRT(1);
>>
>> Wha?! Amazing. And this really works?
>> How the hell can this result in 16 bits of data followed by 16 bits of
>> zeroes, twice :)
>
> Don't ask :) If we could explain in detail what these registers mean,
> I'd rather make them macro values, but unfortunately, the comment is
> correct ...
>
>>
>> > +                       break;
>> > +               default:
>> > +                       /* Cleared when the DAI format is set */
>> > +                       sspsp |= SSPSP_SFRMWDTH(width);
>>
>> Not good for DSP_A/B.
>
> This is the SND_SOC_DAIFMT_I2S case, so DSP modes will be unaffected.
>
> Could you try the patches on your board?

Done. They don't interfere with my setup.

regards
Philipp