[PATCHv4 0/9] ASoC: fsl-asoc-card: compatibility integration of a generic codec use case for use with S/PDIF controller

[PATCHv4 0/9] ASoC: fsl-asoc-card: compatibility integration of a generic codec use case for use with S/PDIF controller

[Elinor Montmasson]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=quoted-printable, Size: 5074 bytes --]

Hello,

This is the v4 of the series of patch aiming to make the machine driver
"fsl-asoc-card" compatible with use cases where there is no real codec
driver. It proposes to use the "spdif_receiver" and "spdif_transmitter"
drivers instead of the dummy codec.
This is a first step in using the S/PDIF controller with the ASRC.

The five first patches add compatibility with the pair of codecs
"spdif_receiver" and "spdif_transmitter" with a new compatible,
"fsl,imx-audio-generic". Codec parameters are set with default values.
Consequently, the driver is modified to work with multi-codec use cases.
It can get 2 codecs phandles from the device tree, and the
"fsl_asoc_card_priv" struct now has 2 "codec_priv" to store properties
of both codecs. It is fixed to 2 codecs as only "fsl,imx-audio-generic"
uses multiple codecs at the moment.
However, the driver now uses "for_each_codecs" macros when possible to
ease future implementations of multi-codec configurations.

The three following patches add configuration options for the
devicetree. They configure the CPU DAI when using
"fsl,imx-audio-generic". These options are usually hard-coded in
"fsl-asoc-card.c" for each audio codec. Because the generic codec could
be used with other CPU DAIs than the S/PDIF controller, setting these
parameters could be required.
These new options try to follow the style of the simple-card driver:
* standard TDM properties are used, as defined in "tdm-slot.txt".
* the CPU DAI system-clock can be specified, allowing the codec to
retrieve its frequency.
* the CPU DAI system-clock direction can be specified through a new
binding, the same way it is done in simple-card.

The last commit updates the DT bindings documentation and add a new
example for the generic codec use case.

This series of patch was successfully built for arm64 and x86 on top of
the latest "for-next" branch of the ASoC git tree on the 14th of May
2024.
These modifications have also been tested on an i.MX8MN evaluation
board, with a linux kernel RT v6.1.26-rt8.

If you have any question or remark about these commits, don't hesitate
to reply.

Best regards,
Elinor Montmasson


Changelog:
v3 -> v4:
* Use the standard TDM bidings, as defined in "tdm-slot.txt", for the
new optional DT bindings setting the TDM slots number and width.
* Use the clock DT bindings to optionally specify the CPU DAI system
clock frequency, instead of a dedicated new binding.
* Rename the new DT binding "cpu-sysclk-dir-out" to
"cpu-system-clock-direction-out" to better follow the style of the
simple-card driver.
* Merge TX an RX bindings for CPU DAI system-clock, to better follow the
style of the simple-card driver, and also as there was no use case in
fsl-asoc-card where TX and RX settings had to be different.
* Add the documentation for the new bindings in the new DT schema
bindings documentation. Also add an example with the generic codec.
* v3 patch series at :
https://lore.kernel.org/alsa-devel/20231218124058.2047167-1-elinor.montmasson@savoirfairelinux.com/

v2 -> v3:
* When the bitmaster or framemaster are retrieved from the device tree,
the driver will now compare them with the two codecs possibly given in
device tree, and not just the first codec.
* Improve driver modifications to use multiple codecs for better
integration of future multi-codec use cases:
    * Use "for_each_codec" macros when possible.
    * "fsl_asoc_card_priv" struct now has 2 "codec_priv" as the driver
    can currently retrieve 2 codec phandles from the device tree.
* Fix subject of patch 10/10 to follow the style of the subsystem
* v2 patch series at:
https://lore.kernel.org/alsa-devel/20231027144734.3654829-1-elinor.montmasson@savoirfairelinux.com/

v1 -> v2:
* Replace use of the dummy codec by the pair of codecs spdif_receiver /
spdif_transmitter.
* Adapt how dai links codecs are used to take into account the
possibility for multiple codecs per link.
* Change compatible name.
* Adapt driver to be able to register two codecs given in the device
tree.
* v1 patch series at:
https://lore.kernel.org/alsa-devel/20230901144550.520072-1-elinor.montmasson@savoirfairelinux.com/


Elinor Montmasson (9):
  ASoC: fsl-asoc-card: add support for dai links with multiple codecs
  ASoC: fsl-asoc-card: add second dai link component for codecs
  ASoC: fsl-asoc-card: add compatibility to use 2 codecs in dai-links
  ASoC: fsl-asoc-card: add new compatible for a generic codec use case
  ASoC: fsl-asoc-card: set generic codec as clock provider
  ASoC: fsl-asoc-card: add use of devicetree TDM slot properties
  ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec
  ASoC: fsl-asoc-card: add DT property "cpu-system-clock-direction-out"
  ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

 .../bindings/sound/fsl-asoc-card.yaml         |  96 +++++-
 sound/soc/fsl/fsl-asoc-card.c                 | 306 +++++++++++-------
 2 files changed, 287 insertions(+), 115 deletions(-)

-- 
2.34.1

[PATCHv4 1/9] ASoC: fsl-asoc-card: add support for dai links with multiple codecs

[Elinor Montmasson]

Add support for dai links using multiple codecs for multi-codec
use cases.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
Co-authored-by: Philip-Dylan Gleonec <philip-dylan.gleonec@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 5ddc0c2fe53f..8a2a6e5461dc 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -815,10 +815,10 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 
 	/* Normal DAI Link */
 	priv->dai_link[0].cpus->of_node = cpu_np;
-	priv->dai_link[0].codecs->dai_name = codec_dai_name;
+	priv->dai_link[0].codecs[0].dai_name = codec_dai_name;
 
 	if (!fsl_asoc_card_is_ac97(priv))
-		priv->dai_link[0].codecs->of_node = codec_np;
+		priv->dai_link[0].codecs[0].of_node = codec_np;
 	else {
 		u32 idx;
 
@@ -829,11 +829,11 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 			goto asrc_fail;
 		}
 
-		priv->dai_link[0].codecs->name =
+		priv->dai_link[0].codecs[0].name =
 				devm_kasprintf(&pdev->dev, GFP_KERNEL,
 					       "ac97-codec.%u",
 					       (unsigned int)idx);
