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

[Daniel Mack <daniel@caiaq.de>]

Hi Paul,

sorry for the delay.

On Thu, Jun 04, 2009 at 05:44:55PM +0800, Paul Shen wrote:
> > The tdm time slot configuration needs to be set again in my board support
> > code just like in your example: snd_soc_set_tdm_slot(cpu_dai, 3, 2). And
> > the PXA_SSP_DIV_SCR value needed to be doubled from 4 to 8.
> >
> I tested  with below  codes to set the cpu_dai  :
> 
> format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
>                          SND_SOC_DAIFMT_CBS_CFS | PXA_SSP_FRM_64FS;

Ah - the PXA_SSP_FRM_64FS is the detail I missed. With that option set,
my system works well, so I'm generally fine with the changes.

However, I proposed a generic way (not pxa/ssp specific) to let the
format flags carry the information about such frame format details some
months ago and the approach was rejected because there are already too
many ways to let the DAI know about this. In particular, the DIV_SCR
(serial clock rate divider) is used to propagate the base frequency
ratio and together with other options, you can find out the desired
frame format.

In my implementation, the '32 bit over 64fs case' was detected with this
condition in pxa_ssp_hw_params():

  (scr == 4) && (snd_pcm_format_physical_width(params_format(params)) == 16)

Now, SCR isn't used at all any more but instead PXA_SSP_FRM_WIDTH()
handles the case. Eventually this might be a matter of taste, but if we
go this way, why shouldn't that definitions be in ASoC's generic
headers?

Anyway, the code looks much cleaner now :)

Thanks,
Daniel