On 24.07.2019 14:15, mirq-linux@rere.qmqm.pl wrote:
External E-Mail
On Wed, Jul 24, 2019 at 10:35:29AM +0000, Codrin.Ciubotariu@microchip.com wrote:
On 22.07.2019 21:27, Michał Mirosław wrote:
Rework DAI format calculation in preparation for adding more formats later.
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
sound/soc/atmel/atmel_ssc_dai.c | 283 +++++++++----------------------- 1 file changed, 79 insertions(+), 204 deletions(-)
diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c index 6f89483ac88c..b2992496e52f 100644 --- a/sound/soc/atmel/atmel_ssc_dai.c +++ b/sound/soc/atmel/atmel_ssc_dai.c
[...]
- 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);You can also add here SSC_BF(RCMR_CKO, SSC_CKO_NONE) and remove it from the if-else above;
I left it to keep symmetry between TX and RX code. I can pull it here if you prefer that way.
Right, you can leave it then.
- 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);
You are adding support for SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_CBM_CFS. If this is intended, please make a separate patch. If not, then:
printk(KERN_WARNING "atmel_ssc_dai: unsupported DAI format 0x%x\n", ssc_p->daifmt); return -EINVAL;
Hmm. I guess this is actually a good side effect. I can't see a way to contain this change that doesn't involve adding code that's immediately removed in next patch. So would you agree to just mentioning this in commit message?
I prefer a separate patch, for clarity mostly, but I don't have a strong opinion on this. Later, it might prove trickier to investigate a bug this case and review this patch. Also, we should test and see that this format works indeed...
Best regards, Codrin