Re: [alsa-devel] [PATCH] ASoC: qcom: add sdm845 sound card support

Thanks Rohit for the patch!

On 18/06/18 12:16, Rohit kumar wrote:

This patch adds sdm845 audio machine driver support.

Signed-off-by: Rohit kumar rohitkr@codeaurora.org

.../devicetree/bindings/sound/qcom,sdm845.txt | 87 ++++ sound/soc/qcom/Kconfig | 9 + sound/soc/qcom/Makefile | 2 + sound/soc/qcom/sdm845.c | 539 +++++++++++++++++++++ 4 files changed, 637 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/qcom,sdm845.txt

Split the bindings into a separate patch!

create mode 100644 sound/soc/qcom/sdm845.c

diff --git a/Documentation/devicetree/bindings/sound/qcom,sdm845.txt b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt new file mode 100644 index 0000000..fc0e98c --- /dev/null +++ b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt @@ -0,0 +1,87 @@ +* Qualcomm Technologies Inc. SDM845 ASoC sound card driver

+This binding describes the SDM845 sound card, which uses qdsp for audio.

+- compatible:

• Usage: required
• Value type: <stringlist>
• Definition: must be "qcom,sdm845-sndcard"

+- qcom,audio-routing:

• Usage: Optional
• Value type: <stringlist>
• Definition: A list of the connections between audio components.

•   Each entry is a pair of strings, the first being the
  

•   connection's sink, the second being the connection's
  

•   source. Valid names could be power supplies, MicBias
  

•   of codec and the jacks on the board.
  

+- cdc-vdd-supply:

• Usage: Optional
• Value type: <phandle>
• Definition: phandle of regulator supply required for codec vdd.

+= dailinks +Each subnode of sndcard represents either a dailink, and subnodes of each +dailinks would be cpu/codec/platform dais.

+- link-name:

• Usage: required
• Value type: <string>
• Definition: User friendly name for dai link

+= CPU, PLATFORM, CODEC dais subnodes +- cpu:

• Usage: required
• Value type: <subnode>
• Definition: cpu dai sub-node

+- codec:

• Usage: required
• Value type: <subnode>
• Definition: codec dai sub-node

+- platform:

• Usage: opional

Optional

• Value type: <subnode>
• Definition: platform dai sub-node

+- sound-dai:

• Usage: required
• Value type: <phandle>
• Definition: dai phandle/s and port of CPU/CODEC/PLATFORM node.

+Example:

+audio {

• compatible = "qcom,sdm845-sndcard";
• qcom,model = "sdm845-snd-card";
• pinctrl-names = "pri_active", "pri_sleep";
• pinctrl-0 = <&pri_mi2s_active &pri_mi2s_ws_active>;
• pinctrl-1 = <&pri_mi2s_sleep &pri_mi2s_ws_sleep>;
• qcom,audio-routing = "PRI_MI2S_RX Audio Mixer", "Pri-mi2s-gpio";
• cdc-vdd-supply = <&pm8998_l14>;
• fe@1 {

Lets not use fe or be reference here, its just a link.

• link-name = "MultiMedia1";
  

• cpu {
  

• 	sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
  

• };
  

• platform {
  

• 	sound-dai = <&q6asmdai>;
  

• };
  

asmdai has sound-cell specifier as 1, so this will dtc will throw warning for this.

have a look at how 8996 is done.

• };
• be@1 {

• link-name = "PRI MI2S Playback";
  

• cpu {
  

• 	sound-dai = <&q6afedai PRIMARY_MI2S_RX>;
  

• };
  

• platform {
  

• 	sound-dai = <&q6routing>;
  

• };
  

• };

+}; diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index 87838fa..09de50d 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -90,3 +90,12 @@ config SND_SOC_MSM8996 Support for Qualcomm Technologies LPASS audio block in APQ8096 SoC-based systems. Say Y if you want to use audio device on this SoCs

+config SND_SOC_SDM845

• tristate "SoC Machine driver for SDM845 boards"
• depends on QCOM_APR
• select SND_SOC_QDSP6
• help

•  To add support for audio on Qualcomm Technologies Inc.
  

•  SDM845 SoC-based systems.
  

•      Say Y if you want to use audio device on this SoCs
  

diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile index 206945b..ac9609e 100644 --- a/sound/soc/qcom/Makefile +++ b/sound/soc/qcom/Makefile @@ -14,10 +14,12 @@ obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o snd-soc-storm-objs := storm.o snd-soc-apq8016-sbc-objs := apq8016_sbc.o snd-soc-apq8096-objs := apq8096.o +snd-soc-sdm845-objs := sdm845.o

