da7213 questions

da7213 questions

[Peter Meerwald-Stadler]

Hello,

I have a couple of questions/comments regarding the da7213 driver in 
mainline:

(1) the da7213 driver enables mclk in BIAS_SUSPEND, so essentially the 
clock is always running (since idle_bias_off is not set); at least on my 
platform, clk_set_rate() fails when the clock is already enabled

(2) da7213's srm_en variable is redundant and can be removed

(3) for 32 kHz PLL mode (DAI master), section 13.28 of the datasheet 
indicates that PLL_SRM_EN should be asserted; the da7213 drivers goesn't

(4) is only S16_LE actually supported?
#define DA7213_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
but in da7213_set_dai_fmt() we have
	/* By default only 32 BCLK per WCLK is supported */
	dai_clk_mode |= DA7213_DAI_BCLKS_PER_WCLK_32;

thanks, regards, p.

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: da7213 questions

[Adam Thomson]

On 13 July 2016 14:33, Peter Meerwald-Stadler wrote:

> I have a couple of questions/comments regarding the da7213 driver in
> mainline:
> 
> (1) the da7213 driver enables mclk in BIAS_SUSPEND, so essentially the
> clock is always running (since idle_bias_off is not set); at least on my
> platform, clk_set_rate() fails when the clock is already enabled

I guess depending on the clock being used, some allow for setting rate whilst
enabled and some do not. I had a quick look and it wasn't completely clear as to
how you would determine this. With regards to MCLK running in BIAS_SUSPEND
though, I agree it's not the most efficient. Will take a look at this.

> (2) da7213's srm_en variable is redundant and can be removed

I'd like to look at improving the PLL function a little, so will address this
at that time.

> 
> (3) for 32 kHz PLL mode (DAI master), section 13.28 of the datasheet
> indicates that PLL_SRM_EN should be asserted; the da7213 drivers goesn't
> 

This was noted recently, and is on my TODO list to update. I'll address it as
part of the PLL function improvements.

> (4) is only S16_LE actually supported?
> #define DA7213_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |
> SNDRV_PCM_FMTBIT_S20_3LE |\
> 			SNDRV_PCM_FMTBIT_S24_LE |
> SNDRV_PCM_FMTBIT_S32_LE)
> but in da7213_set_dai_fmt() we have
> 	/* By default only 32 BCLK per WCLK is supported */
> 	dai_clk_mode |= DA7213_DAI_BCLKS_PER_WCLK_32;
>

Yes, you're correct here. It's a mistake on my part. Should be 64 not 32. Will
make sure this is updated.