[PATCH v3 0/5] Add support for audio on SDM845 SoC

[PATCH v3 0/5] Add support for audio on SDM845 SoC

[Rohit kumar]

This provides initial patchset to support audio on
Qualcomm Techonologies Inc. SDM845 SoC. Currently, it supports
audio playback/capture over Primary MI2S and Quaternary
TDM ports.
Changes since v2:
- Added new file to parse dai-links from device tree
  and use it in sdm845 and apq8096 machine driver.
- Removed qcom prefix from device tree properties as
  suggested by Robb.
- Updated apq8096 machine driver to use common APIs.

Rohit kumar (5):
  ASoC: qcom: dt-bindings: Add sdm845 machine bindings
  ASoC: qcom: Add support to parse common audio device nodes
  ASoC: qcom: add sdm845 sound card support
  ASoC: dt-bindings: Update dt binding name for apq8096
  ASoC: qcom: apq8096: Use common APIs to parse device nodes

 .../devicetree/bindings/sound/qcom,apq8096.txt     |  15 +-
 .../devicetree/bindings/sound/qcom,sdm845.txt      |  80 +++++
 sound/soc/qcom/Kconfig                             |  14 +
 sound/soc/qcom/Makefile                            |   3 +
 sound/soc/qcom/apq8096.c                           | 151 +-------
 sound/soc/qcom/common.c                            | 150 ++++++++
 sound/soc/qcom/common.h                            |  14 +
 sound/soc/qcom/sdm845.c                            | 390 +++++++++++++++++++++
 8 files changed, 674 insertions(+), 143 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,sdm845.txt
 create mode 100644 sound/soc/qcom/common.c
 create mode 100644 sound/soc/qcom/common.h
 create mode 100644 sound/soc/qcom/sdm845.c

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v3 1/5] ASoC: qcom: dt-bindings: Add sdm845 machine bindings

[Rohit kumar]

Add devicetree bindings documentation file for SDM845 sound card.

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 .../devicetree/bindings/sound/qcom,sdm845.txt      | 80 ++++++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,sdm845.txt

diff --git a/Documentation/devicetree/bindings/sound/qcom,sdm845.txt b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt
new file mode 100644
index 0000000..408c483
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt
@@ -0,0 +1,80 @@
+* 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"
+
+- 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.
+
+- model:
+	Usage: required
+	Value type: <stringlist>
+	Definition: The user-visible name of this sound card.
+
+= 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: 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";
+	model = "sdm845-snd-card";
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&pri_mi2s_active &pri_mi2s_ws_active>;
+	pinctrl-1 = <&pri_mi2s_sleep &pri_mi2s_ws_sleep>;
+
+	mm1-dai-link {
+		link-name = "MultiMedia1";
+		cpu {
+			sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
+		};
+	};
+
+	pri-mi2s-dai-link {
+		link-name = "PRI MI2S Playback";
+		cpu {
+			sound-dai = <&q6afedai PRIMARY_MI2S_RX>;
+		};
+
+		platform {
+			sound-dai = <&q6routing>;
+		};
+	};
+};
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v3 2/5] ASoC: qcom: Add support to parse common audio device nodes

[Rohit kumar]

This adds support to parse cpu, platform and codec
device nodes and add them in dai-links. Also, add
API to add slave components associated with machine
driver.

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 sound/soc/qcom/Kconfig  |   3 +
 sound/soc/qcom/Makefile |   1 +
 sound/soc/qcom/common.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/qcom/common.h |  14 +++++
 4 files changed, 168 insertions(+)
 create mode 100644 sound/soc/qcom/common.c
 create mode 100644 sound/soc/qcom/common.h

diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 87838fa..0e364b4 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -90,3 +90,6 @@ 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_QCOM_COMMON
+	tristate
diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile
index 206945b..1bcdbee 100644
--- a/sound/soc/qcom/Makefile
+++ b/sound/soc/qcom/Makefile
@@ -19,5 +19,6 @@ 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_QCOM_COMMON) += common.o
 #DSP lib
 obj-$(CONFIG_SND_SOC_QDSP6) += qdsp6/
diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
new file mode 100644
index 0000000..be62fca
--- /dev/null
+++ b/sound/soc/qcom/common.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018, Linaro Limited.
+// Copyright (c) 2018, The Linux Foundation. All rights reserved.
+
+#include "common.h"
+
+int qcom_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;
+	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 = kcalloc(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) {
+		cpu = of_get_child_by_name(np, "cpu");
+		if (!cpu) {
+			dev_err(dev, "Can't find cpu DT node\n");
+			ret = -EINVAL;
+			goto err;
+		}
+
+		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 err;
+		}
+
+		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 err;
+		}
+
+		platform = of_get_child_by_name(np, "platform");
+		codec = of_get_child_by_name(np, "codec");
+		if (codec && platform) {
+			link->platform_of_node = of_parse_phandle(platform,
+					"sound-dai",
+					0);
+			if (!link->platform_of_node) {
+				dev_err(card->dev, "platform dai not found\n");
+				ret = -EINVAL;
+				goto err;
+			}
+
+			ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);
+			if (ret < 0) {
+				dev_err(card->dev, "codec dai not found\n");
+				goto err;
+			}
+			link->no_pcm = 1;
+			link->ignore_pmdown_time = 1;
+		} else {
+			link->platform_of_node = link->cpu_of_node;
+			link->codec_dai_name = "snd-soc-dummy-dai";
+			link->codec_name = "snd-soc-dummy";
+			link->dynamic = 1;
+		}
+
+		link->ignore_suspend = 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;
+		}
+
+		link->dpcm_playback = 1;
+		link->dpcm_capture = 1;
+		link->stream_name = link->name;
+		link++;
+	}
+
+	return 0;
+err:
+	of_node_put(cpu);
+	of_node_put(codec);
+	of_node_put(platform);
+	kfree(card->dai_link);
+	return ret;
+}
+EXPORT_SYMBOL(qcom_snd_parse_of);
+
+static int qcom_snd_compare_of(struct device *dev, void *data)
+{
+	return dev->of_node == data;
+}
+
+static void qcom_snd_release_of(struct device *dev, void *data)
+{
+	of_node_put(data);
+}
+
+int qcom_snd_add_components(struct device *dev, struct component_match **mptr)
+{
+	struct device_node *np, *platform, *cpu, *node, *dai_node;
+
+	node = dev->of_node;
+
+	for_each_child_of_node(node, np) {
+		cpu = of_get_child_by_name(np, "cpu");
+		if (cpu) {
+			dai_node = of_parse_phandle(cpu, "sound-dai", 0);
+			of_node_get(dai_node);
+			component_match_add_release(dev, mptr,
+					qcom_snd_release_of,
+					qcom_snd_compare_of,
+					dai_node);
+		}
+
+		platform = of_get_child_by_name(np, "platform");
+		if (platform) {
+			dai_node = of_parse_phandle(platform, "sound-dai", 0);
+			component_match_add_release(dev, mptr,
+					qcom_snd_release_of,
+					qcom_snd_compare_of,
+					dai_node);
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(qcom_snd_add_components);
diff --git a/sound/soc/qcom/common.h b/sound/soc/qcom/common.h
new file mode 100644
index 0000000..860c235
--- /dev/null
+++ b/sound/soc/qcom/common.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// Copyright (c) 2018, The Linux Foundation. All rights reserved.
+
+#ifndef __QCOM_SND_COMMON_H__
+#define __QCOM_SND_COMMON_H__
+
+#include <linux/component.h>
+#include <sound/soc.h>
+
+int qcom_snd_parse_of(struct snd_soc_card *card);
+int qcom_snd_add_components(struct device *dev,
+			    struct component_match **matchptr);
+
+#endif
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Rohit kumar]

This patch adds sdm845 audio machine driver support.

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 sound/soc/qcom/Kconfig  |  10 ++
 sound/soc/qcom/Makefile |   2 +
 sound/soc/qcom/sdm845.c | 390 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 402 insertions(+)
 create mode 100644 sound/soc/qcom/sdm845.c

diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 0e364b4..cf55f0a 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -93,3 +93,13 @@ config SND_SOC_MSM8996
 
 config SND_SOC_QCOM_COMMON
 	tristate
+
+config SND_SOC_SDM845
+	tristate "SoC Machine driver for SDM845 boards"
+	select SND_SOC_QDSP6
+	select SND_SOC_QCOM_COMMON
+	default n
+	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 1bcdbee..c9699fb 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
 
 obj-$(CONFIG_SND_SOC_QCOM_COMMON) += common.o
 #DSP lib
diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
new file mode 100644
index 0000000..9ee3558
--- /dev/null
+++ b/sound/soc/qcom/sdm845.c
@@ -0,0 +1,390 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/module.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 <linux/soc/qcom/apr.h>
+#include "common.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;
+	uint32_t pri_mi2s_clk_count;
+	uint32_t quat_tdm_clk_count;
+};
+
+
+static unsigned int tdm_slot_offset[8] = {0, 4, 8, 12, 16, 20, 24, 28};
+
+static int sdm845_tdm_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;
+	int channels, slot_width;
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S32_LE:
+	case SNDRV_PCM_FORMAT_S24_LE:
+	case SNDRV_PCM_FORMAT_S16_LE:
+		slot_width = 32;
+		break;
+	default:
+		dev_err(rtd->dev, "%s: invalid param format 0x%x\n",
+				__func__, params_format(params));
+		return -EINVAL;
+	}
+
+	channels = params_channels(params);
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0x3,
+				channels, slot_width);
+		if (ret < 0) {
+			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
+					__func__, ret);
+			goto end;
+		}
+
+		ret = snd_soc_dai_set_channel_map(cpu_dai, 0, NULL,
+				channels, tdm_slot_offset);
+		if (ret < 0) {
+			dev_err(rtd->dev, "%s: failed to set channel map, err:%d\n",
+					__func__, ret);
+			goto end;
+		}
+	} else {
+		ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0xf, 0,
+				channels, slot_width);
+		if (ret < 0) {
+			dev_err(rtd->dev, "%s: failed to set tdm slot, err:%d\n",
+					__func__, ret);
+			goto end;
+		}
+
+		ret = snd_soc_dai_set_channel_map(cpu_dai, channels,
+				tdm_slot_offset, 0, NULL);
+		if (ret < 0) {
+			dev_err(rtd->dev, "%s: failed to set channel map, err:%d\n",
+					__func__, ret);
+			goto end;
+		}
+	}
+end:
+	return ret;
+}
+
+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_card *card = rtd->card;
+	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+
+	switch (cpu_dai->id) {
+	case PRIMARY_MI2S_RX:
+	case PRIMARY_MI2S_TX:
+		if (++(data->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);
+		}
+		snd_soc_dai_set_fmt(cpu_dai, fmt);
+		break;
+
+	case QUATERNARY_TDM_RX_0:
+	case QUATERNARY_TDM_TX_0:
+		if (++(data->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);
+		}
+		break;
+
+	default:
+		pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
+		break;
+	}
+	return 0;
+}
+
+static void  sdm845_snd_shutdown(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+
+	switch (cpu_dai->id) {
+	case PRIMARY_MI2S_RX:
+	case PRIMARY_MI2S_TX:
+		if (--(data->pri_mi2s_clk_count) == 0) {
+			snd_soc_dai_set_sysclk(cpu_dai,
+				Q6AFE_LPASS_CLK_ID_MCLK_1,
+				0, SNDRV_PCM_STREAM_PLAYBACK);
+			snd_soc_dai_set_sysclk(cpu_dai,
+				Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT,
+				0, SNDRV_PCM_STREAM_PLAYBACK);
+		};
+		break;
+
+	case QUATERNARY_TDM_RX_0:
+	case QUATERNARY_TDM_TX_0:
+		if (--(data->quat_tdm_clk_count) == 0) {
+			snd_soc_dai_set_sysclk(cpu_dai,
+				Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT,
+				0, SNDRV_PCM_STREAM_PLAYBACK);
+		}
+		break;
+
+	default:
+		pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
+		break;
+	}
+}
+
+static struct snd_soc_ops sdm845_be_ops = {
+	.hw_params = sdm845_snd_hw_params,
+	.startup = sdm845_snd_startup,
+	.shutdown = sdm845_snd_shutdown,
+};
+
+static int sdm845_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
+				struct snd_pcm_hw_params *params)
+{
+	struct snd_interval *rate = hw_param_interval(params,
+					SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval *channels = hw_param_interval(params,
+					SNDRV_PCM_HW_PARAM_CHANNELS);
+
+	rate->min = rate->max = DEFAULT_SAMPLE_RATE_48K;
+	channels->min = channels->max = 2;
+
+	return 0;
+}
+
+static void sdm845_init_supplies(struct device *dev)
+{
+	struct snd_soc_card *card = dev_get_drvdata(dev);
+	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
+
+	data->vdd_supply = regulator_get(dev, "cdc-vdd");
+	if (IS_ERR(data->vdd_supply)) {
+		dev_err(dev, "Unable to get regulator supplies\n");
+		data->vdd_supply = NULL;
+		return;
+	}
+
+	if (regulator_enable(data->vdd_supply))
+		dev_err(dev, "Unable to enable vdd supply\n");
+}
+
+static void sdm845_deinit_supplies(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)
+		return;
+
+	regulator_disable(data->vdd_supply);
+	regulator_put(data->vdd_supply);
+}
+
+static void sdm845_add_be_ops(struct snd_soc_card *card)
+{
+	struct snd_soc_dai_link *link = card->dai_link;
+	int i, num_links = card->num_links;
+
+	for (i = 0; i < num_links; i++) {
+		if (link->no_pcm == 1) {
+			link->ops = &sdm845_be_ops;
+			link->be_hw_params_fixup = sdm845_be_hw_params_fixup;
+		}
+		link++;
+	}
+}
+
+static int sdm845_bind(struct device *dev)
+{
+	struct snd_soc_card *card;
+	struct sdm845_snd_data *data;
+	int ret;
+
+	card = kzalloc(sizeof(*card), GFP_KERNEL);
+	if (!card)
+		return -ENOMEM;
+
+	/* Allocate the private data */
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	ret = component_bind_all(dev, card);
+	if (ret) {
+		dev_err(dev, "Audio components bind failed: %d\n", ret);
+		goto bind_fail;
+	}
+
+	dev_set_drvdata(dev, card);
+	card->dev = dev;
+	ret = qcom_snd_parse_of(card);
+	if (ret) {
+		dev_err(dev, "Error parsing OF data\n");
+		goto parse_dt_fail;
+	}
+
+	data->card = card;
+	snd_soc_card_set_drvdata(card, data);
+
+	sdm845_add_be_ops(card);
+
+	sdm845_init_supplies(dev);
+
+	ret = snd_soc_register_card(card);
+	if (ret) {
+		dev_err(dev, "Sound card registration failed\n");
+		goto register_card_fail;
+	}
+	return ret;
+
+register_card_fail:
+	sdm845_deinit_supplies(dev);
+	kfree(card->dai_link);
+parse_dt_fail:
+	component_unbind_all(dev, card);
+bind_fail:
+	kfree(data);
+	kfree(card);
+	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);
+
+	if (data->vdd_supply)
+		regulator_put(data->vdd_supply);
+	component_unbind_all(dev, card);
+	snd_soc_unregister_card(card);
+	kfree(card->dai_link);
+	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 int sdm845_runtime_suspend(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_disable(data->vdd_supply))
+		dev_err(dev, "Disable regulator supply failed\n");
+
+	return 0;
+}
+
+static const struct dev_pm_ops sdm845_pm_ops = {
+	SET_RUNTIME_PM_OPS(sdm845_runtime_suspend,
+			sdm845_runtime_resume, NULL)
+};
+
+static int sdm845_snd_platform_probe(struct platform_device *pdev)
+{
+	struct component_match *match = NULL;
+	int ret;
+
+	ret = qcom_snd_add_components(&pdev->dev, &match);
+	if (ret)
+		return ret;
+
+	return component_master_add_with_match(&pdev->dev, &sdm845_ops, match);
+}
+
+static int sdm845_snd_platform_remove(struct platform_device *pdev)
+{
+	component_master_del(&pdev->dev, &sdm845_ops);
+	return 0;
+}
+
+static const struct of_device_id sdm845_snd_device_id[]  = {
+	{ .compatible = "qcom,sdm845-sndcard" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sdm845_snd_device_id);
+
+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,
+		.of_match_table = sdm845_snd_device_id,
+	},
+};
+module_platform_driver(sdm845_snd_driver);
+
+MODULE_DESCRIPTION("sdm845 ASoC Machine Driver");
+MODULE_LICENSE("GPL v2");
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v3 4/5] ASoC: dt-bindings: Update dt binding name for apq8096

