alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant
@ 2020-07-08  5:08 Rohit kumar
  2020-07-08  5:08 ` [PATCH v3 1/8] ASoC: qcom: Add common array to initialize soc based core clocks Rohit kumar
                   ` (7 more replies)
  0 siblings, 8 replies; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Rohit kumar

This patch chain add audio support for SC7180 soc by doing the required
modification in existing common lpass-cpu/lpass-platform driver.
Below is a brief summary of patch series:

PATCH v3 0001 ... 0005: Update lpass-cpu, lpass-platform drivers to make it more generic
and support newer soc registers configuration. This also updates existing lpass-apq8096.c
and lpass-ipq806x.c.
PATCH v2 0005 ... 0007: Add documentation and platform driver for newer SC7180 SOC variant.

Changes since v2:
	- Moved yaml conversion of Documentation to the end of patch series
	- Used REG_FIELD_ID instead of REG_FIELD for DMACTL and I2SCTL registers.
Move reg_fields to struct lpass_variant as suggested by Srinivas.

Ajit Pandey (5):
  ASoC: qcom: Add common array to initialize soc based core clocks
  include: dt-bindings: sound: Add sc7180-lpass bindings header
  ASoC: qcom: lpass-platform: Replace card->dev with component->dev
  ASoC: qcom: lpass-sc7180: Add platform driver for lpass audio
  dt-bindings: sound: lpass-cpu: Move to yaml format

Rohit kumar (3):
  ASoC: qcom: lpass-cpu: Move ahbix clk to platform specific function
  ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers
  dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node

 .../devicetree/bindings/sound/qcom,lpass-cpu.txt   |  79 --------
 .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 154 +++++++++++++++
 include/dt-bindings/sound/sc7180-lpass.h           |  10 +
 sound/soc/qcom/Kconfig                             |   5 +
 sound/soc/qcom/Makefile                            |   2 +
 sound/soc/qcom/lpass-apq8016.c                     |  86 ++++++--
 sound/soc/qcom/lpass-cpu.c                         | 193 +++++++++---------
 sound/soc/qcom/lpass-ipq806x.c                     |  67 +++++++
 sound/soc/qcom/lpass-lpaif-reg.h                   | 157 ++++++++-------
 sound/soc/qcom/lpass-platform.c                    | 156 +++++++++++----
 sound/soc/qcom/lpass-sc7180.c                      | 216 +++++++++++++++++++++
 sound/soc/qcom/lpass.h                             |  63 +++++-
 12 files changed, 886 insertions(+), 302 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
 create mode 100644 include/dt-bindings/sound/sc7180-lpass.h
 create mode 100644 sound/soc/qcom/lpass-sc7180.c

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v3 1/8] ASoC: qcom: Add common array to initialize soc based core clocks
  2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
@ 2020-07-08  5:08 ` Rohit kumar
  2020-07-09  9:26   ` Srinivas Kandagatla
  2020-07-08  5:08 ` [PATCH v3 2/8] ASoC: qcom: lpass-cpu: Move ahbix clk to platform specific function Rohit kumar
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Rohit kumar, Ajit Pandey

From: Ajit Pandey <ajitp@codeaurora.org>

LPASS variants have their own soc specific clocks that needs to be
enabled for MI2S audio support. Added a common variable in drvdata to
initialize such clocks using bulk clk api. Such clock names is
defined in variants specific data and needs to fetched during init.

Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 sound/soc/qcom/lpass-apq8016.c | 39 +++++++++++++++++++--------------------
 sound/soc/qcom/lpass.h         | 10 +++++++---
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/sound/soc/qcom/lpass-apq8016.c b/sound/soc/qcom/lpass-apq8016.c
index b3610d0..8210e37 100644
--- a/sound/soc/qcom/lpass-apq8016.c
+++ b/sound/soc/qcom/lpass-apq8016.c
@@ -161,32 +161,27 @@ static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
 static int apq8016_lpass_init(struct platform_device *pdev)
 {
 	struct lpass_data *drvdata = platform_get_drvdata(pdev);
+	struct lpass_variant *variant = drvdata->variant;
 	struct device *dev = &pdev->dev;
-	int ret;
+	int ret, i;
 
-	drvdata->pcnoc_mport_clk = devm_clk_get(dev, "pcnoc-mport-clk");
-	if (IS_ERR(drvdata->pcnoc_mport_clk)) {
-		dev_err(dev, "error getting pcnoc-mport-clk: %ld\n",
-			PTR_ERR(drvdata->pcnoc_mport_clk));
-		return PTR_ERR(drvdata->pcnoc_mport_clk);
-	}
 
-	ret = clk_prepare_enable(drvdata->pcnoc_mport_clk);
+	drvdata->clks = devm_kcalloc(dev, variant->num_clks,
+				     sizeof(*drvdata->clks), GFP_KERNEL);
+	drvdata->num_clks = variant->num_clks;
+
+	for (i = 0; i < drvdata->num_clks; i++)
+		drvdata->clks[i].id = variant->clk_name[i];
+
+	ret = devm_clk_bulk_get(dev, drvdata->num_clks, drvdata->clks);
 	if (ret) {
-		dev_err(dev, "Error enabling pcnoc-mport-clk: %d\n", ret);
+		dev_err(dev, "Failed to get clocks %d\n", ret);
 		return ret;
 	}
 
-	drvdata->pcnoc_sway_clk = devm_clk_get(dev, "pcnoc-sway-clk");
-	if (IS_ERR(drvdata->pcnoc_sway_clk)) {
-		dev_err(dev, "error getting pcnoc-sway-clk: %ld\n",
-			PTR_ERR(drvdata->pcnoc_sway_clk));
-		return PTR_ERR(drvdata->pcnoc_sway_clk);
-	}
-
-	ret = clk_prepare_enable(drvdata->pcnoc_sway_clk);
+	ret = clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
 	if (ret) {
-		dev_err(dev, "Error enabling pcnoc_sway_clk: %d\n", ret);
+		dev_err(dev, "apq8016 clk_enable failed\n");
 		return ret;
 	}
 
@@ -197,8 +192,7 @@ static int apq8016_lpass_exit(struct platform_device *pdev)
 {
 	struct lpass_data *drvdata = platform_get_drvdata(pdev);
 
-	clk_disable_unprepare(drvdata->pcnoc_mport_clk);
-	clk_disable_unprepare(drvdata->pcnoc_sway_clk);
+	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
 
 	return 0;
 }
@@ -219,6 +213,11 @@ static struct lpass_variant apq8016_data = {
 	.wrdma_reg_stride	= 0x1000,
 	.wrdma_channel_start	= 5,
 	.wrdma_channels		= 2,
+	.clk_name		= (const char*[]) {
+				   "pcnoc-mport-clk",
+				   "pcnoc-sway-clk",
+				  },
+	.num_clks		= 2,
 	.dai_driver		= apq8016_lpass_cpu_dai_driver,
 	.num_dai		= ARRAY_SIZE(apq8016_lpass_cpu_dai_driver),
 	.dai_osr_clk_names	= (const char *[]) {
diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
index bd19ec5..450020e 100644
--- a/sound/soc/qcom/lpass.h
+++ b/sound/soc/qcom/lpass.h
@@ -51,9 +51,9 @@ struct lpass_data {
 	/* used it for handling interrupt per dma channel */
 	struct snd_pcm_substream *substream[LPASS_MAX_DMA_CHANNELS];
 
-	/* 8016 specific */
-	struct clk *pcnoc_mport_clk;
-	struct clk *pcnoc_sway_clk;
+	/* SOC specific clock list */
+	struct clk_bulk_data *clks;
+	int num_clks;
 
 };
 
@@ -89,6 +89,10 @@ struct lpass_variant {
 	int num_dai;
 	const char * const *dai_osr_clk_names;
 	const char * const *dai_bit_clk_names;
+
+	/* SOC specific clocks configuration */
+	const char **clk_name;
+	int num_clks;
 };
 
 /* register the platform driver from the CPU DAI driver */
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 2/8] ASoC: qcom: lpass-cpu: Move ahbix clk to platform specific function
  2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
  2020-07-08  5:08 ` [PATCH v3 1/8] ASoC: qcom: Add common array to initialize soc based core clocks Rohit kumar
@ 2020-07-08  5:08 ` Rohit kumar
  2020-07-09  9:26   ` Srinivas Kandagatla
  2020-07-08  5:08 ` [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers Rohit kumar
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Rohit kumar

Ahbix clock is optional clock and not needed for all platforms.
Move it to lpass-apq8016/ipq806x as it is not needed for sc7180.

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 sound/soc/qcom/lpass-apq8016.c | 27 ++++++++++++++++++++++++++
 sound/soc/qcom/lpass-cpu.c     | 40 ++++++++++-----------------------------
 sound/soc/qcom/lpass-ipq806x.c | 43 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 30 deletions(-)

diff --git a/sound/soc/qcom/lpass-apq8016.c b/sound/soc/qcom/lpass-apq8016.c
index 8210e37..fe4c258 100644
--- a/sound/soc/qcom/lpass-apq8016.c
+++ b/sound/soc/qcom/lpass-apq8016.c
@@ -185,7 +185,33 @@ static int apq8016_lpass_init(struct platform_device *pdev)
 		return ret;
 	}
 
+	drvdata->ahbix_clk = devm_clk_get(dev, "ahbix-clk");
+	if (IS_ERR(drvdata->ahbix_clk)) {
+		dev_err(dev, "error getting ahbix-clk: %ld\n",
+				PTR_ERR(drvdata->ahbix_clk));
+		ret = PTR_ERR(drvdata->ahbix_clk);
+		goto err_ahbix_clk;
+	}
+
+	ret = clk_set_rate(drvdata->ahbix_clk, LPASS_AHBIX_CLOCK_FREQUENCY);
+	if (ret) {
+		dev_err(dev, "error setting rate on ahbix_clk: %d\n", ret);
+		goto err_ahbix_clk;
+	}
+	dev_dbg(dev, "set ahbix_clk rate to %lu\n",
+			clk_get_rate(drvdata->ahbix_clk));
+
+	ret = clk_prepare_enable(drvdata->ahbix_clk);
+	if (ret) {
+		dev_err(dev, "error enabling ahbix_clk: %d\n", ret);
+		goto err_ahbix_clk;
+	}
+
 	return 0;
+
+err_ahbix_clk:
+	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
+	return ret;
 }
 
 static int apq8016_lpass_exit(struct platform_device *pdev)
@@ -193,6 +219,7 @@ static int apq8016_lpass_exit(struct platform_device *pdev)
 	struct lpass_data *drvdata = platform_get_drvdata(pdev);
 
 	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
+	clk_disable_unprepare(drvdata->ahbix_clk);
 
 	return 0;
 }
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index e00a4af..f0c7e93 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -566,8 +566,13 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 		return PTR_ERR(drvdata->lpaif_map);
 	}
 
-	if (variant->init)
-		variant->init(pdev);
+	if (variant->init) {
+		ret = variant->init(pdev);
+		if (ret) {
+			dev_err(dev, "error initializing variant: %d\n", ret);
+			return ret;
+		}
+	}
 
 	for (i = 0; i < variant->num_dai; i++) {
 		dai_id = variant->dai_driver[i].id;
@@ -594,46 +599,22 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 		}
 	}
 
-	drvdata->ahbix_clk = devm_clk_get(dev, "ahbix-clk");
-	if (IS_ERR(drvdata->ahbix_clk)) {
-		dev_err(dev, "error getting ahbix-clk: %ld\n",
-			PTR_ERR(drvdata->ahbix_clk));
-		return PTR_ERR(drvdata->ahbix_clk);
-	}
-
-	ret = clk_set_rate(drvdata->ahbix_clk, LPASS_AHBIX_CLOCK_FREQUENCY);
-	if (ret) {
-		dev_err(dev, "error setting rate on ahbix_clk: %d\n", ret);
-		return ret;
-	}
-	dev_dbg(dev, "set ahbix_clk rate to %lu\n",
-		clk_get_rate(drvdata->ahbix_clk));
-
-	ret = clk_prepare_enable(drvdata->ahbix_clk);
-	if (ret) {
-		dev_err(dev, "error enabling ahbix_clk: %d\n", ret);
-		return ret;
-	}
-
 	ret = devm_snd_soc_register_component(dev,
 					      &lpass_cpu_comp_driver,
 					      variant->dai_driver,
 					      variant->num_dai);
 	if (ret) {
 		dev_err(dev, "error registering cpu driver: %d\n", ret);
-		goto err_clk;
+		goto err;
 	}
 
 	ret = asoc_qcom_lpass_platform_register(pdev);
 	if (ret) {
 		dev_err(dev, "error registering platform driver: %d\n", ret);
-		goto err_clk;
+		goto err;
 	}
 
-	return 0;
-
-err_clk:
-	clk_disable_unprepare(drvdata->ahbix_clk);
+err:
 	return ret;
 }
 EXPORT_SYMBOL_GPL(asoc_qcom_lpass_cpu_platform_probe);
@@ -645,7 +626,6 @@ int asoc_qcom_lpass_cpu_platform_remove(struct platform_device *pdev)
 	if (drvdata->variant->exit)
 		drvdata->variant->exit(pdev);
 
-	clk_disable_unprepare(drvdata->ahbix_clk);
 
 	return 0;
 }
diff --git a/sound/soc/qcom/lpass-ipq806x.c b/sound/soc/qcom/lpass-ipq806x.c
index 1987605..b7c0586 100644
--- a/sound/soc/qcom/lpass-ipq806x.c
+++ b/sound/soc/qcom/lpass-ipq806x.c
@@ -55,6 +55,47 @@ static struct snd_soc_dai_driver ipq806x_lpass_cpu_dai_driver = {
 	.ops    = &asoc_qcom_lpass_cpu_dai_ops,
 };
 
+static int ipq806x_lpass_init(struct platform_device *pdev)
+{
+	struct lpass_data *drvdata = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	drvdata->ahbix_clk = devm_clk_get(dev, "ahbix-clk");
+	if (IS_ERR(drvdata->ahbix_clk)) {
+		dev_err(dev, "error getting ahbix-clk: %ld\n",
+				PTR_ERR(drvdata->ahbix_clk));
+		ret = PTR_ERR(drvdata->ahbix_clk);
+		goto err_ahbix_clk;
+	}
+
+	ret = clk_set_rate(drvdata->ahbix_clk, LPASS_AHBIX_CLOCK_FREQUENCY);
+	if (ret) {
+		dev_err(dev, "error setting rate on ahbix_clk: %d\n", ret);
+		goto err_ahbix_clk;
+	}
+	dev_dbg(dev, "set ahbix_clk rate to %lu\n",
+			clk_get_rate(drvdata->ahbix_clk));
+
+	ret = clk_prepare_enable(drvdata->ahbix_clk);
+	if (ret) {
+		dev_err(dev, "error enabling ahbix_clk: %d\n", ret);
+		goto err_ahbix_clk;
+	}
+
+err_ahbix_clk:
+	return ret;
+}
+
+static int ipq806x_lpass_exit(struct platform_device *pdev)
+{
+	struct lpass_data *drvdata = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(drvdata->ahbix_clk);
+
+	return 0;
+}
+
 static int ipq806x_lpass_alloc_dma_channel(struct lpass_data *drvdata, int dir)
 {
 	if (dir == SNDRV_PCM_STREAM_PLAYBACK)
@@ -90,6 +131,8 @@ static struct lpass_variant ipq806x_data = {
 	.dai_bit_clk_names	= (const char *[]) {
 				"mi2s-bit-clk",
 				},
+	.init			= ipq806x_lpass_init,
+	.exit			= ipq806x_lpass_exit,
 	.alloc_dma_channel	= ipq806x_lpass_alloc_dma_channel,
 	.free_dma_channel	= ipq806x_lpass_free_dma_channel,
 };
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers
  2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
  2020-07-08  5:08 ` [PATCH v3 1/8] ASoC: qcom: Add common array to initialize soc based core clocks Rohit kumar
  2020-07-08  5:08 ` [PATCH v3 2/8] ASoC: qcom: lpass-cpu: Move ahbix clk to platform specific function Rohit kumar
@ 2020-07-08  5:08 ` Rohit kumar
  2020-07-09  9:26   ` Srinivas Kandagatla
  2020-07-08  5:08 ` [PATCH v3 4/8] include: dt-bindings: sound: Add sc7180-lpass bindings header Rohit kumar
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Rohit kumar

I2SCTL and DMACTL registers has different bits alignment for newer
LPASS variants of SC7180 soc. Use REG_FIELD_ID() to define the
reg_fields in platform specific file and removed shifts and mask
macros for such registers from header file.

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 sound/soc/qcom/lpass-apq8016.c   |  24 ++++++
 sound/soc/qcom/lpass-cpu.c       | 163 +++++++++++++++++++++++----------------
 sound/soc/qcom/lpass-ipq806x.c   |  24 ++++++
 sound/soc/qcom/lpass-lpaif-reg.h | 157 +++++++++++++++++++------------------
 sound/soc/qcom/lpass-platform.c  | 151 +++++++++++++++++++++++++++---------
 sound/soc/qcom/lpass.h           |  53 +++++++++++++
 6 files changed, 398 insertions(+), 174 deletions(-)

