Re: [PATCH V2 2/2] ASoC: fsl_esai: recover the channel swap after xrun
flat view
From: Nicolin Chen <nicoleotsuka@gmail.com>
Date: 2019-07-03 09:32:21
Also in:
alsa-devel, lkml
On Wed, Jul 03, 2019 at 02:42:05PM +0800, shengjiu.wang@nxp.com wrote:
quoted hunk ↗ jump to hunk
From: Shengjiu Wang <redacted> There is chip errata ERR008000, the reference doc is (https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf), The issue is "While using ESAI transmit or receive and an underrun/overrun happens, channel swap may occur. The only recovery mechanism is to reset the ESAI." This issue exist in imx3/imx5/imx6(partial) series. In this commit add a tasklet to handle reset of ESAI after xrun happens to recover the channel swap. Signed-off-by: Shengjiu Wang <redacted> --- sound/soc/fsl/fsl_esai.c | 76 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c index 20039ae9893b..8c92e49ad6d8 100644 --- a/sound/soc/fsl/fsl_esai.c +++ b/sound/soc/fsl/fsl_esai.c
+static void fsl_esai_reset(unsigned long arg)
Similarly fsl_esai_hw_reset? This one isn't really that bad though, yet it feels better to have function naming in a similar style.
quoted hunk ↗ jump to hunk
+{ + struct fsl_esai *esai_priv = (struct fsl_esai *)arg; + u32 saisr, tfcr, rfcr; + + /* save the registers */ + regmap_read(esai_priv->regmap, REG_ESAI_TFCR, &tfcr); + regmap_read(esai_priv->regmap, REG_ESAI_RFCR, &rfcr);
Instead of having this implicit comments, we could have: + bool tx = true, rx = false, enabled[2]; + + regmap_read(esai_priv->regmap, REG_ESAI_TFCR, &tfcr); + regmap_read(esai_priv->regmap, REG_ESAI_RFCR, &rfcr); + enabled[tx] = tfcr & ESAI_xFCR_xFEN; + enabled[rx] = rfcr & ESAI_xFCR_xFEN;
quoted hunk ↗ jump to hunk
+ + /* stop the tx & rx */ + fsl_esai_trigger_stop(esai_priv, 1); + fsl_esai_trigger_stop(esai_priv, 0);
And we could reuse the boolean 'tx' and 'rx' here.
quoted hunk ↗ jump to hunk
+ + /* reset the esai, and restore the registers */ + fsl_esai_init(esai_priv); +
[...]
quoted hunk ↗ jump to hunk
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, + ESAI_xCR_xPR_MASK, + ESAI_xCR_xPR); + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR, + ESAI_xCR_xPR_MASK, + ESAI_xCR_xPR);
Mask and value might fit into one line?
quoted hunk ↗ jump to hunk
+ + /* restore registers by regcache_sync */ + fsl_esai_register_restore(esai_priv); + + regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, + ESAI_xCR_xPR_MASK, 0); + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR, + ESAI_xCR_xPR_MASK, 0);
And just for curious, can (or shall) we stuff this personal reset to the reset() function? I found this one is a part of the reset routine being mentioned in the RM -- it was done after ESAI reset is done via ECR register. [...]
quoted hunk ↗ jump to hunk
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_PRRC, + ESAI_PRRC_PDC_MASK, + ESAI_PRRC_PDC(ESAI_GPIO)); + regmap_update_bits(esai_priv->regmap, REG_ESAI_PCRC, + ESAI_PCRC_PC_MASK, + ESAI_PCRC_PC(ESAI_GPIO));
Mask and value might fit into one line?