[Rohit kumar]

Remove qcom prefix from machine driver dt bindings of
apq8096 SoC.

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 Documentation/devicetree/bindings/sound/qcom,apq8096.txt | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8096.txt b/Documentation/devicetree/bindings/sound/qcom,apq8096.txt
index c7600a9..c814e86 100644
--- a/Documentation/devicetree/bindings/sound/qcom,apq8096.txt
+++ b/Documentation/devicetree/bindings/sound/qcom,apq8096.txt
@@ -7,7 +7,7 @@ This binding describes the APQ8096 sound card, which uses qdsp for audio.
 	Value type: <stringlist>
 	Definition: must be "qcom,apq8096-sndcard"
 
-- qcom,audio-routing:
+- audio-routing:
 	Usage: Optional
 	Value type: <stringlist>
 	Definition:  A list of the connections between audio components.
@@ -49,6 +49,12 @@ This binding describes the APQ8096 sound card, which uses qdsp for audio.
 			"DMIC1"
 			"DMIC2"
 			"DMIC3"
+
+- model:
+	Usage: required
+	Value type: <stringlist>
+	Definition: The user-visible name of this sound card.
+
 = dailinks
 Each subnode of sndcard represents either a dailink, and subnodes of each
 dailinks would be cpu/codec/platform dais.