-		if (!priv->dai_link[0].codecs->name) {
+		if (!priv->dai_link[0].codecs[0].name) {
 			ret = -ENOMEM;
 			goto asrc_fail;
 		}
@@ -844,13 +844,19 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	priv->card.num_links = 1;
 
 	if (asrc_pdev) {
+		int i;
+		struct snd_soc_dai_link_component *codec;
+		struct snd_soc_dai_link *link;
+
 		/* DPCM DAI Links only if ASRC exists */
 		priv->dai_link[1].cpus->of_node = asrc_np;
 		priv->dai_link[1].platforms->of_node = asrc_np;
-		priv->dai_link[2].codecs->dai_name = codec_dai_name;
-		priv->dai_link[2].codecs->of_node = codec_np;
-		priv->dai_link[2].codecs->name =
-				priv->dai_link[0].codecs->name;
+		link = &(priv->dai_link[2]);
+		for_each_link_codecs(link, i, codec) {
+			codec->dai_name = priv->dai_link[0].codecs[i].dai_name;
+			codec->of_node = priv->dai_link[0].codecs[i].of_node;
+			codec->name = priv->dai_link[0].codecs[i].name;
+		}
 		priv->dai_link[2].cpus->of_node = cpu_np;
 		priv->dai_link[2].dai_fmt = priv->dai_fmt;
 		priv->card.num_links = 3;
-- 
2.34.1

[PATCHv4 2/9] ASoC: fsl-asoc-card: add second dai link component for codecs

[Elinor Montmasson]

Add a second dai link component for codecs that will be used for the
generic codec use case.
It will use spdif_receiver and spdif_transmitter drivers as dummy codec
drivers, needing 2 codecs slots for the links.

To prevent deferring in use cases using only one codec, also set
by default the number of codecs to 1 for the relevant dai links.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
Co-authored-by: Philip-Dylan Gleonec <philip-dylan.gleonec@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 8a2a6e5461dc..c83492e7cec2 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -296,7 +296,7 @@ static int be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 
 SND_SOC_DAILINK_DEFS(hifi,
 	DAILINK_COMP_ARRAY(COMP_EMPTY()),
-	DAILINK_COMP_ARRAY(COMP_EMPTY()),
+	DAILINK_COMP_ARRAY(COMP_EMPTY(), COMP_EMPTY()),
 	DAILINK_COMP_ARRAY(COMP_EMPTY()));
 
 SND_SOC_DAILINK_DEFS(hifi_fe,
@@ -306,7 +306,7 @@ SND_SOC_DAILINK_DEFS(hifi_fe,
 
 SND_SOC_DAILINK_DEFS(hifi_be,
 	DAILINK_COMP_ARRAY(COMP_EMPTY()),
-	DAILINK_COMP_ARRAY(COMP_EMPTY()));
+	DAILINK_COMP_ARRAY(COMP_EMPTY(), COMP_EMPTY()));
 
 static const struct snd_soc_dai_link fsl_asoc_card_dai[] = {
 	/* Default ASoC DAI Link*/
@@ -618,6 +618,8 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 
 	memcpy(priv->dai_link, fsl_asoc_card_dai,
 	       sizeof(struct snd_soc_dai_link) * ARRAY_SIZE(priv->dai_link));
+	priv->dai_link[0].num_codecs = 1;
+	priv->dai_link[2].num_codecs = 1;
 
 	priv->card.dapm_routes = audio_map;
 	priv->card.num_dapm_routes = ARRAY_SIZE(audio_map);
-- 
2.34.1

[PATCHv4 3/9] ASoC: fsl-asoc-card: add compatibility to use 2 codecs in dai-links

[Elinor Montmasson]

Adapt the driver to work with configurations using two codecs or more.
Modify fsl_asoc_card_probe() to handle use cases where 2 codecs are
given in the device tree.
This will be needed for the generic codec case.

Use cases using one codec will ignore any given codecs other than the
first.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
Co-authored-by: Philip-Dylan Gleonec <philip-dylan.gleonec@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 239 ++++++++++++++++++++--------------
 1 file changed, 139 insertions(+), 100 deletions(-)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index c83492e7cec2..620a25eb068a 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -99,7 +99,7 @@ struct fsl_asoc_card_priv {
 	struct simple_util_jack hp_jack;
 	struct simple_util_jack mic_jack;
 	struct platform_device *pdev;
-	struct codec_priv codec_priv;
+	struct codec_priv codec_priv[2];
 	struct cpu_priv cpu_priv;
 	struct snd_soc_card card;
 	u8 streams;
@@ -172,11 +172,13 @@ static int fsl_asoc_card_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
 	struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(rtd->card);
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-	struct codec_priv *codec_priv = &priv->codec_priv;
+	struct codec_priv *codec_priv;
+	struct snd_soc_dai *codec_dai;
 	struct cpu_priv *cpu_priv = &priv->cpu_priv;
 	struct device *dev = rtd->card->dev;
 	unsigned int pll_out;
 	int ret;
+	int i;
 
 	priv->sample_rate = params_rate(params);
 	priv->sample_format = params_format(params);
@@ -208,28 +210,32 @@ static int fsl_asoc_card_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	/* Specific configuration for PLL */
-	if (codec_priv->pll_id >= 0 && codec_priv->fll_id >= 0) {
-		if (priv->sample_format == SNDRV_PCM_FORMAT_S24_LE)
-			pll_out = priv->sample_rate * 384;
-		else
-			pll_out = priv->sample_rate * 256;
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		codec_priv = &priv->codec_priv[i];
 
-		ret = snd_soc_dai_set_pll(snd_soc_rtd_to_codec(rtd, 0),
-					  codec_priv->pll_id,
-					  codec_priv->mclk_id,
-					  codec_priv->mclk_freq, pll_out);
-		if (ret) {
-			dev_err(dev, "failed to start FLL: %d\n", ret);
-			goto fail;
-		}
+		if (codec_priv->pll_id >= 0 && codec_priv->fll_id >= 0) {
+			if (priv->sample_format == SNDRV_PCM_FORMAT_S24_LE)
+				pll_out = priv->sample_rate * 384;
+			else
+				pll_out = priv->sample_rate * 256;
 
-		ret = snd_soc_dai_set_sysclk(snd_soc_rtd_to_codec(rtd, 0),
-					     codec_priv->fll_id,
-					     pll_out, SND_SOC_CLOCK_IN);
+			ret = snd_soc_dai_set_pll(codec_dai,
+						codec_priv->pll_id,
+						codec_priv->mclk_id,
+						codec_priv->mclk_freq, pll_out);
+			if (ret) {
+				dev_err(dev, "failed to start FLL: %d\n", ret);
+				goto fail;
+			}
 
-		if (ret && ret != -ENOTSUPP) {
-			dev_err(dev, "failed to set SYSCLK: %d\n", ret);
-			goto fail;
+			ret = snd_soc_dai_set_sysclk(codec_dai,
+						codec_priv->fll_id,
+						pll_out, SND_SOC_CLOCK_IN);
+
+			if (ret && ret != -ENOTSUPP) {
+				dev_err(dev, "failed to set SYSCLK: %d\n", ret);
+				goto fail;
+			}
 		}
 	}
 
@@ -244,28 +250,34 @@ static int fsl_asoc_card_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
 	struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(rtd->card);
-	struct codec_priv *codec_priv = &priv->codec_priv;
+	struct codec_priv *codec_priv;
+	struct snd_soc_dai *codec_dai;
 	struct device *dev = rtd->card->dev;
 	int ret;
+	int i;
 
 	priv->streams &= ~BIT(substream->stream);
 
-	if (!priv->streams && codec_priv->pll_id >= 0 && codec_priv->fll_id >= 0) {
-		/* Force freq to be free_freq to avoid error message in codec */
-		ret = snd_soc_dai_set_sysclk(snd_soc_rtd_to_codec(rtd, 0),
-					     codec_priv->mclk_id,
-					     codec_priv->free_freq,
-					     SND_SOC_CLOCK_IN);
-		if (ret) {
-			dev_err(dev, "failed to switch away from FLL: %d\n", ret);
-			return ret;
-		}
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		codec_priv = &priv->codec_priv[i];
 
-		ret = snd_soc_dai_set_pll(snd_soc_rtd_to_codec(rtd, 0),
-					  codec_priv->pll_id, 0, 0, 0);
-		if (ret && ret != -ENOTSUPP) {
-			dev_err(dev, "failed to stop FLL: %d\n", ret);
-			return ret;
+		if (!priv->streams && codec_priv->pll_id >= 0 && codec_priv->fll_id >= 0) {
+			/* Force freq to be free_freq to avoid error message in codec */
+			ret = snd_soc_dai_set_sysclk(codec_dai,
+						codec_priv->mclk_id,
+						codec_priv->free_freq,
+						SND_SOC_CLOCK_IN);
+			if (ret) {
+				dev_err(dev, "failed to switch away from FLL: %d\n", ret);
+				return ret;
+			}
+
+			ret = snd_soc_dai_set_pll(codec_dai,
+						codec_priv->pll_id, 0, 0, 0);
+			if (ret && ret != -ENOTSUPP) {
+				dev_err(dev, "failed to stop FLL: %d\n", ret);
+				return ret;
+			}
 		}
 	}
 
@@ -504,10 +516,11 @@ static int fsl_asoc_card_late_probe(struct snd_soc_card *card)
 	struct fsl_asoc_card_priv *priv = snd_soc_card_get_drvdata(card);
 	struct snd_soc_pcm_runtime *rtd = list_first_entry(
 			&card->rtd_list, struct snd_soc_pcm_runtime, list);
-	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
-	struct codec_priv *codec_priv = &priv->codec_priv;
+	struct snd_soc_dai *codec_dai;
+	struct codec_priv *codec_priv;
 	struct device *dev = card->dev;
 	int ret;
+	int i;
 
 	if (fsl_asoc_card_is_ac97(priv)) {
 #if IS_ENABLED(CONFIG_SND_AC97_CODEC)
@@ -526,31 +539,36 @@ static int fsl_asoc_card_late_probe(struct snd_soc_card *card)
 		return 0;
 	}
 
-	ret = snd_soc_dai_set_sysclk(codec_dai, codec_priv->mclk_id,
-				     codec_priv->mclk_freq, SND_SOC_CLOCK_IN);
-	if (ret && ret != -ENOTSUPP) {
-		dev_err(dev, "failed to set sysclk in %s\n", __func__);
-		return ret;
-	}
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		codec_priv = &priv->codec_priv[i];
 
-	if (!IS_ERR_OR_NULL(codec_priv->mclk))
-		clk_prepare_enable(codec_priv->mclk);
+		ret = snd_soc_dai_set_sysclk(codec_dai, codec_priv->mclk_id,
+					codec_priv->mclk_freq, SND_SOC_CLOCK_IN);
+		if (ret && ret != -ENOTSUPP) {
+			dev_err(dev, "failed to set sysclk in %s\n", __func__);
+			return ret;
+		}
+
+		if (!IS_ERR_OR_NULL(codec_priv->mclk))
+			clk_prepare_enable(codec_priv->mclk);
+	}
 
 	return 0;
 }
 
 static int fsl_asoc_card_probe(struct platform_device *pdev)
 {
-	struct device_node *cpu_np, *codec_np, *asrc_np;
+	struct device_node *cpu_np, *asrc_np;
+	struct device_node *codec_np[2];
 	struct device_node *np = pdev->dev.of_node;
 	struct platform_device *asrc_pdev = NULL;
 	struct device_node *bitclkprovider = NULL;
 	struct device_node *frameprovider = NULL;
 	struct platform_device *cpu_pdev;
 	struct fsl_asoc_card_priv *priv;
-	struct device *codec_dev = NULL;
+	struct device *codec_dev[2] = { NULL, NULL };
 	const char *codec_dai_name;
-	const char *codec_dev_name;
+	const char *codec_dev_name[2];
 	u32 asrc_fmt = 0;
 	u32 width;
 	int ret;
@@ -576,21 +594,25 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	codec_np = of_parse_phandle(np, "audio-codec", 0);
-	if (codec_np) {
-		struct platform_device *codec_pdev;
-		struct i2c_client *codec_i2c;
+	codec_np[0] = of_parse_phandle(np, "audio-codec", 0);
+	codec_np[1] = of_parse_phandle(np, "audio-codec", 1);
 
-		codec_i2c = of_find_i2c_device_by_node(codec_np);
-		if (codec_i2c) {
-			codec_dev = &codec_i2c->dev;
-			codec_dev_name = codec_i2c->name;
-		}
-		if (!codec_dev) {
-			codec_pdev = of_find_device_by_node(codec_np);
-			if (codec_pdev) {
-				codec_dev = &codec_pdev->dev;
-				codec_dev_name = codec_pdev->name;
+	for (int i = 0; i < 2; i++) {
+		if (codec_np[i]) {
+			struct platform_device *codec_pdev;
+			struct i2c_client *codec_i2c;
+
+			codec_i2c = of_find_i2c_device_by_node(codec_np[i]);
+			if (codec_i2c) {
+				codec_dev[i] = &codec_i2c->dev;
+				codec_dev_name[i] = codec_i2c->name;
+			}
+			if (!codec_dev[i]) {
+				codec_pdev = of_find_device_by_node(codec_np[i]);
+				if (codec_pdev) {
+					codec_dev[i] = &codec_pdev->dev;
+					codec_dev_name[i] = codec_pdev->name;
+				}
 			}
 		}
 	}
@@ -600,12 +622,14 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		asrc_pdev = of_find_device_by_node(asrc_np);
 
 	/* Get the MCLK rate only, and leave it controlled by CODEC drivers */
-	if (codec_dev) {
-		struct clk *codec_clk = clk_get(codec_dev, NULL);
+	for (int i = 0; i < 2; i++) {
+		if (codec_dev[i]) {
+			struct clk *codec_clk = clk_get(codec_dev[i], NULL);
 
-		if (!IS_ERR(codec_clk)) {
-			priv->codec_priv.mclk_freq = clk_get_rate(codec_clk);
-			clk_put(codec_clk);
+			if (!IS_ERR(codec_clk)) {
+				priv->codec_priv[i].mclk_freq = clk_get_rate(codec_clk);
+				clk_put(codec_clk);
+			}
 		}
 	}
 
@@ -625,25 +649,27 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	priv->card.num_dapm_routes = ARRAY_SIZE(audio_map);
 	priv->card.driver_name = DRIVER_NAME;
 
-	priv->codec_priv.fll_id = -1;
-	priv->codec_priv.pll_id = -1;
+	for (int i = 0; i < 2; i++) {
+		priv->codec_priv[i].fll_id = -1;
+		priv->codec_priv[i].pll_id = -1;
+	}
 
 	/* Diversify the card configurations */
 	if (of_device_is_compatible(np, "fsl,imx-audio-cs42888")) {
 		codec_dai_name = "cs42888";
-		priv->cpu_priv.sysclk_freq[TX] = priv->codec_priv.mclk_freq;
-		priv->cpu_priv.sysclk_freq[RX] = priv->codec_priv.mclk_freq;
+		priv->cpu_priv.sysclk_freq[TX] = priv->codec_priv[0].mclk_freq;
+		priv->cpu_priv.sysclk_freq[RX] = priv->codec_priv[0].mclk_freq;
 		priv->cpu_priv.sysclk_dir[TX] = SND_SOC_CLOCK_OUT;
 		priv->cpu_priv.sysclk_dir[RX] = SND_SOC_CLOCK_OUT;
 		priv->cpu_priv.slot_width = 32;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBC_CFC;
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-cs427x")) {
 		codec_dai_name = "cs4271-hifi";
-		priv->codec_priv.mclk_id = CS427x_SYSCLK_MCLK;
+		priv->codec_priv[0].mclk_id = CS427x_SYSCLK_MCLK;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-sgtl5000")) {
 		codec_dai_name = "sgtl5000";
-		priv->codec_priv.mclk_id = SGTL5000_SYSCLK;
+		priv->codec_priv[0].mclk_id = SGTL5000_SYSCLK;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-tlv320aic32x4")) {
 		codec_dai_name = "tlv320aic32x4-hifi";
@@ -659,14 +685,14 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_tx);
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8962")) {
 		codec_dai_name = "wm8962";
-		priv->codec_priv.mclk_id = WM8962_SYSCLK_MCLK;
-		priv->codec_priv.fll_id = WM8962_SYSCLK_FLL;
-		priv->codec_priv.pll_id = WM8962_FLL;
+		priv->codec_priv[0].mclk_id = WM8962_SYSCLK_MCLK;
+		priv->codec_priv[0].fll_id = WM8962_SYSCLK_FLL;
+		priv->codec_priv[0].pll_id = WM8962_FLL;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8960")) {
 		codec_dai_name = "wm8960-hifi";
-		priv->codec_priv.fll_id = WM8960_SYSCLK_AUTO;
-		priv->codec_priv.pll_id = WM8960_SYSCLK_AUTO;
+		priv->codec_priv[0].fll_id = WM8960_SYSCLK_AUTO;
+		priv->codec_priv[0].pll_id = WM8960_SYSCLK_AUTO;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-ac97")) {
 		codec_dai_name = "ac97-hifi";
@@ -698,25 +724,25 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8958")) {
 		codec_dai_name = "wm8994-aif1";
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
-		priv->codec_priv.mclk_id = WM8994_FLL_SRC_MCLK1;
-		priv->codec_priv.fll_id = WM8994_SYSCLK_FLL1;
-		priv->codec_priv.pll_id = WM8994_FLL1;
-		priv->codec_priv.free_freq = priv->codec_priv.mclk_freq;
+		priv->codec_priv[0].mclk_id = WM8994_FLL_SRC_MCLK1;
+		priv->codec_priv[0].fll_id = WM8994_SYSCLK_FLL1;
+		priv->codec_priv[0].pll_id = WM8994_FLL1;
+		priv->codec_priv[0].free_freq = priv->codec_priv[0].mclk_freq;
 		priv->card.dapm_routes = NULL;
 		priv->card.num_dapm_routes = 0;
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-nau8822")) {
 		codec_dai_name = "nau8822-hifi";
-		priv->codec_priv.mclk_id = NAU8822_CLK_MCLK;
-		priv->codec_priv.fll_id = NAU8822_CLK_PLL;
-		priv->codec_priv.pll_id = NAU8822_CLK_PLL;
+		priv->codec_priv[0].mclk_id = NAU8822_CLK_MCLK;
+		priv->codec_priv[0].fll_id = NAU8822_CLK_PLL;
+		priv->codec_priv[0].pll_id = NAU8822_CLK_PLL;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
-		if (codec_dev)
-			priv->codec_priv.mclk = devm_clk_get(codec_dev, NULL);
+		if (codec_dev[0])
+			priv->codec_priv[0].mclk = devm_clk_get(codec_dev[0], NULL);
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8904")) {
 		codec_dai_name = "wm8904-hifi";
-		priv->codec_priv.mclk_id = WM8904_FLL_MCLK;
-		priv->codec_priv.fll_id = WM8904_CLK_FLL;
-		priv->codec_priv.pll_id = WM8904_FLL_MCLK;
+		priv->codec_priv[0].mclk_id = WM8904_FLL_MCLK;
+		priv->codec_priv[0].fll_id = WM8904_CLK_FLL;
+		priv->codec_priv[0].pll_id = WM8904_FLL_MCLK;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
@@ -728,18 +754,30 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	 * Allow setting mclk-id from the device-tree node. Otherwise, the
 	 * default value for each card configuration is used.
 	 */
-	of_property_read_u32(np, "mclk-id", &priv->codec_priv.mclk_id);
+	for (int i = 0; i < 2; i++) {
+		of_property_read_u32_index(np, "mclk-id", i,
+					&priv->codec_priv[i].mclk_id);
+	}
 
 	/* Format info from DT is optional. */
 	snd_soc_daifmt_parse_clock_provider_as_phandle(np, NULL, &bitclkprovider, &frameprovider);
 	if (bitclkprovider || frameprovider) {
 		unsigned int daifmt = snd_soc_daifmt_parse_format(np, NULL);
+		bool codec_bitclkprovider = false;
+		bool codec_frameprovider = false;
+
+		for (int i = 0; i < 2; i++) {
+			if (bitclkprovider && codec_np[i] == bitclkprovider)
+				codec_bitclkprovider = true;
+			if (frameprovider && codec_np[i] == frameprovider)
+				codec_frameprovider = true;
+		}
 
-		if (codec_np == bitclkprovider)
-			daifmt |= (codec_np == frameprovider) ?
+		if (codec_bitclkprovider)
+			daifmt |= (codec_frameprovider) ?
 				SND_SOC_DAIFMT_CBP_CFP : SND_SOC_DAIFMT_CBP_CFC;
 		else
-			daifmt |= (codec_np == frameprovider) ?
+			daifmt |= (codec_frameprovider) ?
 				SND_SOC_DAIFMT_CBC_CFP : SND_SOC_DAIFMT_CBC_CFC;
 
 		/* Override dai_fmt with value from DT */
@@ -755,7 +793,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	of_node_put(bitclkprovider);
 	of_node_put(frameprovider);
 
-	if (!fsl_asoc_card_is_ac97(priv) && !codec_dev) {
+	if (!fsl_asoc_card_is_ac97(priv) && !codec_dev[0]) {
 		dev_dbg(&pdev->dev, "failed to find codec device\n");
 		ret = -EPROBE_DEFER;
 		goto asrc_fail;
@@ -795,7 +833,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	ret = snd_soc_of_parse_card_name(&priv->card, "model");
 	if (ret) {
 		snprintf(priv->name, sizeof(priv->name), "%s-audio",
-			 fsl_asoc_card_is_ac97(priv) ? "ac97" : codec_dev_name);
+			 fsl_asoc_card_is_ac97(priv) ? "ac97" : codec_dev_name[0]);
 		priv->card.name = priv->name;
 	}
 	priv->card.dai_link = priv->dai_link;
@@ -820,7 +858,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	priv->dai_link[0].codecs[0].dai_name = codec_dai_name;
 
 	if (!fsl_asoc_card_is_ac97(priv))
-		priv->dai_link[0].codecs[0].of_node = codec_np;
+		priv->dai_link[0].codecs[0].of_node = codec_np[0];
 	else {
 		u32 idx;
 
@@ -928,7 +966,8 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 
 asrc_fail:
 	of_node_put(asrc_np);
-	of_node_put(codec_np);
+	of_node_put(codec_np[0]);
+	of_node_put(codec_np[1]);
 	put_device(&cpu_pdev->dev);
 fail:
 	of_node_put(cpu_np);
-- 
2.34.1

[PATCHv4 4/9] ASoC: fsl-asoc-card: add new compatible for a generic codec use case

[Elinor Montmasson]

Add the new compatible "fsl,imx-audio-generic" for a generic codec
use case. It allows using the fsl-asoc-card driver with the
spdif_receiver and spdif_transmitter codec drivers used as dummy codecs.
It can be used for cases where there is no real codec or codecs which do
not require declaring controls.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
Co-authored-by: Philip-Dylan Gleonec <philip-dylan.gleonec@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 620a25eb068a..a4ecc9093558 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -567,6 +567,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	struct platform_device *cpu_pdev;
 	struct fsl_asoc_card_priv *priv;
 	struct device *codec_dev[2] = { NULL, NULL };
+	const char *generic_codec_dai_names[2];
 	const char *codec_dai_name;
 	const char *codec_dev_name[2];
 	u32 asrc_fmt = 0;
@@ -744,6 +745,11 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		priv->codec_priv[0].fll_id = WM8904_CLK_FLL;
 		priv->codec_priv[0].pll_id = WM8904_FLL_MCLK;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
+	} else if (of_device_is_compatible(np, "fsl,imx-audio-generic")) {
+		generic_codec_dai_names[0] = "dit-hifi";
+		generic_codec_dai_names[1] = "dir-hifi";
+		priv->dai_link[0].num_codecs = 2;
+		priv->dai_link[2].num_codecs = 2;
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
 		ret = -EINVAL;
@@ -798,6 +804,12 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		ret = -EPROBE_DEFER;
 		goto asrc_fail;
 	}
+	if (of_device_is_compatible(np, "fsl,imx-audio-generic")
+	  && !codec_dev[1]) {
+		dev_dbg(&pdev->dev, "failed to find second codec device\n");
+		ret = -EPROBE_DEFER;
+		goto asrc_fail;
+	}
 
 	/* Common settings for corresponding Freescale CPU DAI driver */
 	if (of_node_name_eq(cpu_np, "ssi")) {
@@ -855,11 +867,21 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 
 	/* Normal DAI Link */
 	priv->dai_link[0].cpus->of_node = cpu_np;
-	priv->dai_link[0].codecs[0].dai_name = codec_dai_name;
 
-	if (!fsl_asoc_card_is_ac97(priv))
+	if (of_device_is_compatible(np, "fsl,imx-audio-generic")) {
+		priv->dai_link[0].codecs[0].dai_name =
+			generic_codec_dai_names[0];
+		priv->dai_link[0].codecs[1].dai_name =
+			generic_codec_dai_names[1];
+	} else {
+		priv->dai_link[0].codecs[0].dai_name = codec_dai_name;
+	}
+
+	if (!fsl_asoc_card_is_ac97(priv)) {
 		priv->dai_link[0].codecs[0].of_node = codec_np[0];
-	else {
+		if (of_device_is_compatible(np, "fsl,imx-audio-generic"))
+			priv->dai_link[0].codecs[1].of_node = codec_np[1];
+	} else {
 		u32 idx;
 
 		ret = of_property_read_u32(cpu_np, "cell-index", &idx);
@@ -990,6 +1012,7 @@ static const struct of_device_id fsl_asoc_card_dt_ids[] = {
 	{ .compatible = "fsl,imx-audio-wm8958", },
 	{ .compatible = "fsl,imx-audio-nau8822", },
 	{ .compatible = "fsl,imx-audio-wm8904", },
+	{ .compatible = "fsl,imx-audio-generic", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, fsl_asoc_card_dt_ids);
-- 
2.34.1

[PATCHv4 5/9] ASoC: fsl-asoc-card: set generic codec as clock provider

[Elinor Montmasson]

The default dai format defined by DAI_FMT_BASE doesn't set if the codec
is consumer or provider of the bit and frame clocks.

S/PDIF DIR usually converts audio signal to an asynchronous I2S/PCM
stream, and doesn't consume a bit or frame clock.

As S/PDIF DIR and DIT are used as codecs for the generic use case,
set codecs as provider of both bit and frame clocks by default.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
Co-authored-by: Philip-Dylan Gleonec <philip-dylan.gleonec@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index a4ecc9093558..82ed7f4e81a1 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -750,6 +750,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		generic_codec_dai_names[1] = "dir-hifi";
 		priv->dai_link[0].num_codecs = 2;
 		priv->dai_link[2].num_codecs = 2;
+		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
 		ret = -EINVAL;
-- 
2.34.1

[PATCHv4 6/9] ASoC: fsl-asoc-card: add use of devicetree TDM slot properties

[Elinor Montmasson]

Add use of optional TDM slot properties "dai-tdm-slot-num" and
"dai-tdm-slot-width" through snd_soc_of_parse_tdm_slot().
They allow setting a custom TDM slot width in bits and number of slots
for the CPU DAI when using the generic codec.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 82ed7f4e81a1..9aca8ad15372 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -751,6 +751,9 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		priv->dai_link[0].num_codecs = 2;
 		priv->dai_link[2].num_codecs = 2;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
+		snd_soc_of_parse_tdm_slot(np, NULL, NULL,
+						&priv->cpu_priv.slot_num,
+						&priv->cpu_priv.slot_width);
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
 		ret = -EINVAL;
-- 
2.34.1

[PATCHv4 7/9] ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec

[Elinor Montmasson]

Add an optional DT clock "cpu_sysclk" to get the CPU DAI system-clock
frequency when using the generic codec.
It is set for both Tx and Rx.
The way the frequency value is used is up to the CPU DAI driver
implementation.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 9aca8ad15372..c7fc9c16f761 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -754,6 +754,12 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		snd_soc_of_parse_tdm_slot(np, NULL, NULL,
 						&priv->cpu_priv.slot_num,
 						&priv->cpu_priv.slot_width);
+		struct clk *cpu_sysclk = clk_get(&pdev->dev, "cpu_sysclk");
+		if (!IS_ERR(cpu_sysclk)) {
+			priv->cpu_priv.sysclk_freq[TX] = clk_get_rate(cpu_sysclk);
+			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
+			clk_put(cpu_sysclk);
+		}
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
 		ret = -EINVAL;
-- 
2.34.1

[PATCHv4 8/9] ASoC: fsl-asoc-card: add DT property "cpu-system-clock-direction-out"

[Elinor Montmasson]

Add new optional DT property "cpu-system-clock-direction-out" to set
sysclk direction as "out" for the CPU DAI when using the generic codec.
It is set for both Tx and Rx.
If not set, the direction is "in".
The way the direction value is used is up to the CPU DAI driver
implementation.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index c7fc9c16f761..f3fc2b29c92f 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -760,6 +760,10 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
 			clk_put(cpu_sysclk);
 		}
+		priv->cpu_priv.sysclk_dir[TX] =
+			of_property_read_bool(np, "cpu-system-clock-direction-out") ?
+			SND_SOC_CLOCK_OUT : SND_SOC_CLOCK_IN;
+		priv->cpu_priv.sysclk_dir[RX] = priv->cpu_priv.sysclk_dir[TX];
 	} else {
 		dev_err(&pdev->dev, "unknown Device Tree compatible\n");
 		ret = -EINVAL;
-- 
2.34.1

[PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Elinor Montmasson]

Add documentation about new dts bindings following new support
for compatible "fsl,imx-audio-generic".

Some CPU DAI don't require a real audio codec. The new compatible
"fsl,imx-audio-generic" allows using the driver with codec drivers
SPDIF DIT and SPDIF DIR as dummy codecs.
It also allows using not pre-configured audio codecs which
don't require specific control through a codec driver.

The new dts properties give the possibility to set some parameters
about the CPU DAI usually set through the codec configuration.

Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
---
 .../bindings/sound/fsl-asoc-card.yaml         | 96 ++++++++++++++++++-
 1 file changed, 92 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
index 9922664d5ccc..332d8bf96e06 100644
--- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
@@ -23,6 +23,16 @@ description:
   and PCM DAI formats. However, it'll be also possible to support those non
   AC'97/I2S/PCM type sound cards, such as S/PDIF audio and HDMI audio, as
   long as the driver has been properly upgraded.
+  To use CPU DAIs that do not require a codec such as an S/PDIF controller,
+  or to use a DAI to output or capture raw I2S/TDM data, you can
+  use the compatible "fsl,imx-audio-generic".
+
+definitions:
+  imx-audio-generic-dependency:
+    properties:
+      compatible:
+        contains:
+          const: fsl,imx-audio-generic
 
 maintainers:
   - Shengjiu Wang <shengjiu.wang@nxp.com>
@@ -81,6 +91,7 @@ properties:
               - fsl,imx-audio-wm8960
               - fsl,imx-audio-wm8962
               - fsl,imx-audio-wm8958
+              - fsl,imx-audio-generic
 
   model:
     $ref: /schemas/types.yaml#/definitions/string
@@ -93,8 +104,14 @@ properties:
       need to add ASRC support via DPCM.
 
   audio-codec:
-    $ref: /schemas/types.yaml#/definitions/phandle
-    description: The phandle of an audio codec
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      The phandle of an audio codec.
+      If using the "fsl,imx-audio-generic" compatible, give instead a pair of
+      phandles with the spdif_transmitter first (driver SPDIF DIT) and the
+      spdif_receiver second (driver SPDIF DIR).
+    items:
+      maxItems: 1
 
   audio-cpu:
     $ref: /schemas/types.yaml#/definitions/phandle
@@ -150,8 +167,8 @@ properties:
     description: dai-link uses bit clock inversion.
 
   mclk-id:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: main clock id, specific for each card configuration.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: Main clock id for each codec, specific for each card configuration.
 
   mux-int-port:
     $ref: /schemas/types.yaml#/definitions/uint32
@@ -167,10 +184,68 @@ properties:
     $ref: /schemas/types.yaml#/definitions/phandle
     description: The phandle of an CPU DAI controller
 
+  # Properties relevant only with "fsl,imx-audio-generic" compatible
+  dai-tdm-slot-width:
+    description: See tdm-slot.txt.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  dai-tdm-slot-num:
+    description: See tdm-slot.txt.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  clocks:
+    description: |
+      The CPU DAI system clock, used to retrieve
+      the CPU DAI system clock frequency with the generic codec.
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: cpu_sysclk
+
+  cpu-system-clock-direction-out:
+    description: |
+      Specifies cpu system clock direction as 'out' on initialization.
+      If not set, direction is 'in'.
+    $ref: /schemas/types.yaml#/definitions/flag
+
+dependencies:
+  dai-tdm-slot-width:
+    $ref: "#/definitions/imx-audio-generic-dependency"
+  dai-tdm-slot-num:
+    $ref: "#/definitions/imx-audio-generic-dependency"
+  clocks:
+    $ref: "#/definitions/imx-audio-generic-dependency"
+  cpu-system-clock-direction-out:
+    $ref: "#/definitions/imx-audio-generic-dependency"
+
 required:
   - compatible
   - model
 
+allOf:
+  - if:
+      $ref: "#/definitions/imx-audio-generic-dependency"
+    then:
+      properties:
+        audio-codec:
+          items:
+            - description: SPDIF DIT phandle
+            - description: SPDIF DIR phandle
+        mclk-id:
+          maxItems: 1
+          items:
+            minItems: 1
+            maxItems: 2
+    else:
+      properties:
+        audio-codec:
+          maxItems: 1
+        mclk-id:
+          maxItems: 1
+          items:
+            maxItems: 1
+
 unevaluatedProperties: false
 
 examples:
@@ -195,3 +270,16 @@ examples:
              "AIN2L", "Line In Jack",
              "AIN2R", "Line In Jack";
     };
+
+  - |
+    #include <dt-bindings/clock/imx8mn-clock.h>
+    sound-spdif-asrc {
+      compatible = "fsl,imx-audio-generic";
+      model = "spdif-asrc-audio";
+      audio-cpu = <&spdif>;
+      audio-asrc = <&easrc>;
+      audio-codec = <&spdifdit>, <&spdifdir>;
+      clocks = <&clk IMX8MN_CLK_SAI5_ROOT>;
+      clock-names = "cpu_sysclk";
+      cpu-system-clock-direction-out;
+    };
-- 
2.34.1

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

On Wed, May 15, 2024 at 03:54:11PM +0200, Elinor Montmasson wrote:

> Add documentation about new dts bindings following new support
> for compatible "fsl,imx-audio-generic".

>    audio-codec:
> -    $ref: /schemas/types.yaml#/definitions/phandle
> -    description: The phandle of an audio codec
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: |
> +      The phandle of an audio codec.
> +      If using the "fsl,imx-audio-generic" compatible, give instead a pair of
> +      phandles with the spdif_transmitter first (driver SPDIF DIT) and the
> +      spdif_receiver second (driver SPDIF DIR).
> +    items:
> +      maxItems: 1

This description (and the code) don't feel like they're actually generic
- they're clearly specific to the bidrectional S/PDIF case.  I'd expect
something called -generic to cope with single CODECs as well as double,
and not to have any constraints on what those are.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 7/9] ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

On Wed, May 15, 2024 at 03:54:09PM +0200, Elinor Montmasson wrote:

> Add an optional DT clock "cpu_sysclk" to get the CPU DAI system-clock
> frequency when using the generic codec.
> It is set for both Tx and Rx.
> The way the frequency value is used is up to the CPU DAI driver
> implementation.

> +		struct clk *cpu_sysclk = clk_get(&pdev->dev, "cpu_sysclk");
> +		if (!IS_ERR(cpu_sysclk)) {
> +			priv->cpu_priv.sysclk_freq[TX] = clk_get_rate(cpu_sysclk);
> +			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
> +			clk_put(cpu_sysclk);
> +		}

I don't really understand the goal here - this is just reading whatever
frequency happens to be set in the hardware when the driver starts up
which if nothing else seems rather fragile?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 8/9] ASoC: fsl-asoc-card: add DT property "cpu-system-clock-direction-out"

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

On Wed, May 15, 2024 at 03:54:10PM +0200, Elinor Montmasson wrote:
> Add new optional DT property "cpu-system-clock-direction-out" to set
> sysclk direction as "out" for the CPU DAI when using the generic codec.
> It is set for both Tx and Rx.
> If not set, the direction is "in".
> The way the direction value is used is up to the CPU DAI driver
> implementation.

This feels like we should be using the clock bindings to specify the
clock input of whatever is using the output from the SoC, though that's
a lot more work.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 7/9] ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec

[Elinor Montmasson]

From: "Mark Brown" <broonie@kernel.org>
Sent: Thursday, 16 May, 2024 14:13:19
> On Wed, May 15, 2024 at 03:54:09PM +0200, Elinor Montmasson wrote:
> 
>> Add an optional DT clock "cpu_sysclk" to get the CPU DAI system-clock
>> frequency when using the generic codec.
>> It is set for both Tx and Rx.
>> The way the frequency value is used is up to the CPU DAI driver
>> implementation.
> 
>> +		struct clk *cpu_sysclk = clk_get(&pdev->dev, "cpu_sysclk");
>> +		if (!IS_ERR(cpu_sysclk)) {
>> +			priv->cpu_priv.sysclk_freq[TX] = clk_get_rate(cpu_sysclk);
>> +			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
>> +			clk_put(cpu_sysclk);
>> +		}
> 
> I don't really understand the goal here - this is just reading whatever
> frequency happens to be set in the hardware when the driver starts up
> which if nothing else seems rather fragile?

The driver allow to set the sysclk frequency
of the CPU DAI through `priv->cpu_priv.sysclk_freq` when calling
`fsl_asoc_card_hw_params()`.
Currently it is hard-coded per use-case in the driver.

My reasoning was that with a generic codec/compatible, there might
be use-cases needing to use this parameter, so I exposed it here via DT.

Is it a bad idea to expose this parameter ? This is not a requirement for the
driver to work, most of the current compatibles do not use this parameter.
It is currently used only for `fsl,imx-audio-cs42888`.
In that case I can remove this commit.

Best regards,
Elinor Montmasson

Re: [PATCHv4 8/9] ASoC: fsl-asoc-card: add DT property "cpu-system-clock-direction-out"

[Elinor Montmasson]

From: "Mark Brown" <broonie@kernel.org>
Sent: Thursday, 16 May, 2024 14:18:00
> On Wed, May 15, 2024 at 03:54:10PM +0200, Elinor Montmasson wrote:
>> Add new optional DT property "cpu-system-clock-direction-out" to set
>> sysclk direction as "out" for the CPU DAI when using the generic codec.
>> It is set for both Tx and Rx.
>> If not set, the direction is "in".
>> The way the direction value is used is up to the CPU DAI driver
>> implementation.
> 
> This feels like we should be using the clock bindings to specify the
> clock input of whatever is using the output from the SoC, though that's
> a lot more work.

Similarly to patch 7/9, I exposed this parameter because the driver has it, and
because there might be CPU DAIs needing this parameter.
Otherwise the cpu sysclk direction will always be IN as a default.
This parameter could be needed for cases with CPU DAIs, such as an SAI,
which can provide or consume Tx/Rx clocks.
For these devices I know the sysclk direction should correspond to
what was set in the dai format.

This new compatible is intended to be used when there is no codec
device/driver. There is technically no codec device/driver for which
the clock input can be set.

Is it a bad idea to allow setting the cpu sysclk direction only ?
Should the compatible be limited to use-cases where the cpu sysclk
direction cannot be set by the machine driver ?

Best regards,
Elinor Montmasson

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Elinor Montmasson]

From: "Mark Brown" <broonie@kernel.org>
Sent: Thursday, 16 May, 2024 14:11:11
> On Wed, May 15, 2024 at 03:54:11PM +0200, Elinor Montmasson wrote:
> 
>> Add documentation about new dts bindings following new support
>> for compatible "fsl,imx-audio-generic".
> 
>>    audio-codec:
>> -    $ref: /schemas/types.yaml#/definitions/phandle
>> -    description: The phandle of an audio codec
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description: |
>> +      The phandle of an audio codec.
>> +      If using the "fsl,imx-audio-generic" compatible, give instead a pair of
>> +      phandles with the spdif_transmitter first (driver SPDIF DIT) and the
>> +      spdif_receiver second (driver SPDIF DIR).
>> +    items:
>> +      maxItems: 1
> 
> This description (and the code) don't feel like they're actually generic
> - they're clearly specific to the bidrectional S/PDIF case.  I'd expect
> something called -generic to cope with single CODECs as well as double,
> and not to have any constraints on what those are.

I proposed, in an reply of the v3 patch series to Krzysztof Kozlowski,
the compatible "fsl,imx-audio-no-codec" instead of "generic".
Krzysztof thought it was too generic, but it would convey more clearly
that it is for cases without codec driver.
Would this other compatible string be more appropriate ?

Best regards,
Elinor Montmasson

Re: [PATCHv4 8/9] ASoC: fsl-asoc-card: add DT property "cpu-system-clock-direction-out"

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Fri, May 17, 2024 at 05:05:38AM -0400, Elinor Montmasson wrote:

> This new compatible is intended to be used when there is no codec
> device/driver. There is technically no codec device/driver for which
> the clock input can be set.

This is obviously not true, there clearly is a driver.

> Is it a bad idea to allow setting the cpu sysclk direction only ?
> Should the compatible be limited to use-cases where the cpu sysclk
> direction cannot be set by the machine driver ?

When I said "this should use the clock bindings" I meant that we should
use the clock bindings for configuration here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 899 bytes --]

On Fri, May 17, 2024 at 05:05:41AM -0400, Elinor Montmasson wrote:
> From: "Mark Brown" <broonie@kernel.org>

> > This description (and the code) don't feel like they're actually generic
> > - they're clearly specific to the bidrectional S/PDIF case.  I'd expect
> > something called -generic to cope with single CODECs as well as double,
> > and not to have any constraints on what those are.

> I proposed, in an reply of the v3 patch series to Krzysztof Kozlowski,
> the compatible "fsl,imx-audio-no-codec" instead of "generic".
> Krzysztof thought it was too generic, but it would convey more clearly
> that it is for cases without codec driver.
> Would this other compatible string be more appropriate ?

No.  There is very clearly a CODEC here, it physically exists, we can
point at it on the board and it has a software representation.  Your
code is also very specific to the two CODEC case.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 7/9] ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

On Fri, May 17, 2024 at 05:05:35AM -0400, Elinor Montmasson wrote:
> From: "Mark Brown" <broonie@kernel.org>
> > On Wed, May 15, 2024 at 03:54:09PM +0200, Elinor Montmasson wrote:

> >> +		struct clk *cpu_sysclk = clk_get(&pdev->dev, "cpu_sysclk");
> >> +		if (!IS_ERR(cpu_sysclk)) {
> >> +			priv->cpu_priv.sysclk_freq[TX] = clk_get_rate(cpu_sysclk);
> >> +			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
> >> +			clk_put(cpu_sysclk);
> >> +		}

> > I don't really understand the goal here - this is just reading whatever
> > frequency happens to be set in the hardware when the driver starts up
> > which if nothing else seems rather fragile?

> The driver allow to set the sysclk frequency
> of the CPU DAI through `priv->cpu_priv.sysclk_freq` when calling
> `fsl_asoc_card_hw_params()`.
> Currently it is hard-coded per use-case in the driver.

> My reasoning was that with a generic codec/compatible, there might
> be use-cases needing to use this parameter, so I exposed it here via DT.

> Is it a bad idea to expose this parameter ? This is not a requirement for the
> driver to work, most of the current compatibles do not use this parameter.
> It is currently used only for `fsl,imx-audio-cs42888`.
> In that case I can remove this commit.

I'm having a hard time connecting your reply here with my comment.  This
isn't as far as I can see allowing the frequency to be explicitly
configured, it's just using whatever value happens to be programmed in
the clock when the driver starts.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Rob Herring]

On Wed, May 15, 2024 at 03:54:11PM +0200, Elinor Montmasson wrote:
> Add documentation about new dts bindings following new support
> for compatible "fsl,imx-audio-generic".
> 
> Some CPU DAI don't require a real audio codec. The new compatible
> "fsl,imx-audio-generic" allows using the driver with codec drivers
> SPDIF DIT and SPDIF DIR as dummy codecs.
> It also allows using not pre-configured audio codecs which
> don't require specific control through a codec driver.
> 
> The new dts properties give the possibility to set some parameters
> about the CPU DAI usually set through the codec configuration.
> 
> Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
> ---
>  .../bindings/sound/fsl-asoc-card.yaml         | 96 ++++++++++++++++++-
>  1 file changed, 92 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
> index 9922664d5ccc..332d8bf96e06 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
> @@ -23,6 +23,16 @@ description:
>    and PCM DAI formats. However, it'll be also possible to support those non
>    AC'97/I2S/PCM type sound cards, such as S/PDIF audio and HDMI audio, as
>    long as the driver has been properly upgraded.
> +  To use CPU DAIs that do not require a codec such as an S/PDIF controller,
> +  or to use a DAI to output or capture raw I2S/TDM data, you can
> +  use the compatible "fsl,imx-audio-generic".
> +
> +definitions:
> +  imx-audio-generic-dependency:
> +    properties:
> +      compatible:
> +        contains:
> +          const: fsl,imx-audio-generic
>  
>  maintainers:
>    - Shengjiu Wang <shengjiu.wang@nxp.com>
> @@ -81,6 +91,7 @@ properties:
>                - fsl,imx-audio-wm8960
>                - fsl,imx-audio-wm8962
>                - fsl,imx-audio-wm8958
> +              - fsl,imx-audio-generic
>  
>    model:
>      $ref: /schemas/types.yaml#/definitions/string
> @@ -93,8 +104,14 @@ properties:
>        need to add ASRC support via DPCM.
>  
>    audio-codec:
> -    $ref: /schemas/types.yaml#/definitions/phandle
> -    description: The phandle of an audio codec
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: |
> +      The phandle of an audio codec.
> +      If using the "fsl,imx-audio-generic" compatible, give instead a pair of
> +      phandles with the spdif_transmitter first (driver SPDIF DIT) and the
> +      spdif_receiver second (driver SPDIF DIR).

       minItems: 1
       maxItems: 2

> +    items:
> +      maxItems: 1
>  
>    audio-cpu:
>      $ref: /schemas/types.yaml#/definitions/phandle
> @@ -150,8 +167,8 @@ properties:
>      description: dai-link uses bit clock inversion.
>  
>    mclk-id:
> -    $ref: /schemas/types.yaml#/definitions/uint32
> -    description: main clock id, specific for each card configuration.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: Main clock id for each codec, specific for each card configuration.

       minItems: 1
       maxItems: 2
>  
>    mux-int-port:
>      $ref: /schemas/types.yaml#/definitions/uint32
> @@ -167,10 +184,68 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description: The phandle of an CPU DAI controller
>  
> +  # Properties relevant only with "fsl,imx-audio-generic" compatible
> +  dai-tdm-slot-width:
> +    description: See tdm-slot.txt.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  dai-tdm-slot-num:
> +    description: See tdm-slot.txt.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  clocks:
> +    description: |
> +      The CPU DAI system clock, used to retrieve
> +      the CPU DAI system clock frequency with the generic codec.
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: cpu_sysclk
> +
> +  cpu-system-clock-direction-out:
> +    description: |
> +      Specifies cpu system clock direction as 'out' on initialization.
> +      If not set, direction is 'in'.
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +dependencies:
> +  dai-tdm-slot-width:
> +    $ref: "#/definitions/imx-audio-generic-dependency"
> +  dai-tdm-slot-num:
> +    $ref: "#/definitions/imx-audio-generic-dependency"
> +  clocks:
> +    $ref: "#/definitions/imx-audio-generic-dependency"
> +  cpu-system-clock-direction-out:
> +    $ref: "#/definitions/imx-audio-generic-dependency"

This works, but is an unusual pattern...

> +
>  required:
>    - compatible
>    - model
>  
> +allOf:
> +  - if:
> +      $ref: "#/definitions/imx-audio-generic-dependency"
> +    then:
> +      properties:
> +        audio-codec:
> +          items:
> +            - description: SPDIF DIT phandle
> +            - description: SPDIF DIR phandle
> +        mclk-id:
> +          maxItems: 1
> +          items:
> +            minItems: 1
> +            maxItems: 2
> +    else:
> +      properties:
> +        audio-codec:
> +          maxItems: 1
> +        mclk-id:
> +          maxItems: 1
> +          items:
> +            maxItems: 1


You can handle the dependency like this instead:

           dai-tdm-slot-width: false
           dai-tdm-slot-num: false


And then you don't need the definitions.

> +
>  unevaluatedProperties: false
>  
>  examples:
> @@ -195,3 +270,16 @@ examples:
>               "AIN2L", "Line In Jack",
>               "AIN2R", "Line In Jack";
>      };
> +
> +  - |
> +    #include <dt-bindings/clock/imx8mn-clock.h>
> +    sound-spdif-asrc {
> +      compatible = "fsl,imx-audio-generic";
> +      model = "spdif-asrc-audio";
> +      audio-cpu = <&spdif>;
> +      audio-asrc = <&easrc>;
> +      audio-codec = <&spdifdit>, <&spdifdir>;
> +      clocks = <&clk IMX8MN_CLK_SAI5_ROOT>;
> +      clock-names = "cpu_sysclk";
> +      cpu-system-clock-direction-out;
> +    };
> -- 
> 2.34.1
> 

Re: [PATCHv4 7/9] ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec

[Elinor Montmasson]

From: "Mark Brown" <broonie@kernel.org>
Sent: Friday, 17 May, 2024 13:17:20
> On Fri, May 17, 2024 at 05:05:35AM -0400, Elinor Montmasson wrote:
>> From: "Mark Brown" <broonie@kernel.org>
>> > On Wed, May 15, 2024 at 03:54:09PM +0200, Elinor Montmasson wrote:
> 
>> >> +		struct clk *cpu_sysclk = clk_get(&pdev->dev, "cpu_sysclk");
>> >> +		if (!IS_ERR(cpu_sysclk)) {
>> >> +			priv->cpu_priv.sysclk_freq[TX] = clk_get_rate(cpu_sysclk);
>> >> +			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
>> >> +			clk_put(cpu_sysclk);
>> >> +		}
> 
>> > I don't really understand the goal here - this is just reading whatever
>> > frequency happens to be set in the hardware when the driver starts up
>> > which if nothing else seems rather fragile?
> 
>> The driver allow to set the sysclk frequency
>> of the CPU DAI through `priv->cpu_priv.sysclk_freq` when calling
>> `fsl_asoc_card_hw_params()`.
>> Currently it is hard-coded per use-case in the driver.
> 
>> My reasoning was that with a generic codec/compatible, there might
>> be use-cases needing to use this parameter, so I exposed it here via DT.
> 
>> Is it a bad idea to expose this parameter ? This is not a requirement for the
>> driver to work, most of the current compatibles do not use this parameter.
>> It is currently used only for `fsl,imx-audio-cs42888`.
>> In that case I can remove this commit.
> 
> I'm having a hard time connecting your reply here with my comment.  This
> isn't as far as I can see allowing the frequency to be explicitly
> configured, it's just using whatever value happens to be programmed in
> the clock when the driver starts.

In v3 I used parameters `cpu-sysclk-freq-rx/tx` to explicitly
set the frequency.
In its review Rob Herring said that the clock bindings should
be used, so that's why I changed it to use this `cpu_sysclk` clock.

Re: [PATCHv4 8/9] ASoC: fsl-asoc-card: add DT property "cpu-system-clock-direction-out"

[Elinor Montmasson]

From: "Mark Brown" <broonie@kernel.org>
Sent: Friday, 17 May, 2024 13:06:03
> On Fri, May 17, 2024 at 05:05:38AM -0400, Elinor Montmasson wrote:
> 
>> This new compatible is intended to be used when there is no codec
>> device/driver. There is technically no codec device/driver for which
>> the clock input can be set.
> 
> This is obviously not true, there clearly is a driver.
> 
>> Is it a bad idea to allow setting the cpu sysclk direction only ?
>> Should the compatible be limited to use-cases where the cpu sysclk
>> direction cannot be set by the machine driver ?
> 
> When I said "this should use the clock bindings" I meant that we should
> use the clock bindings for configuration here.

As far I as know, it's not possible to set the direction with
the clock bindings, but maybe there is and I missed something ?

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Elinor Montmasson]

From: "Mark Brown" <broonie@kernel.org>
Sent: Friday, 17 May, 2024 13:11:43
> On Fri, May 17, 2024 at 05:05:41AM -0400, Elinor Montmasson wrote:
>> From: "Mark Brown" <broonie@kernel.org>
> 
>> > This description (and the code) don't feel like they're actually generic
>> > - they're clearly specific to the bidrectional S/PDIF case.  I'd expect
>> > something called -generic to cope with single CODECs as well as double,
>> > and not to have any constraints on what those are.
> 
>> I proposed, in an reply of the v3 patch series to Krzysztof Kozlowski,
>> the compatible "fsl,imx-audio-no-codec" instead of "generic".
>> Krzysztof thought it was too generic, but it would convey more clearly
>> that it is for cases without codec driver.
>> Would this other compatible string be more appropriate ?
> 
> No.  There is very clearly a CODEC here, it physically exists, we can
> point at it on the board and it has a software representation.  Your
> code is also very specific to the two CODEC case.

Then maybe it's not be a good idea to make this compatible generic
for this contribution.
The original intention is to bring support for the S/PDIF,
so maybe the contribution should focus on this use case?
In that case, would changing the compatible for "fsl,imx-audio-spdif-card"
be acceptable?
"fsl,imx-audio-spdif" is already used for the `imx-spdif.c`
which does not use the ASRC.

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Elinor Montmasson]

From: "Rob Herring" <robh@kernel.org>
Sent: Monday, 20 May, 2024 20:31:48
> On Wed, May 15, 2024 at 03:54:11PM +0200, Elinor Montmasson wrote:
>> Add documentation about new dts bindings following new support
>> for compatible "fsl,imx-audio-generic".
>> 
>> Some CPU DAI don't require a real audio codec. The new compatible
>> "fsl,imx-audio-generic" allows using the driver with codec drivers
>> SPDIF DIT and SPDIF DIR as dummy codecs.
>> It also allows using not pre-configured audio codecs which
>> don't require specific control through a codec driver.
>> 
>> The new dts properties give the possibility to set some parameters
>> about the CPU DAI usually set through the codec configuration.
>> 
>> Signed-off-by: Elinor Montmasson <elinor.montmasson@savoirfairelinux.com>
>> ---
>>  .../bindings/sound/fsl-asoc-card.yaml         | 96 ++++++++++++++++++-
>>  1 file changed, 92 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>> b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>> index 9922664d5ccc..332d8bf96e06 100644
>> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>> @@ -23,6 +23,16 @@ description:
>>    and PCM DAI formats. However, it'll be also possible to support those non
>>    AC'97/I2S/PCM type sound cards, such as S/PDIF audio and HDMI audio, as
>>    long as the driver has been properly upgraded.
>> +  To use CPU DAIs that do not require a codec such as an S/PDIF controller,
>> +  or to use a DAI to output or capture raw I2S/TDM data, you can
>> +  use the compatible "fsl,imx-audio-generic".
>> +
>> +definitions:
>> +  imx-audio-generic-dependency:
>> +    properties:
>> +      compatible:
>> +        contains:
>> +          const: fsl,imx-audio-generic
>>  
>>  maintainers:
>>    - Shengjiu Wang <shengjiu.wang@nxp.com>
>> @@ -81,6 +91,7 @@ properties:
>>                - fsl,imx-audio-wm8960
>>                - fsl,imx-audio-wm8962
>>                - fsl,imx-audio-wm8958
>> +              - fsl,imx-audio-generic
>>  
>>    model:
>>      $ref: /schemas/types.yaml#/definitions/string
>> @@ -93,8 +104,14 @@ properties:
>>        need to add ASRC support via DPCM.
>>  
>>    audio-codec:
>> -    $ref: /schemas/types.yaml#/definitions/phandle
>> -    description: The phandle of an audio codec
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description: |
>> +      The phandle of an audio codec.
>> +      If using the "fsl,imx-audio-generic" compatible, give instead a pair of
>> +      phandles with the spdif_transmitter first (driver SPDIF DIT) and the
>> +      spdif_receiver second (driver SPDIF DIR).
> 
>       minItems: 1
>       maxItems: 2
> 
>> +    items:
>> +      maxItems: 1
>>  
>>    audio-cpu:
>>      $ref: /schemas/types.yaml#/definitions/phandle
>> @@ -150,8 +167,8 @@ properties:
>>      description: dai-link uses bit clock inversion.
>>  
>>    mclk-id:
>> -    $ref: /schemas/types.yaml#/definitions/uint32
>> -    description: main clock id, specific for each card configuration.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description: Main clock id for each codec, specific for each card
>> configuration.
> 
>       minItems: 1
>       maxItems: 2
>>  
>>    mux-int-port:
>>      $ref: /schemas/types.yaml#/definitions/uint32
>> @@ -167,10 +184,68 @@ properties:
>>      $ref: /schemas/types.yaml#/definitions/phandle
>>      description: The phandle of an CPU DAI controller
>>  
>> +  # Properties relevant only with "fsl,imx-audio-generic" compatible
>> +  dai-tdm-slot-width:
>> +    description: See tdm-slot.txt.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +  dai-tdm-slot-num:
>> +    description: See tdm-slot.txt.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +  clocks:
>> +    description: |
>> +      The CPU DAI system clock, used to retrieve
>> +      the CPU DAI system clock frequency with the generic codec.
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    items:
>> +      - const: cpu_sysclk
>> +
>> +  cpu-system-clock-direction-out:
>> +    description: |
>> +      Specifies cpu system clock direction as 'out' on initialization.
>> +      If not set, direction is 'in'.
>> +    $ref: /schemas/types.yaml#/definitions/flag
>> +
>> +dependencies:
>> +  dai-tdm-slot-width:
>> +    $ref: "#/definitions/imx-audio-generic-dependency"
>> +  dai-tdm-slot-num:
>> +    $ref: "#/definitions/imx-audio-generic-dependency"
>> +  clocks:
>> +    $ref: "#/definitions/imx-audio-generic-dependency"
>> +  cpu-system-clock-direction-out:
>> +    $ref: "#/definitions/imx-audio-generic-dependency"
> 
> This works, but is an unusual pattern...
> 
>> +
>>  required:
>>    - compatible
>>    - model
>>  
>> +allOf:
>> +  - if:
>> +      $ref: "#/definitions/imx-audio-generic-dependency"
>> +    then:
>> +      properties:
>> +        audio-codec:
>> +          items:
>> +            - description: SPDIF DIT phandle
>> +            - description: SPDIF DIR phandle
>> +        mclk-id:
>> +          maxItems: 1
>> +          items:
>> +            minItems: 1
>> +            maxItems: 2
>> +    else:
>> +      properties:
>> +        audio-codec:
>> +          maxItems: 1
>> +        mclk-id:
>> +          maxItems: 1
>> +          items:
>> +            maxItems: 1
> 
> 
> You can handle the dependency like this instead:
> 
>           dai-tdm-slot-width: false
>           dai-tdm-slot-num: false
> 
> 
> And then you don't need the definitions.
> 
>> +
>>  unevaluatedProperties: false
>>  
>>  examples:
>> @@ -195,3 +270,16 @@ examples:
>>               "AIN2L", "Line In Jack",
>>               "AIN2R", "Line In Jack";
>>      };
>> +
>> +  - |
>> +    #include <dt-bindings/clock/imx8mn-clock.h>
>> +    sound-spdif-asrc {
>> +      compatible = "fsl,imx-audio-generic";
>> +      model = "spdif-asrc-audio";
>> +      audio-cpu = <&spdif>;
>> +      audio-asrc = <&easrc>;
>> +      audio-codec = <&spdifdit>, <&spdifdir>;
>> +      clocks = <&clk IMX8MN_CLK_SAI5_ROOT>;
>> +      clock-names = "cpu_sysclk";
>> +      cpu-system-clock-direction-out;
>> +    };
>> --
>> 2.34.1

Ok, thank you for the review, I'll make these modifications in v5.

Re: [PATCHv4 7/9] ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]

On Fri, May 31, 2024 at 08:46:55AM -0400, Elinor Montmasson wrote:
> From: "Mark Brown" <broonie@kernel.org>
> > On Fri, May 17, 2024 at 05:05:35AM -0400, Elinor Montmasson wrote:
> >> From: "Mark Brown" <broonie@kernel.org>
> >> > On Wed, May 15, 2024 at 03:54:09PM +0200, Elinor Montmasson wrote:

> >> >> +		struct clk *cpu_sysclk = clk_get(&pdev->dev, "cpu_sysclk");
> >> >> +		if (!IS_ERR(cpu_sysclk)) {
> >> >> +			priv->cpu_priv.sysclk_freq[TX] = clk_get_rate(cpu_sysclk);
> >> >> +			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
> >> >> +			clk_put(cpu_sysclk);
> >> >> +		}

> >> > I don't really understand the goal here - this is just reading whatever
> >> > frequency happens to be set in the hardware when the driver starts up
> >> > which if nothing else seems rather fragile?

> >> The driver allow to set the sysclk frequency
> >> of the CPU DAI through `priv->cpu_priv.sysclk_freq` when calling
> >> `fsl_asoc_card_hw_params()`.
> >> Currently it is hard-coded per use-case in the driver.

> >> My reasoning was that with a generic codec/compatible, there might
> >> be use-cases needing to use this parameter, so I exposed it here via DT.
> > 
> >> Is it a bad idea to expose this parameter ? This is not a requirement for the
> >> driver to work, most of the current compatibles do not use this parameter.
> >> It is currently used only for `fsl,imx-audio-cs42888`.
> >> In that case I can remove this commit.

> > I'm having a hard time connecting your reply here with my comment.  This
> > isn't as far as I can see allowing the frequency to be explicitly
> > configured, it's just using whatever value happens to be programmed in
> > the clock when the driver starts.

> In v3 I used parameters `cpu-sysclk-freq-rx/tx` to explicitly
> set the frequency.
> In its review Rob Herring said that the clock bindings should
> be used, so that's why I changed it to use this `cpu_sysclk` clock.

So you're trying to use this as the audio clock?  There's no code that
enables the clock which seems worrying, and I'd expect that if the
device is using it's own clock the device would be querying it directly
via the clock API rather than this.  This all seems really confused.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 8/9] ASoC: fsl-asoc-card: add DT property "cpu-system-clock-direction-out"

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]

On Fri, May 31, 2024 at 08:47:22AM -0400, Elinor Montmasson wrote:

> > When I said "this should use the clock bindings" I meant that we should
> > use the clock bindings for configuration here.

> As far I as know, it's not possible to set the direction with
> the clock bindings, but maybe there is and I missed something ?

If a given clock has an input configured then it can't function as an
output and vice versa.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Fri, May 31, 2024 at 08:48:04AM -0400, Elinor Montmasson wrote:

> Then maybe it's not be a good idea to make this compatible generic
> for this contribution.
> The original intention is to bring support for the S/PDIF,
> so maybe the contribution should focus on this use case?
> In that case, would changing the compatible for "fsl,imx-audio-spdif-card"
> be acceptable?
> "fsl,imx-audio-spdif" is already used for the `imx-spdif.c`
> which does not use the ASRC.

Why not just use the existing compatible - why would someone not want to
be able to use the ASRC if it's available in their system?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 7/9] ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec

[Elinor Montmasson]

From: "Mark Brown" <broonie@kernel.org>
Sent: Friday, 31 May, 2024 15:05:28
> On Fri, May 31, 2024 at 08:46:55AM -0400, Elinor Montmasson wrote:
>> From: "Mark Brown" <broonie@kernel.org>
>> > On Fri, May 17, 2024 at 05:05:35AM -0400, Elinor Montmasson wrote:
>> >> From: "Mark Brown" <broonie@kernel.org>
>> >> > On Wed, May 15, 2024 at 03:54:09PM +0200, Elinor Montmasson wrote:
> 
>> >> >> +		struct clk *cpu_sysclk = clk_get(&pdev->dev, "cpu_sysclk");
>> >> >> +		if (!IS_ERR(cpu_sysclk)) {
>> >> >> +			priv->cpu_priv.sysclk_freq[TX] = clk_get_rate(cpu_sysclk);
>> >> >> +			priv->cpu_priv.sysclk_freq[RX] = priv->cpu_priv.sysclk_freq[TX];
>> >> >> +			clk_put(cpu_sysclk);
>> >> >> +		}
> 
>> >> > I don't really understand the goal here - this is just reading whatever
>> >> > frequency happens to be set in the hardware when the driver starts up
>> >> > which if nothing else seems rather fragile?
> 
>> >> The driver allow to set the sysclk frequency
>> >> of the CPU DAI through `priv->cpu_priv.sysclk_freq` when calling
>> >> `fsl_asoc_card_hw_params()`.
>> >> Currently it is hard-coded per use-case in the driver.
> 
>> >> My reasoning was that with a generic codec/compatible, there might
>> >> be use-cases needing to use this parameter, so I exposed it here via DT.
>> > 
>> >> Is it a bad idea to expose this parameter ? This is not a requirement for the
>> >> driver to work, most of the current compatibles do not use this parameter.
>> >> It is currently used only for `fsl,imx-audio-cs42888`.
>> >> In that case I can remove this commit.
> 
>> > I'm having a hard time connecting your reply here with my comment.  This
>> > isn't as far as I can see allowing the frequency to be explicitly
>> > configured, it's just using whatever value happens to be programmed in
>> > the clock when the driver starts.
> 
>> In v3 I used parameters `cpu-sysclk-freq-rx/tx` to explicitly
>> set the frequency.
>> In its review Rob Herring said that the clock bindings should
>> be used, so that's why I changed it to use this `cpu_sysclk` clock.
> 
> So you're trying to use this as the audio clock?  There's no code that
> enables the clock which seems worrying, and I'd expect that if the
> device is using it's own clock the device would be querying it directly
> via the clock API rather than this.  This all seems really confused.

It's not specifically the audio clock, I am merely using this
in the machine driver to let the user the possibility
to configure the CPU DAI sysclock frequency.
The CPU DAI and codec drivers already manage their
own clocks.

I agree it is confused, I am trying to expose a driver option
for this generic compatible without really knowing a use case where it
would be needed.
With the S/PDIF it isn't needed, so I should probably remove this commit.

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Elinor Montmasson]

