On 11.01.2018 07:43, Nicolin Chen wrote:
The _fsl_ssi_set_dai_fmt() is a helper function being called from fsl_ssi_set_dai_fmt() as an ASoC operation and fsl_ssi_hw_init() mainly for AC97 format initialization.
This patch cleans the _fsl_ssi_set_dai_fmt() in following ways:
- Removing *dev pointer in the parameters as it's included in the *ssi pointer of struct fsl_ssi.
- Using regmap_update_bits() instead of regmap_read() with masking the value manually.
- Removing TXBIT0 configurations since this bit is set to 1 as its reset value and there is no use case so far to unset it. And it is safe to remove since regmap_update_bits() won't touch it.
The old code set this bit in any mode other than AC'97 (where the hardware always treats this bit as set regardless of the actual value). I would play safe here and not rely on this bit being set by a SSI reset on all SSI models.
- Moving baudclk check to the switch-case routine to skip the I2S master check. And moving SxCCR.DC settings after baudclk check.
- Adding format settings for SND_SOC_DAIFMT_AC97 like others.
Signed-off-by: Nicolin Chen nicoleotsuka@gmail.com Tested-by: Caleb Crome caleb@crome.org
sound/soc/fsl/fsl_ssi.c | 70 ++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 39 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 178c192..213962a 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -855,42 +855,27 @@ static int fsl_ssi_hw_free(struct snd_pcm_substream *substream, return 0; }
-static int _fsl_ssi_set_dai_fmt(struct device *dev,
struct fsl_ssi *ssi, unsigned int fmt)+static int _fsl_ssi_set_dai_fmt(struct fsl_ssi *ssi, unsigned int fmt) {
- struct regmap *regs = ssi->regs;
- u32 strcr = 0, stcr, srcr, scr, mask;
u32 strcr = 0, scr = 0, stcr, srcr, mask;
ssi->dai_fmt = fmt;
if (fsl_ssi_is_i2s_master(ssi) && IS_ERR(ssi->baudclk)) {
dev_err(dev, "missing baudclk for master mode\n");return -EINVAL;}
regmap_read(regs, REG_SSI_SCR, &scr);
scr &= ~(SSI_SCR_SYN | SSI_SCR_I2S_MODE_MASK); /* Synchronize frame sync clock for TE to avoid data slipping */ scr |= SSI_SCR_SYNC_TX_FS;
mask = SSI_STCR_TXBIT0 | SSI_STCR_TFDIR | SSI_STCR_TXDIR |
SSI_STCR_TSCKP | SSI_STCR_TFSI | SSI_STCR_TFSL | SSI_STCR_TEFS;regmap_read(regs, REG_SSI_STCR, &stcr);
regmap_read(regs, REG_SSI_SRCR, &srcr);
stcr &= ~mask;
srcr &= ~mask;
/* Use Network mode as default */ ssi->i2s_net = SSI_SCR_NET; switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { case SND_SOC_DAIFMT_I2S:
regmap_update_bits(regs, REG_SSI_STCCR,SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));regmap_update_bits(regs, REG_SSI_SRCCR,SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2));switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { case SND_SOC_DAIFMT_CBM_CFS: case SND_SOC_DAIFMT_CBS_CFS:
if (IS_ERR(ssi->baudclk)) {dev_err(ssi->dev,"missing baudclk for master mode\n");return -EINVAL;}
The original code did this check only for fsl_ssi_is_i2s_master(ssi), that is, only for SND_SOC_DAIFMT_CBS_CFS while here you also do it for SND_SOC_DAIFMT_CBM_CFS. Was this changed on purpose?
Maciej