@@ -79,11 +85,16 @@ dailinks would be cpu/codec/platform dais.
 	Value type: <phandle with arguments>
 	Definition: dai phandle/s and port of CPU/CODEC/PLATFORM node.
 
+Obsolete:
+	qcom,model: String for soundcard name (Use model instead)
+	qcom,audio-routing: A list of the connections between audio components.
+			    (Use audio-routing instead)
+
 Example:
 
 audio {
 	compatible = "qcom,apq8096-sndcard";
-	qcom,model = "DB820c";
+	model = "DB820c";
 
 	mm1-dai-link {
 		link-name = "MultiMedia1";
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v3 5/5] ASoC: qcom: apq8096: Use common APIs to parse device nodes

[Rohit kumar]

Use generic APIs exposed by common.c for parsing
dai link device tree nodes and adding slave components.

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 sound/soc/qcom/Kconfig   |   1 +
 sound/soc/qcom/apq8096.c | 151 ++++-------------------------------------------
 2 files changed, 11 insertions(+), 141 deletions(-)

diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index cf55f0a..2be9cee 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -86,6 +86,7 @@ config SND_SOC_MSM8996
 	tristate "SoC Machine driver for MSM8996 and APQ8096 boards"
 	depends on QCOM_APR
 	select SND_SOC_QDSP6
+	select SND_SOC_QCOM_COMMON
 	help
           Support for Qualcomm Technologies LPASS audio block in
           APQ8096 SoC-based systems.
diff --git a/sound/soc/qcom/apq8096.c b/sound/soc/qcom/apq8096.c
index cab8c4f..726fb73 100644
--- a/sound/soc/qcom/apq8096.c
+++ b/sound/soc/qcom/apq8096.c
@@ -9,6 +9,7 @@
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
 #include <sound/pcm.h>
+#include "common.h"
 
 static int apq8096_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 				      struct snd_pcm_hw_params *params)
@@ -24,109 +25,16 @@ static int apq8096_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 	return 0;
 }
 
-static int apq8096_sbc_parse_of(struct snd_soc_card *card)
+static void apq8096_add_be_ops(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;
-	int ret, num_links;
-
-	ret = snd_soc_of_parse_card_name(card, "qcom,model");
-	if (ret) {
-		dev_err(dev, "Error parsing card name: %d\n", ret);
-		return ret;
-	}
+	struct snd_soc_dai_link *link = card->dai_link;
+	int i, num_links = card->num_links;
 
-	/* DAPM routes */
-	if (of_property_read_bool(dev->of_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(dev->of_node);
-
-	/* Allocate the DAI link array */
-	card->dai_link = kcalloc(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) {
-		cpu = of_get_child_by_name(np, "cpu");
-		if (!cpu) {
-			dev_err(dev, "Can't find cpu DT node\n");
-			ret = -EINVAL;
-			goto err;
-		}
-
-		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 err;
-		}
-
-		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 err;
-		}
-
-		platform = of_get_child_by_name(np, "platform");
-		codec = of_get_child_by_name(np, "codec");
-		if (codec && platform) {
-			link->platform_of_node = of_parse_phandle(platform,
-								  "sound-dai",
-								   0);
-			if (!link->platform_of_node) {
-				dev_err(card->dev, "platform dai not found\n");
-				ret = -EINVAL;
-				goto err;
-			}
-
-			ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);
-			if (ret < 0) {
-				dev_err(card->dev, "codec dai not found\n");
-				goto err;
-			}
-			link->no_pcm = 1;
-			link->ignore_pmdown_time = 1;
+	for (i = 0; i < num_links; i++) {
+		if (link->no_pcm == 1)
 			link->be_hw_params_fixup = apq8096_be_hw_params_fixup;
-		} else {
-			link->platform_of_node = link->cpu_of_node;
-			link->codec_dai_name = "snd-soc-dummy-dai";
-			link->codec_name = "snd-soc-dummy";
-			link->dynamic = 1;
-		}
-
-		link->ignore_suspend = 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;
-		}
-
-		link->dpcm_playback = 1;
-		link->dpcm_capture = 1;
-		link->stream_name = link->name;
 		link++;
 	}
