From: <Codrin.Ciubotariu@microchip.com>
To: mirq-linux@rere.qmqm.pl, alsa-devel@alsa-project.org,
devicetree@vger.kernel.org
Cc: alexandre.belloni@bootlin.com, arnd@arndb.de, 3chas3@gmail.com,
gregkh@linuxfoundation.org, perex@perex.cz, lgirdwood@gmail.com,
Ludovic.Desroches@microchip.com, broonie@kernel.org,
mark.rutland@arm.com, Nicolas.Ferre@microchip.com,
robh-dt@kernel.org, tiwai@suse.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/6] ASoC: atmel_ssc_dai: rework DAI format configuration
Date: Mon, 26 Aug 2019 14:04:13 +0000 [thread overview]
Message-ID: <7961d3da-5504-ae16-9342-979c12f9d028@microchip.com> (raw)
In-Reply-To: <f5949b0326fdcdca072f3ed03f77de9e207631cd.1566677788.git.mirq-linux@rere.qmqm.pl>
On 24.08.2019 23:26, Michał Mirosław wrote:
> Rework DAI format calculation in preparation for adding more formats
> later. As a side-effect this enables all CBM/CBS x CFM/CFS combinations
> for supported formats. (Note: the additional modes are not tested.)
The only mode added (and not tested) is for SND_SOC_DAIFMT_DSP_A |
SND_SOC_DAIFMT_CBM_CFS. I would mention it explicitly, instead of
something generic. Also, SND_SOC_DAIFMT_CBM_CFS is not supported for any
format, please see atmel_ssc_hw_rule_rate().
>
> Note: this changes FSEDGE to POSITIVE for I2S CBM_CFS mode as the TXSYN
> interrupt is not used anyway.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
> ---
> v2: added note about extended modes' status
> incorporated common FS (LRCLK) configuration
>
> ---
> sound/soc/atmel/atmel_ssc_dai.c | 286 +++++++++-----------------------
> 1 file changed, 80 insertions(+), 206 deletions(-)
>
> diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
> index 6f89483ac88c..7dc6ec9b8c7a 100644
> --- a/sound/soc/atmel/atmel_ssc_dai.c
> +++ b/sound/soc/atmel/atmel_ssc_dai.c
> @@ -471,7 +471,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
> int dir, channels, bits;
> u32 tfmr, rfmr, tcmr, rcmr;
> int ret;
> - int fslen, fslen_ext;
> + int fslen, fslen_ext, fs_osync, fs_edge;
> u32 cmr_div;
> u32 tcmr_period;
> u32 rcmr_period;
> @@ -558,226 +558,36 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
> /*
> * Compute SSC register settings.
> */
> - switch (ssc_p->daifmt
> - & (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_MASTER_MASK)) {
>
> - case SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBS_CFS:
> - /*
> - * I2S format, SSC provides BCLK and LRC clocks.
> - *
> - * The SSC transmit and receive clocks are generated
> - * from the MCK divider, and the BCLK signal
> - * is output on the SSC TK line.
> - */
> -
> - if (bits > 16 && !ssc->pdata->has_fslen_ext) {
> - dev_err(dai->dev,
> - "sample size %d is too large for SSC device\n",
> - bits);
> - return -EINVAL;
> - }
> -
> - fslen_ext = (bits - 1) / 16;
> - fslen = (bits - 1) % 16;
> -
> - rcmr = SSC_BF(RCMR_PERIOD, rcmr_period)
> - | SSC_BF(RCMR_STTDLY, START_DELAY)
> - | SSC_BF(RCMR_START, SSC_START_FALLING_RF)
> - | SSC_BF(RCMR_CKI, SSC_CKI_RISING)
> - | SSC_BF(RCMR_CKO, SSC_CKO_NONE)
> - | SSC_BF(RCMR_CKS, SSC_CKS_DIV);
> -
> - rfmr = SSC_BF(RFMR_FSLEN_EXT, fslen_ext)
> - | SSC_BF(RFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(RFMR_FSOS, SSC_FSOS_NEGATIVE)
> - | SSC_BF(RFMR_FSLEN, fslen)
> - | SSC_BF(RFMR_DATNB, (channels - 1))
> - | SSC_BIT(RFMR_MSBF)
> - | SSC_BF(RFMR_LOOP, 0)
> - | SSC_BF(RFMR_DATLEN, (bits - 1));
> -
> - tcmr = SSC_BF(TCMR_PERIOD, tcmr_period)
> - | SSC_BF(TCMR_STTDLY, START_DELAY)
> - | SSC_BF(TCMR_START, SSC_START_FALLING_RF)
> - | SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
> - | SSC_BF(TCMR_CKO, SSC_CKO_CONTINUOUS)
> - | SSC_BF(TCMR_CKS, SSC_CKS_DIV);
> -
> - tfmr = SSC_BF(TFMR_FSLEN_EXT, fslen_ext)
> - | SSC_BF(TFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(TFMR_FSDEN, 0)
> - | SSC_BF(TFMR_FSOS, SSC_FSOS_NEGATIVE)
> - | SSC_BF(TFMR_FSLEN, fslen)
> - | SSC_BF(TFMR_DATNB, (channels - 1))
> - | SSC_BIT(TFMR_MSBF)
> - | SSC_BF(TFMR_DATDEF, 0)
> - | SSC_BF(TFMR_DATLEN, (bits - 1));
> - break;
> -
> - case SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM:
> - /* I2S format, CODEC supplies BCLK and LRC clocks. */
> - rcmr = SSC_BF(RCMR_PERIOD, 0)
> - | SSC_BF(RCMR_STTDLY, START_DELAY)
> - | SSC_BF(RCMR_START, SSC_START_FALLING_RF)
> - | SSC_BF(RCMR_CKI, SSC_CKI_RISING)
> - | SSC_BF(RCMR_CKO, SSC_CKO_NONE)
> - | SSC_BF(RCMR_CKS, ssc->clk_from_rk_pin ?
> - SSC_CKS_PIN : SSC_CKS_CLOCK);
> -
> - rfmr = SSC_BF(RFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(RFMR_FSOS, SSC_FSOS_NONE)
> - | SSC_BF(RFMR_FSLEN, 0)
> - | SSC_BF(RFMR_DATNB, (channels - 1))
> - | SSC_BIT(RFMR_MSBF)
> - | SSC_BF(RFMR_LOOP, 0)
> - | SSC_BF(RFMR_DATLEN, (bits - 1));
> -
> - tcmr = SSC_BF(TCMR_PERIOD, 0)
> - | SSC_BF(TCMR_STTDLY, START_DELAY)
> - | SSC_BF(TCMR_START, SSC_START_FALLING_RF)
> - | SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
> - | SSC_BF(TCMR_CKO, SSC_CKO_NONE)
> - | SSC_BF(TCMR_CKS, ssc->clk_from_rk_pin ?
> - SSC_CKS_CLOCK : SSC_CKS_PIN);
> + fslen_ext = (bits - 1) / 16;
> + fslen = (bits - 1) % 16;
>
> - tfmr = SSC_BF(TFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(TFMR_FSDEN, 0)
> - | SSC_BF(TFMR_FSOS, SSC_FSOS_NONE)
> - | SSC_BF(TFMR_FSLEN, 0)
> - | SSC_BF(TFMR_DATNB, (channels - 1))
> - | SSC_BIT(TFMR_MSBF)
> - | SSC_BF(TFMR_DATDEF, 0)
> - | SSC_BF(TFMR_DATLEN, (bits - 1));
> - break;
> -
> - case SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFS:
> - /* I2S format, CODEC supplies BCLK, SSC supplies LRCLK. */
> - if (bits > 16 && !ssc->pdata->has_fslen_ext) {
> - dev_err(dai->dev,
> - "sample size %d is too large for SSC device\n",
> - bits);
> - return -EINVAL;
> - }
> -
> - fslen_ext = (bits - 1) / 16;
> - fslen = (bits - 1) % 16;
> -
> - rcmr = SSC_BF(RCMR_PERIOD, rcmr_period)
> - | SSC_BF(RCMR_STTDLY, START_DELAY)
> - | SSC_BF(RCMR_START, SSC_START_FALLING_RF)
> - | SSC_BF(RCMR_CKI, SSC_CKI_RISING)
> - | SSC_BF(RCMR_CKO, SSC_CKO_NONE)
> - | SSC_BF(RCMR_CKS, ssc->clk_from_rk_pin ?
> - SSC_CKS_PIN : SSC_CKS_CLOCK);
> -
> - rfmr = SSC_BF(RFMR_FSLEN_EXT, fslen_ext)
> - | SSC_BF(RFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(RFMR_FSOS, SSC_FSOS_NEGATIVE)
> - | SSC_BF(RFMR_FSLEN, fslen)
> - | SSC_BF(RFMR_DATNB, (channels - 1))
> - | SSC_BIT(RFMR_MSBF)
> - | SSC_BF(RFMR_LOOP, 0)
> - | SSC_BF(RFMR_DATLEN, (bits - 1));
> -
> - tcmr = SSC_BF(TCMR_PERIOD, tcmr_period)
> - | SSC_BF(TCMR_STTDLY, START_DELAY)
> - | SSC_BF(TCMR_START, SSC_START_FALLING_RF)
> - | SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
> - | SSC_BF(TCMR_CKO, SSC_CKO_NONE)
> - | SSC_BF(TCMR_CKS, ssc->clk_from_rk_pin ?
> - SSC_CKS_CLOCK : SSC_CKS_PIN);
> -
> - tfmr = SSC_BF(TFMR_FSLEN_EXT, fslen_ext)
> - | SSC_BF(TFMR_FSEDGE, SSC_FSEDGE_NEGATIVE)
> - | SSC_BF(TFMR_FSDEN, 0)
> - | SSC_BF(TFMR_FSOS, SSC_FSOS_NEGATIVE)
> - | SSC_BF(TFMR_FSLEN, fslen)
> - | SSC_BF(TFMR_DATNB, (channels - 1))
> - | SSC_BIT(TFMR_MSBF)
> - | SSC_BF(TFMR_DATDEF, 0)
> - | SSC_BF(TFMR_DATLEN, (bits - 1));
> - break;
> -
> - case SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_CBS_CFS:
> - /*
> - * DSP/PCM Mode A format, SSC provides BCLK and LRC clocks.
> - *
> - * The SSC transmit and receive clocks are generated from the
> - * MCK divider, and the BCLK signal is output
> - * on the SSC TK line.
> - */
> - rcmr = SSC_BF(RCMR_PERIOD, rcmr_period)
> - | SSC_BF(RCMR_STTDLY, 1)
> - | SSC_BF(RCMR_START, SSC_START_RISING_RF)
> - | SSC_BF(RCMR_CKI, SSC_CKI_RISING)
> - | SSC_BF(RCMR_CKO, SSC_CKO_NONE)
> - | SSC_BF(RCMR_CKS, SSC_CKS_DIV);
> + switch (ssc_p->daifmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>
> - rfmr = SSC_BF(RFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(RFMR_FSOS, SSC_FSOS_POSITIVE)
> - | SSC_BF(RFMR_FSLEN, 0)
> - | SSC_BF(RFMR_DATNB, (channels - 1))
> - | SSC_BIT(RFMR_MSBF)
> - | SSC_BF(RFMR_LOOP, 0)
> - | SSC_BF(RFMR_DATLEN, (bits - 1));
> + case SND_SOC_DAIFMT_I2S:
> + fs_osync = SSC_FSOS_NEGATIVE;
> + fs_edge = SSC_START_FALLING_RF;
>
> - tcmr = SSC_BF(TCMR_PERIOD, tcmr_period)
> - | SSC_BF(TCMR_STTDLY, 1)
> - | SSC_BF(TCMR_START, SSC_START_RISING_RF)
> - | SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
> - | SSC_BF(TCMR_CKO, SSC_CKO_CONTINUOUS)
> - | SSC_BF(TCMR_CKS, SSC_CKS_DIV);
> + rcmr = SSC_BF(RCMR_STTDLY, 1);
> + tcmr = SSC_BF(TCMR_STTDLY, 1);
>
> - tfmr = SSC_BF(TFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(TFMR_FSDEN, 0)
> - | SSC_BF(TFMR_FSOS, SSC_FSOS_POSITIVE)
> - | SSC_BF(TFMR_FSLEN, 0)
> - | SSC_BF(TFMR_DATNB, (channels - 1))
> - | SSC_BIT(TFMR_MSBF)
> - | SSC_BF(TFMR_DATDEF, 0)
> - | SSC_BF(TFMR_DATLEN, (bits - 1));
> break;
>
> - case SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_CBM_CFM:
> + case SND_SOC_DAIFMT_DSP_A:
> /*
> - * DSP/PCM Mode A format, CODEC supplies BCLK and LRC clocks.
> + * DSP/PCM Mode A format
> *
> * Data is transferred on first BCLK after LRC pulse rising
> * edge.If stereo, the right channel data is contiguous with
> * the left channel data.
> */
> - rcmr = SSC_BF(RCMR_PERIOD, 0)
> - | SSC_BF(RCMR_STTDLY, START_DELAY)
> - | SSC_BF(RCMR_START, SSC_START_RISING_RF)
> - | SSC_BF(RCMR_CKI, SSC_CKI_RISING)
> - | SSC_BF(RCMR_CKO, SSC_CKO_NONE)
> - | SSC_BF(RCMR_CKS, ssc->clk_from_rk_pin ?
> - SSC_CKS_PIN : SSC_CKS_CLOCK);
> + fs_osync = SSC_FSOS_POSITIVE;
> + fs_edge = SSC_START_RISING_RF;
> + fslen = fslen_ext = 0;
>
> - rfmr = SSC_BF(RFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(RFMR_FSOS, SSC_FSOS_NONE)
> - | SSC_BF(RFMR_FSLEN, 0)
> - | SSC_BF(RFMR_DATNB, (channels - 1))
> - | SSC_BIT(RFMR_MSBF)
> - | SSC_BF(RFMR_LOOP, 0)
> - | SSC_BF(RFMR_DATLEN, (bits - 1));
> + rcmr = SSC_BF(RCMR_STTDLY, 1);
> + tcmr = SSC_BF(TCMR_STTDLY, 1);
>
> - tcmr = SSC_BF(TCMR_PERIOD, 0)
> - | SSC_BF(TCMR_STTDLY, START_DELAY)
> - | SSC_BF(TCMR_START, SSC_START_RISING_RF)
> - | SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
> - | SSC_BF(TCMR_CKO, SSC_CKO_NONE)
> - | SSC_BF(RCMR_CKS, ssc->clk_from_rk_pin ?
> - SSC_CKS_CLOCK : SSC_CKS_PIN);
> -
> - tfmr = SSC_BF(TFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> - | SSC_BF(TFMR_FSDEN, 0)
> - | SSC_BF(TFMR_FSOS, SSC_FSOS_NONE)
> - | SSC_BF(TFMR_FSLEN, 0)
> - | SSC_BF(TFMR_DATNB, (channels - 1))
> - | SSC_BIT(TFMR_MSBF)
> - | SSC_BF(TFMR_DATDEF, 0)
> - | SSC_BF(TFMR_DATLEN, (bits - 1));
> break;
>
> default:
> @@ -785,6 +595,70 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
> ssc_p->daifmt);
> return -EINVAL;
> }
> +
> + if (!atmel_ssc_cfs(ssc_p)) {
> + fslen = fslen_ext = 0;
> + rcmr_period = tcmr_period = 0;
> + fs_osync = SSC_FSOS_NONE;
> + }
> +
> + rcmr |= SSC_BF(RCMR_START, fs_edge);
> + tcmr |= SSC_BF(TCMR_START, fs_edge);
Why not move these below in the code, where all the other bits of these
two registers are set?
> +
> + if (atmel_ssc_cbs(ssc_p)) {
> + /*
> + * SSC provides BCLK
> + *
> + * The SSC transmit and receive clocks are generated from the
> + * MCK divider, and the BCLK signal is output
> + * on the SSC TK line.
> + */
> + rcmr |= SSC_BF(RCMR_CKS, SSC_CKS_DIV)
> + | SSC_BF(RCMR_CKO, SSC_CKO_NONE);
> +
> + tcmr |= SSC_BF(TCMR_CKS, SSC_CKS_DIV)
> + | SSC_BF(TCMR_CKO, SSC_CKO_CONTINUOUS);
> + } else {
> + rcmr |= SSC_BF(RCMR_CKS, ssc->clk_from_rk_pin ?
> + SSC_CKS_PIN : SSC_CKS_CLOCK)
> + | SSC_BF(RCMR_CKO, SSC_CKO_NONE);
> +
> + tcmr |= SSC_BF(TCMR_CKS, ssc->clk_from_rk_pin ?
> + SSC_CKS_CLOCK : SSC_CKS_PIN)
> + | SSC_BF(TCMR_CKO, SSC_CKO_NONE);
> + }
> +
> + rcmr |= SSC_BF(RCMR_PERIOD, rcmr_period)
> + | SSC_BF(RCMR_CKI, SSC_CKI_RISING);
> +
> + tcmr |= SSC_BF(TCMR_PERIOD, tcmr_period)
> + | SSC_BF(TCMR_CKI, SSC_CKI_FALLING);
> +
> + rfmr = SSC_BF(RFMR_FSLEN_EXT, fslen_ext)
> + | SSC_BF(RFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> + | SSC_BF(RFMR_FSOS, fs_osync)
> + | SSC_BF(RFMR_FSLEN, fslen)
> + | SSC_BF(RFMR_DATNB, (channels - 1))
> + | SSC_BIT(RFMR_MSBF)
> + | SSC_BF(RFMR_LOOP, 0)
> + | SSC_BF(RFMR_DATLEN, (bits - 1));
> +
> + tfmr = SSC_BF(TFMR_FSLEN_EXT, fslen_ext)
> + | SSC_BF(TFMR_FSEDGE, SSC_FSEDGE_POSITIVE)
> + | SSC_BF(TFMR_FSDEN, 0)
> + | SSC_BF(TFMR_FSOS, fs_osync)
> + | SSC_BF(TFMR_FSLEN, fslen)
> + | SSC_BF(TFMR_DATNB, (channels - 1))
> + | SSC_BIT(TFMR_MSBF)
> + | SSC_BF(TFMR_DATDEF, 0)
> + | SSC_BF(TFMR_DATLEN, (bits - 1));
> +
> + if (fslen_ext && !ssc->pdata->has_fslen_ext) {
> + dev_err(dai->dev, "sample size %d is too large for SSC device\n",
> + bits);
> + return -EINVAL;
> + }
> +
> pr_debug("atmel_ssc_hw_params: "
> "RCMR=%08x RFMR=%08x TCMR=%08x TFMR=%08x\n",
> rcmr, rfmr, tcmr, tfmr);
>
next prev parent reply other threads:[~2019-08-26 14:04 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-24 20:26 [PATCH v2 0/6] ] ASoC: atmel: extend SSC support Michał Mirosław
2019-08-24 20:26 ` [PATCH v2 1/6] ASoC: atmel: enable SOC_SSC_PDC and SOC_SSC_DMA in Kconfig Michał Mirosław
2019-08-26 13:45 ` Codrin.Ciubotariu
2019-08-26 14:23 ` Alexandre Belloni
2019-08-24 20:26 ` [PATCH v2 2/6] ASoC: atmel_ssc_dai: rework DAI format configuration Michał Mirosław
2019-08-26 14:04 ` Codrin.Ciubotariu [this message]
2019-08-24 20:26 ` [PATCH v2 3/6] ASoC: atmel_ssc_dai: implement left-justified data mode Michał Mirosław
2019-08-26 14:10 ` Codrin.Ciubotariu
2019-08-24 20:26 ` [PATCH v2 4/6] dt-bindings: misc: atmel-ssc: LRCLK from TF/RF pin option Michał Mirosław
2019-08-27 22:37 ` Rob Herring
2019-08-28 13:02 ` Michał Mirosław
2019-08-28 18:34 ` Rob Herring
2019-08-24 20:26 ` [PATCH v2 5/6] misc: atmel-ssc: get LRCLK pin selection from DT Michał Mirosław
2019-08-26 14:38 ` Codrin.Ciubotariu
2019-08-24 20:26 ` [PATCH v2 6/6] ASoC: atmel_ssc_dai: Enable shared FSYNC source in frame-slave mode Michał Mirosław
2019-08-26 15:05 ` Codrin.Ciubotariu
2019-09-08 13:39 ` [alsa-devel] " mirq-linux
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7961d3da-5504-ae16-9342-979c12f9d028@microchip.com \
--to=codrin.ciubotariu@microchip.com \
--cc=3chas3@gmail.com \
--cc=Ludovic.Desroches@microchip.com \
--cc=Nicolas.Ferre@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=alsa-devel@alsa-project.org \
--cc=arnd@arndb.de \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mirq-linux@rere.qmqm.pl \
--cc=perex@perex.cz \
--cc=robh-dt@kernel.org \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).