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

[pHilipp Zabel]

On Sat, Jun 6, 2009 at 10:12 PM, Mark
Brown<broonie at opensource.wolfsonmicro.com> wrote:
> On Wed, Jun 03, 2009 at 08:33:42PM +0800, Eric Miao wrote:
>
>> +/* frame size definitions for I2S and Left_J formats - default is
>> + * 32fs, other possibilities are 48fs, 64fs and 96fs
>> + */
>> +#define PXA_SSP_FRM_32FS     (0 << 16)
>> +#define PXA_SSP_FRM_48FS     (1 << 16)
>> +#define PXA_SSP_FRM_64FS     (2 << 16)
>> +#define PXA_SSP_FRM_96FS     (3 << 16)
>> +#define PXA_SSP_FRM_WIDTH(x) (((((x) >> 16) & 0x3) + 2) << 4)
>> +
>
> I still haven't checked this on Zylonite but I wanted to mention these
> new DAI format bits just now - as previously discussed I'm not
> enthusiastic about this.

Agreed. Also, if I had to use this to configure magician with DSP_A
(right now it says I2S and LEFT_J only, but why limit it to that?),
I'd need an additional PXA_SSP_FRM_16FS.

> As well as the previous issues I raised with
> this approach I also meant to mention is that the use of a bitfield will
> inevitably restrict what can be expressed, causing real problems for
> some systems.  For example, the WM9081 supports systems using up to 3
> stereo TDM slots and up to 32 bit samples so could be used in a system
> which needs a 192fs bit clock.
>
> If we did decide to adopt this approach then these defines should be in
> the ASoC headers rather than private to this driver - apart from
> anything else, there's every chance that additions to the standard bits
> could end up colliding with this.
>
> I'm still not sure what the best way to handle this is due to the
> interaction with TDM mode.  The fact that TDM mode really wants to
> always explicitly specify the frame width in order to allow the sample
> size in each slot to vary suggests that one option would be to add a
> sample width parameter to set_tdm_slot() - given the very small number
> of in tree users it'd cause little disruption.  A set_sample_width()
> operation could also be added.

set_sample_width or set_frame_width?
I'd prefer a separate operation over a parameter to set_tdm_slot because
in my setup I just need to force the (SSP) frame width to 16 bits.
Enabling network mode and calling set_tdm_slot(..,1,1) wouldn't be
necessary at all, then.

regards
Philipp