Re: [PATCH v3 14/17] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
On Mon, Jan 15, 2018 at 10:16:28PM +0100, Maciej S. Szmigiero wrote: > > * 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. > > I didn't get any response to a comment I've written about the point I neglected the comments in the middle. Sorry. Will add it back then.
Re: [PATCH v3 14/17] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
On Mon, Jan 15, 2018 at 10:16:28PM +0100, Maciej S. Szmigiero wrote: > > * 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. > > I didn't get any response to a comment I've written about the point I neglected the comments in the middle. Sorry. Will add it back then.
Re: [PATCH v3 14/17] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
On 15.01.2018 05:21, 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. I didn't get any response to a comment I've written about the point above during the previous patch iteration:> 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. Maciej > * 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> Tested-by: Caleb Crome > --- > Changelog > v3 > * Put CBM_CFS behind the baudclk check to keep the same program >flow as before > > sound/soc/fsl/fsl_ssi.c | 73 > ++--- > 1 file changed, 33 insertions(+), 40 deletions(-) >
Re: [PATCH v3 14/17] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
On 15.01.2018 05:21, 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. I didn't get any response to a comment I've written about the point above during the previous patch iteration:> 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. Maciej > * 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 > Tested-by: Caleb Crome > --- > Changelog > v3 > * Put CBM_CFS behind the baudclk check to keep the same program >flow as before > > sound/soc/fsl/fsl_ssi.c | 73 > ++--- > 1 file changed, 33 insertions(+), 40 deletions(-) >
[PATCH v3 14/17] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
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. * 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 ChenTested-by: Caleb Crome --- Changelog v3 * Put CBM_CFS behind the baudclk check to keep the same program flow as before sound/soc/fsl/fsl_ssi.c | 73 ++--- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index ebb3eb9..9ff6734 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -860,42 +860,28 @@ 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 &= ~(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, ); - regmap_read(regs, REG_SSI_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; + } + /* fall through */ + case SND_SOC_DAIFMT_CBM_CFS: ssi->i2s_net |= SSI_SCR_I2S_MODE_MASTER; break; case SND_SOC_DAIFMT_CBM_CFM: @@ -905,30 +891,34 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev, return -EINVAL; } + regmap_update_bits(ssi->regs, REG_SSI_STCCR, + SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2)); + regmap_update_bits(ssi->regs, REG_SSI_SRCCR, + SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2)); + /* Data on rising edge of bclk, frame low, 1clk before data */ - strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP | -SSI_STCR_TXBIT0 | SSI_STCR_TEFS; + strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP | SSI_STCR_TEFS; break; case SND_SOC_DAIFMT_LEFT_J: /* Data on rising edge of bclk, frame high */ - strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP; + strcr |= SSI_STCR_TSCKP; break; case SND_SOC_DAIFMT_DSP_A: /* Data on rising edge of bclk, frame high, 1clk before data */ - strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP | -SSI_STCR_TXBIT0 | SSI_STCR_TEFS; + strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP | SSI_STCR_TEFS; break; case SND_SOC_DAIFMT_DSP_B: /* Data on rising edge of bclk, frame high */ - strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP | SSI_STCR_TXBIT0; + strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP; break; case
[PATCH v3 14/17] ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
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. * 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 Tested-by: Caleb Crome --- Changelog v3 * Put CBM_CFS behind the baudclk check to keep the same program flow as before sound/soc/fsl/fsl_ssi.c | 73 ++--- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index ebb3eb9..9ff6734 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -860,42 +860,28 @@ 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 &= ~(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, ); - regmap_read(regs, REG_SSI_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; + } + /* fall through */ + case SND_SOC_DAIFMT_CBM_CFS: ssi->i2s_net |= SSI_SCR_I2S_MODE_MASTER; break; case SND_SOC_DAIFMT_CBM_CFM: @@ -905,30 +891,34 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev, return -EINVAL; } + regmap_update_bits(ssi->regs, REG_SSI_STCCR, + SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2)); + regmap_update_bits(ssi->regs, REG_SSI_SRCCR, + SSI_SxCCR_DC_MASK, SSI_SxCCR_DC(2)); + /* Data on rising edge of bclk, frame low, 1clk before data */ - strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP | -SSI_STCR_TXBIT0 | SSI_STCR_TEFS; + strcr |= SSI_STCR_TFSI | SSI_STCR_TSCKP | SSI_STCR_TEFS; break; case SND_SOC_DAIFMT_LEFT_J: /* Data on rising edge of bclk, frame high */ - strcr |= SSI_STCR_TXBIT0 | SSI_STCR_TSCKP; + strcr |= SSI_STCR_TSCKP; break; case SND_SOC_DAIFMT_DSP_A: /* Data on rising edge of bclk, frame high, 1clk before data */ - strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP | -SSI_STCR_TXBIT0 | SSI_STCR_TEFS; + strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP | SSI_STCR_TEFS; break; case SND_SOC_DAIFMT_DSP_B: /* Data on rising edge of bclk, frame high */ - strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP | SSI_STCR_TXBIT0; + strcr |= SSI_STCR_TFSL | SSI_STCR_TSCKP; break; case SND_SOC_DAIFMT_AC97: /* Data