On Wed, Jul 03, 2019 at 02:42:05PM +0800, shengjiu.wang@nxp.com wrote:
From: Shengjiu Wang shengjiu.wang@nxp.com
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 shengjiu.wang@nxp.com
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.
+{
- 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;
- /* 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.
- /* reset the esai, and restore the registers */
- fsl_esai_init(esai_priv);
[...]
- 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?
- /* 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.
[...]
- 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?