From: "Mark Brown" <broonie@kernel.org>
Sent: Friday, 31 May, 2024 15:14:13

> On Fri, May 31, 2024 at 08:48:04AM -0400, Elinor Montmasson wrote:
> 
>> Then maybe it's not be a good idea to make this compatible generic
>> for this contribution.
>> The original intention is to bring support for the S/PDIF,
>> so maybe the contribution should focus on this use case?
>> In that case, would changing the compatible for "fsl,imx-audio-spdif-card"
>> be acceptable?
>> "fsl,imx-audio-spdif" is already used for the `imx-spdif.c`
>> which does not use the ASRC.
> 
> Why not just use the existing compatible - why would someone not want to
> be able to use the ASRC if it's available in their system?

That's true but it will be a problem if both `fsl-asoc-card.c` and
`imx-spdif.c` drivers have the same compatible, and they don't
have the same DT properties.

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 440 bytes --]

On Fri, May 31, 2024 at 10:48:14AM -0400, Elinor Montmasson wrote:
> From: "Mark Brown" <broonie@kernel.org>

> > Why not just use the existing compatible - why would someone not want to
> > be able to use the ASRC if it's available in their system?

> That's true but it will be a problem if both `fsl-asoc-card.c` and
> `imx-spdif.c` drivers have the same compatible, and they don't
> have the same DT properties.

