diff mbox series

[v2,16/16] ASoC: qcom: sm8250: Add audioreach support

Message ID 20210714153039.28373-17-srinivas.kandagatla@linaro.org
State New
Headers show
Series ASoC: qcom: Add AudioReach support | expand

Commit Message

Srinivas Kandagatla July 14, 2021, 3:30 p.m. UTC
This patch adds support for parsing dt for AudioReach based soundcards
which only have backend DAI links in DT.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

---
 sound/soc/qcom/sm8250.c | 144 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 143 insertions(+), 1 deletion(-)

-- 
2.21.0

Comments

Pierre-Louis Bossart July 14, 2021, 5:12 p.m. UTC | #1
On 7/14/21 10:30 AM, Srinivas Kandagatla wrote:
> This patch adds support for parsing dt for AudioReach based soundcards

> which only have backend DAI links in DT.

> 

> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

> ---

>  sound/soc/qcom/sm8250.c | 144 +++++++++++++++++++++++++++++++++++++++-

>  1 file changed, 143 insertions(+), 1 deletion(-)

> 

> diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c

> index fe8fd7367e21..421f9d1d2bed 100644

> --- a/sound/soc/qcom/sm8250.c

> +++ b/sound/soc/qcom/sm8250.c

> @@ -20,6 +20,141 @@ struct sm8250_snd_data {

>  	struct sdw_stream_runtime *sruntime[AFE_PORT_MAX];

>  };

>  

> +static int qcom_audioreach_snd_parse_of(struct snd_soc_card *card)

> +{

> +	struct device_node *np;

> +	struct device_node *codec = NULL;

> +	struct device_node *platform = NULL;

> +	struct device_node *cpu = NULL;

> +	struct device *dev = card->dev;

> +	struct snd_soc_dai_link *link;

> +	struct of_phandle_args args;

> +	struct snd_soc_dai_link_component *dlc;

> +	int ret, num_links;

> +

> +	ret = snd_soc_of_parse_card_name(card, "model");

> +	if (ret) {

> +		dev_err(dev, "Error parsing card name: %d\n", ret);

> +		return ret;

> +	}

> +

> +	/* DAPM routes */

> +	if (of_property_read_bool(dev->of_node, "audio-routing")) {

> +		ret = snd_soc_of_parse_audio_routing(card, "audio-routing");

> +		if (ret)

> +			return ret;

> +	}

> +

> +	/* Populate links */

> +	num_links = of_get_child_count(dev->of_node);

> +

> +	/* Allocate the DAI link array */

> +	card->dai_link = devm_kcalloc(dev, num_links, sizeof(*link), GFP_KERNEL);

> +	if (!card->dai_link)

> +		return -ENOMEM;

> +

> +	card->num_links = num_links;

> +	link = card->dai_link;

> +

> +	for_each_child_of_node(dev->of_node, np) {

> +

> +		dlc = devm_kzalloc(dev, 2 * sizeof(*dlc), GFP_KERNEL);

> +		if (!dlc) {

> +			ret = -ENOMEM;

> +			goto err_put_np;

> +		}

> +

> +		link->cpus	= &dlc[0];

> +		link->platforms	= &dlc[1];

> +

> +		link->num_cpus		= 1;

> +		link->num_platforms	= 1;

> +

> +

> +		ret = of_property_read_string(np, "link-name", &link->name);

> +		if (ret) {

> +			dev_err(card->dev, "error getting codec dai_link name\n");

> +			goto err_put_np;

> +		}

> +

> +		cpu = of_get_child_by_name(np, "cpu");

> +		platform = of_get_child_by_name(np, "platform");

> +		codec = of_get_child_by_name(np, "codec");

> +		if (!cpu) {

> +			dev_err(dev, "%s: Can't find cpu DT node\n", link->name);

> +			ret = -EINVAL;

> +			goto err;

> +		}

> +

> +		if (!platform) {

> +			dev_err(dev, "%s: Can't find platform DT node\n", link->name);

> +			ret = -EINVAL;

> +			goto err;

> +		}

> +

> +		if (!codec) {

> +			dev_err(dev, "%s: Can't find codec DT node\n", link->name);

> +			ret = -EINVAL;

> +			goto err;

> +		}

> +

> +		ret = of_parse_phandle_with_args(cpu, "sound-dai", "#sound-dai-cells", 0, &args);

> +		if (ret) {

> +			dev_err(card->dev, "%s: error getting cpu phandle\n", link->name);

> +			goto err;

> +		}

> +

> +		link->cpus->of_node = args.np;

> +		link->id = args.args[0];

> +

> +		ret = snd_soc_of_get_dai_name(cpu, &link->cpus->dai_name);

> +		if (ret) {

> +			if (ret != -EPROBE_DEFER)

> +				dev_err(card->dev, "%s: error getting cpu dai name: %d\n",

> +					link->name, ret);

> +			goto err;

> +		}

> +

> +		link->platforms->of_node = of_parse_phandle(platform, "sound-dai", 0);

> +		if (!link->platforms->of_node) {

> +			dev_err(card->dev, "%s: platform dai not found\n", link->name);

> +			ret = -EINVAL;

> +			goto err;

> +		}

> +

> +		ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);

> +		if (ret < 0) {

> +			if (ret != -EPROBE_DEFER)

> +				dev_err(card->dev, "%s: codec dai not found: %d\n",

> +					link->name, ret);

> +			goto err;

> +		}

> +

> +		/* DPCM backend */

> +		link->no_pcm = 1;

> +		link->ignore_pmdown_time = 1;

> +		link->ignore_suspend = 1;


why are those two fields set unconditionally?

If you parse information from DT shouldn't those links be explicitly tagged as requiring those fields to be set?

It's a recurring battle for me to ask why people set them in Intel machine drivers, I find it really odd that you would set them since they aren't without side effect on clocks and suspend.

> +

> +		link->stream_name = link->name;

> +		snd_soc_dai_link_set_capabilities(link);

> +		link++;

> +

> +		of_node_put(cpu);

> +		of_node_put(codec);

> +		of_node_put(platform);

> +

> +	}

