> > 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@xxxxxxx>
> > > ---
> > > 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.
>
> For sync mode, only RX is enabled, the register of tx should be set, so call the
> Set_dai_sysclk again.
Yea, I understood that. But why not just replace RX with TX on the
register-writing level? Do we need to set both TCCR and RCCR? Your
change in hw_params() only sets TCCR inside fsl_esai_set_bclk(), so
we probably only need to change TCCR for recordings running in sync
mode, right?
>From the commit message, it feels like that only the clock-related
fields in the TX registers need to be set. Things like calculation
and setting the direction of HCKx pin don't need to run again.
> > > @@ -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?
> Both TCR and RCR. RCR will be set in normal flow.
OK. Settings both xCRs makes sense. Would you please try this:
===============
@@ -537,14 +552,20 @@ 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);
+ /* Synchronous mode uses TX clock generator */
+ ret = fsl_esai_set_bclk(dai, esai_priv->synchronous || tx, bclk);
if (ret)
return ret;
+ mask = ESAI_xCR_xMOD_MASK | ESAI_xCR_xSWS_MASK;
+ val = ESAI_xCR_xSWS(slot_width, width);
/* 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 ?
- ESAI_xCR_xMOD_NETWORK : 0);
+ val |= params_channels(params) > 1 ? ESAI_xCR_xMOD_NETWORK : 0;
+
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx), mask, val);
+ /* Recording in synchronous mode needs to set TCR also */
+ if (!tx && esai_priv->synchronous)
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, mask, val);
regmap_update_bits(esai_priv->regmap, REG_ESAI_xFCR(tx),
ESAI_xFCR_xFR_MASK, ESAI_xFCR_xFR);
@@ -556,10 +577,10 @@ static int fsl_esai_hw_params(struct snd_pcm_substream *substream,
regmap_update_bits(esai_priv->regmap, REG_ESAI_xFCR(tx), mask, val);
- mask = ESAI_xCR_xSWS_MASK | (tx ? ESAI_xCR_PADC : 0);
- val = ESAI_xCR_xSWS(slot_width, width) | (tx ? ESAI_xCR_PADC : 0);
-
- regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx), mask, val);
+ /* Only TCR has padding bit and needs to be set for synchronous mode */
+ if (tx || esai_priv->synchronous)
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
+ ESAI_xCR_PADC, ESAI_xCR_PADC);
/* Remove ESAI personal reset by configuring ESAI_PCRC and ESAI_PRRC */
regmap_update_bits(esai_priv->regmap, REG_ESAI_PRRC,
===============
It merges those plain settings so we won't need to change both
sides if there's a modification of xCR settings in the future.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