obj-$(CONFIG_SND_SOC_STORM) += snd-soc-storm.o obj-$(CONFIG_SND_SOC_APQ8016_SBC) += snd-soc-apq8016-sbc.o obj-$(CONFIG_SND_SOC_MSM8996) += snd-soc-apq8096.o ++obj-$(CONFIG_SND_SOC_SDM845) += snd-soc-sdm845.o

?? looks like typo here.

#DSP lib obj-$(CONFIG_SND_SOC_QDSP6) += qdsp6/ diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c new file mode 100644 index 0000000..70d2611 --- /dev/null +++ b/sound/soc/qcom/sdm845.c @@ -0,0 +1,539 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• • Copyright (c) 2018, The Linux Foundation. All rights reserved.
• */

+#include <linux/module.h> +#include <linux/component.h> +#include <linux/platform_device.h> +#include <linux/regulator/consumer.h> +#include <linux/atomic.h>

+#include <linux/of_device.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <linux/soc/qcom/apr.h> +#include "qdsp6/q6afe.h"

+#define DEFAULT_SAMPLE_RATE_48K 48000 +#define DEFAULT_MCLK_RATE 24576000 +#define DEFAULT_BCLK_RATE 1536000

+struct sdm845_snd_data {

• struct snd_soc_card *card;
• struct regulator *vdd_supply;
• struct snd_soc_dai_link dai_link[];

+};

+static struct mutex pri_mi2s_res_lock; +static struct mutex quat_tdm_res_lock; +static atomic_t pri_mi2s_clk_count; +static atomic_t quat_tdm_clk_count;

+static unsigned int tdm_slot_offset[8] = {0, 4, 8, 12, 16, 20, 24, 28}; +static int sdm845_snd_hw_params(struct snd_pcm_substream *substream,

• 			struct snd_pcm_hw_params *params)
  

+{

• struct snd_soc_pcm_runtime *rtd = substream->private_data;
• struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
• int ret = 0;
• switch (cpu_dai->id) {
• case QUATERNARY_TDM_RX_0:
• case QUATERNARY_TDM_TX_0:

• ret = sdm845_tdm_snd_hw_params(substream, params);
  

• break;
  

• default:

• pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
  

• break;
  

• }
• return ret;

+}