-
-	return 0;
-err:
-	of_node_put(cpu);
-	of_node_put(codec);
-	of_node_put(platform);
-	kfree(card->dai_link);
-	return ret;
 }
 
 static int apq8096_bind(struct device *dev)
@@ -141,12 +49,14 @@ static int apq8096_bind(struct device *dev)
 	component_bind_all(dev, card);
 	card->dev = dev;
 	dev_set_drvdata(dev, card);
-	ret = apq8096_sbc_parse_of(card);
+	ret = qcom_snd_parse_of(card);
 	if (ret) {
 		dev_err(dev, "Error parsing OF data\n");
 		goto err;
 	}
 
+	apq8096_add_be_ops(card);
+
 	ret = snd_soc_register_card(card);
 	if (ret)
 		goto err;
@@ -174,53 +84,12 @@ static void apq8096_unbind(struct device *dev)
 	.unbind = apq8096_unbind,
 };
 
-static int apq8016_compare_of(struct device *dev, void *data)
-{
-	return dev->of_node == data;
-}
-
-static void apq8016_release_of(struct device *dev, void *data)
-{
-	of_node_put(data);
-}
-
-static int add_audio_components(struct device *dev,
-				struct component_match **matchptr)
-{
-	struct device_node *np, *platform, *cpu, *node, *dai_node;
-
-	node = dev->of_node;
-
-	for_each_child_of_node(node, np) {
-		cpu = of_get_child_by_name(np, "cpu");
-		if (cpu) {
-			dai_node = of_parse_phandle(cpu, "sound-dai", 0);
-			of_node_get(dai_node);
-			component_match_add_release(dev, matchptr,
-						    apq8016_release_of,
-						    apq8016_compare_of,
-						    dai_node);
-		}
-
-		platform = of_get_child_by_name(np, "platform");
-		if (platform) {
-			dai_node = of_parse_phandle(platform, "sound-dai", 0);
-			component_match_add_release(dev, matchptr,
-						    apq8016_release_of,
-						    apq8016_compare_of,
-						    dai_node);
-		}
-	}
-
-	return 0;
-}
-
 static int apq8096_platform_probe(struct platform_device *pdev)
 {
 	struct component_match *match = NULL;
 	int ret;
 
-	ret = add_audio_components(&pdev->dev, &match);
+	ret = qcom_snd_add_components(&pdev->dev, &match);
 	if (ret)
 		return ret;
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3 4/5] ASoC: dt-bindings: Update dt binding name for apq8096

[Srinivas Kandagatla]

On 06/07/18 10:43, Rohit kumar wrote:
> Remove qcom prefix from machine driver dt bindings of
> apq8096 SoC.
Yes, this is a good move to make everything inline!

> 
> Signed-off-by: Rohit kumar<rohitkr@codeaurora.org>
> ---
>   Documentation/devicetree/bindings/sound/qcom,apq8096.txt | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)

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

Re: [PATCH v3 2/5] ASoC: qcom: Add support to parse common audio device nodes

[Srinivas Kandagatla]

On 06/07/18 10:43, Rohit kumar wrote:
> This adds support to parse cpu, platform and codec
> device nodes and add them in dai-links. Also, add
> API to add slave components associated with machine
> driver.
> 
> Signed-off-by: Rohit kumar<rohitkr@codeaurora.org>
> ---
>   sound/soc/qcom/Kconfig  |   3 +
>   sound/soc/qcom/Makefile |   1 +
>   sound/soc/qcom/common.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
>   sound/soc/qcom/common.h |  14 +++++
>   4 files changed, 168 insertions(+)
>   create mode 100644 sound/soc/qcom/common.c

common.c seems to miss

MODULE_LICENSE("GPL v2");

this will be an issue for snd card module auto-loading!

Also patch 5/5 can go with this.

Other than that it looks good.

Once these 2 fixed you can add my Ack

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

--srini

>   create mode 100644 sound/soc/qcom/common.h

Re: [alsa-devel] [PATCH v3 2/5] ASoC: qcom: Add support to parse common audio device nodes

[Vinod]

On 06-07-18, 18:51, Srinivas Kandagatla wrote:
> 
> 
> On 06/07/18 10:43, Rohit kumar wrote:
> > This adds support to parse cpu, platform and codec
> > device nodes and add them in dai-links. Also, add
> > API to add slave components associated with machine
> > driver.
> > 
> > Signed-off-by: Rohit kumar<rohitkr@codeaurora.org>
> > ---
> >   sound/soc/qcom/Kconfig  |   3 +
> >   sound/soc/qcom/Makefile |   1 +
> >   sound/soc/qcom/common.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++
> >   sound/soc/qcom/common.h |  14 +++++
> >   4 files changed, 168 insertions(+)
> >   create mode 100644 sound/soc/qcom/common.c
> 
> common.c seems to miss

Btw do you need a new Kconfig symbol for this?

Alternate option would be to just add common.o to wherever it is needed

-- 
~Vinod

Re: [alsa-devel] [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Vinod]

On 06-07-18, 15:13, Rohit kumar wrote:

> +static void sdm845_init_supplies(struct device *dev)
> +{
> +	struct snd_soc_card *card = dev_get_drvdata(dev);
> +	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
> +
> +	data->vdd_supply = regulator_get(dev, "cdc-vdd");
> +	if (IS_ERR(data->vdd_supply)) {
> +		dev_err(dev, "Unable to get regulator supplies\n");
> +		data->vdd_supply = NULL;
> +		return;
> +	}
> +
> +	if (regulator_enable(data->vdd_supply))
> +		dev_err(dev, "Unable to enable vdd supply\n");
> +}
> +
> +static void sdm845_deinit_supplies(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)
> +		return;
> +
> +	regulator_disable(data->vdd_supply);
> +	regulator_put(data->vdd_supply);
> +}

these two can be made generic, cant we make these common when we have
supplies present?

> +static int sdm845_bind(struct device *dev)
> +{
> +	struct snd_soc_card *card;
> +	struct sdm845_snd_data *data;
> +	int ret;
> +
> +	card = kzalloc(sizeof(*card), GFP_KERNEL);
> +	if (!card)
> +		return -ENOMEM;
> +
> +	/* Allocate the private data */
> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	ret = component_bind_all(dev, card);
> +	if (ret) {
> +		dev_err(dev, "Audio components bind failed: %d\n", ret);
> +		goto bind_fail;
> +	}
> +
> +	dev_set_drvdata(dev, card);
> +	card->dev = dev;
> +	ret = qcom_snd_parse_of(card);
> +	if (ret) {
> +		dev_err(dev, "Error parsing OF data\n");
> +		goto parse_dt_fail;
> +	}
> +
> +	data->card = card;
> +	snd_soc_card_set_drvdata(card, data);
> +
> +	sdm845_add_be_ops(card);
> +
> +	sdm845_init_supplies(dev);
> +
> +	ret = snd_soc_register_card(card);
> +	if (ret) {
> +		dev_err(dev, "Sound card registration failed\n");
> +		goto register_card_fail;
> +	}
> +	return ret;
> +
> +register_card_fail:
> +	sdm845_deinit_supplies(dev);
> +	kfree(card->dai_link);
> +parse_dt_fail:
> +	component_unbind_all(dev, card);
> +bind_fail:
> +	kfree(data);
> +	kfree(card);
> +	return ret;
> +}

I would make a case for this to be moved into common too :)
-- 
~Vinod

Re: [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Srinivas Kandagatla]

Minor Nits...

On 06/07/18 10:43, Rohit kumar wrote:
> +struct sdm845_snd_data {
> +	struct snd_soc_card *card;
> +	struct regulator *vdd_supply;
> +	uint32_t pri_mi2s_clk_count;
> +	uint32_t quat_tdm_clk_count;
> +};
> +
> +
Unnecessary extra line..
> +static unsigned int tdm_slot_offset[8] = {0, 4, 8, 12, 16, 20, 24, 28};
> +
> 
> +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,
setting owner is not required here!

> +		.of_match_table = sdm845_snd_device_id,
> +	},
> +};

Re: [alsa-devel] [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Rohit Kumar]

Thanks Vinod for reviewing.


On 7/9/2018 1:18 PM, Vinod wrote:
> On 06-07-18, 15:13, Rohit kumar wrote:
>
>> +static void sdm845_init_supplies(struct device *dev)
>> +{
>> +	struct snd_soc_card *card = dev_get_drvdata(dev);
>> +	struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
>> +
>> +	data->vdd_supply = regulator_get(dev, "cdc-vdd");
>> +	if (IS_ERR(data->vdd_supply)) {
>> +		dev_err(dev, "Unable to get regulator supplies\n");
>> +		data->vdd_supply = NULL;
>> +		return;
>> +	}
>> +
>> +	if (regulator_enable(data->vdd_supply))
>> +		dev_err(dev, "Unable to enable vdd supply\n");
>> +}
>> +
>> +static void sdm845_deinit_supplies(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)
>> +		return;
>> +
>> +	regulator_disable(data->vdd_supply);
>> +	regulator_put(data->vdd_supply);
>> +}
> these two can be made generic, cant we make these common when we have
> supplies present?

Actually we  need to move it to codec driver as suggested by Rob in v2 
patchset.
I will remove this in next spin.

>> +static int sdm845_bind(struct device *dev)
>> +{
>> +	struct snd_soc_card *card;
>> +	struct sdm845_snd_data *data;
>> +	int ret;
>> +
>> +	card = kzalloc(sizeof(*card), GFP_KERNEL);
>> +	if (!card)
>> +		return -ENOMEM;
>> +
>> +	/* Allocate the private data */
>> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	ret = component_bind_all(dev, card);
>> +	if (ret) {
>> +		dev_err(dev, "Audio components bind failed: %d\n", ret);
>> +		goto bind_fail;
>> +	}
>> +
>> +	dev_set_drvdata(dev, card);
>> +	card->dev = dev;
>> +	ret = qcom_snd_parse_of(card);
>> +	if (ret) {
>> +		dev_err(dev, "Error parsing OF data\n");
>> +		goto parse_dt_fail;
>> +	}
>> +
>> +	data->card = card;
>> +	snd_soc_card_set_drvdata(card, data);
>> +
>> +	sdm845_add_be_ops(card);
>> +
>> +	sdm845_init_supplies(dev);
>> +
>> +	ret = snd_soc_register_card(card);
>> +	if (ret) {
>> +		dev_err(dev, "Sound card registration failed\n");
>> +		goto register_card_fail;
>> +	}
>> +	return ret;
>> +
>> +register_card_fail:
>> +	sdm845_deinit_supplies(dev);
>> +	kfree(card->dai_link);
>> +parse_dt_fail:
>> +	component_unbind_all(dev, card);
>> +bind_fail:
>> +	kfree(data);
>> +	kfree(card);
>> +	return ret;
>> +}
> I would make a case for this to be moved into common too :)