> +

> +	return 0;

> +err:

> +	of_node_put(cpu);

> +	of_node_put(codec);

> +	of_node_put(platform);

> +err_put_np:

> +	of_node_put(np);

> +	return ret;

> +}

> +
Srinivas Kandagatla July 15, 2021, 10:32 a.m. UTC | #2
On 14/07/2021 18:12, Pierre-Louis Bossart wrote:
> 

> 

> On 7/14/21 10:30 AM, Srinivas Kandagatla wrote:

>> This patch adds support for parsing dt for AudioReach based soundcards

>> which only have backend DAI links in DT.

>>

>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

>> ---

>>   sound/soc/qcom/sm8250.c | 144 +++++++++++++++++++++++++++++++++++++++-

>>   1 file changed, 143 insertions(+), 1 deletion(-)

>>

>> diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c

>> index fe8fd7367e21..421f9d1d2bed 100644

>> --- a/sound/soc/qcom/sm8250.c

>> +++ b/sound/soc/qcom/sm8250.c

>> @@ -20,6 +20,141 @@ struct sm8250_snd_data {

>>   	struct sdw_stream_runtime *sruntime[AFE_PORT_MAX];

>>   };

>>   

>> +static int qcom_audioreach_snd_parse_of(struct snd_soc_card *card)

