[alsa-devel] [PATCH 5/5] ASoC: atmel_ssc_dai: Enable shared FSYNC source in frame-slave mode

[Codrin.Ciubotariu at microchip.com]

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 at 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