There are few platform specific APIs and structs here like struct 
sdm845_snd_data,
sdm845_add_be_ops() which needs to be initialized and assigned before 
soundcard
registration. Moving this complete API to common will restrict it. 
Please suggest.

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

On Fri, Jul 06, 2018 at 03:13:13PM +0530, Rohit kumar wrote:

> @@ -0,0 +1,390 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */

Please make the entire comment block a C++ comment, it makes it look
more intentional.

> +static const struct component_master_ops sdm845_ops = {
> +	.bind = sdm845_bind,
> +	.unbind = sdm845_unbind,
> +};

Why is this using the component stuff rather than the normal support for
finding the components of audio cards?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [alsa-devel] [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

On Mon, Jul 09, 2018 at 05:31:04PM +0530, Rohit Kumar wrote:
> On 7/9/2018 4:44 PM, Mark Brown wrote:
> > On Fri, Jul 06, 2018 at 03:13:13PM +0530, Rohit kumar wrote:

> > > +static const struct component_master_ops sdm845_ops = {
> > > +	.bind = sdm845_bind,
> > > +	.unbind = sdm845_unbind,
> > > +};

> > Why is this using the component stuff rather than the normal support for
> > finding the components of audio cards?

> QCOM soundcard is dependent on platform and cpu dais which gets registered
> only when ADSP is up.

This doesn't answer the question...

> This is the design being followed in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/qcom/apq8096.c?h=v4.18-rc4#n132

Oh dear, missed that :(  This is also broken and should be fixed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Srinivas Kandagatla]

On 09/07/18 12:14, Mark Brown wrote:
>> +static const struct component_master_ops sdm845_ops = {
>> +	.bind = sdm845_bind,
>> +	.unbind = sdm845_unbind,
>> +};
> Why is this using the component stuff rather than the normal support for
> finding the components of audio cards?
Could you elaborate this please?
Do you mean something like snd_soc_lookup_component()?  Or in general 
audio card binding during startup.

AFAIU, The issue with that mechanism or EPROBEDEFER is that it works 
only for first time.. for the second time(restart usecase) there are no 
hooks like bind/unbind.

The reason why we chose to use component framework is because of bind 
and unbind functionality. Am more than happy to rework on this if there 
is already a alternative mechanism in ASoC which can provide this.

The design we are aiming at is:
1> audio card to deregister when any of the DSP audio services go DOWN.
2> re-register audio card only when all the DSP audio services are UP.

Usecase is something like DSP start-stop or restart due to fatal errors.

thanks,
srini

Re: [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

On Mon, Jul 09, 2018 at 01:34:42PM +0100, Srinivas Kandagatla wrote:
> On 09/07/18 12:14, Mark Brown wrote:
> > > +static const struct component_master_ops sdm845_ops = {
> > > +	.bind = sdm845_bind,
> > > +	.unbind = sdm845_unbind,
> > > +};

> > Why is this using the component stuff rather than the normal support for
> > finding the components of audio cards?

> Could you elaborate this please?
> Do you mean something like snd_soc_lookup_component()?  Or in general audio
> card binding during startup.

Just the normal audio card binding during startup.

> AFAIU, The issue with that mechanism or EPROBEDEFER is that it works only
> for first time.. for the second time(restart usecase) there are no hooks
> like bind/unbind.

This is not the case, the card will be unbound at the ASoC level when
any of the components are removed and then probed again when they
reappear.

> The reason why we chose to use component framework is because of bind and
> unbind functionality. Am more than happy to rework on this if there is
> already a alternative mechanism in ASoC which can provide this.

The component framework is a generalization of what ASoC does.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Srinivas Kandagatla]

On 09/07/18 13:41, Mark Brown wrote:
> This is not the case, the card will be unbound at the ASoC level when
> any of the components are removed and then probed again when they
> reappear.
> 
I will give that a try now!

--srini

Re: [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Srinivas Kandagatla]

On 09/07/18 13:41, Mark Brown wrote:
>> AFAIU, The issue with that mechanism or EPROBEDEFER is that it works only
>> for first time.. for the second time(restart usecase) there are no hooks
>> like bind/unbind.
> This is not the case, the card will be unbound at the ASoC level when
> any of the components are removed and then probed again when they
> reappear.
> 
I did try this and It works only for first time! May be am missing 
something!

snd_soc_component_del_unlocked() unregisters the sound card totally. so 
for the second time (After DSP stop) there is no registered sound card 
in place.. Am not sure how this is supposed to work?

The reason I think it works for the first time is because of EPROBEDEFER 
from the machine driver.

Here are the steps I do with DSP:

Step1: Start DSP, can see sound card after all the services are ON.
Step2: Stop DSP, there is no audio card.
Step3: Start DSP, I can see all the components in debugfs but not the 
sound card.

This is what I do w.r.t code:
 From machine driver in snd_soc_register_card() in probe()
 From Audio services driver snd_soc_register_component() in probe() and
snd_soc_unregister_component() in remove()

When DSP is stopped the audio services disappear and 
snd_soc_unregister_component() is invoked.


thanks,
srini

Re: [alsa-devel] [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Vinod]

On 09-07-18, 16:16, Rohit Kumar wrote:

> > > +static int sdm845_bind(struct device *dev)
> > > +{
> > > +	struct snd_soc_card *card;
> > > +	struct sdm845_snd_data *data;
> > > +	int ret;
> > > +
> > > +	card = kzalloc(sizeof(*card), GFP_KERNEL);
> > > +	if (!card)
> > > +		return -ENOMEM;
> > > +
> > > +	/* Allocate the private data */
> > > +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> > > +	if (!data)
> > > +		return -ENOMEM;
> > > +
> > > +	ret = component_bind_all(dev, card);
> > > +	if (ret) {
> > > +		dev_err(dev, "Audio components bind failed: %d\n", ret);
> > > +		goto bind_fail;
> > > +	}
> > > +
> > > +	dev_set_drvdata(dev, card);
> > > +	card->dev = dev;
> > > +	ret = qcom_snd_parse_of(card);
> > > +	if (ret) {
> > > +		dev_err(dev, "Error parsing OF data\n");
> > > +		goto parse_dt_fail;
> > > +	}
> > > +
> > > +	data->card = card;
> > > +	snd_soc_card_set_drvdata(card, data);
> > > +
> > > +	sdm845_add_be_ops(card);
> > > +
> > > +	sdm845_init_supplies(dev);
> > > +
> > > +	ret = snd_soc_register_card(card);
> > > +	if (ret) {
> > > +		dev_err(dev, "Sound card registration failed\n");
> > > +		goto register_card_fail;
> > > +	}
> > > +	return ret;
> > > +
> > > +register_card_fail:
> > > +	sdm845_deinit_supplies(dev);
> > > +	kfree(card->dai_link);
> > > +parse_dt_fail:
> > > +	component_unbind_all(dev, card);
> > > +bind_fail:
> > > +	kfree(data);
> > > +	kfree(card);
> > > +	return ret;
> > > +}
> > I would make a case for this to be moved into common too :)
> 
> There are few platform specific APIs and structs here like struct
> sdm845_snd_data,
> sdm845_add_be_ops() which needs to be initialized and assigned before
> soundcard
> registration. Moving this complete API to common will restrict it. Please
> suggest.