>> +{

>> +	struct device_node *np;

>> +	struct device_node *codec = NULL;

>> +	struct device_node *platform = NULL;

>> +	struct device_node *cpu = NULL;

>> +	struct device *dev = card->dev;

>> +	struct snd_soc_dai_link *link;

>> +	struct of_phandle_args args;

>> +	struct snd_soc_dai_link_component *dlc;

>> +	int ret, num_links;

>> +

>> +	ret = snd_soc_of_parse_card_name(card, "model");

>> +	if (ret) {

>> +		dev_err(dev, "Error parsing card name: %d\n", ret);

>> +		return ret;

>> +	}

>> +

>> +	/* DAPM routes */

>> +	if (of_property_read_bool(dev->of_node, "audio-routing")) {

>> +		ret = snd_soc_of_parse_audio_routing(card, "audio-routing");

>> +		if (ret)

>> +			return ret;

>> +	}

>> +

>> +	/* Populate links */

>> +	num_links = of_get_child_count(dev->of_node);

>> +

>> +	/* Allocate the DAI link array */

>> +	card->dai_link = devm_kcalloc(dev, num_links, sizeof(*link), GFP_KERNEL);

>> +	if (!card->dai_link)

>> +		return -ENOMEM;

>> +

>> +	card->num_links = num_links;

>> +	link = card->dai_link;

>> +

>> +	for_each_child_of_node(dev->of_node, np) {

>> +

>> +		dlc = devm_kzalloc(dev, 2 * sizeof(*dlc), GFP_KERNEL);

>> +		if (!dlc) {

>> +			ret = -ENOMEM;

>> +			goto err_put_np;

>> +		}

>> +

>> +		link->cpus	= &dlc[0];

>> +		link->platforms	= &dlc[1];

>> +

>> +		link->num_cpus		= 1;

>> +		link->num_platforms	= 1;

>> +

>> +

>> +		ret = of_property_read_string(np, "link-name", &link->name);

>> +		if (ret) {

>> +			dev_err(card->dev, "error getting codec dai_link name\n");

>> +			goto err_put_np;

>> +		}

>> +

>> +		cpu = of_get_child_by_name(np, "cpu");

>> +		platform = of_get_child_by_name(np, "platform");

>> +		codec = of_get_child_by_name(np, "codec");

>> +		if (!cpu) {

>> +			dev_err(dev, "%s: Can't find cpu DT node\n", link->name);

>> +			ret = -EINVAL;

>> +			goto err;

>> +		}

>> +

>> +		if (!platform) {

>> +			dev_err(dev, "%s: Can't find platform DT node\n", link->name);

>> +			ret = -EINVAL;

>> +			goto err;

>> +		}

>> +

>> +		if (!codec) {

>> +			dev_err(dev, "%s: Can't find codec DT node\n", link->name);

>> +			ret = -EINVAL;

>> +			goto err;

>> +		}

>> +

>> +		ret = of_parse_phandle_with_args(cpu, "sound-dai", "#sound-dai-cells", 0, &args);

>> +		if (ret) {

>> +			dev_err(card->dev, "%s: error getting cpu phandle\n", link->name);

>> +			goto err;

>> +		}

>> +

>> +		link->cpus->of_node = args.np;

>> +		link->id = args.args[0];

>> +

>> +		ret = snd_soc_of_get_dai_name(cpu, &link->cpus->dai_name);

>> +		if (ret) {

>> +			if (ret != -EPROBE_DEFER)

>> +				dev_err(card->dev, "%s: error getting cpu dai name: %d\n",

>> +					link->name, ret);

>> +			goto err;

>> +		}

>> +

>> +		link->platforms->of_node = of_parse_phandle(platform, "sound-dai", 0);

>> +		if (!link->platforms->of_node) {

>> +			dev_err(card->dev, "%s: platform dai not found\n", link->name);

>> +			ret = -EINVAL;

>> +			goto err;

>> +		}

>> +

>> +		ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);

>> +		if (ret < 0) {

>> +			if (ret != -EPROBE_DEFER)

>> +				dev_err(card->dev, "%s: codec dai not found: %d\n",

>> +					link->name, ret);

>> +			goto err;

>> +		}

>> +

>> +		/* DPCM backend */

>> +		link->no_pcm = 1;


Currently we only allow backend dailinks from DT, as the rest of pcm dai 
links come from topology.

>> +		link->ignore_pmdown_time = 1;

>> +		link->ignore_suspend = 1;

> 

> why are those two fields set unconditionally?


These two flags have been inherited from ./sound/soc/qcom/common.c

> 

> If you parse information from DT shouldn't those links be explicitly tagged as requiring those fields to be set?


Currently we do not have any DT bindings for these generic flags. May be 
I/we should think about adding binding for this soon.

> 

> It's a recurring battle for me to ask why people set them in Intel machine drivers, I find it really odd that you would set them since they aren't without side effect on clocks and suspend.


Noted, Thanks for hints, I think we are yet to hit these issues for 
wrong reasons, As most of the qcom audio drivers are not tested with PM. 
Some of the Google hw did test full PM though.
Currently am working on getting PM support for various qcom codecs, and 
soundwire controller.

--srini

> 

>> +

>> +		link->stream_name = link->name;

>> +		snd_soc_dai_link_set_capabilities(link);

>> +		link++;

>> +

>> +		of_node_put(cpu);

>> +		of_node_put(codec);

>> +		of_node_put(platform);

>> +

>> +	}

>> +

>> +	return 0;

>> +err:

>> +	of_node_put(cpu);

>> +	of_node_put(codec);

>> +	of_node_put(platform);

>> +err_put_np:

>> +	of_node_put(np);

>> +	return ret;

>> +}

>> +

>
diff mbox series

Patch

diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c
index fe8fd7367e21..421f9d1d2bed 100644
--- a/sound/soc/qcom/sm8250.c
+++ b/sound/soc/qcom/sm8250.c
@@ -20,6 +20,141 @@  struct sm8250_snd_data {
 	struct sdw_stream_runtime *sruntime[AFE_PORT_MAX];
 };
 
