[PATCH] ASoC: fsl_esai: Support synchronous mode

[Nicolin Chen]

Shengjiu,

On Mon, Apr 01, 2019 at 11:39:10AM +0000, S.j. Wang wrote:
> In ESAI synchronous mode, the clock is generated by Tx, So
> we should always set registers of Tx which relate with the
> bit clock and frame clock generation (TCCR, TCR, ECR), even
> there is only Rx is working.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang at nxp.com>
> ---
>  sound/soc/fsl/fsl_esai.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
> index 3623aa9a6f2e..d9fcddd55c02 100644
> --- a/sound/soc/fsl/fsl_esai.c
> +++ b/sound/soc/fsl/fsl_esai.c
> @@ -230,6 +230,21 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
>  		return -EINVAL;
>  	}
>  
> +	if (esai_priv->synchronous && !tx) {
> +		switch (clk_id) {
> +		case ESAI_HCKR_FSYS:
> +			fsl_esai_set_dai_sysclk(dai, ESAI_HCKT_FSYS,
> +								freq, dir);
> +			break;
> +		case ESAI_HCKR_EXTAL:
> +			fsl_esai_set_dai_sysclk(dai, ESAI_HCKT_EXTAL,
> +								freq, dir);

Not sure why you call set_dai_sysclk inside set_dai_sysclk again. It
feels very confusing to do so, especially without a comments.

> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
>  	/* Bypass divider settings if the requirement doesn't change */
>  	if (freq == esai_priv->hck_rate[tx] && dir == esai_priv->hck_dir[tx])
>  		return 0;
> @@ -537,10 +552,21 @@ static int fsl_esai_hw_params(struct snd_pcm_substream *substream,
>  
>  	bclk = params_rate(params) * slot_width * esai_priv->slots;
>  
> -	ret = fsl_esai_set_bclk(dai, tx, bclk);
> +	ret = fsl_esai_set_bclk(dai, esai_priv->synchronous ? true : tx, bclk);
>  	if (ret)
>  		return ret;
>  
> +	if (esai_priv->synchronous && !tx) {
> +		/* Use Normal mode to support monaural audio */
> +		regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
> +			   ESAI_xCR_xMOD_MASK, params_channels(params) > 1 ?
> +			   ESAI_xCR_xMOD_NETWORK : 0);
> +
> +		mask = ESAI_xCR_xSWS_MASK | ESAI_xCR_PADC;
> +		val = ESAI_xCR_xSWS(slot_width, width) | ESAI_xCR_PADC;
> +		regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, mask, val);
> +	}

Does synchronous mode require to set both TCR and RCR? or just TCR?
The code behind this part is doing the same setting to RCR. If that
is not needed any more for a synchronous recording, we should reuse
it instead of inserting a piece of redundant code. Otherwise, if we
need to set both, we should have two regmap_update_bits operations
back-to-back for TCR and RCR (and other registers too).

> +
>  	/* Use Normal mode to support monaural audio */
>  	regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
>  			   ESAI_xCR_xMOD_MASK, params_channels(params) > 1 ?

In case that we only need to set TCR (more likely I feel), it would
feel less confusing to me, if we changed REG_ESAI_xCR(tx) here, for
example, to REG_ESAI_xCR(tx || sync). Yea, please add to the top a
'bool sync = esai_priv->synchronous;'.

Similarly, for ECR_ETO and ECR_ERO:
	(tx || sync) ? ESAI_ECR_ETO : ESAI_ECR_ERO;