| Message ID | 107e0cfd11a31ce1558e941612e183100022930d.1563819483.git.mirq-linux@rere.qmqm.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: atmel: extend SSC support | expand |
On 22.07.2019 21:27, Michał Mirosław wrote: > SSC driver allows only synchronous TX and RX. In slave mode for BCLK > it uses only one of TK or RK pin, but for LRCLK it configured separate > inputs from TF and RF pins. Allow configuration with common FS signal. > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> > --- > .../devicetree/bindings/misc/atmel-ssc.txt | 4 ++ > drivers/misc/atmel-ssc.c | 2 + > include/linux/atmel-ssc.h | 1 + > sound/soc/atmel/atmel_ssc_dai.c | 48 ++++++++++++------- > 4 files changed, 38 insertions(+), 17 deletions(-) > > diff --git a/Documentation/devicetree/bindings/misc/atmel-ssc.txt b/Documentation/devicetree/bindings/misc/atmel-ssc.txt > index f9fb412642fe..89133c5b82cb 100644 > --- a/Documentation/devicetree/bindings/misc/atmel-ssc.txt > +++ b/Documentation/devicetree/bindings/misc/atmel-ssc.txt > @@ -24,6 +24,10 @@ Optional properties: > this parameter to choose where the clock from. > - By default the clock is from TK pin, if the clock from RK pin, this > property is needed. > + - atmel,shared-fs-pin: bool property. > + - When SSC works in slave mode, by default it gets separate LRCLK signals > + from RF and TF. This property makes SSC use a single pin for both > + RX and TX. TF is used unless atmel,clk-from-rk-pin is also present. > - #sound-dai-cells: Should contain <0>. > - This property makes the SSC into an automatically registered DAI. the binding changes should be sent in a separate patch, please see Documentation/devicetree/bindings/submitting-patches.txt for details. > > diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c > index ab4144ea1f11..da63eee1cdf5 100644 > --- a/drivers/misc/atmel-ssc.c > +++ b/drivers/misc/atmel-ssc.c > @@ -210,6 +210,8 @@ static int ssc_probe(struct platform_device *pdev) > struct device_node *np = pdev->dev.of_node; > ssc->clk_from_rk_pin = > of_property_read_bool(np, "atmel,clk-from-rk-pin"); > + ssc->shared_fs_pin = > + of_property_read_bool(np, "atmel,shared-fs-pin"); > } > > regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); > diff --git a/include/linux/atmel-ssc.h b/include/linux/atmel-ssc.h > index 6091d2abc1eb..46fdff2dfbb0 100644 > --- a/include/linux/atmel-ssc.h > +++ b/include/linux/atmel-ssc.h > @@ -21,6 +21,7 @@ struct ssc_device { > int user; > int irq; > bool clk_from_rk_pin; > + bool shared_fs_pin; > bool sound_dai; > }; These changes should also be in a separate patch, since drivers/misc/* files are in a different subsystem, with different maintainers. > > diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c > index cf2cfc345676..6f595a748618 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, fs_osync; > + int fslen, fslen_ext, fs_osync, fs_edge; > u32 cmr_div; > u32 tcmr_period; > u32 rcmr_period; > @@ -567,24 +567,20 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, > case SND_SOC_DAIFMT_LEFT_J: > /* left-justified format */ > fs_osync = SSC_FSOS_POSITIVE; > + fs_edge = SSC_START_RISING_TF; > > - rcmr = SSC_BF(RCMR_STTDLY, 0) > - | SSC_BF(RCMR_START, SSC_START_RISING_RF); > - > - tcmr = SSC_BF(TCMR_STTDLY, 0) > - | SSC_BF(TCMR_START, SSC_START_RISING_TF); > + rcmr = SSC_BF(RCMR_STTDLY, 0); > + tcmr = SSC_BF(TCMR_STTDLY, 0); > > break; > > case SND_SOC_DAIFMT_I2S: > /* I2S format = left-justified with start bit and inverted LRCLK */ > fs_osync = SSC_FSOS_NEGATIVE; > + fs_edge = SSC_START_FALLING_TF; > > - rcmr = SSC_BF(RCMR_STTDLY, 1) > - | SSC_BF(RCMR_START, SSC_START_FALLING_RF); > - > - tcmr = SSC_BF(TCMR_STTDLY, 1) > - | SSC_BF(TCMR_START, SSC_START_FALLING_TF); > + rcmr = SSC_BF(RCMR_STTDLY, 1); > + tcmr = SSC_BF(TCMR_STTDLY, 1); > > break; > > @@ -597,13 +593,11 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, > * the left channel data. > */ > fs_osync = SSC_FSOS_POSITIVE; > + fs_edge = SSC_START_RISING_TF; > fslen = fslen_ext = 0; > > - rcmr = SSC_BF(RCMR_STTDLY, 1) > - | SSC_BF(RCMR_START, SSC_START_RISING_RF); > - > - tcmr = SSC_BF(TCMR_STTDLY, 1) > - | SSC_BF(TCMR_START, SSC_START_RISING_TF); > + rcmr = SSC_BF(RCMR_STTDLY, 1); > + tcmr = SSC_BF(TCMR_STTDLY, 1); > > break; > > @@ -613,10 +607,30 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, > return -EINVAL; > } > > - if (!atmel_ssc_cfs(ssc_p)) { > + if (atmel_ssc_cfs(ssc_p)) { > + /* > + * SSC provides LRCLK > + * > + * Both TF and RF are generated, so use them directly. > + */ > + rcmr |= SSC_BF(RCMR_START, fs_edge); > + tcmr |= SSC_BF(TCMR_START, fs_edge); > + } else { > fslen = fslen_ext = 0; > rcmr_period = tcmr_period = 0; > fs_osync = SSC_FSOS_NONE; > + if (!ssc->shared_fs_pin) { > + rcmr |= SSC_BF(RCMR_START, fs_edge); > + tcmr |= SSC_BF(TCMR_START, fs_edge); > + } else if (ssc->clk_from_rk_pin) { > + /* assume RF is to be used when RK is used as BCLK input */ > + /* Note: won't work correctly on SAMA5D2 due to errata */ > + rcmr |= SSC_BF(RCMR_START, fs_edge); > + tcmr |= SSC_BF(TCMR_START, SSC_START_RECEIVE); Did you find a platform in which this mode works? > + } else { > + rcmr |= SSC_BF(RCMR_START, SSC_START_TRANSMIT); > + tcmr |= SSC_BF(TCMR_START, fs_edge); > + } > } > > if (atmel_ssc_cbs(ssc_p)) { > I like this feature, just separate the changes in different patches. Thanks and best regards, Codrin
On Thu, Jul 25, 2019 at 03:02:34PM +0000, Codrin.Ciubotariu@microchip.com wrote: > On 22.07.2019 21:27, Michał Mirosław wrote: > > SSC driver allows only synchronous TX and RX. In slave mode for BCLK > > it uses only one of TK or RK pin, but for LRCLK it configured separate > > inputs from TF and RF pins. Allow configuration with common FS signal. [...] > > @@ -613,10 +607,30 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, > > return -EINVAL; > > } > > > > - if (!atmel_ssc_cfs(ssc_p)) { > > + if (atmel_ssc_cfs(ssc_p)) { > > + /* > > + * SSC provides LRCLK > > + * > > + * Both TF and RF are generated, so use them directly. > > + */ > > + rcmr |= SSC_BF(RCMR_START, fs_edge); > > + tcmr |= SSC_BF(TCMR_START, fs_edge); > > + } else { > > fslen = fslen_ext = 0; > > rcmr_period = tcmr_period = 0; > > fs_osync = SSC_FSOS_NONE; > > + if (!ssc->shared_fs_pin) { > > + rcmr |= SSC_BF(RCMR_START, fs_edge); > > + tcmr |= SSC_BF(TCMR_START, fs_edge); > > + } else if (ssc->clk_from_rk_pin) { > > + /* assume RF is to be used when RK is used as BCLK input */ > > + /* Note: won't work correctly on SAMA5D2 due to errata */ > > + rcmr |= SSC_BF(RCMR_START, fs_edge); > > + tcmr |= SSC_BF(TCMR_START, SSC_START_RECEIVE); > > Did you find a platform in which this mode works? To be exact: according to the errata, TX is delayed improperly. So if you use only RX (SSC side receives) direction, you're fine. Best Regards, Michał Mirosław
On 25.07.2019 21:24, mirq-linux@rere.qmqm.pl wrote: > External E-Mail > > > On Thu, Jul 25, 2019 at 03:02:34PM +0000, Codrin.Ciubotariu@microchip.com wrote: >> On 22.07.2019 21:27, Michał Mirosław wrote: >>> SSC driver allows only synchronous TX and RX. In slave mode for BCLK >>> it uses only one of TK or RK pin, but for LRCLK it configured separate >>> inputs from TF and RF pins. Allow configuration with common FS signal. > [...] >>> @@ -613,10 +607,30 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, >>> return -EINVAL; >>> } >>> >>> - if (!atmel_ssc_cfs(ssc_p)) { >>> + if (atmel_ssc_cfs(ssc_p)) { >>> + /* >>> + * SSC provides LRCLK >>> + * >>> + * Both TF and RF are generated, so use them directly. >>> + */ >>> + rcmr |= SSC_BF(RCMR_START, fs_edge); >>> + tcmr |= SSC_BF(TCMR_START, fs_edge); >>> + } else { >>> fslen = fslen_ext = 0; >>> rcmr_period = tcmr_period = 0; >>> fs_osync = SSC_FSOS_NONE; >>> + if (!ssc->shared_fs_pin) { >>> + rcmr |= SSC_BF(RCMR_START, fs_edge); >>> + tcmr |= SSC_BF(TCMR_START, fs_edge); >>> + } else if (ssc->clk_from_rk_pin) { >>> + /* assume RF is to be used when RK is used as BCLK input */ >>> + /* Note: won't work correctly on SAMA5D2 due to errata */ >>> + rcmr |= SSC_BF(RCMR_START, fs_edge); >>> + tcmr |= SSC_BF(TCMR_START, SSC_START_RECEIVE); >> >> Did you find a platform in which this mode works? > > To be exact: according to the errata, TX is delayed improperly. So if you > use only RX (SSC side receives) direction, you're fine. I know, but there are other platforms with SSC, which don't have this errata, like sam9x35 or sama5d3. Have you tested this mode, RK input, RF input, RD starts on edge detect, TF input, TD starts synchronously with receiver? Best regards, Codrin
On Fri, Jul 26, 2019 at 10:33:29AM +0000, Codrin.Ciubotariu@microchip.com wrote: > On 25.07.2019 21:24, mirq-linux@rere.qmqm.pl wrote: > > On Thu, Jul 25, 2019 at 03:02:34PM +0000, Codrin.Ciubotariu@microchip.com wrote: > >> On 22.07.2019 21:27, Michał Mirosław wrote: > >>> SSC driver allows only synchronous TX and RX. In slave mode for BCLK > >>> it uses only one of TK or RK pin, but for LRCLK it configured separate > >>> inputs from TF and RF pins. Allow configuration with common FS signal. > > [...] > >>> @@ -613,10 +607,30 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, > >>> return -EINVAL; > >>> } > >>> > >>> - if (!atmel_ssc_cfs(ssc_p)) { > >>> + if (atmel_ssc_cfs(ssc_p)) { > >>> + /* > >>> + * SSC provides LRCLK > >>> + * > >>> + * Both TF and RF are generated, so use them directly. > >>> + */ > >>> + rcmr |= SSC_BF(RCMR_START, fs_edge); > >>> + tcmr |= SSC_BF(TCMR_START, fs_edge); > >>> + } else { > >>> fslen = fslen_ext = 0; > >>> rcmr_period = tcmr_period = 0; > >>> fs_osync = SSC_FSOS_NONE; > >>> + if (!ssc->shared_fs_pin) { > >>> + rcmr |= SSC_BF(RCMR_START, fs_edge); > >>> + tcmr |= SSC_BF(TCMR_START, fs_edge); > >>> + } else if (ssc->clk_from_rk_pin) { > >>> + /* assume RF is to be used when RK is used as BCLK input */ > >>> + /* Note: won't work correctly on SAMA5D2 due to errata */ > >>> + rcmr |= SSC_BF(RCMR_START, fs_edge); > >>> + tcmr |= SSC_BF(TCMR_START, SSC_START_RECEIVE); > >> > >> Did you find a platform in which this mode works? > > > > To be exact: according to the errata, TX is delayed improperly. So if you > > use only RX (SSC side receives) direction, you're fine. > > I know, but there are other platforms with SSC, which don't have this > errata, like sam9x35 or sama5d3. Have you tested this mode, RK input, RF > input, RD starts on edge detect, TF input, TD starts synchronously with > receiver? No, I have only SAMA5D2 available to test. Best Regards, Michał Mirosław
diff --git a/Documentation/devicetree/bindings/misc/atmel-ssc.txt b/Documentation/devicetree/bindings/misc/atmel-ssc.txt index f9fb412642fe..89133c5b82cb 100644 --- a/Documentation/devicetree/bindings/misc/atmel-ssc.txt +++ b/Documentation/devicetree/bindings/misc/atmel-ssc.txt @@ -24,6 +24,10 @@ Optional properties: this parameter to choose where the clock from. - By default the clock is from TK pin, if the clock from RK pin, this property is needed. + - atmel,shared-fs-pin: bool property. + - When SSC works in slave mode, by default it gets separate LRCLK signals + from RF and TF. This property makes SSC use a single pin for both + RX and TX. TF is used unless atmel,clk-from-rk-pin is also present. - #sound-dai-cells: Should contain <0>. - This property makes the SSC into an automatically registered DAI. diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index ab4144ea1f11..da63eee1cdf5 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -210,6 +210,8 @@ static int ssc_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; ssc->clk_from_rk_pin = of_property_read_bool(np, "atmel,clk-from-rk-pin"); + ssc->shared_fs_pin = + of_property_read_bool(np, "atmel,shared-fs-pin"); } regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); diff --git a/include/linux/atmel-ssc.h b/include/linux/atmel-ssc.h index 6091d2abc1eb..46fdff2dfbb0 100644 --- a/include/linux/atmel-ssc.h +++ b/include/linux/atmel-ssc.h @@ -21,6 +21,7 @@ struct ssc_device { int user; int irq; bool clk_from_rk_pin; + bool shared_fs_pin; bool sound_dai; }; diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c index cf2cfc345676..6f595a748618 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, fs_osync; + int fslen, fslen_ext, fs_osync, fs_edge; u32 cmr_div; u32 tcmr_period; u32 rcmr_period; @@ -567,24 +567,20 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, case SND_SOC_DAIFMT_LEFT_J: /* left-justified format */ fs_osync = SSC_FSOS_POSITIVE; + fs_edge = SSC_START_RISING_TF; - rcmr = SSC_BF(RCMR_STTDLY, 0) - | SSC_BF(RCMR_START, SSC_START_RISING_RF); - - tcmr = SSC_BF(TCMR_STTDLY, 0) - | SSC_BF(TCMR_START, SSC_START_RISING_TF); + rcmr = SSC_BF(RCMR_STTDLY, 0); + tcmr = SSC_BF(TCMR_STTDLY, 0); break; case SND_SOC_DAIFMT_I2S: /* I2S format = left-justified with start bit and inverted LRCLK */ fs_osync = SSC_FSOS_NEGATIVE; + fs_edge = SSC_START_FALLING_TF; - rcmr = SSC_BF(RCMR_STTDLY, 1) - | SSC_BF(RCMR_START, SSC_START_FALLING_RF); - - tcmr = SSC_BF(TCMR_STTDLY, 1) - | SSC_BF(TCMR_START, SSC_START_FALLING_TF); + rcmr = SSC_BF(RCMR_STTDLY, 1); + tcmr = SSC_BF(TCMR_STTDLY, 1); break; @@ -597,13 +593,11 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, * the left channel data. */ fs_osync = SSC_FSOS_POSITIVE; + fs_edge = SSC_START_RISING_TF; fslen = fslen_ext = 0; - rcmr = SSC_BF(RCMR_STTDLY, 1) - | SSC_BF(RCMR_START, SSC_START_RISING_RF); - - tcmr = SSC_BF(TCMR_STTDLY, 1) - | SSC_BF(TCMR_START, SSC_START_RISING_TF); + rcmr = SSC_BF(RCMR_STTDLY, 1); + tcmr = SSC_BF(TCMR_STTDLY, 1); break; @@ -613,10 +607,30 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream, return -EINVAL; } - if (!atmel_ssc_cfs(ssc_p)) { + if (atmel_ssc_cfs(ssc_p)) { + /* + * SSC provides LRCLK + * + * Both TF and RF are generated, so use them directly. + */ + rcmr |= SSC_BF(RCMR_START, fs_edge); + tcmr |= SSC_BF(TCMR_START, fs_edge); + } else { fslen = fslen_ext = 0; rcmr_period = tcmr_period = 0; fs_osync = SSC_FSOS_NONE; + if (!ssc->shared_fs_pin) { + rcmr |= SSC_BF(RCMR_START, fs_edge); + tcmr |= SSC_BF(TCMR_START, fs_edge); + } else if (ssc->clk_from_rk_pin) { + /* assume RF is to be used when RK is used as BCLK input */ + /* Note: won't work correctly on SAMA5D2 due to errata */ + rcmr |= SSC_BF(RCMR_START, fs_edge); + tcmr |= SSC_BF(TCMR_START, SSC_START_RECEIVE); + } else { + rcmr |= SSC_BF(RCMR_START, SSC_START_TRANSMIT); + tcmr |= SSC_BF(TCMR_START, fs_edge); + } } if (atmel_ssc_cbs(ssc_p)) {
SSC driver allows only synchronous TX and RX. In slave mode for BCLK it uses only one of TK or RK pin, but for LRCLK it configured separate inputs from TF and RF pins. Allow configuration with common FS signal. Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> --- .../devicetree/bindings/misc/atmel-ssc.txt | 4 ++ drivers/misc/atmel-ssc.c | 2 + include/linux/atmel-ssc.h | 1 + sound/soc/atmel/atmel_ssc_dai.c | 48 ++++++++++++------- 4 files changed, 38 insertions(+), 17 deletions(-)