+static int qcom_audioreach_snd_parse_of(struct snd_soc_card *card)
+{
+	struct device_node *np;
+	struct device_node *codec = NULL;
+	struct device_node *platform = NULL;
+	struct device_node *cpu = NULL;
+	struct device *dev = card->dev;
+	struct snd_soc_dai_link *link;
+	struct of_phandle_args args;
+	struct snd_soc_dai_link_component *dlc;
+	int ret, num_links;
+
+	ret = snd_soc_of_parse_card_name(card, "model");
+	if (ret) {
+		dev_err(dev, "Error parsing card name: %d\n", ret);
+		return ret;
+	}
+
+	/* DAPM routes */
+	if (of_property_read_bool(dev->of_node, "audio-routing")) {
+		ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
+		if (ret)
+			return ret;
+	}
+
+	/* Populate links */
+	num_links = of_get_child_count(dev->of_node);
+
+	/* Allocate the DAI link array */
+	card->dai_link = devm_kcalloc(dev, num_links, sizeof(*link), GFP_KERNEL);
+	if (!card->dai_link)
+		return -ENOMEM;
+
+	card->num_links = num_links;
+	link = card->dai_link;
+
+	for_each_child_of_node(dev->of_node, np) {
+
+		dlc = devm_kzalloc(dev, 2 * sizeof(*dlc), GFP_KERNEL);
+		if (!dlc) {
+			ret = -ENOMEM;
+			goto err_put_np;
+		}
+
+		link->cpus	= &dlc[0];
+		link->platforms	= &dlc[1];
+
+		link->num_cpus		= 1;
+		link->num_platforms	= 1;
+
+
+		ret = of_property_read_string(np, "link-name", &link->name);
+		if (ret) {
+			dev_err(card->dev, "error getting codec dai_link name\n");
+			goto err_put_np;
+		}
+
+		cpu = of_get_child_by_name(np, "cpu");
+		platform = of_get_child_by_name(np, "platform");
+		codec = of_get_child_by_name(np, "codec");
+		if (!cpu) {
+			dev_err(dev, "%s: Can't find cpu DT node\n", link->name);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		if (!platform) {
+			dev_err(dev, "%s: Can't find platform DT node\n", link->name);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		if (!codec) {
+			dev_err(dev, "%s: Can't find codec DT node\n", link->name);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		ret = of_parse_phandle_with_args(cpu, "sound-dai", "#sound-dai-cells", 0, &args);
+		if (ret) {
+			dev_err(card->dev, "%s: error getting cpu phandle\n", link->name);
+			goto err;
+		}
+
+		link->cpus->of_node = args.np;
+		link->id = args.args[0];
+
+		ret = snd_soc_of_get_dai_name(cpu, &link->cpus->dai_name);
+		if (ret) {
+			if (ret != -EPROBE_DEFER)
+				dev_err(card->dev, "%s: error getting cpu dai name: %d\n",
+					link->name, ret);
+			goto err;
+		}
+
+		link->platforms->of_node = of_parse_phandle(platform, "sound-dai", 0);
+		if (!link->platforms->of_node) {
+			dev_err(card->dev, "%s: platform dai not found\n", link->name);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);
+		if (ret < 0) {
+			if (ret != -EPROBE_DEFER)
+				dev_err(card->dev, "%s: codec dai not found: %d\n",
+					link->name, ret);
+			goto err;
+		}
+
+		/* DPCM backend */
+		link->no_pcm = 1;
+		link->ignore_pmdown_time = 1;
+		link->ignore_suspend = 1;
+
+		link->stream_name = link->name;
+		snd_soc_dai_link_set_capabilities(link);
+		link++;
+
+		of_node_put(cpu);
+		of_node_put(codec);
+		of_node_put(platform);
+
+	}
+
+	return 0;
+err:
+	of_node_put(cpu);
+	of_node_put(codec);
+	of_node_put(platform);
+err_put_np:
+	of_node_put(np);
+	return ret;
+}
+
 static int sm8250_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 				     struct snd_pcm_hw_params *params)
 {
@@ -199,7 +334,12 @@  static int sm8250_platform_probe(struct platform_device *pdev)
 	card->dev = dev;
 	dev_set_drvdata(dev, card);
 	snd_soc_card_set_drvdata(card, data);
-	ret = qcom_snd_parse_of(card);
+	if (of_device_is_compatible(dev->of_node, "qcom,sm8250-audioreach-sndcard") ||
+		of_device_is_compatible(dev->of_node, "qcom,qrb5165-rb5-audioreach-sndcard"))
+		ret = qcom_audioreach_snd_parse_of(card);
+	else
+		ret = qcom_snd_parse_of(card);
+
 	if (ret)
 		return ret;
 
@@ -211,6 +351,8 @@  static int sm8250_platform_probe(struct platform_device *pdev)
 static const struct of_device_id snd_sm8250_dt_match[] = {
 	{.compatible = "qcom,sm8250-sndcard"},
 	{.compatible = "qcom,qrb5165-rb5-sndcard"},
+	{.compatible = "qcom,sm8250-audioreach-sndcard" },
+	{.compatible = "qcom,qrb5165-rb5-audioreach-sndcard" },
 	{}
 };