diff --git a/sound/soc/qcom/lpass-apq8016.c b/sound/soc/qcom/lpass-apq8016.c
index fe4c258..dd9e3dd 100644
--- a/sound/soc/qcom/lpass-apq8016.c
+++ b/sound/soc/qcom/lpass-apq8016.c
@@ -240,6 +240,30 @@ static struct lpass_variant apq8016_data = {
 	.wrdma_reg_stride	= 0x1000,
 	.wrdma_channel_start	= 5,
 	.wrdma_channels		= 2,
+	.loopback		= REG_FIELD_ID(0x1000, 15, 15, 4, 0x1000),
+	.spken			= REG_FIELD_ID(0x1000, 14, 14, 4, 0x1000),
+	.spkmode		= REG_FIELD_ID(0x1000, 10, 13, 4, 0x1000),
+	.spkmono		= REG_FIELD_ID(0x1000, 9, 9, 4, 0x1000),
+	.micen			= REG_FIELD_ID(0x1000, 8, 8, 4, 0x1000),
+	.micmode		= REG_FIELD_ID(0x1000, 4, 7, 4, 0x1000),
+	.micmono		= REG_FIELD_ID(0x1000, 3, 3, 4, 0x1000),
+	.wssrc			= REG_FIELD_ID(0x1000, 2, 2, 4, 0x1000),
+	.bitwidth		= REG_FIELD_ID(0x1000, 0, 0, 4, 0x1000),
+
+	.rdma_dyncclk		= REG_FIELD_ID(0x8400, 12, 12, 2, 0x1000),
+	.rdma_bursten		= REG_FIELD_ID(0x8400, 11, 11, 2, 0x1000),
+	.rdma_wpscnt		= REG_FIELD_ID(0x8400, 8, 10, 2, 0x1000),
+	.rdma_intf		= REG_FIELD_ID(0x8400, 4, 7, 2, 0x1000),
+	.rdma_fifowm		= REG_FIELD_ID(0x8400, 1, 3, 2, 0x1000),
+	.rdma_enable		= REG_FIELD_ID(0x8400, 0, 0, 2, 0x1000),
+
+	.wrdma_dyncclk		= REG_FIELD_ID(0xB000, 12, 12, 2, 0x1000),
+	.wrdma_bursten		= REG_FIELD_ID(0xB000, 11, 11, 2, 0x1000),
+	.wrdma_wpscnt		= REG_FIELD_ID(0xB000, 8, 10, 2, 0x1000),
+	.wrdma_intf		= REG_FIELD_ID(0xB000, 4, 7, 2, 0x1000),
+	.wrdma_fifowm		= REG_FIELD_ID(0xB000, 1, 3, 2, 0x1000),
+	.wrdma_enable		= REG_FIELD_ID(0xB000, 0, 0, 2, 0x1000),
+
 	.clk_name		= (const char*[]) {
 				   "pcnoc-mport-clk",
 				   "pcnoc-sway-clk",
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index f0c7e93..f358d12 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -29,6 +29,32 @@
 #define LPASS_CPU_I2S_SD0_1_2_MASK	GENMASK(2, 0)
 #define LPASS_CPU_I2S_SD0_1_2_3_MASK	GENMASK(3, 0)
 
+static int lpass_cpu_init_i2sctl_bitfields(struct device *dev,
+			struct lpaif_i2sctl *i2sctl, struct regmap *map)
+{
+	struct lpass_data *drvdata = dev_get_drvdata(dev);
+	struct lpass_variant *v = drvdata->variant;
+
+	i2sctl->loopback = devm_regmap_field_alloc(dev, map, v->loopback);
+	i2sctl->spken = devm_regmap_field_alloc(dev, map, v->spken);
+	i2sctl->spkmode = devm_regmap_field_alloc(dev, map, v->spkmode);
+	i2sctl->spkmono = devm_regmap_field_alloc(dev, map, v->spkmono);
+	i2sctl->micen = devm_regmap_field_alloc(dev, map, v->micen);
+	i2sctl->micmode = devm_regmap_field_alloc(dev, map, v->micmode);
+	i2sctl->micmono = devm_regmap_field_alloc(dev, map, v->micmono);
+	i2sctl->wssrc = devm_regmap_field_alloc(dev, map, v->wssrc);
+	i2sctl->bitwidth = devm_regmap_field_alloc(dev, map, v->bitwidth);
+
+	if (IS_ERR(i2sctl->loopback) || IS_ERR(i2sctl->spken) ||
+	    IS_ERR(i2sctl->spkmode) || IS_ERR(i2sctl->spkmono) ||
+	    IS_ERR(i2sctl->micen) || IS_ERR(i2sctl->micmode) ||
+	    IS_ERR(i2sctl->micmono) || IS_ERR(i2sctl->wssrc) ||
+	    IS_ERR(i2sctl->bitwidth))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int lpass_cpu_daiops_set_sysclk(struct snd_soc_dai *dai, int clk_id,
 		unsigned int freq, int dir)
 {
@@ -79,12 +105,13 @@ static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
 		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
 {
 	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+	struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
+	unsigned int id = dai->driver->id;
 	snd_pcm_format_t format = params_format(params);
 	unsigned int channels = params_channels(params);
 	unsigned int rate = params_rate(params);
 	unsigned int mode;
-	unsigned int regval;
-	int bitwidth, ret;
+	int bitwidth, ret, regval;
 
 	bitwidth = snd_pcm_format_width(format);
 	if (bitwidth < 0) {
@@ -92,28 +119,45 @@ static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
 		return bitwidth;
 	}
 
-	regval = LPAIF_I2SCTL_LOOPBACK_DISABLE |
-			LPAIF_I2SCTL_WSSRC_INTERNAL;
+	ret = regmap_fields_write(i2sctl->loopback, id,
+				 LPAIF_I2SCTL_LOOPBACK_DISABLE);
+	if (ret) {
+		dev_err(dai->dev, "error updating loopback field: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_fields_write(i2sctl->wssrc, id,
+				 LPAIF_I2SCTL_WSSRC_INTERNAL);
+	if (ret) {
+		dev_err(dai->dev, "error updating wssrc field: %d\n", ret);
+		return ret;
+	}
 
 	switch (bitwidth) {
 	case 16:
-		regval |= LPAIF_I2SCTL_BITWIDTH_16;
+		regval = LPAIF_I2SCTL_BITWIDTH_16;
 		break;
 	case 24:
-		regval |= LPAIF_I2SCTL_BITWIDTH_24;
+		regval = LPAIF_I2SCTL_BITWIDTH_24;
 		break;
 	case 32:
-		regval |= LPAIF_I2SCTL_BITWIDTH_32;
+		regval = LPAIF_I2SCTL_BITWIDTH_32;
 		break;
 	default:
 		dev_err(dai->dev, "invalid bitwidth given: %d\n", bitwidth);
 		return -EINVAL;
 	}
 
+	ret = regmap_fields_write(i2sctl->bitwidth, id, regval);
+	if (ret) {
+		dev_err(dai->dev, "error updating bitwidth field: %d\n", ret);
+		return ret;
+	}
+
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		mode = drvdata->mi2s_playback_sd_mode[dai->driver->id];
+		mode = drvdata->mi2s_playback_sd_mode[id];
 	else
-		mode = drvdata->mi2s_capture_sd_mode[dai->driver->id];
+		mode = drvdata->mi2s_capture_sd_mode[id];
 
 	if (!mode) {
 		dev_err(dai->dev, "no line is assigned\n");
@@ -175,30 +219,34 @@ static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		regval |= LPAIF_I2SCTL_SPKMODE(mode);
+		ret = regmap_fields_write(i2sctl->spkmode, id,
+					 LPAIF_I2SCTL_SPKMODE(mode));
 
 		if (channels >= 2)
-			regval |= LPAIF_I2SCTL_SPKMONO_STEREO;
+			ret = regmap_fields_write(i2sctl->spkmono, id,
+						 LPAIF_I2SCTL_SPKMONO_STEREO);
 		else
-			regval |= LPAIF_I2SCTL_SPKMONO_MONO;
+			ret = regmap_fields_write(i2sctl->spkmono, id,
+						 LPAIF_I2SCTL_SPKMONO_MONO);
 	} else {
-		regval |= LPAIF_I2SCTL_MICMODE(mode);
+		ret = regmap_fields_write(i2sctl->micmode, id,
+					 LPAIF_I2SCTL_MICMODE(mode));
 
 		if (channels >= 2)
-			regval |= LPAIF_I2SCTL_MICMONO_STEREO;
+			ret = regmap_fields_write(i2sctl->micmono, id,
+						 LPAIF_I2SCTL_MICMONO_STEREO);
 		else
-			regval |= LPAIF_I2SCTL_MICMONO_MONO;
+			ret = regmap_fields_write(i2sctl->micmono, id,
+						 LPAIF_I2SCTL_MICMONO_MONO);
 	}
 
-	ret = regmap_write(drvdata->lpaif_map,
-			   LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
-			   regval);
 	if (ret) {
-		dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
+		dev_err(dai->dev, "error writing to i2sctl channels mode: %d\n",
+			ret);
 		return ret;
 	}
 
-	ret = clk_set_rate(drvdata->mi2s_bit_clk[dai->driver->id],
+	ret = clk_set_rate(drvdata->mi2s_bit_clk[id],
 			   rate * bitwidth * 2);
 	if (ret) {
 		dev_err(dai->dev, "error setting mi2s bitclk to %u: %d\n",
@@ -209,41 +257,24 @@ static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int lpass_cpu_daiops_hw_free(struct snd_pcm_substream *substream,
-		struct snd_soc_dai *dai)
-{
-	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
-	int ret;
-
-	ret = regmap_write(drvdata->lpaif_map,
-			   LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
-			   0);
-	if (ret)
-		dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
-
-	return ret;
-}
-
 static int lpass_cpu_daiops_prepare(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
 	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+	struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
+	unsigned int id = dai->driver->id;
 	int ret;
-	unsigned int val, mask;
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		val = LPAIF_I2SCTL_SPKEN_ENABLE;
-		mask = LPAIF_I2SCTL_SPKEN_MASK;
-	} else  {
-		val = LPAIF_I2SCTL_MICEN_ENABLE;
-		mask = LPAIF_I2SCTL_MICEN_MASK;
+		ret = regmap_fields_write(i2sctl->spken, id,
+					 LPAIF_I2SCTL_SPKEN_ENABLE);
+	} else {
+		ret = regmap_fields_write(i2sctl->micen, id,
+					 LPAIF_I2SCTL_MICEN_ENABLE);
 	}
 
-	ret = regmap_update_bits(drvdata->lpaif_map,
-			LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
-			mask, val);
 	if (ret)
-		dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
+		dev_err(dai->dev, "error writing to i2sctl enable: %d\n", ret);
 
 	return ret;
 }
@@ -252,25 +283,21 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
 		int cmd, struct snd_soc_dai *dai)
 {
 	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+	struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
+	unsigned int id = dai->driver->id;
 	int ret = -EINVAL;
-	unsigned int val, mask;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-			val = LPAIF_I2SCTL_SPKEN_ENABLE;
-			mask = LPAIF_I2SCTL_SPKEN_MASK;
+			ret = regmap_fields_write(i2sctl->spken, id,
+						 LPAIF_I2SCTL_SPKEN_ENABLE);
 		} else  {
-			val = LPAIF_I2SCTL_MICEN_ENABLE;
-			mask = LPAIF_I2SCTL_MICEN_MASK;
+			ret = regmap_fields_write(i2sctl->micen, id,
+						 LPAIF_I2SCTL_MICEN_ENABLE);
 		}
-
-		ret = regmap_update_bits(drvdata->lpaif_map,
-				LPAIF_I2SCTL_REG(drvdata->variant,
-						dai->driver->id),
-				mask, val);
 		if (ret)
 			dev_err(dai->dev, "error writing to i2sctl reg: %d\n",
 				ret);
@@ -279,17 +306,12 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-			val = LPAIF_I2SCTL_SPKEN_DISABLE;
-			mask = LPAIF_I2SCTL_SPKEN_MASK;
+			ret = regmap_fields_write(i2sctl->spken, id,
+						 LPAIF_I2SCTL_SPKEN_DISABLE);
 		} else  {
-			val = LPAIF_I2SCTL_MICEN_DISABLE;
-			mask = LPAIF_I2SCTL_MICEN_MASK;
+			ret = regmap_fields_write(i2sctl->micen, id,
+						 LPAIF_I2SCTL_MICEN_DISABLE);
 		}
-
-		ret = regmap_update_bits(drvdata->lpaif_map,
-				LPAIF_I2SCTL_REG(drvdata->variant,
-						dai->driver->id),
-				mask, val);
 		if (ret)
 			dev_err(dai->dev, "error writing to i2sctl reg: %d\n",
 				ret);
@@ -304,7 +326,6 @@ const struct snd_soc_dai_ops asoc_qcom_lpass_cpu_dai_ops = {
 	.startup	= lpass_cpu_daiops_startup,
 	.shutdown	= lpass_cpu_daiops_shutdown,
 	.hw_params	= lpass_cpu_daiops_hw_params,
-	.hw_free	= lpass_cpu_daiops_hw_free,
 	.prepare	= lpass_cpu_daiops_prepare,
 	.trigger	= lpass_cpu_daiops_trigger,
 };
@@ -599,6 +620,18 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Allocation for i2sctl regmap fields */
+	drvdata->i2sctl = devm_kzalloc(&pdev->dev, sizeof(struct lpaif_i2sctl),
+					GFP_KERNEL);
+
+	/* Initialize bitfields for dai I2SCTL register */
+	ret = lpass_cpu_init_i2sctl_bitfields(dev, drvdata->i2sctl,
+						drvdata->lpaif_map);
+	if (ret) {
+		dev_err(dev, "error init i2sctl field: %d\n", ret);
+		return ret;
+	}
+
 	ret = devm_snd_soc_register_component(dev,
 					      &lpass_cpu_comp_driver,
 					      variant->dai_driver,
diff --git a/sound/soc/qcom/lpass-ipq806x.c b/sound/soc/qcom/lpass-ipq806x.c
index b7c0586..72f09b3 100644
--- a/sound/soc/qcom/lpass-ipq806x.c
+++ b/sound/soc/qcom/lpass-ipq806x.c
@@ -123,6 +123,30 @@ static struct lpass_variant ipq806x_data = {
 	.wrdma_reg_stride	= 0x1000,
 	.wrdma_channel_start	= 5,
 	.wrdma_channels		= 4,
+	.loopback		= REG_FIELD_ID(0x0010, 15, 15, 5, 0x4),
+	.spken			= REG_FIELD_ID(0x0010, 14, 14, 5, 0x4),
+	.spkmode		= REG_FIELD_ID(0x0010, 10, 13, 5, 0x4),
+	.spkmono		= REG_FIELD_ID(0x0010, 9, 9, 5, 0x4),
+	.micen			= REG_FIELD_ID(0x0010, 8, 8, 5, 0x4),
+	.micmode		= REG_FIELD_ID(0x0010, 4, 7, 5, 0x4),
+	.micmono		= REG_FIELD_ID(0x0010, 3, 3, 5, 0x4),
+	.wssrc			= REG_FIELD_ID(0x0010, 2, 2, 5, 0x4),
+	.bitwidth		= REG_FIELD_ID(0x0010, 0, 0, 5, 0x4),
+
+	.rdma_dyncclk		= REG_FIELD_ID(0x6000, 12, 12, 4, 0x1000),
+	.rdma_bursten		= REG_FIELD_ID(0x6000, 11, 11, 4, 0x1000),
+	.rdma_wpscnt		= REG_FIELD_ID(0x6000, 8, 10, 4, 0x1000),
+	.rdma_intf		= REG_FIELD_ID(0x6000, 4, 7, 4, 0x1000),
+	.rdma_fifowm		= REG_FIELD_ID(0x6000, 1, 3, 4, 0x1000),
+	.rdma_enable		= REG_FIELD_ID(0x6000, 0, 0, 4, 0x1000),
+
+	.wrdma_dyncclk		= REG_FIELD_ID(0xB000, 12, 12, 4, 0x1000),
+	.wrdma_bursten		= REG_FIELD_ID(0xB000, 11, 11, 4, 0x1000),
+	.wrdma_wpscnt		= REG_FIELD_ID(0xB000, 8, 10, 4, 0x1000),
+	.wrdma_intf		= REG_FIELD_ID(0xB000, 4, 7, 4, 0x1000),
+	.wrdma_fifowm		= REG_FIELD_ID(0xB000, 1, 3, 4, 0x1000),
+	.wrdma_enable		= REG_FIELD_ID(0xB000, 0, 0, 4, 0x1000),
+
 	.dai_driver		= &ipq806x_lpass_cpu_dai_driver,
 	.num_dai		= 1,
 	.dai_osr_clk_names	= (const char *[]) {
diff --git a/sound/soc/qcom/lpass-lpaif-reg.h b/sound/soc/qcom/lpass-lpaif-reg.h
index 72a3e2f..5258e60 100644
--- a/sound/soc/qcom/lpass-lpaif-reg.h
+++ b/sound/soc/qcom/lpass-lpaif-reg.h
@@ -12,15 +12,12 @@
 	(v->i2sctrl_reg_base + (addr) + v->i2sctrl_reg_stride * (port))
 
 #define LPAIF_I2SCTL_REG(v, port)	LPAIF_I2SCTL_REG_ADDR(v, 0x0, (port))
-#define LPAIF_I2SCTL_LOOPBACK_MASK	0x8000
-#define LPAIF_I2SCTL_LOOPBACK_SHIFT	15
-#define LPAIF_I2SCTL_LOOPBACK_DISABLE	(0 << LPAIF_I2SCTL_LOOPBACK_SHIFT)
-#define LPAIF_I2SCTL_LOOPBACK_ENABLE	(1 << LPAIF_I2SCTL_LOOPBACK_SHIFT)
 
-#define LPAIF_I2SCTL_SPKEN_MASK		0x4000
-#define LPAIF_I2SCTL_SPKEN_SHIFT	14
-#define LPAIF_I2SCTL_SPKEN_DISABLE	(0 << LPAIF_I2SCTL_SPKEN_SHIFT)
-#define LPAIF_I2SCTL_SPKEN_ENABLE	(1 << LPAIF_I2SCTL_SPKEN_SHIFT)
+#define LPAIF_I2SCTL_LOOPBACK_DISABLE	0
+#define LPAIF_I2SCTL_LOOPBACK_ENABLE	1
+
+#define LPAIF_I2SCTL_SPKEN_DISABLE	0
+#define LPAIF_I2SCTL_SPKEN_ENABLE	1
 
 #define LPAIF_I2SCTL_MODE_NONE		0
 #define LPAIF_I2SCTL_MODE_SD0		1
@@ -31,40 +28,37 @@
 #define LPAIF_I2SCTL_MODE_QUAD23	6
 #define LPAIF_I2SCTL_MODE_6CH		7
 #define LPAIF_I2SCTL_MODE_8CH		8
+#define LPAIF_I2SCTL_MODE_10CH		9
+#define LPAIF_I2SCTL_MODE_12CH		10
+#define LPAIF_I2SCTL_MODE_14CH		11
+#define LPAIF_I2SCTL_MODE_16CH		12
+#define LPAIF_I2SCTL_MODE_SD4		13
+#define LPAIF_I2SCTL_MODE_SD5		14
+#define LPAIF_I2SCTL_MODE_SD6		15
+#define LPAIF_I2SCTL_MODE_SD7		16
+#define LPAIF_I2SCTL_MODE_QUAD45	17
+#define LPAIF_I2SCTL_MODE_QUAD47	18
+#define LPAIF_I2SCTL_MODE_8CH_2		19
 
-#define LPAIF_I2SCTL_SPKMODE_MASK	0x3C00
-#define LPAIF_I2SCTL_SPKMODE_SHIFT	10
-#define LPAIF_I2SCTL_SPKMODE(mode)	((mode) << LPAIF_I2SCTL_SPKMODE_SHIFT)
+#define LPAIF_I2SCTL_SPKMODE(mode)	mode
 
-#define LPAIF_I2SCTL_SPKMONO_MASK	0x0200
-#define LPAIF_I2SCTL_SPKMONO_SHIFT	9
-#define LPAIF_I2SCTL_SPKMONO_STEREO	(0 << LPAIF_I2SCTL_SPKMONO_SHIFT)
-#define LPAIF_I2SCTL_SPKMONO_MONO	(1 << LPAIF_I2SCTL_SPKMONO_SHIFT)
+#define LPAIF_I2SCTL_SPKMONO_STEREO	0
+#define LPAIF_I2SCTL_SPKMONO_MONO	1
 
-#define LPAIF_I2SCTL_MICEN_MASK		GENMASK(8, 8)
-#define LPAIF_I2SCTL_MICEN_SHIFT	8
-#define LPAIF_I2SCTL_MICEN_DISABLE	(0 << LPAIF_I2SCTL_MICEN_SHIFT)
-#define LPAIF_I2SCTL_MICEN_ENABLE	(1 << LPAIF_I2SCTL_MICEN_SHIFT)
+#define LPAIF_I2SCTL_MICEN_DISABLE	0
+#define LPAIF_I2SCTL_MICEN_ENABLE	1
 
-#define LPAIF_I2SCTL_MICMODE_MASK	GENMASK(7, 4)
-#define LPAIF_I2SCTL_MICMODE_SHIFT	4
-#define LPAIF_I2SCTL_MICMODE(mode)	((mode) << LPAIF_I2SCTL_MICMODE_SHIFT)
+#define LPAIF_I2SCTL_MICMODE(mode)	mode
 
-#define LPAIF_I2SCTL_MIMONO_MASK	GENMASK(3, 3)
-#define LPAIF_I2SCTL_MICMONO_SHIFT	3
-#define LPAIF_I2SCTL_MICMONO_STEREO	(0 << LPAIF_I2SCTL_MICMONO_SHIFT)
-#define LPAIF_I2SCTL_MICMONO_MONO	(1 << LPAIF_I2SCTL_MICMONO_SHIFT)
+#define LPAIF_I2SCTL_MICMONO_STEREO	0
+#define LPAIF_I2SCTL_MICMONO_MONO	1
 
-#define LPAIF_I2SCTL_WSSRC_MASK		0x0004
-#define LPAIF_I2SCTL_WSSRC_SHIFT	2
-#define LPAIF_I2SCTL_WSSRC_INTERNAL	(0 << LPAIF_I2SCTL_WSSRC_SHIFT)
-#define LPAIF_I2SCTL_WSSRC_EXTERNAL	(1 << LPAIF_I2SCTL_WSSRC_SHIFT)
+#define LPAIF_I2SCTL_WSSRC_INTERNAL	0
+#define LPAIF_I2SCTL_WSSRC_EXTERNAL	1
 
-#define LPAIF_I2SCTL_BITWIDTH_MASK	0x0003
-#define LPAIF_I2SCTL_BITWIDTH_SHIFT	0
-#define LPAIF_I2SCTL_BITWIDTH_16	(0 << LPAIF_I2SCTL_BITWIDTH_SHIFT)
-#define LPAIF_I2SCTL_BITWIDTH_24	(1 << LPAIF_I2SCTL_BITWIDTH_SHIFT)
-#define LPAIF_I2SCTL_BITWIDTH_32	(2 << LPAIF_I2SCTL_BITWIDTH_SHIFT)
+#define LPAIF_I2SCTL_BITWIDTH_16	0
+#define LPAIF_I2SCTL_BITWIDTH_24	1
+#define LPAIF_I2SCTL_BITWIDTH_32	2
 
 /* LPAIF IRQ */
 #define LPAIF_IRQ_REG_ADDR(v, addr, port) \
@@ -121,42 +115,59 @@
 #define	LPAIF_DMAPER_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, PER)
 #define	LPAIF_DMAPERCNT_REG(v, chan, dir) __LPAIF_DMA_REG(v, chan, dir, PERCNT)
 
-#define LPAIF_DMACTL_BURSTEN_MASK	0x800
-#define LPAIF_DMACTL_BURSTEN_SHIFT	11
-#define LPAIF_DMACTL_BURSTEN_SINGLE	(0 << LPAIF_DMACTL_BURSTEN_SHIFT)
-#define LPAIF_DMACTL_BURSTEN_INCR4	(1 << LPAIF_DMACTL_BURSTEN_SHIFT)
-
-#define LPAIF_DMACTL_WPSCNT_MASK	0x700
-#define LPAIF_DMACTL_WPSCNT_SHIFT	8
-#define LPAIF_DMACTL_WPSCNT_ONE	(0 << LPAIF_DMACTL_WPSCNT_SHIFT)
-#define LPAIF_DMACTL_WPSCNT_TWO	(1 << LPAIF_DMACTL_WPSCNT_SHIFT)
-#define LPAIF_DMACTL_WPSCNT_THREE	(2 << LPAIF_DMACTL_WPSCNT_SHIFT)
-#define LPAIF_DMACTL_WPSCNT_FOUR	(3 << LPAIF_DMACTL_WPSCNT_SHIFT)
-#define LPAIF_DMACTL_WPSCNT_SIX	(5 << LPAIF_DMACTL_WPSCNT_SHIFT)
-#define LPAIF_DMACTL_WPSCNT_EIGHT	(7 << LPAIF_DMACTL_WPSCNT_SHIFT)
-
-#define LPAIF_DMACTL_AUDINTF_MASK	0x0F0
-#define LPAIF_DMACTL_AUDINTF_SHIFT	4
-#define LPAIF_DMACTL_AUDINTF(id)	(id << LPAIF_DMACTL_AUDINTF_SHIFT)
-
-#define LPAIF_DMACTL_FIFOWM_MASK	0x00E
-#define LPAIF_DMACTL_FIFOWM_SHIFT	1
-#define LPAIF_DMACTL_FIFOWM_1		(0 << LPAIF_DMACTL_FIFOWM_SHIFT)
-#define LPAIF_DMACTL_FIFOWM_2		(1 << LPAIF_DMACTL_FIFOWM_SHIFT)
-#define LPAIF_DMACTL_FIFOWM_3		(2 << LPAIF_DMACTL_FIFOWM_SHIFT)
-#define LPAIF_DMACTL_FIFOWM_4		(3 << LPAIF_DMACTL_FIFOWM_SHIFT)
-#define LPAIF_DMACTL_FIFOWM_5		(4 << LPAIF_DMACTL_FIFOWM_SHIFT)
-#define LPAIF_DMACTL_FIFOWM_6		(5 << LPAIF_DMACTL_FIFOWM_SHIFT)
-#define LPAIF_DMACTL_FIFOWM_7		(6 << LPAIF_DMACTL_FIFOWM_SHIFT)
-#define LPAIF_DMACTL_FIFOWM_8		(7 << LPAIF_DMACTL_FIFOWM_SHIFT)
-
-#define LPAIF_DMACTL_ENABLE_MASK	0x1
-#define LPAIF_DMACTL_ENABLE_SHIFT	0
-#define LPAIF_DMACTL_ENABLE_OFF	(0 << LPAIF_DMACTL_ENABLE_SHIFT)
-#define LPAIF_DMACTL_ENABLE_ON		(1 << LPAIF_DMACTL_ENABLE_SHIFT)
-
-#define LPAIF_DMACTL_DYNCLK_MASK	BIT(12)
-#define LPAIF_DMACTL_DYNCLK_SHIFT	12
-#define LPAIF_DMACTL_DYNCLK_OFF	(0 << LPAIF_DMACTL_DYNCLK_SHIFT)
-#define LPAIF_DMACTL_DYNCLK_ON		(1 << LPAIF_DMACTL_DYNCLK_SHIFT)
+#define LPAIF_DMACTL_BURSTEN_SINGLE	0
+#define LPAIF_DMACTL_BURSTEN_INCR4	1
+
+#define LPAIF_DMACTL_WPSCNT_ONE		0
+#define LPAIF_DMACTL_WPSCNT_TWO		1
+#define LPAIF_DMACTL_WPSCNT_THREE	2
+#define LPAIF_DMACTL_WPSCNT_FOUR	3
+#define LPAIF_DMACTL_WPSCNT_SIX		5
+#define LPAIF_DMACTL_WPSCNT_EIGHT	7
+#define LPAIF_DMACTL_WPSCNT_TEN		9
+#define LPAIF_DMACTL_WPSCNT_TWELVE	11
+#define LPAIF_DMACTL_WPSCNT_FOURTEEN	13
+#define LPAIF_DMACTL_WPSCNT_SIXTEEN	15
+
+#define LPAIF_DMACTL_AUDINTF(id)	id
+
+#define LPAIF_DMACTL_FIFOWM_1		0
+#define LPAIF_DMACTL_FIFOWM_2		1
+#define LPAIF_DMACTL_FIFOWM_3		2
+#define LPAIF_DMACTL_FIFOWM_4		3
+#define LPAIF_DMACTL_FIFOWM_5		4
+#define LPAIF_DMACTL_FIFOWM_6		5
+#define LPAIF_DMACTL_FIFOWM_7		6
+#define LPAIF_DMACTL_FIFOWM_8		7
+#define LPAIF_DMACTL_FIFOWM_9		8
+#define LPAIF_DMACTL_FIFOWM_10		9
+#define LPAIF_DMACTL_FIFOWM_11		10
+#define LPAIF_DMACTL_FIFOWM_12		11
+#define LPAIF_DMACTL_FIFOWM_13		12
+#define LPAIF_DMACTL_FIFOWM_14		13
+#define LPAIF_DMACTL_FIFOWM_15		14
+#define LPAIF_DMACTL_FIFOWM_16		15
+#define LPAIF_DMACTL_FIFOWM_17		16
+#define LPAIF_DMACTL_FIFOWM_18		17
+#define LPAIF_DMACTL_FIFOWM_19		18
+#define LPAIF_DMACTL_FIFOWM_20		19
+#define LPAIF_DMACTL_FIFOWM_21		20
+#define LPAIF_DMACTL_FIFOWM_22		21
+#define LPAIF_DMACTL_FIFOWM_23		22
+#define LPAIF_DMACTL_FIFOWM_24		23
+#define LPAIF_DMACTL_FIFOWM_25		24
+#define LPAIF_DMACTL_FIFOWM_26		25
+#define LPAIF_DMACTL_FIFOWM_27		26
+#define LPAIF_DMACTL_FIFOWM_28		27
+#define LPAIF_DMACTL_FIFOWM_29		28
+#define LPAIF_DMACTL_FIFOWM_30		29
+#define LPAIF_DMACTL_FIFOWM_31		30
+#define LPAIF_DMACTL_FIFOWM_32		31
+
+#define LPAIF_DMACTL_ENABLE_OFF		0
+#define LPAIF_DMACTL_ENABLE_ON		1
+
+#define LPAIF_DMACTL_DYNCLK_OFF		0
+#define LPAIF_DMACTL_DYNCLK_ON		1
+
 #endif /* __LPASS_LPAIF_REG_H__ */
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index 34f7fd1..445ca193 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -50,6 +50,53 @@ static const struct snd_pcm_hardware lpass_platform_pcm_hardware = {
 	.fifo_size		=	0,
 };
 
+static int lpass_platform_alloc_dmactl_fields(struct device *dev,
+					 struct regmap *map)
+{
+	struct lpass_data *drvdata = dev_get_drvdata(dev);
+	struct lpass_variant *v = drvdata->variant;
+	struct lpaif_dmactl *rd_dmactl, *wr_dmactl;
+
+	drvdata->rd_dmactl = devm_kzalloc(dev, sizeof(struct lpaif_dmactl),
+					  GFP_KERNEL);
+	if (drvdata->rd_dmactl == NULL)
+		return -ENOMEM;
+
+	drvdata->wr_dmactl = devm_kzalloc(dev, sizeof(struct lpaif_dmactl),
+					  GFP_KERNEL);
+	if (drvdata->wr_dmactl == NULL)
+		return -ENOMEM;
+
+	rd_dmactl = drvdata->rd_dmactl;
+	wr_dmactl = drvdata->wr_dmactl;
+
+	rd_dmactl->bursten = devm_regmap_field_alloc(dev, map, v->rdma_bursten);
+	rd_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, v->rdma_wpscnt);
+	rd_dmactl->fifowm = devm_regmap_field_alloc(dev, map, v->rdma_fifowm);
+	rd_dmactl->intf = devm_regmap_field_alloc(dev, map, v->rdma_intf);
+	rd_dmactl->enable = devm_regmap_field_alloc(dev, map, v->rdma_enable);
+	rd_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, v->rdma_dyncclk);
+
+	if (IS_ERR(rd_dmactl->bursten) || IS_ERR(rd_dmactl->wpscnt) ||
+	    IS_ERR(rd_dmactl->fifowm) || IS_ERR(rd_dmactl->intf) ||
+	    IS_ERR(rd_dmactl->enable) || IS_ERR(rd_dmactl->dyncclk))
+		return -EINVAL;
+
+	wr_dmactl->bursten = devm_regmap_field_alloc(dev, map, v->wrdma_bursten);
+	wr_dmactl->wpscnt = devm_regmap_field_alloc(dev, map, v->wrdma_wpscnt);
+	wr_dmactl->fifowm = devm_regmap_field_alloc(dev, map, v->wrdma_fifowm);
+	wr_dmactl->intf = devm_regmap_field_alloc(dev, map, v->wrdma_intf);
+	wr_dmactl->enable = devm_regmap_field_alloc(dev, map, v->wrdma_enable);
+	wr_dmactl->dyncclk = devm_regmap_field_alloc(dev, map, v->wrdma_dyncclk);
+
+	if (IS_ERR(wr_dmactl->bursten) || IS_ERR(wr_dmactl->wpscnt) ||
+	    IS_ERR(wr_dmactl->fifowm) || IS_ERR(wr_dmactl->intf) ||
+	    IS_ERR(wr_dmactl->enable) || IS_ERR(wr_dmactl->dyncclk))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int lpass_platform_pcmops_open(struct snd_soc_component *component,
 				      struct snd_pcm_substream *substream)
 {
@@ -59,9 +106,9 @@ static int lpass_platform_pcmops_open(struct snd_soc_component *component,
 	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
 	struct lpass_variant *v = drvdata->variant;
 	int ret, dma_ch, dir = substream->stream;
-	struct lpass_pcm_data *data;
+	struct lpass_pcm_data *data = NULL;
 
-	data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL);
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
@@ -111,13 +158,13 @@ static int lpass_platform_pcmops_close(struct snd_soc_component *component,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
 	struct lpass_variant *v = drvdata->variant;
-	struct lpass_pcm_data *data;
+	struct lpass_pcm_data *data = runtime->private_data;
 
-	data = runtime->private_data;
 	drvdata->substream[data->dma_ch] = NULL;
 	if (v->free_dma_channel)
 		v->free_dma_channel(drvdata, data->dma_ch);
 
+	kfree(data);
 	return 0;
 }
 
@@ -132,12 +179,19 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 	struct lpass_variant *v = drvdata->variant;
 	snd_pcm_format_t format = params_format(params);
 	unsigned int channels = params_channels(params);
-	unsigned int regval;
-	int ch, dir = substream->stream;
-	int bitwidth;
+	struct lpaif_dmactl *dmactl;
+	int dir = substream->stream;
+	int id, bitwidth;
 	int ret, dma_port = pcm_data->i2s_port + v->dmactl_audif_start;
 
-	ch = pcm_data->dma_ch;
+
+	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
+		dmactl = drvdata->rd_dmactl;
+		id = pcm_data->dma_ch;
+	} else {
+		dmactl = drvdata->wr_dmactl;
+		id = pcm_data->dma_ch - v->wrdma_channel_start;
+	}
 
 	bitwidth = snd_pcm_format_width(format);
 	if (bitwidth < 0) {
@@ -146,25 +200,29 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 		return bitwidth;
 	}
 
-	regval = LPAIF_DMACTL_BURSTEN_INCR4 |
-			LPAIF_DMACTL_AUDINTF(dma_port) |
-			LPAIF_DMACTL_FIFOWM_8;
+	ret = regmap_fields_write(dmactl->bursten, id, LPAIF_DMACTL_BURSTEN_INCR4);
+	ret = regmap_fields_write(dmactl->fifowm, id, LPAIF_DMACTL_FIFOWM_8);
+	ret = regmap_fields_write(dmactl->intf, id, LPAIF_DMACTL_AUDINTF(dma_port));
 
 	switch (bitwidth) {
 	case 16:
 		switch (channels) {
 		case 1:
 		case 2:
-			regval |= LPAIF_DMACTL_WPSCNT_ONE;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_ONE);
 			break;
 		case 4:
-			regval |= LPAIF_DMACTL_WPSCNT_TWO;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_TWO);
 			break;
 		case 6:
-			regval |= LPAIF_DMACTL_WPSCNT_THREE;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_THREE);
 			break;
 		case 8:
-			regval |= LPAIF_DMACTL_WPSCNT_FOUR;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_FOUR);
 			break;
 		default:
 			dev_err(soc_runtime->dev,
@@ -177,19 +235,24 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 	case 32:
 		switch (channels) {
 		case 1:
-			regval |= LPAIF_DMACTL_WPSCNT_ONE;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_ONE);
 			break;
 		case 2:
-			regval |= LPAIF_DMACTL_WPSCNT_TWO;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_TWO);
 			break;
 		case 4:
-			regval |= LPAIF_DMACTL_WPSCNT_FOUR;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_FOUR);
 			break;
 		case 6:
-			regval |= LPAIF_DMACTL_WPSCNT_SIX;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_SIX);
 			break;
 		case 8:
-			regval |= LPAIF_DMACTL_WPSCNT_EIGHT;
+			ret = regmap_fields_write(dmactl->wpscnt, id,
+						 LPAIF_DMACTL_WPSCNT_EIGHT);
 			break;
 		default:
 			dev_err(soc_runtime->dev,
@@ -204,10 +267,8 @@ static int lpass_platform_pcmops_hw_params(struct snd_soc_component *component,
 		return -EINVAL;
 	}
 
-	ret = regmap_write(drvdata->lpaif_map,
-			LPAIF_DMACTL_REG(v, ch, dir), regval);
 	if (ret) {
-		dev_err(soc_runtime->dev, "error writing to rdmactl reg: %d\n",
+		dev_err(soc_runtime->dev, "error writing to dmactl reg: %d\n",
 			ret);
 		return ret;
 	}
@@ -244,9 +305,17 @@ static int lpass_platform_pcmops_prepare(struct snd_soc_component *component,
 	struct snd_pcm_runtime *rt = substream->runtime;
 	struct lpass_pcm_data *pcm_data = rt->private_data;
 	struct lpass_variant *v = drvdata->variant;
-	int ret, ch, dir = substream->stream;
+	struct lpaif_dmactl *dmactl;
+	int ret, id, ch, dir = substream->stream;
 
 	ch = pcm_data->dma_ch;
+	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
+		dmactl = drvdata->rd_dmactl;
+		id = pcm_data->dma_ch;
+	} else {
+		dmactl = drvdata->wr_dmactl;
+		id = pcm_data->dma_ch - v->wrdma_channel_start;
+	}
 
 	ret = regmap_write(drvdata->lpaif_map,
 			LPAIF_DMABASE_REG(v, ch, dir),
@@ -275,9 +344,7 @@ static int lpass_platform_pcmops_prepare(struct snd_soc_component *component,
 		return ret;
 	}
 
-	ret = regmap_update_bits(drvdata->lpaif_map,
-			LPAIF_DMACTL_REG(v, ch, dir),
-			LPAIF_DMACTL_ENABLE_MASK, LPAIF_DMACTL_ENABLE_ON);
+	ret = regmap_fields_write(dmactl->enable, id, LPAIF_DMACTL_ENABLE_ON);
 	if (ret) {
 		dev_err(soc_runtime->dev, "error writing to rdmactl reg: %d\n",
 			ret);
@@ -296,9 +363,18 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 	struct snd_pcm_runtime *rt = substream->runtime;
 	struct lpass_pcm_data *pcm_data = rt->private_data;
 	struct lpass_variant *v = drvdata->variant;
-	int ret, ch, dir = substream->stream;
+	struct lpaif_dmactl *dmactl;
+	int ret, ch, id;
+	int dir = substream->stream;
 
 	ch = pcm_data->dma_ch;
+	if (dir ==  SNDRV_PCM_STREAM_PLAYBACK) {
+		dmactl = drvdata->rd_dmactl;
+		id = pcm_data->dma_ch;
+	} else {
+		dmactl = drvdata->wr_dmactl;
+		id = pcm_data->dma_ch - v->wrdma_channel_start;
+	}
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -324,10 +400,8 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 			return ret;
 		}
 
-		ret = regmap_update_bits(drvdata->lpaif_map,
-				LPAIF_DMACTL_REG(v, ch, dir),
-				LPAIF_DMACTL_ENABLE_MASK,
-				LPAIF_DMACTL_ENABLE_ON);
+		ret = regmap_fields_write(dmactl->enable, id,
+					 LPAIF_DMACTL_ENABLE_ON);
 		if (ret) {
 			dev_err(soc_runtime->dev,
 				"error writing to rdmactl reg: %d\n", ret);
@@ -337,10 +411,8 @@ static int lpass_platform_pcmops_trigger(struct snd_soc_component *component,
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		ret = regmap_update_bits(drvdata->lpaif_map,
-				LPAIF_DMACTL_REG(v, ch, dir),
-				LPAIF_DMACTL_ENABLE_MASK,
-				LPAIF_DMACTL_ENABLE_OFF);
+		ret = regmap_fields_write(dmactl->enable, id,
+					 LPAIF_DMACTL_ENABLE_OFF);
 		if (ret) {
 			dev_err(soc_runtime->dev,
 				"error writing to rdmactl reg: %d\n", ret);
@@ -580,6 +652,13 @@ int asoc_qcom_lpass_platform_register(struct platform_device *pdev)
 		return ret;
 	}
 
+	ret = lpass_platform_alloc_dmactl_fields(&pdev->dev,
+						 drvdata->lpaif_map);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"error initializing dmactl fields: %d\n", ret);
+		return ret;
+	}
 
 	return devm_snd_soc_register_component(&pdev->dev,
 			&lpass_component_driver, NULL, 0);
diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
index 450020e..4294ec2 100644
--- a/sound/soc/qcom/lpass.h
+++ b/sound/soc/qcom/lpass.h
@@ -17,6 +17,28 @@
 #define LPASS_MAX_MI2S_PORTS			(8)
 #define LPASS_MAX_DMA_CHANNELS			(8)
 
+struct lpaif_i2sctl {
+	struct regmap_field *loopback;
+	struct regmap_field *spken;
+	struct regmap_field *spkmode;
+	struct regmap_field *spkmono;
+	struct regmap_field *micen;
+	struct regmap_field *micmode;
+	struct regmap_field *micmono;
+	struct regmap_field *wssrc;
+	struct regmap_field *bitwidth;
+};
+
+
+struct lpaif_dmactl {
+	struct regmap_field *bursten;
+	struct regmap_field *wpscnt;
+	struct regmap_field *intf;
+	struct regmap_field *fifowm;
+	struct regmap_field *enable;
+	struct regmap_field *dyncclk;
+};
+
 /* Both the CPU DAI and platform drivers will access this data */
 struct lpass_data {
 
@@ -55,6 +77,10 @@ struct lpass_data {
 	struct clk_bulk_data *clks;
 	int num_clks;
 
+	/* Regmap fields of I2SCTL & DMACTL registers bitfields */
+	struct lpaif_i2sctl *i2sctl;
+	struct lpaif_dmactl *rd_dmactl;
+	struct lpaif_dmactl *wr_dmactl;
 };
 
 /* Vairant data per each SOC */
@@ -72,6 +98,33 @@ struct lpass_variant {
 	u32	wrdma_reg_stride;
 	u32	wrdma_channels;
 
+	/* I2SCTL Register fields */
+	struct reg_field loopback;
+	struct reg_field spken;
+	struct reg_field spkmode;
+	struct reg_field spkmono;
+	struct reg_field micen;
+	struct reg_field micmode;
+	struct reg_field micmono;
+	struct reg_field wssrc;
+	struct reg_field bitwidth;
+
+	/* RD_DMA Register fields */
+	struct reg_field rdma_bursten;
+	struct reg_field rdma_wpscnt;
+	struct reg_field rdma_intf;
+	struct reg_field rdma_fifowm;
+	struct reg_field rdma_enable;
+	struct reg_field rdma_dyncclk;
+
+	/* RD_DMA Register fields */
+	struct reg_field wrdma_bursten;
+	struct reg_field wrdma_wpscnt;
+	struct reg_field wrdma_intf;
+	struct reg_field wrdma_fifowm;
+	struct reg_field wrdma_enable;
+	struct reg_field wrdma_dyncclk;
+
 	/**
 	 * on SOCs like APQ8016 the channel control bits start
 	 * at different offset to ipq806x
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 4/8] include: dt-bindings: sound: Add sc7180-lpass bindings header
  2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
                   ` (2 preceding siblings ...)
  2020-07-08  5:08 ` [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers Rohit kumar
@ 2020-07-08  5:08 ` Rohit kumar
  2020-07-08 14:45   ` Mark Brown
  2020-07-08  5:08 ` [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev Rohit kumar
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Ajit Pandey

From: Ajit Pandey <ajitp@codeaurora.org>

Add header defining dai-id and mclk id for SC7180 lpass soc.

Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
---
 include/dt-bindings/sound/sc7180-lpass.h | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 include/dt-bindings/sound/sc7180-lpass.h

diff --git a/include/dt-bindings/sound/sc7180-lpass.h b/include/dt-bindings/sound/sc7180-lpass.h
new file mode 100644
index 00000000..7d988f6
--- /dev/null
+++ b/include/dt-bindings/sound/sc7180-lpass.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DT_SC7180_LPASS_H
+#define __DT_SC7180_LPASS_H
+
+#define MI2S_PRIMARY	0
+#define MI2S_SECONDARY	1
+
+#define LPASS_MCLK0	0
+
+#endif /* __DT_APQ8016_LPASS_H */
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev
  2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
                   ` (3 preceding siblings ...)
  2020-07-08  5:08 ` [PATCH v3 4/8] include: dt-bindings: sound: Add sc7180-lpass bindings header Rohit kumar
@ 2020-07-08  5:08 ` Rohit kumar
  2020-07-08 16:50   ` Mark Brown
  2020-07-09  9:26   ` Srinivas Kandagatla
  2020-07-08  5:08 ` [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node Rohit kumar
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Ajit Pandey

From: Ajit Pandey <ajitp@codeaurora.org>

We are allocating dma memory for component->dev but trying to mmap
such memory for substream->pcm->card->dev. Replace device argument
in mmap with component->dev to fix this.

Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
---
 sound/soc/qcom/lpass-platform.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index 445ca193..f9424cc 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -472,9 +472,8 @@ static int lpass_platform_pcmops_mmap(struct snd_soc_component *component,
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
-	return dma_mmap_coherent(substream->pcm->card->dev, vma,
-			runtime->dma_area, runtime->dma_addr,
-			runtime->dma_bytes);
+	return dma_mmap_coherent(component->dev, vma, runtime->dma_area,
+				 runtime->dma_addr, runtime->dma_bytes);
 }
 
 static irqreturn_t lpass_dma_interrupt_handler(
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node
  2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
                   ` (4 preceding siblings ...)
  2020-07-08  5:08 ` [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev Rohit kumar
@ 2020-07-08  5:08 ` Rohit kumar
  2020-07-09  9:27   ` Srinivas Kandagatla
  2020-07-08  5:08 ` [PATCH v3 7/8] ASoC: qcom: lpass-sc7180: Add platform driver for lpass audio Rohit kumar
  2020-07-08  5:08 ` [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format Rohit kumar
  7 siblings, 1 reply; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Rohit kumar

Add dt-bindings to support "qcom,lpass-cpu-sc7180" node.

Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
index 32c2cdb..04e34cc 100644
--- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
@@ -4,7 +4,8 @@ This node models the Qualcomm Technologies Low-Power Audio SubSystem (LPASS).
 
 Required properties:
 
-- compatible		: "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu"
+- compatible		: "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
+			  "qcom,lpass-cpu-sc7180"
 - clocks		: Must contain an entry for each entry in clock-names.
 - clock-names		: A list which must include the following entries:
 				* "ahbix-clk"
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 7/8] ASoC: qcom: lpass-sc7180: Add platform driver for lpass audio
  2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
                   ` (5 preceding siblings ...)
  2020-07-08  5:08 ` [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node Rohit kumar
@ 2020-07-08  5:08 ` Rohit kumar
  2020-07-09  9:27   ` Srinivas Kandagatla
  2020-07-08  5:08 ` [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format Rohit kumar
  7 siblings, 1 reply; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Rohit kumar, Ajit Pandey

From: Ajit Pandey <ajitp@codeaurora.org>

Add platform driver for configuring sc7180 lpass core I2S and
DMA configuration to support playback & capture to external codecs
connected over primary & secondary MI2S interfaces.

Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
Signed-off-by: Rohit kumar <rohkumar@qti.qualcomm.com>
---
 sound/soc/qcom/Kconfig        |   5 +
 sound/soc/qcom/Makefile       |   2 +
 sound/soc/qcom/lpass-sc7180.c | 216 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 223 insertions(+)
 create mode 100644 sound/soc/qcom/lpass-sc7180.c

diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index 0ea4cde..87bec7f 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -24,6 +24,11 @@ config SND_SOC_LPASS_APQ8016
 	select SND_SOC_LPASS_CPU
 	select SND_SOC_LPASS_PLATFORM
 
+config SND_SOC_LPASS_SC7180
+	tristate
+	select SND_SOC_LPASS_CPU
+	select SND_SOC_LPASS_PLATFORM
+
 config SND_SOC_STORM
 	tristate "ASoC I2S support for Storm boards"
 	depends on SND_SOC_QCOM
diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile
index 41b2c7a..7972c94 100644
--- a/sound/soc/qcom/Makefile
+++ b/sound/soc/qcom/Makefile
@@ -4,11 +4,13 @@ snd-soc-lpass-cpu-objs := lpass-cpu.o
 snd-soc-lpass-platform-objs := lpass-platform.o
 snd-soc-lpass-ipq806x-objs := lpass-ipq806x.o
 snd-soc-lpass-apq8016-objs := lpass-apq8016.o
+snd-soc-lpass-sc7180-objs := lpass-sc7180.o
 
 obj-$(CONFIG_SND_SOC_LPASS_CPU) += snd-soc-lpass-cpu.o
 obj-$(CONFIG_SND_SOC_LPASS_PLATFORM) += snd-soc-lpass-platform.o
 obj-$(CONFIG_SND_SOC_LPASS_IPQ806X) += snd-soc-lpass-ipq806x.o
 obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o
+obj-$(CONFIG_SND_SOC_LPASS_SC7180) += snd-soc-lpass-sc7180.o
 
 # Machine
 snd-soc-storm-objs := storm.o
diff --git a/sound/soc/qcom/lpass-sc7180.c b/sound/soc/qcom/lpass-sc7180.c
new file mode 100644
index 00000000..dd85a97
--- /dev/null
+++ b/sound/soc/qcom/lpass-sc7180.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ *
+ * lpass-sc7180.c -- ALSA SoC platform-machine driver for QTi LPASS
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <dt-bindings/sound/sc7180-lpass.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+
+#include "lpass-lpaif-reg.h"
+#include "lpass.h"
+
+static struct snd_soc_dai_driver sc7180_lpass_cpu_dai_driver[] = {
+	[MI2S_PRIMARY] = {
+		.id = MI2S_PRIMARY,
+		.name = "Primary MI2S",
+		.playback = {
+			.stream_name = "Primary Playback",
+			.formats	= SNDRV_PCM_FMTBIT_S16,
+			.rates = SNDRV_PCM_RATE_48000,
+			.rate_min	= 48000,
+			.rate_max	= 48000,
+			.channels_min	= 2,
+			.channels_max	= 2,
+		},
+		.capture = {
+			.stream_name = "Primary Capture",
+			.formats = SNDRV_PCM_FMTBIT_S16,
+			.rates = SNDRV_PCM_RATE_48000,
+			.rate_min	= 48000,
+			.rate_max	= 48000,
+			.channels_min	= 2,
+			.channels_max	= 2,
+		},
+		.probe	= &asoc_qcom_lpass_cpu_dai_probe,
+		.ops    = &asoc_qcom_lpass_cpu_dai_ops,
+	},
+
+	[MI2S_SECONDARY] = {
+		.id = MI2S_SECONDARY,
+		.name = "Secondary MI2S",
+		.playback = {
+			.stream_name = "Secondary Playback",
+			.formats	= SNDRV_PCM_FMTBIT_S16,
+			.rates = SNDRV_PCM_RATE_48000,
+			.rate_min	= 48000,
+			.rate_max	= 48000,
+			.channels_min	= 2,
+			.channels_max	= 2,
+		},
+		.probe	= &asoc_qcom_lpass_cpu_dai_probe,
+		.ops    = &asoc_qcom_lpass_cpu_dai_ops,
+	},
+};
+
+static int sc7180_lpass_alloc_dma_channel(struct lpass_data *drvdata,
+					   int direction)
+{
+	struct lpass_variant *v = drvdata->variant;
+	int chan = 0;
+
+	if (direction == SNDRV_PCM_STREAM_PLAYBACK) {
+		chan = find_first_zero_bit(&drvdata->dma_ch_bit_map,
+					v->rdma_channels);
+
+		if (chan >= v->rdma_channels)
+			return -EBUSY;
+	} else {
+		chan = find_next_zero_bit(&drvdata->dma_ch_bit_map,
+					v->wrdma_channel_start +
+					v->wrdma_channels,
+					v->wrdma_channel_start);
+
+		if (chan >=  v->wrdma_channel_start + v->wrdma_channels)
+			return -EBUSY;
+	}
+
+	set_bit(chan, &drvdata->dma_ch_bit_map);
+
+	return chan;
+}
+
+static int sc7180_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
+{
+	clear_bit(chan, &drvdata->dma_ch_bit_map);
+
+	return 0;
+}
+
+static int sc7180_lpass_init(struct platform_device *pdev)
+{
+	struct lpass_data *drvdata = platform_get_drvdata(pdev);
+	struct lpass_variant *variant = drvdata->variant;
+	struct device *dev = &pdev->dev;
+	int ret, i;
+
+	drvdata->clks = devm_kcalloc(dev, variant->num_clks,
+				     sizeof(*drvdata->clks), GFP_KERNEL);
+	drvdata->num_clks = variant->num_clks;
+
+	for (i = 0; i < drvdata->num_clks; i++)
+		drvdata->clks[i].id = variant->clk_name[i];
+
+	ret = devm_clk_bulk_get(dev, drvdata->num_clks, drvdata->clks);
+	if (ret) {
+		dev_err(dev, "Failed to get clocks %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
+	if (ret) {
+		dev_err(dev, "sc7180 clk_enable failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int sc7180_lpass_exit(struct platform_device *pdev)
+{
+	struct lpass_data *drvdata = platform_get_drvdata(pdev);
+
+	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
+
+	return 0;
+}
+
+static struct lpass_variant sc7180_data = {
+	.i2sctrl_reg_base	= 0x1000,
+	.i2sctrl_reg_stride	= 0x1000,
+	.i2s_ports		= 3,
+	.irq_reg_base		= 0x9000,
+	.irq_reg_stride		= 0x1000,
+	.irq_ports		= 3,
+	.rdma_reg_base		= 0xC000,
+	.rdma_reg_stride	= 0x1000,
+	.rdma_channels		= 5,
+	.dmactl_audif_start	= 1,
+	.wrdma_reg_base		= 0x18000,
+	.wrdma_reg_stride	= 0x1000,
+	.wrdma_channel_start	= 5,
+	.wrdma_channels		= 4,
+
+	.loopback		= REG_FIELD_ID(0x1000, 17, 17, 3, 0x1000),
+	.spken			= REG_FIELD_ID(0x1000, 16, 16, 3, 0x1000),
+	.spkmode		= REG_FIELD_ID(0x1000, 11, 15, 3, 0x1000),
+	.spkmono		= REG_FIELD_ID(0x1000, 10, 10, 3, 0x1000),
+	.micen			= REG_FIELD_ID(0x1000, 9, 9, 3, 0x1000),
+	.micmode		= REG_FIELD_ID(0x1000, 4, 8, 3, 0x1000),
+	.micmono		= REG_FIELD_ID(0x1000, 3, 3, 3, 0x1000),
+	.wssrc			= REG_FIELD_ID(0x1000, 2, 2, 3, 0x1000),
+	.bitwidth		= REG_FIELD_ID(0x1000, 0, 0, 3, 0x1000),
+
+	.rdma_dyncclk		= REG_FIELD_ID(0xC000, 21, 21, 5, 0x1000),
+	.rdma_bursten		= REG_FIELD_ID(0xC000, 20, 20, 5, 0x1000),
+	.rdma_wpscnt		= REG_FIELD_ID(0xC000, 16, 19, 5, 0x1000),
+	.rdma_intf		= REG_FIELD_ID(0xC000, 12, 15, 5, 0x1000),
+	.rdma_fifowm		= REG_FIELD_ID(0xC000, 1, 5, 5, 0x1000),
+	.rdma_enable		= REG_FIELD_ID(0xC000, 0, 0, 5, 0x1000),
+
+	.wrdma_dyncclk		= REG_FIELD_ID(0x18000, 22, 22, 4, 0x1000),
+	.wrdma_bursten		= REG_FIELD_ID(0x18000, 21, 21, 4, 0x1000),
+	.wrdma_wpscnt		= REG_FIELD_ID(0x18000, 17, 20, 4, 0x1000),
+	.wrdma_intf		= REG_FIELD_ID(0x18000, 12, 16, 4, 0x1000),
+	.wrdma_fifowm		= REG_FIELD_ID(0x18000, 1, 5, 4, 0x1000),
+	.wrdma_enable		= REG_FIELD_ID(0x18000, 0, 0, 4, 0x1000),
+
+	.clk_name		= (const char*[]) {
+				   "noc",
+				   "audio-core",
+				   "sysnoc_mport",
+				},
+	.num_clks		= 3,
+	.dai_driver		= sc7180_lpass_cpu_dai_driver,
+	.num_dai		= ARRAY_SIZE(sc7180_lpass_cpu_dai_driver),
+	.dai_osr_clk_names      = (const char *[]) {
+				   "mclk0",
+				   "null",
+				},
+	.dai_bit_clk_names      = (const char *[]) {
+				   "pri_ibit",
+				   "sec_ibit",
+				},
+	.init			= sc7180_lpass_init,
+	.exit			= sc7180_lpass_exit,
+	.alloc_dma_channel	= sc7180_lpass_alloc_dma_channel,
+	.free_dma_channel	= sc7180_lpass_free_dma_channel,
+};
+
+static const struct of_device_id sc7180_lpass_cpu_device_id[] = {
+	{.compatible = "qcom,lpass-cpu-sc7180", .data = &sc7180_data},
+	{}
+};
+
+static struct platform_driver sc7180_lpass_cpu_platform_driver = {
+	.driver = {
+		.name = "sc7180-lpass-cpu",
+		.of_match_table = of_match_ptr(sc7180_lpass_cpu_device_id),
+	},
+	.probe = asoc_qcom_lpass_cpu_platform_probe,
+	.remove = asoc_qcom_lpass_cpu_platform_probe,
+};
+
+module_platform_driver(sc7180_lpass_cpu_platform_driver);
+
+MODULE_DESCRIPTION("SC7180 LPASS CPU 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.


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format
  2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
                   ` (6 preceding siblings ...)
  2020-07-08  5:08 ` [PATCH v3 7/8] ASoC: qcom: lpass-sc7180: Add platform driver for lpass audio Rohit kumar
@ 2020-07-08  5:08 ` Rohit kumar
  2020-07-13 22:53   ` Rob Herring
  7 siblings, 1 reply; 29+ messages in thread
From: Rohit kumar @ 2020-07-08  5:08 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel
  Cc: Rohit kumar, Ajit Pandey

From: Ajit Pandey <ajitp@codeaurora.org>

Update lpass-cpu binding with yaml formats.

Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
---
 .../devicetree/bindings/sound/qcom,lpass-cpu.txt   |  80 -----------
 .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 154 +++++++++++++++++++++
 2 files changed, 154 insertions(+), 80 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
 create mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml

diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
deleted file mode 100644
index 04e34cc..00000000
--- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
+++ /dev/null
@@ -1,80 +0,0 @@
-* Qualcomm Technologies LPASS CPU DAI
-
-This node models the Qualcomm Technologies Low-Power Audio SubSystem (LPASS).
-
-Required properties:
-
-- compatible		: "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
-			  "qcom,lpass-cpu-sc7180"
-- clocks		: Must contain an entry for each entry in clock-names.
-- clock-names		: A list which must include the following entries:
-				* "ahbix-clk"
-				* "mi2s-osr-clk"
-				* "mi2s-bit-clk"
-			: required clocks for "qcom,lpass-cpu-apq8016"
-				* "ahbix-clk"
-				* "mi2s-bit-clk0"
-				* "mi2s-bit-clk1"
-				* "mi2s-bit-clk2"
-				* "mi2s-bit-clk3"
-				* "pcnoc-mport-clk"
-				* "pcnoc-sway-clk"
-
-- interrupts		: Must contain an entry for each entry in
-			  interrupt-names.
-- interrupt-names	: A list which must include the following entries:
-				* "lpass-irq-lpaif"
-- pinctrl-N		: One property must exist for each entry in
-			  pinctrl-names.  See ../pinctrl/pinctrl-bindings.txt
-			  for details of the property values.
-- pinctrl-names		: Must contain a "default" entry.
-- reg			: Must contain an address for each entry in reg-names.
-- reg-names		: A list which must include the following entries:
-				* "lpass-lpaif"
-- #address-cells	: Must be 1
-- #size-cells		: Must be 0
-
-
-
-Optional properties:
-
-- qcom,adsp		: Phandle for the audio DSP node
-
-By default, the driver uses up to 4 MI2S SD lines, for a total of 8 channels.
-The SD lines to use can be configured by adding subnodes for each of the DAIs.
-
-Required properties for each DAI (represented by a subnode):
-- reg			: Must be one of the DAI IDs
-			  (usually part of dt-bindings header)
-- qcom,playback-sd-lines: List of serial data lines to use for playback
-			  Each SD line should be represented by a number from 0-3.
-- qcom,capture-sd-lines	: List of serial data lines to use for capture
-			  Each SD line should be represented by a number from 0-3.
-
-Note that adding a subnode changes the default to "no lines configured",
-so both playback and capture lines should be configured when a subnode is added.
-
-Example:
-
-lpass@28100000 {
-	compatible = "qcom,lpass-cpu";
-	clocks = <&lcc AHBIX_CLK>, <&lcc MI2S_OSR_CLK>, <&lcc MI2S_BIT_CLK>;
-	clock-names = "ahbix-clk", "mi2s-osr-clk", "mi2s-bit-clk";
-	interrupts = <0 85 1>;
-	interrupt-names = "lpass-irq-lpaif";
-	pinctrl-names = "default", "idle";
-	pinctrl-0 = <&mi2s_default>;
-	pinctrl-1 = <&mi2s_idle>;
-	reg = <0x28100000 0x10000>;
-	reg-names = "lpass-lpaif";
-	qcom,adsp = <&adsp>;
-
-	#address-cells = <1>;
-	#size-cells = <0>;
-
-	/* Optional to set different MI2S SD lines */
-	dai@3 {
-		reg = <MI2S_QUATERNARY>;
-		qcom,playback-sd-lines = <0 1>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
new file mode 100644
index 00000000..9c350bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/qcom,lpass-cpu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm LPASS CPU dai driver bindings
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+  - Rohit kumar <rohitkr@codeaurora.org>
+
+description:
+  Qualcomm SOC Low-Power Audio SubSystem (LPASS) that consist of MI2S interface
+  for audio data transfer on external codecs. LPASS cpu driver is a module to
+  configure Low-Power Audio Interface(LPAIF) core registers across different
+  IP versions.
+
+properties:
+  compatible:
+    enum:
+      - qcom,lpass-cpu
+      - qcom,apq8016-lpass-cpu
+      - qcom,lpass-cpu-sc7180
+
+  reg:
+    items:
+      - description: LPAIF core registers
+
+  reg-names:
+    items:
+      - const: lpass-lpaif
+
+  clocks:
+    items:
+      - description: AHBIX core clock
+      - description: oscillator clock for MI2S external interfaces
+      - description: Bit clock for single MI2S dai
+      - description: Bit clock for MI2S_PRIMARY dai interface
+      - description: Bit clock for MI2S_SECONDARY dai interface
+      - description: Bit clock for MI2S_TERTIARY dai interface
+      - description: Bit clock for MI2S_QUATERNARY dai interface
+      - description: NOC MPORT clock of LPASS core
+      - description: NOC SWAY clock of LPASS core
+
+  clock-names:
+    items:
+      - const: ahbix-clk
+      - const: mi2s-osr-clk
+      - const: mi2s-bit-clk
+      - const: mi2s-bit-clk0
+      - const: mi2s-bit-clk1
+      - const: mi2s-bit-clk2
+      - const: mi2s-bit-clk3
+      - const: pcnoc-mport-clk
+      - const: pcnoc-sway-clk
+
+  interrupts:
+    items:
+      - description: LPAIF DMA buffer interrupt
+
+  interrupt-names:
+    items:
+      - const: lpass-irq-lpaif
+
+  qcom,adsp:
+    maxItems: 1
+    description: Phandle for the audio DSP node
+
+  iommus:
+    maxItems: 1
+    description: Phandle to apps_smmu node with sid mask
+
+  power-domains:
+    maxItems: 1
+    description: Phandle for power domain node
+
+  '#sound-dai-cells':
+    const: 1
+
+  child-node:
+    description: Required properties for each DAI
+    type: object
+    properties:
+      reg:
+        description: Must be one of the DAI ID
+                     (Usually part of dtbindings header)
+      qcom,playback-sd-lines:
+        description: List of serial data lines to use for playback
+                     Each SD line should be represented by a number from 0-3.
+      qcom,capture-sd-lines :
+        description: List of serial data lines to use for capture
+                     Each SD line should be represented by a number from 0-3.
+    required:
+      -reg
+    # Note that adding a subnode changes the default to "no lines configured",
+    # so both playback and capture lines should be configured when a subnode
+    # is added.
+
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - interrupts
+  - interrupt-names
+  - sound-dai-cells
+
+optional:
+  - qcom,adsp
+
+if:
+  properties:
+    compatible:
+      contains:
+        const: qcom,lpass-cpu-sc7180
+
+then:
+  required:
+    - iommus
+    - power-domains
+
+examples:
+  lpass@28100000 {
+	compatible = "qcom,lpass-cpu";
+	clocks = <&lcc AHBIX_CLK>,
+		 <&lcc MI2S_OSR_CLK>,
+		 <&lcc MI2S_BIT_CLK>;
+
+	clock-names = "ahbix-clk",
+		      "mi2s-osr-clk",
+		      "mi2s-bit-clk";
+
+	interrupts = <0 85 1>;
+        interrupt-names = "lpass-irq-lpaif";
+
+	iommus = <&apps_smmu 0x1020 0>;
+	power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>;
+
+	reg = <0x28100000 0x10000>;
+	reg-names = "lpass-lpaif";
+	#sound-dai-cells = <1>;
+	qcom,adsp = <&adsp>;
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	/* Optional to set different MI2S SD lines */
+	mi2s-quaternary@3 {
+		reg = <MI2S_QUATERNARY>;
+		qcom,playback-sd-lines = <0 1>;
+  };
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 4/8] include: dt-bindings: sound: Add sc7180-lpass bindings header
  2020-07-08  5:08 ` [PATCH v3 4/8] include: dt-bindings: sound: Add sc7180-lpass bindings header Rohit kumar
@ 2020-07-08 14:45   ` Mark Brown
  2020-07-09 10:06     ` Rohit Kumar
  0 siblings, 1 reply; 29+ messages in thread
From: Mark Brown @ 2020-07-08 14:45 UTC (permalink / raw)
  To: Rohit kumar
  Cc: devicetree, alsa-devel, bgoswami, linux-arm-msm, plai, tiwai,
	lgirdwood, robh+dt, bjorn.andersson, agross, srinivas.kandagatla,
	Ajit Pandey, linux-kernel

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

On Wed, Jul 08, 2020 at 10:38:12AM +0530, Rohit kumar wrote:
> From: Ajit Pandey <ajitp@codeaurora.org>
> 
> Add header defining dai-id and mclk id for SC7180 lpass soc.
> 
> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
> ---

This one is missing a signoff as well, and I can't seem to see any
reference to this header in the bindings document patches?

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev
  2020-07-08  5:08 ` [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev Rohit kumar
@ 2020-07-08 16:50   ` Mark Brown
  2020-07-09  3:46     ` Rohit Kumar
  2020-07-09  9:26   ` Srinivas Kandagatla
  1 sibling, 1 reply; 29+ messages in thread
From: Mark Brown @ 2020-07-08 16:50 UTC (permalink / raw)
  To: Rohit kumar
  Cc: devicetree, alsa-devel, bgoswami, linux-arm-msm, plai, tiwai,
	lgirdwood, robh+dt, bjorn.andersson, agross, srinivas.kandagatla,
	Ajit Pandey, linux-kernel

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

On Wed, Jul 08, 2020 at 10:38:13AM +0530, Rohit kumar wrote:
> From: Ajit Pandey <ajitp@codeaurora.org>
> 
> We are allocating dma memory for component->dev but trying to mmap
> such memory for substream->pcm->card->dev. Replace device argument
> in mmap with component->dev to fix this.

This is a bug fix and should've been at the start of the series (or sent
separately) so that it can be applied without the rest of the series.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev
  2020-07-08 16:50   ` Mark Brown
@ 2020-07-09  3:46     ` Rohit Kumar
  0 siblings, 0 replies; 29+ messages in thread
From: Rohit Kumar @ 2020-07-09  3:46 UTC (permalink / raw)
  To: Mark Brown
  Cc: devicetree, alsa-devel, bgoswami, linux-arm-msm, plai, tiwai,
	lgirdwood, robh+dt, bjorn.andersson, agross, srinivas.kandagatla,
	Ajit Pandey, linux-kernel


On 7/8/2020 10:20 PM, Mark Brown wrote:
> On Wed, Jul 08, 2020 at 10:38:13AM +0530, Rohit kumar wrote:
>> From: Ajit Pandey <ajitp@codeaurora.org>
>>
>> We are allocating dma memory for component->dev but trying to mmap
>> such memory for substream->pcm->card->dev. Replace device argument
>> in mmap with component->dev to fix this.
> This is a bug fix and should've been at the start of the series (or sent
> separately) so that it can be applied without the rest of the series.

Thanks Mark for the suggestion. I will send it separately.

For other patches in series, I will wait for comments before posting next

patchset.

Thanks,

Rohit

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 2/8] ASoC: qcom: lpass-cpu: Move ahbix clk to platform specific function
  2020-07-08  5:08 ` [PATCH v3 2/8] ASoC: qcom: lpass-cpu: Move ahbix clk to platform specific function Rohit kumar
@ 2020-07-09  9:26   ` Srinivas Kandagatla
  0 siblings, 0 replies; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09  9:26 UTC (permalink / raw)
  To: Rohit kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel



On 08/07/2020 06:08, Rohit kumar wrote:
> Ahbix clock is optional clock and not needed for all platforms.
> Move it to lpass-apq8016/ipq806x as it is not needed for sc7180.
> 
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
>   sound/soc/qcom/lpass-apq8016.c | 27 ++++++++++++++++++++++++++
>   sound/soc/qcom/lpass-cpu.c     | 40 ++++++++++-----------------------------
>   sound/soc/qcom/lpass-ipq806x.c | 43 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 80 insertions(+), 30 deletions(-)
> 

LGTM,

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 1/8] ASoC: qcom: Add common array to initialize soc based core clocks
  2020-07-08  5:08 ` [PATCH v3 1/8] ASoC: qcom: Add common array to initialize soc based core clocks Rohit kumar
@ 2020-07-09  9:26   ` Srinivas Kandagatla
  0 siblings, 0 replies; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09  9:26 UTC (permalink / raw)
  To: Rohit kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel
  Cc: Ajit Pandey



On 08/07/2020 06:08, Rohit kumar wrote:
> From: Ajit Pandey <ajitp@codeaurora.org>
> 
> LPASS variants have their own soc specific clocks that needs to be
> enabled for MI2S audio support. Added a common variable in drvdata to
> initialize such clocks using bulk clk api. Such clock names is
> defined in variants specific data and needs to fetched during init.
> 
> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
>   sound/soc/qcom/lpass-apq8016.c | 39 +++++++++++++++++++--------------------
>   sound/soc/qcom/lpass.h         | 10 +++++++---
>   2 files changed, 26 insertions(+), 23 deletions(-)

LGTM,

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers
  2020-07-08  5:08 ` [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers Rohit kumar
@ 2020-07-09  9:26   ` Srinivas Kandagatla
  2020-07-09  9:57     ` Rohit Kumar
  0 siblings, 1 reply; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09  9:26 UTC (permalink / raw)
  To: Rohit kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel



On 08/07/2020 06:08, Rohit kumar wrote:
> I2SCTL and DMACTL registers has different bits alignment for newer
> LPASS variants of SC7180 soc. Use REG_FIELD_ID() to define the
> reg_fields in platform specific file and removed shifts and mask
> macros for such registers from header file.
> 
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>

Thanks Rohit for doing this, this looks much better now!
I have few minor comments..

> ---
>   sound/soc/qcom/lpass-apq8016.c   |  24 ++++++
>   sound/soc/qcom/lpass-cpu.c       | 163 +++++++++++++++++++++++----------------
>   sound/soc/qcom/lpass-ipq806x.c   |  24 ++++++
>   sound/soc/qcom/lpass-lpaif-reg.h | 157 +++++++++++++++++++------------------
>   sound/soc/qcom/lpass-platform.c  | 151 +++++++++++++++++++++++++++---------
>   sound/soc/qcom/lpass.h           |  53 +++++++++++++
>   6 files changed, 398 insertions(+), 174 deletions(-)
> 

index f0c7e93..f358d12 100644
> --- a/sound/soc/qcom/lpass-cpu.c
> +++ b/sound/soc/qcom/lpass-cpu.c
> @@ -29,6 +29,32 @@
>   #define LPASS_CPU_I2S_SD0_1_2_MASK	GENMASK(2, 0)
>   #define LPASS_CPU_I2S_SD0_1_2_3_MASK	GENMASK(3, 0)
>   


>   static int lpass_cpu_daiops_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>   		unsigned int freq, int dir)
>   {
> @@ -79,12 +105,13 @@ static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
>   		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>   {
>   	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +	struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
> +	unsigned int id = dai->driver->id;
>   	snd_pcm_format_t format = params_format(params);
>   	unsigned int channels = params_channels(params);
>   	unsigned int rate = params_rate(params);
>   	unsigned int mode;
> -	unsigned int regval;
> -	int bitwidth, ret;
> +	int bitwidth, ret, regval;

Why is this change?

>   
>   	bitwidth = snd_pcm_format_width(format);
>   	if (bitwidth < 0) {
> @@ -92,28 +119,45 @@ static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
>   		return bitwidth;
>   	}
>   
> -	regval = LPAIF_I2SCTL_LOOPBACK_DISABLE |
> -			LPAIF_I2SCTL_WSSRC_INTERNAL;
> +	ret = regmap_fields_write(i2sctl->loopback, id,
> +				 LPAIF_I2SCTL_LOOPBACK_DISABLE);
> +	if (ret) {
> +		dev_err(dai->dev, "error updating loopback field: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_fields_write(i2sctl->wssrc, id,
> +				 LPAIF_I2SCTL_WSSRC_INTERNAL);
> +	if (ret) {
> +		dev_err(dai->dev, "error updating wssrc field: %d\n", ret);
> +		return ret;
> +	}
>   
>   	switch (bitwidth) {
>   	case 16:
> -		regval |= LPAIF_I2SCTL_BITWIDTH_16;
> +		regval = LPAIF_I2SCTL_BITWIDTH_16;
>   		break;
>   	case 24:
> -		regval |= LPAIF_I2SCTL_BITWIDTH_24;
> +		regval = LPAIF_I2SCTL_BITWIDTH_24;
>   		break;
>   	case 32:
> -		regval |= LPAIF_I2SCTL_BITWIDTH_32;
> +		regval = LPAIF_I2SCTL_BITWIDTH_32;
>   		break;
>   	default:
>   		dev_err(dai->dev, "invalid bitwidth given: %d\n", bitwidth);
>   		return -EINVAL;
>   	}
>   
> +	ret = regmap_fields_write(i2sctl->bitwidth, id, regval);
> +	if (ret) {
> +		dev_err(dai->dev, "error updating bitwidth field: %d\n", ret);
> +		return ret;
> +	}
> +
>   	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> -		mode = drvdata->mi2s_playback_sd_mode[dai->driver->id];
> +		mode = drvdata->mi2s_playback_sd_mode[id];
>   	else
> -		mode = drvdata->mi2s_capture_sd_mode[dai->driver->id];
> +		mode = drvdata->mi2s_capture_sd_mode[id];
>   
>   	if (!mode) {
>   		dev_err(dai->dev, "no line is assigned\n");
> @@ -175,30 +219,34 @@ static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
>   	}
>   
>   	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> -		regval |= LPAIF_I2SCTL_SPKMODE(mode);
> +		ret = regmap_fields_write(i2sctl->spkmode, id,
> +					 LPAIF_I2SCTL_SPKMODE(mode));
>   
>   		if (channels >= 2)
> -			regval |= LPAIF_I2SCTL_SPKMONO_STEREO;
> +			ret = regmap_fields_write(i2sctl->spkmono, id,
> +						 LPAIF_I2SCTL_SPKMONO_STEREO);
>   		else
> -			regval |= LPAIF_I2SCTL_SPKMONO_MONO;
> +			ret = regmap_fields_write(i2sctl->spkmono, id,
> +						 LPAIF_I2SCTL_SPKMONO_MONO);
>   	} else {
> -		regval |= LPAIF_I2SCTL_MICMODE(mode);
> +		ret = regmap_fields_write(i2sctl->micmode, id,
> +					 LPAIF_I2SCTL_MICMODE(mode));
>   
>   		if (channels >= 2)
> -			regval |= LPAIF_I2SCTL_MICMONO_STEREO;
> +			ret = regmap_fields_write(i2sctl->micmono, id,
> +						 LPAIF_I2SCTL_MICMONO_STEREO);
>   		else
> -			regval |= LPAIF_I2SCTL_MICMONO_MONO;
> +			ret = regmap_fields_write(i2sctl->micmono, id,
> +						 LPAIF_I2SCTL_MICMONO_MONO);
>   	}
>   
> -	ret = regmap_write(drvdata->lpaif_map,
> -			   LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
> -			   regval);
>   	if (ret) {
> -		dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
> +		dev_err(dai->dev, "error writing to i2sctl channels mode: %d\n",
> +			ret);
>   		return ret;
>   	}
>   
> -	ret = clk_set_rate(drvdata->mi2s_bit_clk[dai->driver->id],
> +	ret = clk_set_rate(drvdata->mi2s_bit_clk[id],
>   			   rate * bitwidth * 2);
>   	if (ret) {
>   		dev_err(dai->dev, "error setting mi2s bitclk to %u: %d\n",
> @@ -209,41 +257,24 @@ static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
>   	return 0;
>   }
>   
> -static int lpass_cpu_daiops_hw_free(struct snd_pcm_substream *substream,
> -		struct snd_soc_dai *dai)
> -{
> -	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> -	int ret;
> -
> -	ret = regmap_write(drvdata->lpaif_map,
> -			   LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
> -			   0);
> -	if (ret)
> -		dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
> -
> -	return ret;
> -}

Any particular reason why this function remove
> -
>   static int lpass_cpu_daiops_prepare(struct snd_pcm_substream *substream,
>   		struct snd_soc_dai *dai)
>   {
>   	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +	struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
> +	unsigned int id = dai->driver->id;
>   	int ret;
> -	unsigned int val, mask;
>   
>   	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> -		val = LPAIF_I2SCTL_SPKEN_ENABLE;
> -		mask = LPAIF_I2SCTL_SPKEN_MASK;
> -	} else  {
> -		val = LPAIF_I2SCTL_MICEN_ENABLE;
> -		mask = LPAIF_I2SCTL_MICEN_MASK;
> +		ret = regmap_fields_write(i2sctl->spken, id,
> +					 LPAIF_I2SCTL_SPKEN_ENABLE);
> +	} else {
> +		ret = regmap_fields_write(i2sctl->micen, id,
> +					 LPAIF_I2SCTL_MICEN_ENABLE);
>   	}
>   
> -	ret = regmap_update_bits(drvdata->lpaif_map,
> -			LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
> -			mask, val);
>   	if (ret)
> -		dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
> +		dev_err(dai->dev, "error writing to i2sctl enable: %d\n", ret);
>   
>   	return ret;
>   }
...
> @@ -304,7 +326,6 @@ const struct snd_soc_dai_ops asoc_qcom_lpass_cpu_dai_ops = {
>   	.startup	= lpass_cpu_daiops_startup,
>   	.shutdown	= lpass_cpu_daiops_shutdown,
>   	.hw_params	= lpass_cpu_daiops_hw_params,
> -	.hw_free	= lpass_cpu_daiops_hw_free,
>   	.prepare	= lpass_cpu_daiops_prepare,
>   	.trigger	= lpass_cpu_daiops_trigger,
>   };
> @@ -599,6 +620,18 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> +	/* Allocation for i2sctl regmap fields */
> +	drvdata->i2sctl = devm_kzalloc(&pdev->dev, sizeof(struct lpaif_i2sctl),
> +					GFP_KERNEL);
> +
> +	/* Initialize bitfields for dai I2SCTL register */
> +	ret = lpass_cpu_init_i2sctl_bitfields(dev, drvdata->i2sctl,
> +						drvdata->lpaif_map);
> +	if (ret) {
> +		dev_err(dev, "error init i2sctl field: %d\n", ret);
> +		return ret;
> +	}
> +
>   	ret = devm_snd_soc_register_component(dev,
>   					      &lpass_cpu_comp_driver,
>   					      variant->dai_driver,

> diff --git a/sound/soc/qcom/lpass-lpaif-reg.h b/sound/soc/qcom/lpass-lpaif-reg.h
> index 72a3e2f..5258e60 100644
> --- a/sound/soc/qcom/lpass-lpaif-reg.h
> +++ b/sound/soc/qcom/lpass-lpaif-reg.h
> @@ -12,15 +12,12 @@
...
>   #endif /* __LPASS_LPAIF_REG_H__ */
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index 34f7fd1..445ca193 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -50,6 +50,53 @@ static const struct snd_pcm_hardware lpass_platform_pcm_hardware = {
>   	.fifo_size		=	0,
>   };
>   
...
>   static int lpass_platform_pcmops_open(struct snd_soc_component *component,
>   				      struct snd_pcm_substream *substream)
>   {
> @@ -59,9 +106,9 @@ static int lpass_platform_pcmops_open(struct snd_soc_component *component,
>   	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
>   	struct lpass_variant *v = drvdata->variant;
>   	int ret, dma_ch, dir = substream->stream;
> -	struct lpass_pcm_data *data;
> +	struct lpass_pcm_data *data = NULL;
>   
> -	data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL);
> +	data = kzalloc(sizeof(*data), GFP_KERNEL);

Does this change belong in this patch?

>   	if (!data)
>   		return -ENOMEM;
>   
> @@ -111,13 +158,13 @@ static int lpass_platform_pcmops_close(struct snd_soc_component *component,
>   	struct snd_pcm_runtime *runtime = substream->runtime;
>   	struct lpass_data *drvdata = snd_soc_component_get_drvdata(component);
>   	struct lpass_variant *v = drvdata->variant;
> -	struct lpass_pcm_data *data;
> +	struct lpass_pcm_data *data = runtime->private_data;
>   
> -	data = runtime->private_data;
>   	drvdata->substream[data->dma_ch] = NULL;
>   	if (v->free_dma_channel)
>   		v->free_dma_channel(drvdata, data->dma_ch);
>   
> +	kfree(data);
>   	return 0;
>   }
>   
>   
>   	return devm_snd_soc_register_component(&pdev->dev,
>   			&lpass_component_driver, NULL, 0);
> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
> index 450020e..4294ec2 100644
> --- a/sound/soc/qcom/lpass.h
> +++ b/sound/soc/qcom/lpass.h
> @@ -17,6 +17,28 @@
>   #define LPASS_MAX_MI2S_PORTS			(8)
>   #define LPASS_MAX_DMA_CHANNELS			(8)
>   
...

>   /* Both the CPU DAI and platform drivers will access this data */
>   struct lpass_data {
>   
> @@ -55,6 +77,10 @@ struct lpass_data {
>   	struct clk_bulk_data *clks;
>   	int num_clks;
>   
> +	/* Regmap fields of I2SCTL & DMACTL registers bitfields */
> +	struct lpaif_i2sctl *i2sctl;
> +	struct lpaif_dmactl *rd_dmactl;
> +	struct lpaif_dmactl *wr_dmactl;
>   };
>   
>   /* Vairant data per each SOC */
> @@ -72,6 +98,33 @@ struct lpass_variant {
>   	u32	wrdma_reg_stride;
>   	u32	wrdma_channels;
>   

Above two along with rddma members can be removed, these become 
redundant after adding regmap field!


> +	/* I2SCTL Register fields */
> +	struct reg_field loopback;
> +	struct reg_field spken;
> +	struct reg_field spkmode;
> +	struct reg_field spkmono;
> +	struct reg_field micen;
> +	struct reg_field micmode;
> +	struct reg_field micmono;
> +	struct reg_field wssrc;
> +	struct reg_field bitwidth;
> +
> +	/* RD_DMA Register fields */
> +	struct reg_field rdma_bursten;
> +	struct reg_field rdma_wpscnt;
> +	struct reg_field rdma_intf;
> +	struct reg_field rdma_fifowm;
> +	struct reg_field rdma_enable;
> +	struct reg_field rdma_dyncclk;
> +
> +	/* RD_DMA Register fields */
> +	struct reg_field wrdma_bursten;
> +	struct reg_field wrdma_wpscnt;
> +	struct reg_field wrdma_intf;
> +	struct reg_field wrdma_fifowm;
> +	struct reg_field wrdma_enable;
> +	struct reg_field wrdma_dyncclk;
> +
>   	/**
>   	 * on SOCs like APQ8016 the channel control bits start
>   	 * at different offset to ipq806x
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev
  2020-07-08  5:08 ` [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev Rohit kumar
  2020-07-08 16:50   ` Mark Brown
@ 2020-07-09  9:26   ` Srinivas Kandagatla
  1 sibling, 0 replies; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09  9:26 UTC (permalink / raw)
  To: Rohit kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel
  Cc: Ajit Pandey



On 08/07/2020 06:08, Rohit kumar wrote:
> From: Ajit Pandey <ajitp@codeaurora.org>
> 
> We are allocating dma memory for component->dev but trying to mmap
> such memory for substream->pcm->card->dev. Replace device argument
> in mmap with component->dev to fix this.
> 
> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>


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

> ---
>   sound/soc/qcom/lpass-platform.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index 445ca193..f9424cc 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -472,9 +472,8 @@ static int lpass_platform_pcmops_mmap(struct snd_soc_component *component,
>   {
>   	struct snd_pcm_runtime *runtime = substream->runtime;
>   
> -	return dma_mmap_coherent(substream->pcm->card->dev, vma,
> -			runtime->dma_area, runtime->dma_addr,
> -			runtime->dma_bytes);
> +	return dma_mmap_coherent(component->dev, vma, runtime->dma_area,
> +				 runtime->dma_addr, runtime->dma_bytes);
>   }
>   
>   static irqreturn_t lpass_dma_interrupt_handler(
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node
  2020-07-08  5:08 ` [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node Rohit kumar
@ 2020-07-09  9:27   ` Srinivas Kandagatla
  2020-07-09 10:01     ` Rohit Kumar
  0 siblings, 1 reply; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09  9:27 UTC (permalink / raw)
  To: Rohit kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel



On 08/07/2020 06:08, Rohit kumar wrote:
> Add dt-bindings to support "qcom,lpass-cpu-sc7180" node.
> 
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
>   Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> index 32c2cdb..04e34cc 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> @@ -4,7 +4,8 @@ This node models the Qualcomm Technologies Low-Power Audio SubSystem (LPASS).
>   
>   Required properties:
>   
> -- compatible		: "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu"
> +- compatible		: "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
> +			  "qcom,lpass-cpu-sc7180"
>   - clocks		: Must contain an entry for each entry in clock-names.
>   - clock-names		: A list which must include the following entries:
>   				* "ahbix-clk"

Can you also list the clocks that are mandatory for this SoC.

--srini


> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 7/8] ASoC: qcom: lpass-sc7180: Add platform driver for lpass audio
  2020-07-08  5:08 ` [PATCH v3 7/8] ASoC: qcom: lpass-sc7180: Add platform driver for lpass audio Rohit kumar
@ 2020-07-09  9:27   ` Srinivas Kandagatla
  0 siblings, 0 replies; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09  9:27 UTC (permalink / raw)
  To: Rohit kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel
  Cc: Rohit kumar, Ajit Pandey



On 08/07/2020 06:08, Rohit kumar wrote:
> From: Ajit Pandey <ajitp@codeaurora.org>
> 
> Add platform driver for configuring sc7180 lpass core I2S and
> DMA configuration to support playback & capture to external codecs
> connected over primary & secondary MI2S interfaces.
> 
> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
> Signed-off-by: Rohit kumar <rohkumar@qti.qualcomm.com>
> ---
LGTM,

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



>   sound/soc/qcom/Kconfig        |   5 +
>   sound/soc/qcom/Makefile       |   2 +
>   sound/soc/qcom/lpass-sc7180.c | 216 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 223 insertions(+)
>   create mode 100644 sound/soc/qcom/lpass-sc7180.c
> 
> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
> index 0ea4cde..87bec7f 100644

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers
  2020-07-09  9:26   ` Srinivas Kandagatla
@ 2020-07-09  9:57     ` Rohit Kumar
  2020-07-09 10:06       ` Srinivas Kandagatla
  0 siblings, 1 reply; 29+ messages in thread
From: Rohit Kumar @ 2020-07-09  9:57 UTC (permalink / raw)
  To: Srinivas Kandagatla, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel

Thanks Srini for reviewing.

On 7/9/2020 2:56 PM, Srinivas Kandagatla wrote:
>
>
> On 08/07/2020 06:08, Rohit kumar wrote:
>> I2SCTL and DMACTL registers has different bits alignment for newer
>> LPASS variants of SC7180 soc. Use REG_FIELD_ID() to define the
>> reg_fields in platform specific file and removed shifts and mask
>> macros for such registers from header file.
>>
>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>
> Thanks Rohit for doing this, this looks much better now!
> I have few minor comments..
>
>> ---
>>   sound/soc/qcom/lpass-apq8016.c   |  24 ++++++
>>   sound/soc/qcom/lpass-cpu.c       | 163 
>> +++++++++++++++++++++++----------------
>>   sound/soc/qcom/lpass-ipq806x.c   |  24 ++++++
>>   sound/soc/qcom/lpass-lpaif-reg.h | 157 
>> +++++++++++++++++++------------------
>>   sound/soc/qcom/lpass-platform.c  | 151 
>> +++++++++++++++++++++++++++---------
>>   sound/soc/qcom/lpass.h           |  53 +++++++++++++
>>   6 files changed, 398 insertions(+), 174 deletions(-)
>>
>
> index f0c7e93..f358d12 100644
>> --- a/sound/soc/qcom/lpass-cpu.c
>> +++ b/sound/soc/qcom/lpass-cpu.c
>> @@ -29,6 +29,32 @@
>>   #define LPASS_CPU_I2S_SD0_1_2_MASK    GENMASK(2, 0)
>>   #define LPASS_CPU_I2S_SD0_1_2_3_MASK    GENMASK(3, 0)
>
>
>>   static int lpass_cpu_daiops_set_sysclk(struct snd_soc_dai *dai, int 
>> clk_id,
>>           unsigned int freq, int dir)
>>   {
>> @@ -79,12 +105,13 @@ static int lpass_cpu_daiops_hw_params(struct 
>> snd_pcm_substream *substream,
>>           struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>>   {
>>       struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
>> +    struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
>> +    unsigned int id = dai->driver->id;
>>       snd_pcm_format_t format = params_format(params);
>>       unsigned int channels = params_channels(params);
>>       unsigned int rate = params_rate(params);
>>       unsigned int mode;
>> -    unsigned int regval;
>> -    int bitwidth, ret;
>> +    int bitwidth, ret, regval;
>
> Why is this change?

Sorry, It came as part of previous patchset. regval was removed in 
patchv2  and

bitwidth variable was used to configure both clk and i2sctl register, 
which was not proper.

Added regval again. I will repost this patch without this change.

>
>>         bitwidth = snd_pcm_format_width(format);
>>       if (bitwidth < 0) {
>> @@ -92,28 +119,45 @@ static int lpass_cpu_daiops_hw_params(struct 
>> snd_pcm_substream *substream,
>>           return bitwidth;
>>       }
>>   -    regval = LPAIF_I2SCTL_LOOPBACK_DISABLE |
>> -            LPAIF_I2SCTL_WSSRC_INTERNAL;
>> +    ret = regmap_fields_write(i2sctl->loopback, id,
>> +                 LPAIF_I2SCTL_LOOPBACK_DISABLE);
>> +    if (ret) {
>> +        dev_err(dai->dev, "error updating loopback field: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    ret = regmap_fields_write(i2sctl->wssrc, id,
>> +                 LPAIF_I2SCTL_WSSRC_INTERNAL);
>> +    if (ret) {
>> +        dev_err(dai->dev, "error updating wssrc field: %d\n", ret);
>> +        return ret;
>> +    }
>>         switch (bitwidth) {
>>       case 16:
>> -        regval |= LPAIF_I2SCTL_BITWIDTH_16;
>> +        regval = LPAIF_I2SCTL_BITWIDTH_16;
>>           break;
>>       case 24:
>> -        regval |= LPAIF_I2SCTL_BITWIDTH_24;
>> +        regval = LPAIF_I2SCTL_BITWIDTH_24;
>>           break;
>>       case 32:
>> -        regval |= LPAIF_I2SCTL_BITWIDTH_32;
>> +        regval = LPAIF_I2SCTL_BITWIDTH_32;
>>           break;
>>       default:
>>           dev_err(dai->dev, "invalid bitwidth given: %d\n", bitwidth);
>>           return -EINVAL;
>>       }
>>   +    ret = regmap_fields_write(i2sctl->bitwidth, id, regval);
>> +    if (ret) {
>> +        dev_err(dai->dev, "error updating bitwidth field: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>>       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> -        mode = drvdata->mi2s_playback_sd_mode[dai->driver->id];
>> +        mode = drvdata->mi2s_playback_sd_mode[id];
>>       else
>> -        mode = drvdata->mi2s_capture_sd_mode[dai->driver->id];
>> +        mode = drvdata->mi2s_capture_sd_mode[id];
>>         if (!mode) {
>>           dev_err(dai->dev, "no line is assigned\n");
>> @@ -175,30 +219,34 @@ static int lpass_cpu_daiops_hw_params(struct 
>> snd_pcm_substream *substream,
>>       }
>>         if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> -        regval |= LPAIF_I2SCTL_SPKMODE(mode);
>> +        ret = regmap_fields_write(i2sctl->spkmode, id,
>> +                     LPAIF_I2SCTL_SPKMODE(mode));
>>             if (channels >= 2)
>> -            regval |= LPAIF_I2SCTL_SPKMONO_STEREO;
>> +            ret = regmap_fields_write(i2sctl->spkmono, id,
>> +                         LPAIF_I2SCTL_SPKMONO_STEREO);
>>           else
>> -            regval |= LPAIF_I2SCTL_SPKMONO_MONO;
>> +            ret = regmap_fields_write(i2sctl->spkmono, id,
>> +                         LPAIF_I2SCTL_SPKMONO_MONO);
>>       } else {
>> -        regval |= LPAIF_I2SCTL_MICMODE(mode);
>> +        ret = regmap_fields_write(i2sctl->micmode, id,
>> +                     LPAIF_I2SCTL_MICMODE(mode));
>>             if (channels >= 2)
>> -            regval |= LPAIF_I2SCTL_MICMONO_STEREO;
>> +            ret = regmap_fields_write(i2sctl->micmono, id,
>> +                         LPAIF_I2SCTL_MICMONO_STEREO);
>>           else
>> -            regval |= LPAIF_I2SCTL_MICMONO_MONO;
>> +            ret = regmap_fields_write(i2sctl->micmono, id,
>> +                         LPAIF_I2SCTL_MICMONO_MONO);
>>       }
>>   -    ret = regmap_write(drvdata->lpaif_map,
>> -               LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
>> -               regval);
>>       if (ret) {
>> -        dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
>> +        dev_err(dai->dev, "error writing to i2sctl channels mode: 
>> %d\n",
>> +            ret);
>>           return ret;
>>       }
>>   -    ret = clk_set_rate(drvdata->mi2s_bit_clk[dai->driver->id],
>> +    ret = clk_set_rate(drvdata->mi2s_bit_clk[id],
>>                  rate * bitwidth * 2);
>>       if (ret) {
>>           dev_err(dai->dev, "error setting mi2s bitclk to %u: %d\n",
>> @@ -209,41 +257,24 @@ static int lpass_cpu_daiops_hw_params(struct 
>> snd_pcm_substream *substream,
>>       return 0;
>>   }
>>   -static int lpass_cpu_daiops_hw_free(struct snd_pcm_substream 
>> *substream,
>> -        struct snd_soc_dai *dai)
>> -{
>> -    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
>> -    int ret;
>> -
>> -    ret = regmap_write(drvdata->lpaif_map,
>> -               LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
>> -               0);
>> -    if (ret)
>> -        dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
>> -
>> -    return ret;
>> -}
>
> Any particular reason why this function remove

This was causing issue in playback/capture concurrency. It sets I2SCTL 
register value to 0

when usecase ends. However, playback/capture specific bits are already 
cleared during trigger() stop

function. So, this is not needed.

>> -
>>   static int lpass_cpu_daiops_prepare(struct snd_pcm_substream 
>> *substream,
>>           struct snd_soc_dai *dai)
>>   {
>>       struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
>> +    struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
>> +    unsigned int id = dai->driver->id;
>>       int ret;
>> -    unsigned int val, mask;
>>         if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> -        val = LPAIF_I2SCTL_SPKEN_ENABLE;
>> -        mask = LPAIF_I2SCTL_SPKEN_MASK;
>> -    } else  {
>> -        val = LPAIF_I2SCTL_MICEN_ENABLE;
>> -        mask = LPAIF_I2SCTL_MICEN_MASK;
>> +        ret = regmap_fields_write(i2sctl->spken, id,
>> +                     LPAIF_I2SCTL_SPKEN_ENABLE);
>> +    } else {
>> +        ret = regmap_fields_write(i2sctl->micen, id,
>> +                     LPAIF_I2SCTL_MICEN_ENABLE);
>>       }
>>   -    ret = regmap_update_bits(drvdata->lpaif_map,
>> -            LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
>> -            mask, val);
>>       if (ret)
>> -        dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
>> +        dev_err(dai->dev, "error writing to i2sctl enable: %d\n", ret);
>>         return ret;
>>   }
> ...
>> @@ -304,7 +326,6 @@ const struct snd_soc_dai_ops 
>> asoc_qcom_lpass_cpu_dai_ops = {
>>       .startup    = lpass_cpu_daiops_startup,
>>       .shutdown    = lpass_cpu_daiops_shutdown,
>>       .hw_params    = lpass_cpu_daiops_hw_params,
>> -    .hw_free    = lpass_cpu_daiops_hw_free,
>>       .prepare    = lpass_cpu_daiops_prepare,
>>       .trigger    = lpass_cpu_daiops_trigger,
>>   };
>> @@ -599,6 +620,18 @@ int asoc_qcom_lpass_cpu_platform_probe(struct 
>> platform_device *pdev)
>>           }
>>       }
>>   +    /* Allocation for i2sctl regmap fields */
>> +    drvdata->i2sctl = devm_kzalloc(&pdev->dev, sizeof(struct 
>> lpaif_i2sctl),
>> +                    GFP_KERNEL);
>> +
>> +    /* Initialize bitfields for dai I2SCTL register */
>> +    ret = lpass_cpu_init_i2sctl_bitfields(dev, drvdata->i2sctl,
>> +                        drvdata->lpaif_map);
>> +    if (ret) {
>> +        dev_err(dev, "error init i2sctl field: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>>       ret = devm_snd_soc_register_component(dev,
>>                             &lpass_cpu_comp_driver,
>>                             variant->dai_driver,
>
>> diff --git a/sound/soc/qcom/lpass-lpaif-reg.h 
>> b/sound/soc/qcom/lpass-lpaif-reg.h
>> index 72a3e2f..5258e60 100644
>> --- a/sound/soc/qcom/lpass-lpaif-reg.h
>> +++ b/sound/soc/qcom/lpass-lpaif-reg.h
>> @@ -12,15 +12,12 @@
> ...
>>   #endif /* __LPASS_LPAIF_REG_H__ */
>> diff --git a/sound/soc/qcom/lpass-platform.c 
>> b/sound/soc/qcom/lpass-platform.c
>> index 34f7fd1..445ca193 100644
>> --- a/sound/soc/qcom/lpass-platform.c
>> +++ b/sound/soc/qcom/lpass-platform.c
>> @@ -50,6 +50,53 @@ static const struct snd_pcm_hardware 
>> lpass_platform_pcm_hardware = {
>>       .fifo_size        =    0,
>>   };
> ...
>>   static int lpass_platform_pcmops_open(struct snd_soc_component 
>> *component,
>>                         struct snd_pcm_substream *substream)
>>   {
>> @@ -59,9 +106,9 @@ static int lpass_platform_pcmops_open(struct 
>> snd_soc_component *component,
>>       struct lpass_data *drvdata = 
>> snd_soc_component_get_drvdata(component);
>>       struct lpass_variant *v = drvdata->variant;
>>       int ret, dma_ch, dir = substream->stream;
>> -    struct lpass_pcm_data *data;
>> +    struct lpass_pcm_data *data = NULL;
>>   -    data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL);
>> +    data = kzalloc(sizeof(*data), GFP_KERNEL);
>
> Does this change belong in this patch?


As part of this change, I fixed memory leak too by adding kfree() in close()

However, this was causing issue as memory was allocated using 
devm_kzalloc().

Should I move it to different patch?

>
>>       if (!data)
>>           return -ENOMEM;
>>   @@ -111,13 +158,13 @@ static int lpass_platform_pcmops_close(struct 
>> snd_soc_component *component,
>>       struct snd_pcm_runtime *runtime = substream->runtime;
>>       struct lpass_data *drvdata = 
>> snd_soc_component_get_drvdata(component);
>>       struct lpass_variant *v = drvdata->variant;
>> -    struct lpass_pcm_data *data;
>> +    struct lpass_pcm_data *data = runtime->private_data;
>>   -    data = runtime->private_data;
>>       drvdata->substream[data->dma_ch] = NULL;
>>       if (v->free_dma_channel)
>>           v->free_dma_channel(drvdata, data->dma_ch);
>>   +    kfree(data);
>>       return 0;
>>   }
>>           return devm_snd_soc_register_component(&pdev->dev,
>>               &lpass_component_driver, NULL, 0);
>> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
>> index 450020e..4294ec2 100644
>> --- a/sound/soc/qcom/lpass.h
>> +++ b/sound/soc/qcom/lpass.h
>> @@ -17,6 +17,28 @@
>>   #define LPASS_MAX_MI2S_PORTS            (8)
>>   #define LPASS_MAX_DMA_CHANNELS            (8)
> ...
>
>>   /* Both the CPU DAI and platform drivers will access this data */
>>   struct lpass_data {
>>   @@ -55,6 +77,10 @@ struct lpass_data {
>>       struct clk_bulk_data *clks;
>>       int num_clks;
>>   +    /* Regmap fields of I2SCTL & DMACTL registers bitfields */
>> +    struct lpaif_i2sctl *i2sctl;
>> +    struct lpaif_dmactl *rd_dmactl;
>> +    struct lpaif_dmactl *wr_dmactl;
>>   };
>>     /* Vairant data per each SOC */
>> @@ -72,6 +98,33 @@ struct lpass_variant {
>>       u32    wrdma_reg_stride;
>>       u32    wrdma_channels;
>
> Above two along with rddma members can be removed, these become 
> redundant after adding regmap field!
>
wrdma_channels is used in alloc_dma_channel() to get the channel id.

Also, both are used for other DMA registers such as LPAIF_RDMABASE_REG,

LPAIF_RDMABUFF_REG, LPAIF_RDMACURR_REG, etc.

>
>> +    /* I2SCTL Register fields */
>> +    struct reg_field loopback;
>> +    struct reg_field spken;
>> +    struct reg_field spkmode;
>> +    struct reg_field spkmono;
>> +    struct reg_field micen;
>> +    struct reg_field micmode;
>> +    struct reg_field micmono;
>> +    struct reg_field wssrc;
>> +    struct reg_field bitwidth;
>> +
>> +    /* RD_DMA Register fields */
>> +    struct reg_field rdma_bursten;
>> +    struct reg_field rdma_wpscnt;
>> +    struct reg_field rdma_intf;
>> +    struct reg_field rdma_fifowm;
>> +    struct reg_field rdma_enable;
>> +    struct reg_field rdma_dyncclk;
>> +
>> +    /* RD_DMA Register fields */
>> +    struct reg_field wrdma_bursten;
>> +    struct reg_field wrdma_wpscnt;
>> +    struct reg_field wrdma_intf;
>> +    struct reg_field wrdma_fifowm;
>> +    struct reg_field wrdma_enable;
>> +    struct reg_field wrdma_dyncclk;
>> +
>>       /**
>>        * on SOCs like APQ8016 the channel control bits start
>>        * at different offset to ipq806x
>>
Thanks,

Rohit

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node
  2020-07-09  9:27   ` Srinivas Kandagatla
@ 2020-07-09 10:01     ` Rohit Kumar
  2020-07-09 10:08       ` Srinivas Kandagatla
  0 siblings, 1 reply; 29+ messages in thread
From: Rohit Kumar @ 2020-07-09 10:01 UTC (permalink / raw)
  To: Srinivas Kandagatla, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel


On 7/9/2020 2:57 PM, Srinivas Kandagatla wrote:
>
>
> On 08/07/2020 06:08, Rohit kumar wrote:
>> Add dt-bindings to support "qcom,lpass-cpu-sc7180" node.
>>
>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>> ---
>>   Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt 
>> b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>> index 32c2cdb..04e34cc 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>> @@ -4,7 +4,8 @@ This node models the Qualcomm Technologies Low-Power 
>> Audio SubSystem (LPASS).
>>     Required properties:
>>   -- compatible        : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu"
>> +- compatible        : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
>> +              "qcom,lpass-cpu-sc7180"
>>   - clocks        : Must contain an entry for each entry in clock-names.
>>   - clock-names        : A list which must include the following 
>> entries:
>>                   * "ahbix-clk"
>
> Can you also list the clocks that are mandatory for this SoC.
>
> --srini
>
Will it be fine if I update it in patch 8 only where we have moved to 
yaml format?

Thanks,

Rohit

>
>>
-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 4/8] include: dt-bindings: sound: Add sc7180-lpass bindings header
  2020-07-08 14:45   ` Mark Brown
@ 2020-07-09 10:06     ` Rohit Kumar
  0 siblings, 0 replies; 29+ messages in thread
From: Rohit Kumar @ 2020-07-09 10:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: devicetree, alsa-devel, bgoswami, linux-arm-msm, plai, tiwai,
	lgirdwood, robh+dt, srinivas.kandagatla, agross, Ajit Pandey,
	bjorn.andersson, linux-kernel


On 7/8/2020 8:15 PM, Mark Brown wrote:
> On Wed, Jul 08, 2020 at 10:38:12AM +0530, Rohit kumar wrote:
>> From: Ajit Pandey <ajitp@codeaurora.org>
>>
>> Add header defining dai-id and mclk id for SC7180 lpass soc.
>>
>> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
>> ---
> This one is missing a signoff as well, and I can't seem to see any
> reference to this header in the bindings document patches?

Thanks Mark for reviewing. I will add it documentation in next patchset.

Thanks,

Rohit

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers
  2020-07-09  9:57     ` Rohit Kumar
@ 2020-07-09 10:06       ` Srinivas Kandagatla
  2020-07-09 10:14         ` Rohit Kumar
  0 siblings, 1 reply; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09 10:06 UTC (permalink / raw)
  To: Rohit Kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel



On 09/07/2020 10:57, Rohit Kumar wrote:
> Thanks Srini for reviewing.
> 
> On 7/9/2020 2:56 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 08/07/2020 06:08, Rohit kumar wrote:
>>> I2SCTL and DMACTL registers has different bits alignment for newer
>>> LPASS variants of SC7180 soc. Use REG_FIELD_ID() to define the
>>> reg_fields in platform specific file and removed shifts and mask
>>> macros for such registers from header file.
>>>
>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>>
>> Thanks Rohit for doing this, this looks much better now!
>> I have few minor comments..
>>
>>> ---
>>>   sound/soc/qcom/lpass-apq8016.c   |  24 ++++++
>>>   sound/soc/qcom/lpass-cpu.c       | 163 
>>> +++++++++++++++++++++++----------------
>>>   sound/soc/qcom/lpass-ipq806x.c   |  24 ++++++
>>>   sound/soc/qcom/lpass-lpaif-reg.h | 157 
>>> +++++++++++++++++++------------------
>>>   sound/soc/qcom/lpass-platform.c  | 151 
>>> +++++++++++++++++++++++++++---------
>>>   sound/soc/qcom/lpass.h           |  53 +++++++++++++
>>>   6 files changed, 398 insertions(+), 174 deletions(-)
>>>
>>
>> index f0c7e93..f358d12 100644
>>> --- a/sound/soc/qcom/lpass-cpu.c
>>> +++ b/sound/soc/qcom/lpass-cpu.c
>>> @@ -29,6 +29,32 @@
>>>   #define LPASS_CPU_I2S_SD0_1_2_MASK    GENMASK(2, 0)
>>>   #define LPASS_CPU_I2S_SD0_1_2_3_MASK    GENMASK(3, 0)
>>
>>
>>>   }
>>>   -static int lpass_cpu_daiops_hw_free(struct snd_pcm_substream 
>>> *substream,
>>> -        struct snd_soc_dai *dai)
>>> -{
>>> -    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
>>> -    int ret;
>>> -
>>> -    ret = regmap_write(drvdata->lpaif_map,
>>> -               LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
>>> -               0);
>>> -    if (ret)
>>> -        dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
>>> -
>>> -    return ret;
>>> -}
>>
>> Any particular reason why this function remove
> 
> This was causing issue in playback/capture concurrency. It sets I2SCTL 
> register value to 0
> 
> when usecase ends. However, playback/capture specific bits are already 
> cleared during trigger() stop
> 
> function. So, this is not needed.

This should be sent as separate fix with fixes tag!

> 
>
>>
>>> diff --git a/sound/soc/qcom/lpass-lpaif-reg.h 
>>> b/sound/soc/qcom/lpass-lpaif-reg.h
>>> index 72a3e2f..5258e60 100644
>>> --- a/sound/soc/qcom/lpass-lpaif-reg.h
>>> +++ b/sound/soc/qcom/lpass-lpaif-reg.h
>>> @@ -12,15 +12,12 @@
>> ...
>>>   #endif /* __LPASS_LPAIF_REG_H__ */
>>> diff --git a/sound/soc/qcom/lpass-platform.c 
>>> b/sound/soc/qcom/lpass-platform.c
>>> index 34f7fd1..445ca193 100644
>>> --- a/sound/soc/qcom/lpass-platform.c
>>> +++ b/sound/soc/qcom/lpass-platform.c
>>> @@ -50,6 +50,53 @@ static const struct snd_pcm_hardware 
>>> lpass_platform_pcm_hardware = {
>>>       .fifo_size        =    0,
>>>   };
>> ...
>>>   static int lpass_platform_pcmops_open(struct snd_soc_component 
>>> *component,
>>>                         struct snd_pcm_substream *substream)
>>>   {
>>> @@ -59,9 +106,9 @@ static int lpass_platform_pcmops_open(struct 
>>> snd_soc_component *component,
>>>       struct lpass_data *drvdata = 
>>> snd_soc_component_get_drvdata(component);
>>>       struct lpass_variant *v = drvdata->variant;
>>>       int ret, dma_ch, dir = substream->stream;
>>> -    struct lpass_pcm_data *data;
>>> +    struct lpass_pcm_data *data = NULL;
>>>   -    data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL);
>>> +    data = kzalloc(sizeof(*data), GFP_KERNEL);
>>
>> Does this change belong in this patch?
> 
> 
> As part of this change, I fixed memory leak too by adding kfree() in 
> close()
> 
> However, this was causing issue as memory was allocated using 
> devm_kzalloc().
> 
> Should I move it to different patch?

That would be the right thing to do, can also add fixes tag!


> 
>>
>>>       if (!data)
>>>           return -ENOMEM;
>>>   @@ -111,13 +158,13 @@ static int lpass_platform_pcmops_close(struct 
>>> snd_soc_component *component,
>>>       struct snd_pcm_runtime *runtime = substream->runtime;
>>>       struct lpass_data *drvdata = 
>>> snd_soc_component_get_drvdata(component);
>>>       struct lpass_variant *v = drvdata->variant;

>>
>> Above two along with rddma members can be removed, these become 
>> redundant after adding regmap field!
>>
> wrdma_channels is used in alloc_dma_channel() to get the channel id.
> 
> Also, both are used for other DMA registers such as LPAIF_RDMABASE_REG,
> 
> LPAIF_RDMABUFF_REG, LPAIF_RDMACURR_REG, etc.
> 
Ah I see we are still using this in lpass_cpu_regmap_writeable!
ignore my previous comments about removing them!

--srini

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node
  2020-07-09 10:01     ` Rohit Kumar
@ 2020-07-09 10:08       ` Srinivas Kandagatla
  2020-07-09 10:12         ` Rohit Kumar
  0 siblings, 1 reply; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09 10:08 UTC (permalink / raw)
  To: Rohit Kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel



On 09/07/2020 11:01, Rohit Kumar wrote:
> 
> On 7/9/2020 2:57 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 08/07/2020 06:08, Rohit kumar wrote:
>>> Add dt-bindings to support "qcom,lpass-cpu-sc7180" node.
>>>
>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>>> ---
>>>   Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt 
>>> b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>> index 32c2cdb..04e34cc 100644
>>> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>> @@ -4,7 +4,8 @@ This node models the Qualcomm Technologies Low-Power 
>>> Audio SubSystem (LPASS).
>>>     Required properties:
>>>   -- compatible        : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu"
>>> +- compatible        : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
>>> +              "qcom,lpass-cpu-sc7180"
>>>   - clocks        : Must contain an entry for each entry in clock-names.
>>>   - clock-names        : A list which must include the following 
>>> entries:
>>>                   * "ahbix-clk"
>>
>> Can you also list the clocks that are mandatory for this SoC.
>>
>> --srini
>>
> Will it be fine if I update it in patch 8 only where we have moved to 
> yaml format?
> 
May be reverse the order, Convert to Yaml first and then add sc7180!

--srini
> Thanks,
> 
> Rohit
> 
>>
>>>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node
  2020-07-09 10:08       ` Srinivas Kandagatla
@ 2020-07-09 10:12         ` Rohit Kumar
  2020-07-09 10:14           ` Srinivas Kandagatla
  2020-07-09 10:34           ` Mark Brown
  0 siblings, 2 replies; 29+ messages in thread
From: Rohit Kumar @ 2020-07-09 10:12 UTC (permalink / raw)
  To: Srinivas Kandagatla, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel


On 7/9/2020 3:38 PM, Srinivas Kandagatla wrote:
>
>
> On 09/07/2020 11:01, Rohit Kumar wrote:
>>
>> On 7/9/2020 2:57 PM, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 08/07/2020 06:08, Rohit kumar wrote:
>>>> Add dt-bindings to support "qcom,lpass-cpu-sc7180" node.
>>>>
>>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>>>> ---
>>>>   Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt | 3 ++-
>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git 
>>>> a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt 
>>>> b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>>> index 32c2cdb..04e34cc 100644
>>>> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>>> @@ -4,7 +4,8 @@ This node models the Qualcomm Technologies 
>>>> Low-Power Audio SubSystem (LPASS).
>>>>     Required properties:
>>>>   -- compatible        : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu"
>>>> +- compatible        : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
>>>> +              "qcom,lpass-cpu-sc7180"
>>>>   - clocks        : Must contain an entry for each entry in 
>>>> clock-names.
>>>>   - clock-names        : A list which must include the following 
>>>> entries:
>>>>                   * "ahbix-clk"
>>>
>>> Can you also list the clocks that are mandatory for this SoC.
>>>
>>> --srini
>>>
>> Will it be fine if I update it in patch 8 only where we have moved to 
>> yaml format?
>>
> May be reverse the order, Convert to Yaml first and then add sc7180!

Actually Mark suggested to keep yaml change at the end of patch series 
as there

are pending yaml patch reviews and it might take time. If we keep yaml 
change before sc7180

change, then it will get blocked until yaml review. For now, I can 
update in exisiting

documentation. Please suggest.

Thanks

>
> --srini
>> Thanks,
>>
>> Rohit
>>
>>>
>>>>
-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node
  2020-07-09 10:12         ` Rohit Kumar
@ 2020-07-09 10:14           ` Srinivas Kandagatla
  2020-07-09 10:34           ` Mark Brown
  1 sibling, 0 replies; 29+ messages in thread
From: Srinivas Kandagatla @ 2020-07-09 10:14 UTC (permalink / raw)
  To: Rohit Kumar, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel



On 09/07/2020 11:12, Rohit Kumar wrote:
> 
> On 7/9/2020 3:38 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 09/07/2020 11:01, Rohit Kumar wrote:
>>>
>>> On 7/9/2020 2:57 PM, Srinivas Kandagatla wrote:
>>>>
>>>>
>>>> On 08/07/2020 06:08, Rohit kumar wrote:
>>>>> Add dt-bindings to support "qcom,lpass-cpu-sc7180" node.
>>>>>
>>>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>>>>> ---
>>>>>   Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt | 3 ++-
>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git 
>>>>> a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt 
>>>>> b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>>>> index 32c2cdb..04e34cc 100644
>>>>> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>>>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>>>> @@ -4,7 +4,8 @@ This node models the Qualcomm Technologies 
>>>>> Low-Power Audio SubSystem (LPASS).
>>>>>     Required properties:
>>>>>   -- compatible        : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu"
>>>>> +- compatible        : "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
>>>>> +              "qcom,lpass-cpu-sc7180"
>>>>>   - clocks        : Must contain an entry for each entry in 
>>>>> clock-names.
>>>>>   - clock-names        : A list which must include the following 
>>>>> entries:
>>>>>                   * "ahbix-clk"
>>>>
>>>> Can you also list the clocks that are mandatory for this SoC.
>>>>
>>>> --srini
>>>>
>>> Will it be fine if I update it in patch 8 only where we have moved to 
>>> yaml format?
>>>
>> May be reverse the order, Convert to Yaml first and then add sc7180!
> 
> Actually Mark suggested to keep yaml change at the end of patch series 
> as there
> 
> are pending yaml patch reviews and it might take time. If we keep yaml 
> change before sc7180
> 
> change, then it will get blocked until yaml review. For now, I can 
> update in exisiting
> 
> documentation. Please suggest.
Then these clocks need to be documented in this patch itself!

--srini
> 
> Thanks
> 
>>
>> --srini
>>> Thanks,
>>>
>>> Rohit
>>>
>>>>
>>>>>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers
  2020-07-09 10:06       ` Srinivas Kandagatla
@ 2020-07-09 10:14         ` Rohit Kumar
  0 siblings, 0 replies; 29+ messages in thread
From: Rohit Kumar @ 2020-07-09 10:14 UTC (permalink / raw)
  To: Srinivas Kandagatla, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, linux-arm-msm, alsa-devel,
	devicetree, linux-kernel


On 7/9/2020 3:36 PM, Srinivas Kandagatla wrote:
>
>
> On 09/07/2020 10:57, Rohit Kumar wrote:
>> Thanks Srini for reviewing.
>>
>> On 7/9/2020 2:56 PM, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 08/07/2020 06:08, Rohit kumar wrote:
>>>> I2SCTL and DMACTL registers has different bits alignment for newer
>>>> LPASS variants of SC7180 soc. Use REG_FIELD_ID() to define the
>>>> reg_fields in platform specific file and removed shifts and mask
>>>> macros for such registers from header file.
>>>>
>>>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>>>
>>> Thanks Rohit for doing this, this looks much better now!
>>> I have few minor comments..
>>>
>>>> ---
>>>>   sound/soc/qcom/lpass-apq8016.c   |  24 ++++++
>>>>   sound/soc/qcom/lpass-cpu.c       | 163 
>>>> +++++++++++++++++++++++----------------
>>>>   sound/soc/qcom/lpass-ipq806x.c   |  24 ++++++
>>>>   sound/soc/qcom/lpass-lpaif-reg.h | 157 
>>>> +++++++++++++++++++------------------
>>>>   sound/soc/qcom/lpass-platform.c  | 151 
>>>> +++++++++++++++++++++++++++---------
>>>>   sound/soc/qcom/lpass.h           |  53 +++++++++++++
>>>>   6 files changed, 398 insertions(+), 174 deletions(-)
>>>>
>>>
>>> index f0c7e93..f358d12 100644
>>>> --- a/sound/soc/qcom/lpass-cpu.c
>>>> +++ b/sound/soc/qcom/lpass-cpu.c
>>>> @@ -29,6 +29,32 @@
>>>>   #define LPASS_CPU_I2S_SD0_1_2_MASK    GENMASK(2, 0)
>>>>   #define LPASS_CPU_I2S_SD0_1_2_3_MASK    GENMASK(3, 0)
>>>
>>>
>>>>   }
>>>>   -static int lpass_cpu_daiops_hw_free(struct snd_pcm_substream 
>>>> *substream,
>>>> -        struct snd_soc_dai *dai)
>>>> -{
>>>> -    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
>>>> -    int ret;
>>>> -
>>>> -    ret = regmap_write(drvdata->lpaif_map,
>>>> -               LPAIF_I2SCTL_REG(drvdata->variant, dai->driver->id),
>>>> -               0);
>>>> -    if (ret)
>>>> -        dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
>>>> -
>>>> -    return ret;
>>>> -}
>>>
>>> Any particular reason why this function remove
>>
>> This was causing issue in playback/capture concurrency. It sets 
>> I2SCTL register value to 0
>>
>> when usecase ends. However, playback/capture specific bits are 
>> already cleared during trigger() stop
>>
>> function. So, this is not needed.
>
> This should be sent as separate fix with fixes tag!
Ok. Will create separate patch with fixes tag and post.
>
>>
>>
>>>
>>>> diff --git a/sound/soc/qcom/lpass-lpaif-reg.h 
>>>> b/sound/soc/qcom/lpass-lpaif-reg.h
>>>> index 72a3e2f..5258e60 100644
>>>> --- a/sound/soc/qcom/lpass-lpaif-reg.h
>>>> +++ b/sound/soc/qcom/lpass-lpaif-reg.h
>>>> @@ -12,15 +12,12 @@
>>> ...
>>>>   #endif /* __LPASS_LPAIF_REG_H__ */
>>>> diff --git a/sound/soc/qcom/lpass-platform.c 
>>>> b/sound/soc/qcom/lpass-platform.c
>>>> index 34f7fd1..445ca193 100644
>>>> --- a/sound/soc/qcom/lpass-platform.c
>>>> +++ b/sound/soc/qcom/lpass-platform.c
>>>> @@ -50,6 +50,53 @@ static const struct snd_pcm_hardware 
>>>> lpass_platform_pcm_hardware = {
>>>>       .fifo_size        =    0,
>>>>   };
>>> ...
>>>>   static int lpass_platform_pcmops_open(struct snd_soc_component 
>>>> *component,
>>>>                         struct snd_pcm_substream *substream)
>>>>   {
>>>> @@ -59,9 +106,9 @@ static int lpass_platform_pcmops_open(struct 
>>>> snd_soc_component *component,
>>>>       struct lpass_data *drvdata = 
>>>> snd_soc_component_get_drvdata(component);
>>>>       struct lpass_variant *v = drvdata->variant;
>>>>       int ret, dma_ch, dir = substream->stream;
>>>> -    struct lpass_pcm_data *data;
>>>> +    struct lpass_pcm_data *data = NULL;
>>>>   -    data = devm_kzalloc(soc_runtime->dev, sizeof(*data), 
>>>> GFP_KERNEL);
>>>> +    data = kzalloc(sizeof(*data), GFP_KERNEL);
>>>
>>> Does this change belong in this patch?
>>
>>
>> As part of this change, I fixed memory leak too by adding kfree() in 
>> close()
>>
>> However, this was causing issue as memory was allocated using 
>> devm_kzalloc().
>>
>> Should I move it to different patch?
>
> That would be the right thing to do, can also add fixes tag!


Sure, Will do that in next spin.

>
>
>>
>>>
>>>>       if (!data)
>>>>           return -ENOMEM;
>>>>   @@ -111,13 +158,13 @@ static int 
>>>> lpass_platform_pcmops_close(struct snd_soc_component *component,
>>>>       struct snd_pcm_runtime *runtime = substream->runtime;
>>>>       struct lpass_data *drvdata = 
>>>> snd_soc_component_get_drvdata(component);
>>>>       struct lpass_variant *v = drvdata->variant;
>
>>>
>>> Above two along with rddma members can be removed, these become 
>>> redundant after adding regmap field!
>>>
>> wrdma_channels is used in alloc_dma_channel() to get the channel id.
>>
>> Also, both are used for other DMA registers such as LPAIF_RDMABASE_REG,
>>
>> LPAIF_RDMABUFF_REG, LPAIF_RDMACURR_REG, etc.
>>
> Ah I see we are still using this in lpass_cpu_regmap_writeable!
> ignore my previous comments about removing them!
>
> --srini

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node
  2020-07-09 10:12         ` Rohit Kumar
  2020-07-09 10:14           ` Srinivas Kandagatla
@ 2020-07-09 10:34           ` Mark Brown
  1 sibling, 0 replies; 29+ messages in thread
From: Mark Brown @ 2020-07-09 10:34 UTC (permalink / raw)
  To: Rohit Kumar
  Cc: devicetree, alsa-devel, bgoswami, linux-arm-msm, plai, tiwai,
	lgirdwood, robh+dt, bjorn.andersson, agross, Srinivas Kandagatla,
	linux-kernel

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

On Thu, Jul 09, 2020 at 03:42:38PM +0530, Rohit Kumar wrote:
> On 7/9/2020 3:38 PM, Srinivas Kandagatla wrote:

> > May be reverse the order, Convert to Yaml first and then add sc7180!

> Actually Mark suggested to keep yaml change at the end of patch series as
> there

Right, there's a huge backlog on YAML reviews so they lead to all the
other work getting held up waiting for them.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format
  2020-07-08  5:08 ` [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format Rohit kumar
@ 2020-07-13 22:53   ` Rob Herring
  2020-07-22 10:44     ` Rohit Kumar
  0 siblings, 1 reply; 29+ messages in thread
From: Rob Herring @ 2020-07-13 22:53 UTC (permalink / raw)
  To: Rohit kumar
  Cc: devicetree, alsa-devel, bgoswami, linux-arm-msm, plai, tiwai,
	agross, lgirdwood, broonie, srinivas.kandagatla, Ajit Pandey,
	bjorn.andersson, linux-kernel

On Wed, Jul 08, 2020 at 10:38:16AM +0530, Rohit kumar wrote:
> From: Ajit Pandey <ajitp@codeaurora.org>
> 
> Update lpass-cpu binding with yaml formats.
> 
> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
>  .../devicetree/bindings/sound/qcom,lpass-cpu.txt   |  80 -----------
>  .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 154 +++++++++++++++++++++
>  2 files changed, 154 insertions(+), 80 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>  create mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> deleted file mode 100644
> index 04e34cc..00000000
> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
> +++ /dev/null
> @@ -1,80 +0,0 @@
> -* Qualcomm Technologies LPASS CPU DAI
> -
> -This node models the Qualcomm Technologies Low-Power Audio SubSystem (LPASS).
> -
> -Required properties:
> -
> -- compatible		: "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
> -			  "qcom,lpass-cpu-sc7180"
> -- clocks		: Must contain an entry for each entry in clock-names.
> -- clock-names		: A list which must include the following entries:
> -				* "ahbix-clk"
> -				* "mi2s-osr-clk"
> -				* "mi2s-bit-clk"
> -			: required clocks for "qcom,lpass-cpu-apq8016"
> -				* "ahbix-clk"
> -				* "mi2s-bit-clk0"
> -				* "mi2s-bit-clk1"
> -				* "mi2s-bit-clk2"
> -				* "mi2s-bit-clk3"
> -				* "pcnoc-mport-clk"
> -				* "pcnoc-sway-clk"
> -
> -- interrupts		: Must contain an entry for each entry in
> -			  interrupt-names.
> -- interrupt-names	: A list which must include the following entries:
> -				* "lpass-irq-lpaif"
> -- pinctrl-N		: One property must exist for each entry in
> -			  pinctrl-names.  See ../pinctrl/pinctrl-bindings.txt
> -			  for details of the property values.
> -- pinctrl-names		: Must contain a "default" entry.
> -- reg			: Must contain an address for each entry in reg-names.
> -- reg-names		: A list which must include the following entries:
> -				* "lpass-lpaif"
> -- #address-cells	: Must be 1
> -- #size-cells		: Must be 0
> -
> -
> -
> -Optional properties:
> -
> -- qcom,adsp		: Phandle for the audio DSP node
> -
> -By default, the driver uses up to 4 MI2S SD lines, for a total of 8 channels.
> -The SD lines to use can be configured by adding subnodes for each of the DAIs.
> -
> -Required properties for each DAI (represented by a subnode):
> -- reg			: Must be one of the DAI IDs
> -			  (usually part of dt-bindings header)
> -- qcom,playback-sd-lines: List of serial data lines to use for playback
> -			  Each SD line should be represented by a number from 0-3.
> -- qcom,capture-sd-lines	: List of serial data lines to use for capture
> -			  Each SD line should be represented by a number from 0-3.
> -
> -Note that adding a subnode changes the default to "no lines configured",
> -so both playback and capture lines should be configured when a subnode is added.
> -
> -Example:
> -
> -lpass@28100000 {
> -	compatible = "qcom,lpass-cpu";
> -	clocks = <&lcc AHBIX_CLK>, <&lcc MI2S_OSR_CLK>, <&lcc MI2S_BIT_CLK>;
> -	clock-names = "ahbix-clk", "mi2s-osr-clk", "mi2s-bit-clk";
> -	interrupts = <0 85 1>;
> -	interrupt-names = "lpass-irq-lpaif";
> -	pinctrl-names = "default", "idle";
> -	pinctrl-0 = <&mi2s_default>;
> -	pinctrl-1 = <&mi2s_idle>;
> -	reg = <0x28100000 0x10000>;
> -	reg-names = "lpass-lpaif";
> -	qcom,adsp = <&adsp>;
> -
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> -
> -	/* Optional to set different MI2S SD lines */
> -	dai@3 {
> -		reg = <MI2S_QUATERNARY>;
> -		qcom,playback-sd-lines = <0 1>;
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
> new file mode 100644
> index 00000000..9c350bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/qcom,lpass-cpu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm LPASS CPU dai driver bindings
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> +  - Rohit kumar <rohitkr@codeaurora.org>
> +
> +description:
> +  Qualcomm SOC Low-Power Audio SubSystem (LPASS) that consist of MI2S interface
> +  for audio data transfer on external codecs. LPASS cpu driver is a module to
> +  configure Low-Power Audio Interface(LPAIF) core registers across different
> +  IP versions.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,lpass-cpu
> +      - qcom,apq8016-lpass-cpu
> +      - qcom,lpass-cpu-sc7180
> +
> +  reg:
> +    items:
> +      - description: LPAIF core registers

Just: 'maxItems: 1' for a single entry.

> +
> +  reg-names:
> +    items:
> +      - const: lpass-lpaif

Not all that useful with only 1.

> +
> +  clocks:
> +    items:
> +      - description: AHBIX core clock
> +      - description: oscillator clock for MI2S external interfaces
> +      - description: Bit clock for single MI2S dai
> +      - description: Bit clock for MI2S_PRIMARY dai interface
> +      - description: Bit clock for MI2S_SECONDARY dai interface
> +      - description: Bit clock for MI2S_TERTIARY dai interface
> +      - description: Bit clock for MI2S_QUATERNARY dai interface
> +      - description: NOC MPORT clock of LPASS core
> +      - description: NOC SWAY clock of LPASS core
> +
> +  clock-names:
> +    items:
> +      - const: ahbix-clk
> +      - const: mi2s-osr-clk
> +      - const: mi2s-bit-clk
> +      - const: mi2s-bit-clk0
> +      - const: mi2s-bit-clk1
> +      - const: mi2s-bit-clk2
> +      - const: mi2s-bit-clk3
> +      - const: pcnoc-mport-clk
> +      - const: pcnoc-sway-clk
> +
> +  interrupts:
> +    items:
> +      - description: LPAIF DMA buffer interrupt

maxItems: 1

> +
> +  interrupt-names:
> +    items:
> +      - const: lpass-irq-lpaif
> +
> +  qcom,adsp:
> +    maxItems: 1
> +    description: Phandle for the audio DSP node

Needs a type $ref. And if just a phandle, 'maxItems: 1' is not 
appropriate.

> +
> +  iommus:
> +    maxItems: 1
> +    description: Phandle to apps_smmu node with sid mask
> +
> +  power-domains:
> +    maxItems: 1
> +    description: Phandle for power domain node

Drop. That's every 'power-domains' property.

> +
> +  '#sound-dai-cells':
> +    const: 1
> +
> +  child-node:

I'm sure I said this on some review recently, but you are defining a 
child node named 'child-node'. You need this under patternProperties 
with the actual child node name.

> +    description: Required properties for each DAI
> +    type: object
> +    properties:
> +      reg:
> +        description: Must be one of the DAI ID
> +                     (Usually part of dtbindings header)

Ideally, you'd define the range of values here.

> +      qcom,playback-sd-lines:
> +        description: List of serial data lines to use for playback
> +                     Each SD line should be represented by a number from 0-3.

Needs a type $ref and 0-3 should be expressed as a schema.

'make dt_binding_check' should complain about this. You did run that, 
right?

> +      qcom,capture-sd-lines :
> +        description: List of serial data lines to use for capture
> +                     Each SD line should be represented by a number from 0-3.

ditto

> +    required:
> +      -reg

space     ^

> +    # Note that adding a subnode changes the default to "no lines configured",
> +    # so both playback and capture lines should be configured when a subnode
> +    # is added.
> +
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interrupt-names
> +  - sound-dai-cells

Not a valid property.

> +
> +optional:
> +  - qcom,adsp
> +
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: qcom,lpass-cpu-sc7180
> +
> +then:
> +  required:
> +    - iommus
> +    - power-domains
> +
> +examples:
> +  lpass@28100000 {

This is not valid. 'examples' should be a list.

> +	compatible = "qcom,lpass-cpu";
> +	clocks = <&lcc AHBIX_CLK>,
> +		 <&lcc MI2S_OSR_CLK>,
> +		 <&lcc MI2S_BIT_CLK>;

The example will not build because the includes are missing.

> +
> +	clock-names = "ahbix-clk",
> +		      "mi2s-osr-clk",
> +		      "mi2s-bit-clk";
> +
> +	interrupts = <0 85 1>;
> +        interrupt-names = "lpass-irq-lpaif";
> +
> +	iommus = <&apps_smmu 0x1020 0>;
> +	power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>;
> +
> +	reg = <0x28100000 0x10000>;
> +	reg-names = "lpass-lpaif";
> +	#sound-dai-cells = <1>;
> +	qcom,adsp = <&adsp>;
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	/* Optional to set different MI2S SD lines */
> +	mi2s-quaternary@3 {
> +		reg = <MI2S_QUATERNARY>;
> +		qcom,playback-sd-lines = <0 1>;
> +  };
> -- 
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format
  2020-07-13 22:53   ` Rob Herring
@ 2020-07-22 10:44     ` Rohit Kumar
  0 siblings, 0 replies; 29+ messages in thread
From: Rohit Kumar @ 2020-07-22 10:44 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, alsa-devel, bgoswami, linux-arm-msm, plai, tiwai,
	agross, lgirdwood, broonie, srinivas.kandagatla, Ajit Pandey,
	bjorn.andersson, linux-kernel

Thanks Rob for the review. Posted v4 with fixes.

On 7/14/2020 4:23 AM, Rob Herring wrote:
> On Wed, Jul 08, 2020 at 10:38:16AM +0530, Rohit kumar wrote:
>> From: Ajit Pandey <ajitp@codeaurora.org>
>>
>> Update lpass-cpu binding with yaml formats.
>>
>> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
>> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
>> ---
>>   .../devicetree/bindings/sound/qcom,lpass-cpu.txt   |  80 -----------
>>   .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  | 154 +++++++++++++++++++++
>>   2 files changed, 154 insertions(+), 80 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>>   create mode 100644 Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>> deleted file mode 100644
>> index 04e34cc..00000000
>> --- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
>> +++ /dev/null
>> @@ -1,80 +0,0 @@
>> -* Qualcomm Technologies LPASS CPU DAI
>> -
>> -This node models the Qualcomm Technologies Low-Power Audio SubSystem (LPASS).
>> -
>> -Required properties:
>> -
>> -- compatible		: "qcom,lpass-cpu" or "qcom,apq8016-lpass-cpu" or
>> -			  "qcom,lpass-cpu-sc7180"
>> -- clocks		: Must contain an entry for each entry in clock-names.
>> -- clock-names		: A list which must include the following entries:
>> -				* "ahbix-clk"
>> -				* "mi2s-osr-clk"
>> -				* "mi2s-bit-clk"
>> -			: required clocks for "qcom,lpass-cpu-apq8016"
>> -				* "ahbix-clk"
>> -				* "mi2s-bit-clk0"
>> -				* "mi2s-bit-clk1"
>> -				* "mi2s-bit-clk2"
>> -				* "mi2s-bit-clk3"
>> -				* "pcnoc-mport-clk"
>> -				* "pcnoc-sway-clk"
>> -
>> -- interrupts		: Must contain an entry for each entry in
>> -			  interrupt-names.
>> -- interrupt-names	: A list which must include the following entries:
>> -				* "lpass-irq-lpaif"
>> -- pinctrl-N		: One property must exist for each entry in
>> -			  pinctrl-names.  See ../pinctrl/pinctrl-bindings.txt
>> -			  for details of the property values.
>> -- pinctrl-names		: Must contain a "default" entry.
>> -- reg			: Must contain an address for each entry in reg-names.
>> -- reg-names		: A list which must include the following entries:
>> -				* "lpass-lpaif"
>> -- #address-cells	: Must be 1
>> -- #size-cells		: Must be 0
>> -
>> -
>> -
>> -Optional properties:
>> -
>> -- qcom,adsp		: Phandle for the audio DSP node
>> -
>> -By default, the driver uses up to 4 MI2S SD lines, for a total of 8 channels.
>> -The SD lines to use can be configured by adding subnodes for each of the DAIs.
>> -
>> -Required properties for each DAI (represented by a subnode):
>> -- reg			: Must be one of the DAI IDs
>> -			  (usually part of dt-bindings header)
>> -- qcom,playback-sd-lines: List of serial data lines to use for playback
>> -			  Each SD line should be represented by a number from 0-3.
>> -- qcom,capture-sd-lines	: List of serial data lines to use for capture
>> -			  Each SD line should be represented by a number from 0-3.
>> -
>> -Note that adding a subnode changes the default to "no lines configured",
>> -so both playback and capture lines should be configured when a subnode is added.
>> -
>> -Example:
>> -
>> -lpass@28100000 {
>> -	compatible = "qcom,lpass-cpu";
>> -	clocks = <&lcc AHBIX_CLK>, <&lcc MI2S_OSR_CLK>, <&lcc MI2S_BIT_CLK>;
>> -	clock-names = "ahbix-clk", "mi2s-osr-clk", "mi2s-bit-clk";
>> -	interrupts = <0 85 1>;
>> -	interrupt-names = "lpass-irq-lpaif";
>> -	pinctrl-names = "default", "idle";
>> -	pinctrl-0 = <&mi2s_default>;
>> -	pinctrl-1 = <&mi2s_idle>;
>> -	reg = <0x28100000 0x10000>;
>> -	reg-names = "lpass-lpaif";
>> -	qcom,adsp = <&adsp>;
>> -
>> -	#address-cells = <1>;
>> -	#size-cells = <0>;
>> -
>> -	/* Optional to set different MI2S SD lines */
>> -	dai@3 {
>> -		reg = <MI2S_QUATERNARY>;
>> -		qcom,playback-sd-lines = <0 1>;
>> -	};
>> -};
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
>> new file mode 100644
>> index 00000000..9c350bc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.yaml
>> @@ -0,0 +1,154 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/qcom,lpass-cpu.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm LPASS CPU dai driver bindings
>> +
>> +maintainers:
>> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> +  - Rohit kumar <rohitkr@codeaurora.org>
>> +
>> +description:
>> +  Qualcomm SOC Low-Power Audio SubSystem (LPASS) that consist of MI2S interface
>> +  for audio data transfer on external codecs. LPASS cpu driver is a module to
>> +  configure Low-Power Audio Interface(LPAIF) core registers across different
>> +  IP versions.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,lpass-cpu
>> +      - qcom,apq8016-lpass-cpu
>> +      - qcom,lpass-cpu-sc7180
>> +
>> +  reg:
>> +    items:
>> +      - description: LPAIF core registers
> Just: 'maxItems: 1' for a single entry.
pl
>
>> +
>> +  reg-names:
>> +    items:
>> +      - const: lpass-lpaif
> Not all that useful with only 1.
ok .. Removed in v4.
>
>> +
>> +  clocks:
>> +    items:
>> +      - description: AHBIX core clock
>> +      - description: oscillator clock for MI2S external interfaces
>> +      - description: Bit clock for single MI2S dai
>> +      - description: Bit clock for MI2S_PRIMARY dai interface
>> +      - description: Bit clock for MI2S_SECONDARY dai interface
>> +      - description: Bit clock for MI2S_TERTIARY dai interface
>> +      - description: Bit clock for MI2S_QUATERNARY dai interface
>> +      - description: NOC MPORT clock of LPASS core
>> +      - description: NOC SWAY clock of LPASS core
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ahbix-clk
>> +      - const: mi2s-osr-clk
>> +      - const: mi2s-bit-clk
>> +      - const: mi2s-bit-clk0
>> +      - const: mi2s-bit-clk1
>> +      - const: mi2s-bit-clk2
>> +      - const: mi2s-bit-clk3
>> +      - const: pcnoc-mport-clk
>> +      - const: pcnoc-sway-clk
>> +
>> +  interrupts:
>> +    items:
>> +      - description: LPAIF DMA buffer interrupt
> maxItems: 1
ok
>
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: lpass-irq-lpaif
>> +
>> +  qcom,adsp:
>> +    maxItems: 1
>> +    description: Phandle for the audio DSP node
> Needs a type $ref. And if just a phandle, 'maxItems: 1' is not
> appropriate.
>
>> +
>> +  iommus:
>> +    maxItems: 1
>> +    description: Phandle to apps_smmu node with sid mask
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +    description: Phandle for power domain node
> Drop. That's every 'power-domains' property.
ok
>> +
>> +  '#sound-dai-cells':
>> +    const: 1
>> +
>> +  child-node:
> I'm sure I said this on some review recently, but you are defining a
> child node named 'child-node'. You need this under patternProperties
> with the actual child node name.
Done in v4.
>
>> +    description: Required properties for each DAI
>> +    type: object
>> +    properties:
>> +      reg:
>> +        description: Must be one of the DAI ID
>> +                     (Usually part of dtbindings header)
> Ideally, you'd define the range of values here.
>
>> +      qcom,playback-sd-lines:
>> +        description: List of serial data lines to use for playback
>> +                     Each SD line should be represented by a number from 0-3.
> Needs a type $ref and 0-3 should be expressed as a schema.
>
> 'make dt_binding_check' should complain about this. You did run that,
> right?
Actually I reposted the Ajit's patch. Fixed all issues in v4.
>> +      qcom,capture-sd-lines :
>> +        description: List of serial data lines to use for capture
>> +                     Each SD line should be represented by a number from 0-3.
> ditto
>
>> +    required:
>> +      -reg
> space     ^
>
>> +    # Note that adding a subnode changes the default to "no lines configured",
>> +    # so both playback and capture lines should be configured when a subnode
>> +    # is added.
>> +
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - clocks
>> +  - clock-names
>> +  - interrupts
>> +  - interrupt-names
>> +  - sound-dai-cells
> Not a valid property.
>
>> +
>> +optional:
>> +  - qcom,adsp
>> +
>> +if:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        const: qcom,lpass-cpu-sc7180
>> +
>> +then:
>> +  required:
>> +    - iommus
>> +    - power-domains
>> +
>> +examples:
>> +  lpass@28100000 {
> This is not valid. 'examples' should be a list.
>
>> +	compatible = "qcom,lpass-cpu";
>> +	clocks = <&lcc AHBIX_CLK>,
>> +		 <&lcc MI2S_OSR_CLK>,
>> +		 <&lcc MI2S_BIT_CLK>;
> The example will not build because the includes are missing.
>
>> +
>> +	clock-names = "ahbix-clk",
>> +		      "mi2s-osr-clk",
>> +		      "mi2s-bit-clk";
>> +
>> +	interrupts = <0 85 1>;
>> +        interrupt-names = "lpass-irq-lpaif";
>> +
>> +	iommus = <&apps_smmu 0x1020 0>;
>> +	power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>;
>> +
>> +	reg = <0x28100000 0x10000>;
>> +	reg-names = "lpass-lpaif";
>> +	#sound-dai-cells = <1>;
>> +	qcom,adsp = <&adsp>;
>> +
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	/* Optional to set different MI2S SD lines */
>> +	mi2s-quaternary@3 {
>> +		reg = <MI2S_QUATERNARY>;
>> +		qcom,playback-sd-lines = <0 1>;
>> +  };
>> -- 
>> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
>> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Thanks,

Rohit

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2020-07-22 10:46 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-08  5:08 [PATCH v3 0/8] ASoC: qcom: Add support for SC7180 lpass variant Rohit kumar
2020-07-08  5:08 ` [PATCH v3 1/8] ASoC: qcom: Add common array to initialize soc based core clocks Rohit kumar
2020-07-09  9:26   ` Srinivas Kandagatla
2020-07-08  5:08 ` [PATCH v3 2/8] ASoC: qcom: lpass-cpu: Move ahbix clk to platform specific function Rohit kumar
2020-07-09  9:26   ` Srinivas Kandagatla
2020-07-08  5:08 ` [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers Rohit kumar
2020-07-09  9:26   ` Srinivas Kandagatla
2020-07-09  9:57     ` Rohit Kumar
2020-07-09 10:06       ` Srinivas Kandagatla
2020-07-09 10:14         ` Rohit Kumar
2020-07-08  5:08 ` [PATCH v3 4/8] include: dt-bindings: sound: Add sc7180-lpass bindings header Rohit kumar
2020-07-08 14:45   ` Mark Brown
2020-07-09 10:06     ` Rohit Kumar
2020-07-08  5:08 ` [PATCH v3 5/8] ASoC: qcom: lpass-platform: Replace card->dev with component->dev Rohit kumar
2020-07-08 16:50   ` Mark Brown
2020-07-09  3:46     ` Rohit Kumar
2020-07-09  9:26   ` Srinivas Kandagatla
2020-07-08  5:08 ` [PATCH v3 6/8] dt-bindings: sound: lpass-cpu: Add sc7180 lpass cpu node Rohit kumar
2020-07-09  9:27   ` Srinivas Kandagatla
2020-07-09 10:01     ` Rohit Kumar
2020-07-09 10:08       ` Srinivas Kandagatla
2020-07-09 10:12         ` Rohit Kumar
2020-07-09 10:14           ` Srinivas Kandagatla
2020-07-09 10:34           ` Mark Brown
2020-07-08  5:08 ` [PATCH v3 7/8] ASoC: qcom: lpass-sc7180: Add platform driver for lpass audio Rohit kumar
2020-07-09  9:27   ` Srinivas Kandagatla
2020-07-08  5:08 ` [PATCH v3 8/8] dt-bindings: sound: lpass-cpu: Move to yaml format Rohit kumar
2020-07-13 22:53   ` Rob Herring
2020-07-22 10:44     ` Rohit Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).