So merge the two then?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 7/9] ASoC: fsl-asoc-card: add DT clock "cpu_sysclk" with generic codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 878 bytes --]

On Fri, May 31, 2024 at 10:48:12AM -0400, Elinor Montmasson wrote:
> From: "Mark Brown" <broonie@kernel.org>

> > So you're trying to use this as the audio clock?  There's no code that
> > enables the clock which seems worrying, and I'd expect that if the
> > device is using it's own clock the device would be querying it directly
> > via the clock API rather than this.  This all seems really confused.

> It's not specifically the audio clock, I am merely using this
> in the machine driver to let the user the possibility
> to configure the CPU DAI sysclock frequency.
> The CPU DAI and codec drivers already manage their
> own clocks.

I would expect that if the clocks used by the devices are configured via
the clock API then the drivers for those devices will configure
themselves via the clock API.  I still don't understand what this change
is intended to accomplish.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCHv4 9/9] ASoC: dt-bindings: fsl-asoc-card: add compatible for generic codec

[Elinor Montmasson]

> From: "Mark Brown" <broonie@kernel.org>
> Sent: Friday, 31 May, 2024 18:06:30

> On Fri, May 31, 2024 at 10:48:14AM -0400, Elinor Montmasson wrote:
>> From: "Mark Brown" <broonie@kernel.org>
> 
>> > Why not just use the existing compatible - why would someone not want to
>> > be able to use the ASRC if it's available in their system?
> 
>> That's true but it will be a problem if both `fsl-asoc-card.c` and
>> `imx-spdif.c` drivers have the same compatible, and they don't
>> have the same DT properties.
> 
> So merge the two then?

It would avoid having duplicate drivers yes, I will do this for the v5 of this contribution.
Thank you for the review.