lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1bbdae54-7524-8047-f0de-061df3288bc6@linaro.org>
Date:   Thu, 15 Jul 2021 11:32:45 +0100
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        bjorn.andersson@...aro.org, broonie@...nel.org, robh@...nel.org
Cc:     devicetree@...r.kernel.org, alsa-devel@...a-project.org,
        bgoswami@...eaurora.org, lgirdwood@...il.com, tiwai@...e.de,
        plai@...eaurora.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 16/16] ASoC: qcom: sm8250: Add audioreach support



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@...aro.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;
>> +}
>> +
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