Thread (4 messages) flat view 4 messages, 3 authors, 2020-06-17

Re: [PATCH 2/2] ASoC: fsl-asoc-card: Add MQS support

From: Nicolin Chen <nicoleotsuka@gmail.com>
Date: 2020-06-17 00:49:07
Also in: alsa-devel, linux-devicetree, lkml

On Tue, Jun 16, 2020 at 03:30:37PM +0800, Shengjiu Wang wrote:
The MQS codec isn't an i2c device, so add a new platform device for it.

MQS only support playback, so add a new audio map.

Add there maybe "model" property or no "audio-routing" property insertions
"Add" => "And"
quoted hunk ↗ jump to hunk
devicetree, so add some enhancement for these two property.

Signed-off-by: Shengjiu Wang <redacted>
---
 sound/soc/fsl/fsl-asoc-card.c | 70 ++++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 18 deletions(-)
diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index 00be73900888..2ac8cb9ddd10 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
quoted hunk ↗ jump to hunk
@@ -482,6 +489,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 {
 	struct device_node *cpu_np, *codec_np, *asrc_np;
 	struct device_node *np = pdev->dev.of_node;
+	struct platform_device *codec_pdev = NULL; /* used for non i2c device*/
Having both codec_pdev and codec_dev duplicates things. Actually
only a couple of places really need "codec_dev" -- most of them
need codec_dev->dev pointer instead. So we could have a cleanup:

-	struct i2c_client *codec_dev;
+	struct device *codec_dev = NULL;
quoted hunk ↗ jump to hunk
@@ -512,10 +520,13 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	}
 
 	codec_np = of_parse_phandle(np, "audio-codec", 0);
-	if (codec_np)
+	if (codec_np) {
 		codec_dev = of_find_i2c_device_by_node(codec_np);
-	else
+		if (!codec_dev)
+			codec_pdev = of_find_device_by_node(codec_np);
+	} else {
 		codec_dev = NULL;
+	}
Here can have something like (feel free to simplify):

	if (codec_np) {
		struct platform_device *codec_pdev;
		struct i2c_client *codec_i2c;

		codec_i2c = of_find_i2c_device_by_node(codec_np);
		if (codec_i2c)
			codec_dev = &codec_i2c->dev;

		if (!codec_dev) {
			codec_pdev = of_find_device_by_node(codec_np);
			codec_dev = &codec_pdev->dev;
		}
	}
quoted hunk ↗ jump to hunk
 	asrc_np = of_parse_phandle(np, "audio-asrc", 0);
 	if (asrc_np)
@@ -525,6 +536,13 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	if (codec_dev) {
 		struct clk *codec_clk = clk_get(&codec_dev->dev, NULL);
Then here:

-  		struct clk *codec_clk = clk_get(&codec_dev->dev, NULL);
+  		struct clk *codec_clk = clk_get(codec_dev, NULL);
quoted hunk ↗ jump to hunk
@@ -538,6 +556,11 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 	/* Assign a default DAI format, and allow each card to overwrite it */
 	priv->dai_fmt = DAI_FMT_BASE;
 
+	memcpy(priv->dai_link, fsl_asoc_card_dai,
+	       sizeof(struct snd_soc_dai_link) * ARRAY_SIZE(priv->dai_link));
quoted hunk ↗ jump to hunk
@@ -573,13 +596,25 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		codec_dai_name = "ac97-hifi";
 		priv->card.set_bias_level = NULL;
 		priv->dai_fmt = SND_SOC_DAIFMT_AC97;
+		priv->card.dapm_routes = audio_map_ac97;
+		priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_ac97);
+	} else if (of_device_is_compatible(np, "fsl,imx-audio-mqs")) {
+		codec_dai_name = "fsl-mqs-dai";
+		priv->card.set_bias_level = NULL;
+		priv->dai_fmt = SND_SOC_DAIFMT_LEFT_J |
+				SND_SOC_DAIFMT_CBS_CFS |
+				SND_SOC_DAIFMT_NB_NF;
+		priv->dai_link[1].dpcm_playback = 1;
+		priv->dai_link[2].dpcm_playback = 1;
dpcm_playback = 1? That's the default value in fsl_asoc_card_dai.
quoted hunk ↗ jump to hunk
@@ -601,19 +636,18 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		priv->cpu_priv.sysclk_id[0] = FSL_SAI_CLK_MAST1;
 	}
 
-	snprintf(priv->name, sizeof(priv->name), "%s-audio",
-		 fsl_asoc_card_is_ac97(priv) ? "ac97" :
-		 codec_dev->name);
-
 	/* Initialize sound card */
 	priv->pdev = pdev;
 	priv->card.dev = &pdev->dev;
-	priv->card.name = priv->name;
+	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 ? codec_dev->name : codec_pdev->name));
We can just use dev_name() if codec_dev is struct device *
Or having a codec_dev_name to cache codec_pdev/i2c->name.
-	ret = snd_soc_of_parse_audio_routing(&priv->card, "audio-routing");
-	if (ret) {
-		dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
-		goto asrc_fail;
+	if (of_property_read_bool(np, "audio-routing")) {
+		ret = snd_soc_of_parse_audio_routing(&priv->card, "audio-routing");
+		if (ret) {
+			dev_err(&pdev->dev, "failed to parse audio-routing: %d\n", ret);
+			goto asrc_fail;
Hmm...audio-routing is a required property in DT binding doc.
So you might need to update that too.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help