[PATCH] ASoC: fsl-asoc-card: Allow to describe master or slave mode

[PATCH] ASoC: fsl-asoc-card: Allow to describe master or slave mode

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Provide a mechanism to describe whether the system runs in master or slave mode.

In order to keep compatibilty with existing dtb's let slave mode be default one.

Tested on a imx6q-sabresd board in both modes.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 Documentation/devicetree/bindings/sound/fsl-asoc-card.txt |  5 +++++
 sound/soc/fsl/fsl-asoc-card.c                             | 15 ++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
index a96774c..1bbc828 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
@@ -59,6 +59,10 @@ Optional properties:
   - audio-asrc		: The phandle of ASRC. It can be absent if there's no
 			  need to add ASRC support via DPCM.
 
+  - audio-cpu-mode	: The operating mode for the audio cpu interface.
+			  "slave" - audio clock comes from the external codec
+			  "master" -audio clock is generated from the audio cpu
+
 Example:
 sound-cs42888 {
 	compatible = "fsl,imx-audio-cs42888";
@@ -66,6 +70,7 @@ sound-cs42888 {
 	audio-cpu = <&esai>;
 	audio-asrc = <&asrc>;
 	audio-codec = <&cs42888>;
+	audio-cpu-mode = "master";
 	audio-routing =
 		"Line Out Jack", "AOUT1L",
 		"Line Out Jack", "AOUT1R",
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 007c772..bd903cc 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -393,6 +393,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	struct clk *codec_clk;
 	u32 width;
 	int ret;
+	const char *mode;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -441,6 +442,17 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	/* Assign a default DAI format, and allow each card to overwrite it */
 	priv->dai_fmt = DAI_FMT_BASE;
 
+	mode = of_get_property(np, "audio-cpu-mode", NULL);
+	if (mode) {
+		if (!strcmp(mode, "master"))
+			priv->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
+		else if (!strcmp(mode, "slave"))
+			priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
+	} else {
+		/* Assign slave mode by default */
+		priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
+	}
+
 	/* Diversify the card configurations */
 	if (of_device_is_compatible(np, "fsl,imx-audio-cs42888")) {
 		priv->card.set_bias_level = NULL;
@@ -448,16 +460,13 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		priv->cpu_priv.sysclk_freq[RX] = priv->codec_priv.mclk_freq;
 		priv->cpu_priv.sysclk_dir[TX] = SND_SOC_CLOCK_OUT;
 		priv->cpu_priv.sysclk_dir[RX] = SND_SOC_CLOCK_OUT;
-		priv->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000")) {
 		priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
-		priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8962")) {
 		priv->card.set_bias_level = fsl_asoc_card_set_bias_level;
 		priv->codec_priv.mclk_id = WM8962_SYSCLK_MCLK;
 		priv->codec_priv.fll_id = WM8962_SYSCLK_FLL;
 		priv->codec_priv.pll_id = WM8962_FLL;
-		priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
 		return -EINVAL;
-- 
1.9.1

Re: [PATCH] ASoC: fsl-asoc-card: Allow to describe master or slave mode

[Michael Trimarchi]

Hi

Il 22/set/2014 06:41 "Fabio Estevam" <festevam@gmail.com> ha scritto:
>
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Provide a mechanism to describe whether the system runs in master or
slave mode.
>
> In order to keep compatibilty with existing dtb's let slave mode be
default one.
>
> Tested on a imx6q-sabresd board in both modes.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  Documentation/devicetree/bindings/sound/fsl-asoc-card.txt |  5 +++++
>  sound/soc/fsl/fsl-asoc-card.c                             | 15
++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> index a96774c..1bbc828 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> @@ -59,6 +59,10 @@ Optional properties:
>    - audio-asrc         : The phandle of ASRC. It can be absent if
there's no
>                           need to add ASRC support via DPCM.
>
> +  - audio-cpu-mode     : The operating mode for the audio cpu interface.
> +                         "slave" - audio clock comes from the external
codec
> +                         "master" -audio clock is generated from the
audio cpu
> +
>  Example:
>  sound-cs42888 {
>         compatible = "fsl,imx-audio-cs42888";
> @@ -66,6 +70,7 @@ sound-cs42888 {
>         audio-cpu = <&esai>;
>         audio-asrc = <&asrc>;
>         audio-codec = <&cs42888>;
> +       audio-cpu-mode = "master";
>         audio-routing =
>                 "Line Out Jack", "AOUT1L",
>                 "Line Out Jack", "AOUT1R",
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index 007c772..bd903cc 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -393,6 +393,7 @@ static int fsl_asoc_card_probe(struct platform_device
*pdev)
>         struct clk *codec_clk;
>         u32 width;
>         int ret;
> +       const char *mode;
>
>         priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>         if (!priv)
> @@ -441,6 +442,17 @@ static int fsl_asoc_card_probe(struct
platform_device *pdev)
>         /* Assign a default DAI format, and allow each card to overwrite
it */
>         priv->dai_fmt = DAI_FMT_BASE;
>
> +       mode = of_get_property(np, "audio-cpu-mode", NULL);
> +       if (mode) {
> +               if (!strcmp(mode, "master"))
> +                       priv->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
> +               else if (!strcmp(mode, "slave"))
> +                       priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
> +       } else {
> +               /* Assign slave mode by default */
> +               priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
> +       }
> +

If mode has no error path, what is the clocking in error scenario?

if (mode && is_master)
     set_master
else
     set_slave

Michael

>         /* Diversify the card configurations */
>         if (of_device_is_compatible(np, "fsl,imx-audio-cs42888")) {
>                 priv->card.set_bias_level = NULL;
> @@ -448,16 +460,13 @@ static int fsl_asoc_card_probe(struct
platform_device *pdev)
>                 priv->cpu_priv.sysclk_freq[RX] =
priv->codec_priv.mclk_freq;
>                 priv->cpu_priv.sysclk_dir[TX] = SND_SOC_CLOCK_OUT;
>                 priv->cpu_priv.sysclk_dir[RX] = SND_SOC_CLOCK_OUT;
> -               priv->dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
>         } else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000"))
{
>                 priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
> -               priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
>         } else if (of_device_is_compatible(np, "fsl,imx-audio-wm8962")) {
>                 priv->card.set_bias_level = fsl_asoc_card_set_bias_level;
>                 priv->codec_priv.mclk_id = WM8962_SYSCLK_MCLK;
>                 priv->codec_priv.fll_id = WM8962_SYSCLK_FLL;
>                 priv->codec_priv.pll_id = WM8962_FLL;
> -               priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
>         } else {
>                 dev_err(&pdev->dev, "unknown Device Tree compatible\n");
>                 return -EINVAL;
> --
> 1.9.1
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoC: fsl-asoc-card: Allow to describe master or slave mode

[Nicolin Chen]

Hi Fabio,

On Mon, Sep 22, 2014 at 01:40:54AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Provide a mechanism to describe whether the system runs in master or slave mode.
> 
> In order to keep compatibilty with existing dtb's let slave mode be default one.
> 
> Tested on a imx6q-sabresd board in both modes.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  Documentation/devicetree/bindings/sound/fsl-asoc-card.txt |  5 +++++
>  sound/soc/fsl/fsl-asoc-card.c                             | 15 ++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> index a96774c..1bbc828 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> @@ -59,6 +59,10 @@ Optional properties:
>    - audio-asrc		: The phandle of ASRC. It can be absent if there's no
>  			  need to add ASRC support via DPCM.
>  
> +  - audio-cpu-mode	: The operating mode for the audio cpu interface.
> +			  "slave" - audio clock comes from the external codec
> +			  "master" -audio clock is generated from the audio cpu
> +

This could be a good idea to move the 'hardcode' into the DT binding
while maintaining the compatibility toward imx-sgtl5000 and imx-wm8962.
But at least, it still needs to separate the bit clock and frame sync
clock. Otherwise, the CBS_CFM and CBM_CFS cases will be expelled, as
AUDMUX part in the driver takes care of them even if they are pretty
rare.

And another question I's wondering myself: If they are really worth
adding. Because we may then need I2S/LEFT_J parsing code too while
we can't eradicate all the hard code such as pll_id and mclk_id. So
since the further compatible of the driver still needs some specific
configurations, why not just leave DAI formats configurations in the
driver as well -- So I didn't expect this driver will be enhanced to
become a Freescale Simple Card.


What do you think about it after seeing my idea.

One more thing: we may figure out a better and more explicit name,
rather than 'cpu-mode', if we decide to add DAI formats information
into DT.

Thank you
Nicolin