Re: [PATCH 1/4] ASoC: pxa-ssp: enhance I2S and add Left_J support

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Mon, Jun 08, 2009 at 05:58:54PM +0200, pHilipp Zabel wrote:
> On Mon, Jun 8, 2009 at 2:40 PM, Mark

> > OOI which MUA are you using?  I've noticed several people with this very
> > odd word wrapping the past day.

> This mail was sent with the Google Mail web interface.

Ah.  Why am I not surprised. :/

> > I'm inclined to go with sample here since it seems harder for the
> > machine drivers to get wrong but I've not really thought it through yet?

> I thought sample width is determined by the snd_pcm_hw_params. But
> maybe I'm mixing up alsa sample width vs sample width on the wire?

Essentially what we're doing here is providing a mechanism to specify a
separate wire format.

> I'm leaning towards set_frame_width because that's directly what I
> want to do: override pxa_ssp_hw_params' standard decision to use
> 32-bit frames for S16_LE stereo and set 16-bit frames instead.

Hrm.  Now I remember what you're doing - you're trying to essentially
send your stereo stream as mono data due to your hardware either having
a flip flop or an entertainingly non-standard CODEC which needs a frame
sync per sample rather than per frame.

> > OTOH I don't really see much difference between the two cases - it's
> > just an extra couple of parameters on the end of the call.

> Technically there isn't. It just seems much more obvious to me to
> write something like:
>     /* nonstandard: 16-bit frames, even for 2x 16-bit stereo */
>     if (params_format(params) == ..._S16_LE)
>         set_frame_size(16);

Thing is, I'd expect this would be just as likely to cause the CPU to
discard every other sample since it doesn't have enough clocks to clock
out a stereo sample in the frame.

It occurs to me that this is something that it might be better to work
around this with a user space plugin which rewrites the sample formats
on the way down to the driver so that it claims the stream is configured
as mono even if it's stereo :/  Not sure how practical that is or if
there's a sensible way to do that in kernel space.

> in the machine driver instead of:
>     /* nonstandard: 16-bit frames, even for 2x 16-bit stereo
>      * pxa_ssp_hw_params will check for TTSA==1
>      * and then set the frame size accordingly
>      */
>     set_tdm_slot(1,1);
> especially as I don't really need network mode at all.

Same issue with "it's a surprise it works" applies here.

That TDM configuration ought to disable network mode if the driver
doesn't need it.