+static int sdm845_snd_startup(struct snd_pcm_substream *substream) +{

• unsigned int fmt = SND_SOC_DAIFMT_CBS_CFS;
• struct snd_soc_pcm_runtime *rtd = substream->private_data;
• struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
• pr_debug("%s: dai_id: 0x%x\n", __func__, cpu_dai->id);
• switch (cpu_dai->id) {
• case PRIMARY_MI2S_RX:
• case PRIMARY_MI2S_TX:

• mutex_lock(&pri_mi2s_res_lock);
  

Mutex and atomic variables looks redundant here. Can you explain why do you need both?

• if (atomic_inc_return(&pri_mi2s_clk_count) == 1) { > +			snd_soc_dai_set_sysclk(cpu_dai,
  

• 		Q6AFE_LPASS_CLK_ID_MCLK_1,
  

• 		DEFAULT_MCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
  

• 	snd_soc_dai_set_sysclk(cpu_dai,
  

• 		Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT,
  

• 		DEFAULT_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
  

• }
  

• mutex_unlock(&pri_mi2s_res_lock);
  

• snd_soc_dai_set_fmt(cpu_dai, fmt);
  

• break;
  

• case QUATERNARY_TDM_RX_0:
• case QUATERNARY_TDM_TX_0:

• mutex_lock(&quat_tdm_res_lock);
  

• if (atomic_inc_return(&quat_tdm_clk_count) == 1) {
  

• 	snd_soc_dai_set_sysclk(cpu_dai,
  

• 		Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT,
  

• 		DEFAULT_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
  

• }
  

• mutex_unlock(&quat_tdm_res_lock);
  

• break;
  

• default:

• pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
  

• break;
  

• }
• return 0;

+}

...

+static int sdm845_sbc_parse_of(struct snd_soc_card *card) +{

• struct device *dev = card->dev;
• struct snd_soc_dai_link *link;
• struct device_node *np, *codec, *platform, *cpu, *node;
• int ret, num_links;
• struct sdm845_snd_data *data;
• ret = snd_soc_of_parse_card_name(card, "qcom,model");
• if (ret) {

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

• return ret;
  

• }
• node = dev->of_node;
• /* DAPM routes */
• if (of_property_read_bool(node, "qcom,audio-routing")) {

• ret = snd_soc_of_parse_audio_routing(card,
  

• 			"qcom,audio-routing");
  

• if (ret)
  

• 	return ret;
  

• }
• /* Populate links */
• num_links = of_get_child_count(node);
• dev_info(dev, "Found %d child audio dai links..\n", num_links);

Looks unnessary!

• /* Allocate the private data and the DAI link array */
• data = kzalloc(sizeof(*data) + sizeof(*link) * num_links,

• 	GFP_KERNEL);
  

• if (!data)

• return -ENOMEM;
  

• card->dai_link = &data->dai_link[0];
• card->num_links = num_links;
• card->dapm_widgets = sdm845_widgets;
• card->num_dapm_widgets = ARRAY_SIZE(sdm845_widgets);
• link = data->dai_link;
• data->card = card;
• for_each_child_of_node(node, 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, "Can't find cpu DT node\n");
  

• 	ret = -EINVAL;
  

• 	goto fail;
  

• }
  

• link->cpu_of_node = of_parse_phandle(cpu, "sound-dai", 0);
  

• if (!link->cpu_of_node) {
  

• 	dev_err(card->dev, "error getting cpu phandle\n");
  

• 	ret = -EINVAL;
  

• 	goto fail;
  

• }
  

• link->platform_of_node = of_parse_phandle(platform,
  

• 		"sound-dai", 0);
  

• if (!link->platform_of_node) {
  

• 	dev_err(card->dev, "error getting platform phandle\n");
  

• 	ret = -EINVAL;
  

• 	goto fail;
  

• }
  

• ret = snd_soc_of_get_dai_name(cpu, &link->cpu_dai_name);
  

• if (ret) {
  

• 	dev_err(card->dev, "error getting cpu dai name\n");
  

• 	goto fail;
  

• }
  

• if (codec) {
  

• 	ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);
  

• 	if (ret < 0) {
  

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

• 		goto fail;
  

• 	}
  

• 	link->no_pcm = 1;
  

• 	link->ignore_suspend = 1;
  

• 	link->ignore_pmdown_time = 1;
  

• 	link->ops = &sdm845_be_ops;
  

• 	link->be_hw_params_fixup = sdm845_be_hw_params_fixup;
  

• } else {
  

• 	link->dynamic = 1;
  

• 	link->codec_dai_name = "snd-soc-dummy-dai";
  

• 	link->codec_name = "snd-soc-dummy";
  

• }
  

You could optimize this code, have a look at apq8096.c which does exactly same thing.

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

• if (ret) {
  

• 	dev_err(card->dev,
  

• 		"error getting codec dai_link name\n");
  

• 	goto fail;
  

• }
  

• link->dpcm_playback = 1;
  

• link->dpcm_capture = 1;
  

• link->stream_name = link->name;
  

• link++;
  

• }
• dev_set_drvdata(dev, card);
• snd_soc_card_set_drvdata(card, data);
• return ret;

+fail:

• kfree(data);
• return ret;

+}

...

+static void sdm845_unbind(struct device *dev) +{

• struct snd_soc_card *card = dev_get_drvdata(dev);
• struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
• mutex_destroy(&pri_mi2s_res_lock);
• mutex_destroy(&quat_tdm_res_lock);
• if (data->vdd_supply)

• regulator_put(data->vdd_supply);
  

• component_unbind_all(dev, card);
• snd_soc_unregister_card(card);
• kfree(data);
• kfree(card);

+}

+static const struct component_master_ops sdm845_ops = {

• .bind = sdm845_bind,
• .unbind = sdm845_unbind,

+};

+static int sdm845_runtime_resume(struct device *dev) +{

• struct snd_soc_card *card = dev_get_drvdata(dev);
• struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
• if (!data->vdd_supply) {

• dev_dbg(dev, "no supplies defined\n");
  

• return 0;
  

• }
• if (regulator_enable(data->vdd_supply))

• dev_err(dev, "Enable regulator supply failed\n");
  

• return 0;

+}

+static struct platform_driver sdm845_snd_driver = {

• .probe = sdm845_snd_platform_probe,
• .remove = sdm845_snd_platform_remove,
• .driver = {

• .name = "msm-snd-sdm845",
  

• .pm = &sdm845_pm_ops,
  

• .owner = THIS_MODULE,
  

not necessary!

• .of_match_table = of_match_ptr(sdm845_snd_device_id),
  

• },

+}; +module_platform_driver(sdm845_snd_driver);

+MODULE_DESCRIPTION("sdm845 ASoC Machine Driver"); +MODULE_LICENSE("GPL v2");