Yes indeed, they can be split and done outside while the common stuff
use a 'core' object and use that to initialize. If you need to do some
driver step, you can invoke a callback.

-- 
~Vinod

Re: [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]

On Mon, Jul 09, 2018 at 03:02:11PM +0100, Srinivas Kandagatla wrote:
> On 09/07/18 13:41, Mark Brown wrote:
> > > AFAIU, The issue with that mechanism or EPROBEDEFER is that it works only

> > This is not the case, the card will be unbound at the ASoC level when
> > any of the components are removed and then probed again when they
> > reappear.

> I did try this and It works only for first time! May be am missing
> something!

> snd_soc_component_del_unlocked() unregisters the sound card totally. so for
> the second time (After DSP stop) there is no registered sound card in
> place.. Am not sure how this is supposed to work?

> The reason I think it works for the first time is because of EPROBEDEFER
> from the machine driver.

Ugh, right - we ripped out that code because there's no sensible use
case for it so now we don't keep the cards on a list.  The expectation
is that if someone is going around removing bits of the card they can
probably figure out that they should be removing the card first.

In any case the place to implement this is in the core, there's nothing
special about your cards here.  Either the core should be using the
component framework or the card list should be resurrected and we open
code it.  This isn't something that's unique to your device.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 3/5] ASoC: qcom: add sdm845 sound card support

[Srinivas Kandagatla]

On 09/07/18 17:33, Mark Brown wrote:
> On Mon, Jul 09, 2018 at 03:02:11PM +0100, Srinivas Kandagatla wrote:
>> On 09/07/18 13:41, Mark Brown wrote:
>>>> AFAIU, The issue with that mechanism or EPROBEDEFER is that it works only
> 
>>> This is not the case, the card will be unbound at the ASoC level when
>>> any of the components are removed and then probed again when they
>>> reappear.
> 
>> I did try this and It works only for first time! May be am missing
>> something!
> 
>> snd_soc_component_del_unlocked() unregisters the sound card totally. so for
>> the second time (After DSP stop) there is no registered sound card in
>> place.. Am not sure how this is supposed to work?
> 
>> The reason I think it works for the first time is because of EPROBEDEFER
>> from the machine driver.
> 
> Ugh, right - we ripped out that code because there's no sensible use
> case for it so now we don't keep the cards on a list.  The expectation
> is that if someone is going around removing bits of the card they can
> probably figure out that they should be removing the card first.
> 
> In any case the place to implement this is in the core, there's nothing
> special about your cards here.  Either the core should be using the
> component framework or the card list should be resurrected and we open
> code it.  This isn't something that's unique to your device.
I totally agree with you, this functionality belongs to core!

I will explore both options and see how it goes.

thanks,
srini
> 

Re: [PATCH v3 1/5] ASoC: qcom: dt-bindings: Add sdm845 machine bindings

[Rob Herring]

On Fri, Jul 06, 2018 at 03:13:11PM +0530, Rohit kumar wrote:
> Add devicetree bindings documentation file for SDM845 sound card.
> 
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
>  .../devicetree/bindings/sound/qcom,sdm845.txt      | 80 ++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/qcom,sdm845.txt

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 4/5] ASoC: dt-bindings: Update dt binding name for apq8096

[Rob Herring]

On Fri, Jul 06, 2018 at 03:13:14PM +0530, Rohit kumar wrote:
> Remove qcom prefix from machine driver dt bindings of
> apq8096 SoC.
> 
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/sound/qcom,apq8096.txt | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>