[PATCHv3 RESEND 00/10] ASoC: fsl-asoc-card: compatibility integration of a generic codec use case for use with S/PDIF controller
Hello,
This is the v3 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 remaining patches add configuration options for the device tree. 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.
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 December. These modifications have also been tested on an i.MX8MN evaluation board, with a linux kernel RT v6.1.26-rt8.
We also have a few questions, following remarks made by Krzysztof Kozlowski in a previous email for patch 10/10:
The compatible list for this generic sound card currently: @@ -48,6 +51,8 @@ The compatible list for this generic sound card currently:
"fsl,imx-audio-nau8822"
- "fsl,imx-audio-generic"
Generic does not look like hardware specific.
Even if our end goal is to use it with the S/PDIF controller, this new support can be used with different hardware that doesn't require a codec. Thus, we don't really want to specify "spdif" in it.
Is this compatible string not suitable ? Should we rename it to something else, like "fsl,imx-audio-no-codec" ?
Maybe Mark or Rob will help here, but for me "imx-audio" is just way too generic.
* Which generic name should we use ? Or how should we change it?
Also, you add several new properties, so I really expect either converting old binding to DT schema first or adding new device in DT schema format.
* fsl-asoc-card.txt currently follows the old dt-bindings format. Should we update it to DT schema format in this patch series before adding my new properties?
Best regards, Elinor Montmasson
Changelog: 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 the 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 and previous commits of the file. * v2 patch series at: https://lore.kernel.org/alsa-devel/20231027144734.3654829-1-elinor.montmasso...
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...
Elinor Montmasson (10): 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 dts property "cpu-slot-width" ASoC: fsl-asoc-card: add dts property "cpu-slot-num" ASoC: fsl-asoc-card: add dts properties "cpu-sysclk-freq" ASoC: fsl-asoc-card: add dts properties "cpu-sysclk-dir-out" ASoC: bindings: fsl-asoc-card: add compatible for generic codec
.../bindings/sound/fsl-asoc-card.txt | 28 +- sound/soc/fsl/fsl-asoc-card.c | 299 +++++++++++------- 2 files changed, 218 insertions(+), 109 deletions(-)
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 7518ab9d768e..cde31fd38262 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -809,10 +809,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;
@@ -823,11 +823,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; } @@ -838,13 +838,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;
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 cde31fd38262..a62f26fe9802 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -295,7 +295,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, @@ -305,7 +305,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()), DAILINK_COMP_ARRAY(COMP_DUMMY()));
static const struct snd_soc_dai_link fsl_asoc_card_dai[] = { @@ -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);
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 | 233 ++++++++++++++++++++-------------- 1 file changed, 136 insertions(+), 97 deletions(-)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c index a62f26fe9802..5dd5493cb931 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -98,7 +98,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; @@ -171,11 +171,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); @@ -207,28 +209,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; + } } }
@@ -243,28 +249,34 @@ static int fsl_asoc_card_hw_free(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; 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,20 +724,20 @@ 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 { dev_err(&pdev->dev, "unknown Device Tree compatible\n"); ret = -EINVAL; @@ -722,18 +748,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 */ @@ -749,7 +787,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; @@ -789,7 +827,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; @@ -814,7 +852,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;
@@ -922,7 +960,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);
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 5dd5493cb931..71048c1250ec 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; @@ -738,6 +739,11 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) priv->dai_fmt |= SND_SOC_DAIFMT_CBM_CFM; 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-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; @@ -792,6 +798,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")) { @@ -849,11 +861,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); @@ -983,6 +1005,7 @@ static const struct of_device_id fsl_asoc_card_dt_ids[] = { { .compatible = "fsl,imx-audio-si476x", }, { .compatible = "fsl,imx-audio-wm8958", }, { .compatible = "fsl,imx-audio-nau8822", }, + { .compatible = "fsl,imx-audio-generic", }, {} }; MODULE_DEVICE_TABLE(of, fsl_asoc_card_dt_ids);
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 71048c1250ec..3a57d9bfbb48 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -744,6 +744,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;
Add new optional dts property "cpu-slot-width", which allows setting a custom TDM slot width in bits for the CPU DAI when using the generic codec.
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 3a57d9bfbb48..012c8d3666aa 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -745,6 +745,7 @@ 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; + of_property_read_u32(np, "cpu-slot-width", &priv->cpu_priv.slot_width); } else { dev_err(&pdev->dev, "unknown Device Tree compatible\n"); ret = -EINVAL;
Add new optional dts property "cpu-slot-num", which allows setting a custom number of TDM slots for the CPU DAI when using the generic codec.
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 012c8d3666aa..6f6cc8bd3acd 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -746,6 +746,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) priv->dai_link[2].num_codecs = 2; priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP; of_property_read_u32(np, "cpu-slot-width", &priv->cpu_priv.slot_width); + of_property_read_u32(np, "cpu-slot-num", &priv->cpu_priv.slot_num); } else { dev_err(&pdev->dev, "unknown Device Tree compatible\n"); ret = -EINVAL;
Add new optional dts property "cpu-sysclk-freq" to set custom sysclk frequencies for the CPU DAI with the generic codec. The way values are used is up to the CPU DAI driver implementation.
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 | 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 6f6cc8bd3acd..7b0d7df7ae27 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -747,6 +747,10 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP; of_property_read_u32(np, "cpu-slot-width", &priv->cpu_priv.slot_width); of_property_read_u32(np, "cpu-slot-num", &priv->cpu_priv.slot_num); + of_property_read_u32(np, "cpu-sysclk-freq-rx", + (u32 *)&priv->cpu_priv.sysclk_freq[RX]); + of_property_read_u32(np, "cpu-sysclk-freq-tx", + (u32 *)&priv->cpu_priv.sysclk_freq[TX]); } else { dev_err(&pdev->dev, "unknown Device Tree compatible\n"); ret = -EINVAL;
Add new optional dts properties "cpu-sysclk-dir-out" to set sysclk directions as "out" for the CPU DAI when using the generic codec. This can be set for Tx and Rx. If not set, the direction is "in". The way values are used is up to the CPU DAI driver implementation.
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, 6 insertions(+)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c index 7b0d7df7ae27..5f8fb724e29d 100644 --- a/sound/soc/fsl/fsl-asoc-card.c +++ b/sound/soc/fsl/fsl-asoc-card.c @@ -751,6 +751,12 @@ static int fsl_asoc_card_probe(struct platform_device *pdev) (u32 *)&priv->cpu_priv.sysclk_freq[RX]); of_property_read_u32(np, "cpu-sysclk-freq-tx", (u32 *)&priv->cpu_priv.sysclk_freq[TX]); + priv->cpu_priv.sysclk_dir[RX] = + of_property_read_bool(np, "cpu-sysclk-dir-rx-out") ? + SND_SOC_CLOCK_OUT : SND_SOC_CLOCK_IN; + priv->cpu_priv.sysclk_dir[TX] = + of_property_read_bool(np, "cpu-sysclk-dir-tx-out") ? + SND_SOC_CLOCK_OUT : SND_SOC_CLOCK_IN; } else { dev_err(&pdev->dev, "unknown Device Tree compatible\n"); ret = -EINVAL;
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 Co-authored-by: Philip-Dylan Gleonec philip-dylan.gleonec@savoirfairelinux.com --- .../bindings/sound/fsl-asoc-card.txt | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt index 4e8dbc5abfd1..f137ef2154e3 100644 --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt @@ -17,6 +17,9 @@ Note: The card is initially designed for those sound cards who use AC'97, I2S 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".
The compatible list for this generic sound card currently: @@ -48,6 +51,8 @@ The compatible list for this generic sound card currently:
"fsl,imx-audio-nau8822"
+ "fsl,imx-audio-generic" + Required properties:
- compatible : Contains one of entries in the compatible list. @@ -56,7 +61,11 @@ Required properties:
- audio-cpu : The phandle of an CPU DAI controller
- - audio-codec : The phandle of an audio codec + - audio-codec : 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).
Optional properties:
@@ -87,6 +96,23 @@ Optional properties: - frame-inversion : dai-link uses frame clock inversion, for details see simple-card.yaml. - bitclock-inversion : dai-link uses bit clock inversion, for details see simple-card.yaml. - mclk-id : main clock id, specific for each card configuration. + For multi-codec configurations, an array of ids can be + given, one for each codec. + +Optional, relevant only with the "fsl,imx-audio-generic" compatible: + + - cpu-slot-width : Indicates a specific TDM slot width in bits. + - cpu-slot-num : Indicates a specific number of TDM slots per frame. + + - cpu-sysclk-freq-rx : Frequency of the CPU DAI sys clock for Rx. + - cpu-sysclk-freq-tx : Frequency of the CPU DAI sys clock for Tx. + + - cpu-sysclk-dir-rx-out : Boolean property. Specifies sys clock direction + as 'out' on initialization for Rx. + If not set, default direction is 'in'. + - cpu-sysclk-dir-tx-out : Boolean property. Specifies sys clock direction + as 'out' on initialization for Tx. + If not set, default direction is 'in'.
Optional unless SSI is selected as a CPU DAI:
On Mon, Dec 18, 2023 at 01:40:58PM +0100, 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 Co-authored-by: Philip-Dylan Gleonec philip-dylan.gleonec@savoirfairelinux.com
.../bindings/sound/fsl-asoc-card.txt | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt index 4e8dbc5abfd1..f137ef2154e3 100644 --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt @@ -17,6 +17,9 @@ Note: The card is initially designed for those sound cards who use AC'97, I2S 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 canuse the compatible "fsl,imx-audio-generic".The compatible list for this generic sound card currently: @@ -48,6 +51,8 @@ The compatible list for this generic sound card currently:
"fsl,imx-audio-nau8822"
- "fsl,imx-audio-generic"
Required properties:
- compatible : Contains one of entries in the compatible list.
@@ -56,7 +61,11 @@ Required properties:
audio-cpu : The phandle of an CPU DAI controller
- audio-codec : The phandle of an audio codec
- audio-codec : The phandle of an audio codec.
If using the "fsl,imx-audio-generic" compatible,give instead a pair of phandles with thespdif_transmitter first (driver SPDIF DIT) and thespdif_receiver second (driver SPDIF DIR).Optional properties:
@@ -87,6 +96,23 @@ Optional properties:
- frame-inversion : dai-link uses frame clock inversion, for details see simple-card.yaml.
- bitclock-inversion : dai-link uses bit clock inversion, for details see simple-card.yaml.
- mclk-id : main clock id, specific for each card configuration.
For multi-codec configurations, an array of ids can begiven, one for each codec.+Optional, relevant only with the "fsl,imx-audio-generic" compatible:
- cpu-slot-width : Indicates a specific TDM slot width in bits.
- cpu-slot-num : Indicates a specific number of TDM slots per frame.
Pretty sure I've seen other bindings with TDM slot properties. A sign we need something common if we don't already have something.
- cpu-sysclk-freq-rx : Frequency of the CPU DAI sys clock for Rx.
- cpu-sysclk-freq-tx : Frequency of the CPU DAI sys clock for Tx.
- cpu-sysclk-dir-rx-out : Boolean property. Specifies sys clock direction
as 'out' on initialization for Rx.If not set, default direction is 'in'.
- cpu-sysclk-dir-tx-out : Boolean property. Specifies sys clock direction
as 'out' on initialization for Tx.If not set, default direction is 'in'.
Looks like clock stuff. Use the clock binding.
Rob
Hello,
On Wednesday, 20 December, 2023 23:25:50, Rob Herring wrote:
On Mon, Dec 18, 2023 at 01:40:58PM +0100, Elinor Montmasson wrote:
+Optional, relevant only with the "fsl,imx-audio-generic" compatible:
- cpu-slot-width : Indicates a specific TDM slot width in bits.
- cpu-slot-num : Indicates a specific number of TDM slots per frame.
Pretty sure I've seen other bindings with TDM slot properties. A sign we need something common if we don't already have something.
That's right, "tdm-slot.txt" already defines TDM bindings, I will use them with the utility function snd_soc_of_parse_tdm_slot().
- cpu-sysclk-freq-rx : Frequency of the CPU DAI sys clock for Rx.
- cpu-sysclk-freq-tx : Frequency of the CPU DAI sys clock for Tx.
- cpu-sysclk-dir-rx-out : Boolean property. Specifies sys clock direction
- as 'out' on initialization for Rx.
- If not set, default direction is 'in'.
- cpu-sysclk-dir-tx-out : Boolean property. Specifies sys clock direction
- as 'out' on initialization for Tx.
- If not set, default direction is 'in'.
Looks like clock stuff. Use the clock binding.
simple-card defines similar properties at the dai level: "system-clock-frequency" and "system-clock-direction-out". The first is used if no "clocks" binding is specified for the dai node.
Maybe I could use a similar logic with fsl-asoc-card ? * adding a "clock-cpu" phandle property which should be the cpu clock. It will be used to retreive the frequency for both RX and TX. * keeping "cpu-sysclk-freq-rx"/"cpu-sysclk-freq-tx", which are used if no clock is provided, like "system-clock-frequency" in simple-card. * keep using "cpu-sysclk-dir-rx-out"/"cpu-sysclk-dir-tx-out", like "system-clock-direction-out" in simple card.
Also, maybe I could rename my new properties: cpu-system-clock-frequency-tx, cpu-system-clock-direction-out-rx, ... It would better match those in simple-card as they do the same thing.
Best regards, Elinor Montmasson
On 18/12/2023 13:40, Elinor Montmasson wrote:
Hello,
This is the v3 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 remaining patches add configuration options for the device tree. 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.
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 December. These modifications have also been tested on an i.MX8MN evaluation board, with a linux kernel RT v6.1.26-rt8.
We also have a few questions, following remarks made by Krzysztof Kozlowski in a previous email for patch 10/10:
The compatible list for this generic sound card currently: @@ -48,6 +51,8 @@ The compatible list for this generic sound card currently:
"fsl,imx-audio-nau8822"
- "fsl,imx-audio-generic"
Generic does not look like hardware specific.
Even if our end goal is to use it with the S/PDIF controller, this new support can be used with different hardware that doesn't require a codec. Thus, we don't really want to specify "spdif" in it.
Is this compatible string not suitable ? Should we rename it to something else, like "fsl,imx-audio-no-codec" ?
Maybe Mark or Rob will help here, but for me "imx-audio" is just way too generic.
- Which generic name should we use ? Or how should we change it?
Compatible should be specific to one SoC, even if there is one driver for entire family.
Also, you add several new properties, so I really expect either converting old binding to DT schema first or adding new device in DT schema format.
- fsl-asoc-card.txt currently follows the old dt-bindings format.
Should we update it to DT schema format in this patch series before adding my new properties?
You add six new properties, so from my point of view this cannot be in TXT.
Best regards, Krzysztof
- fsl-asoc-card.txt currently follows the old dt-bindings format.
Should we update it to DT schema format in this patch series before adding my new properties?
I know this is extra-work but we would greatly appreciate if you first convert fsl-asoc-card.txt to yml format and then add your new properties.
On Monday, 18 December, 2023 14:54:03, Daniel Baluta wrote
- fsl-asoc-card.txt currently follows the old dt-bindings format.
Should we update it to DT schema format in this patch series before adding my new properties?
I know this is extra-work but we would greatly appreciate if you first convert fsl-asoc-card.txt to yml format and then add your new properties.
I will take some time next week to do the conversion, then I'll send it in a v4 patch series.
Best regards, Elinor Montmasson
Hello
On Monday, 18 December, 2023 14:54:03, Daniel Baluta wrote
I know this is extra-work but we would greatly appreciate if you first convert fsl-asoc-card.txt to yml format and then add your new properties.
DT schema must have at least one maintainer in the "maintainers" field. Who should I put for fsl-asoc-card.yaml ?
Best regards, Elinor Montmasson
On Fri, Dec 29, 2023 at 3:45 PM Elinor Montmasson elinor.montmasson@savoirfairelinux.com wrote:
Hello
On Monday, 18 December, 2023 14:54:03, Daniel Baluta wrote
I know this is extra-work but we would greatly appreciate if you first convert fsl-asoc-card.txt to yml format and then add your new properties.
DT schema must have at least one maintainer in the "maintainers" field. Who should I put for fsl-asoc-card.yaml ?
I think it should be Shengjiu Wang, if he is OK with that.
participants (4)
-
Daniel Baluta -
Elinor Montmasson -
Krzysztof Kozlowski -
Rob Herring