LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v5 00/10] Add support for audio on SC7280 based targets
@ 2021-11-18 12:48 Srinivasa Rao Mandadapu
  2021-11-18 12:48 ` [PATCH v5 01/10] ASoC: qcom: Move lpass_pcm_data structure to lpass header Srinivasa Rao Mandadapu
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-11-18 12:48 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu

This patch set is to add support for Audio over wcd codec,
digital mics, through digital codecs and without ADSP.
This patch set depends on:
	-- https://patchwork.kernel.org/project/alsa-devel/list/?series=570161
	-- https://patchwork.kernel.org/project/alsa-devel/list/?series=572615
	-- https://patchwork.kernel.org/project/alsa-devel/list/?series=559677

Changes Since V4:
    -- Remove unused variable in lpass-sc7280 platform driver.
Changes Since V3:
    -- Remove redundant power domain controls. As power domains can be configured from dtsi.
Changes Since V2:
    -- Split lpass sc7280 cpu driver patch and create regmap config patch.
    -- Create patches based on latest kernel tip.
    -- Add helper function to get dma control and lpaif handle.
    -- Remove unused variables.
Changes Since V1:
    -- Typo errors fix
    -- CPU driver readable/writable apis optimization.
    -- Add Missing config patch
    -- Add Common api for repeated dmactl initialization.

Srinivasa Rao Mandadapu (10):
  ASoC: qcom: Move lpass_pcm_data structure to lpass header
  ASoC: qcom: lpass: Add dma fields for codec dma lpass interface
  ASoC: qcom: Add register definition for codec rddma and wrdma
  ASoC: qcom: Add lpass CPU driver for codec dma control
  ASoC: qcom: Add helper function to get dma control and lpaif handle
  ASoC: qcom: Add support for codec dma driver
  ASoC: qcom: Add regmap config support for codec dma driver
  ASoC: dt-bindings: Add SC7280 sound card bindings
  ASoC: qcom: lpass-sc7280: Add platform driver for lpass audio
  ASoC: qcom: SC7280: Update config for building codec dma drivers

 .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  |  69 ++-
 sound/soc/qcom/Kconfig                             |  13 +
 sound/soc/qcom/Makefile                            |   4 +
 sound/soc/qcom/common.c                            |  39 ++
 sound/soc/qcom/common.h                            |   1 +
 sound/soc/qcom/lpass-cdc-dma.c                     | 195 ++++++++
 sound/soc/qcom/lpass-cpu.c                         | 245 +++++++++-
 sound/soc/qcom/lpass-lpaif-reg.h                   | 103 ++++-
 sound/soc/qcom/lpass-platform.c                    | 513 ++++++++++++++++++---
 sound/soc/qcom/lpass-sc7280.c                      | 416 +++++++++++++++++
 sound/soc/qcom/lpass.h                             | 150 ++++++
 11 files changed, 1669 insertions(+), 79 deletions(-)
 create mode 100644 sound/soc/qcom/lpass-cdc-dma.c
 create mode 100644 sound/soc/qcom/lpass-sc7280.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] 10+ messages in thread

* [PATCH v5 01/10] ASoC: qcom: Move lpass_pcm_data structure to lpass header
  2021-11-18 12:48 [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivasa Rao Mandadapu
@ 2021-11-18 12:48 ` Srinivasa Rao Mandadapu
  2021-11-23 12:27   ` Srinivas Kandagatla
  2021-11-18 12:48 ` [PATCH v5 02/10] ASoC: qcom: lpass: Add dma fields for codec dma lpass interface Srinivasa Rao Mandadapu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-11-18 12:48 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu, Venkata Prasad Potturu

Declare lpass_pcm_data structure in lpass header file instead of
platform source file to make common use of it by other drivers

Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
---
 sound/soc/qcom/lpass-platform.c | 5 -----
 sound/soc/qcom/lpass.h          | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index a59e9d2..a44162c 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -18,11 +18,6 @@
 
 #define DRV_NAME "lpass-platform"
 
-struct lpass_pcm_data {
-	int dma_ch;
-	int i2s_port;
-};
-
 #define LPASS_PLATFORM_BUFFER_SIZE	(24 *  2 * 1024)
 #define LPASS_PLATFORM_PERIODS		2
 
diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
index 67ef497..63aaa6f 100644
--- a/sound/soc/qcom/lpass.h
+++ b/sound/soc/qcom/lpass.h
@@ -256,6 +256,11 @@ struct lpass_variant {
 	int num_clks;
 };
 
+struct lpass_pcm_data {
+	int dma_ch;
+	int i2s_port;
+};
+
 /* register the platform driver from the CPU DAI driver */
 int asoc_qcom_lpass_platform_register(struct platform_device *);
 int asoc_qcom_lpass_cpu_platform_remove(struct platform_device *pdev);
-- 
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] 10+ messages in thread

* [PATCH v5 02/10] ASoC: qcom: lpass: Add dma fields for codec dma lpass interface
  2021-11-18 12:48 [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivasa Rao Mandadapu
  2021-11-18 12:48 ` [PATCH v5 01/10] ASoC: qcom: Move lpass_pcm_data structure to lpass header Srinivasa Rao Mandadapu
@ 2021-11-18 12:48 ` Srinivasa Rao Mandadapu
  2021-11-18 12:48 ` [PATCH v5 03/10] ASoC: qcom: Add register definition for codec rddma and wrdma Srinivasa Rao Mandadapu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-11-18 12:48 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu, Venkata Prasad Potturu

Add lpass interface memebers to support audio path over codec dma.

Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
---
 sound/soc/qcom/lpass.h | 144 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 144 insertions(+)

diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
index 63aaa6f..b068534 100644
--- a/sound/soc/qcom/lpass.h
+++ b/sound/soc/qcom/lpass.h
@@ -19,6 +19,28 @@
 #define LPASS_MAX_MI2S_PORTS			(8)
 #define LPASS_MAX_DMA_CHANNELS			(8)
 #define LPASS_MAX_HDMI_DMA_CHANNELS		(4)
+#define LPASS_MAX_CDC_DMA_CHANNELS		(8)
+#define LPASS_MAX_VA_CDC_DMA_CHANNELS		(8)
+#define LPASS_CDC_DMA_INTF_ONE_CHANNEL		(0x01)
+#define LPASS_CDC_DMA_INTF_TWO_CHANNEL		(0x03)
+#define LPASS_CDC_DMA_INTF_FOUR_CHANNEL		(0x0F)
+#define LPASS_CDC_DMA_INTF_SIX_CHANNEL		(0x3F)
+#define LPASS_CDC_DMA_INTF_EIGHT_CHANNEL	(0xFF)
+#define LPASS_CDC_DMA_RX0_INTERFACE		(0x1)
+#define LPASS_CDC_DMA_TX0_INTERFACE		(0x1)
+#define LPASS_CDC_DMA_TX1_INTERFACE		(0x2)
+#define LPASS_CDC_DMA_TX2_INTERFACE		(0x3)
+#define LPASS_CDC_DMA_TX3_INTERFACE		(0x4)
+#define LPASS_CDC_DMA_VA0_INTERFACE		(0x1)
+
+#define LPASS_CDC_DMA_INTERFACE(dai_id)	\
+	((dai_id == LPASS_CDC_DMA_TX3) ? \
+	LPASS_CDC_DMA_TX3_INTERFACE : \
+	LPASS_CDC_DMA_VA0_INTERFACE)
+
+#define LPASS_MAX_CDC_CLKS			(9)
+#define LPASS_ACTIVE_PDS			(4)
+#define LPASS_PROXY_PDS			(8)
 
 #define QCOM_REGMAP_FIELD_ALLOC(d, m, f, mf)    \
 	do { \
@@ -50,6 +72,12 @@ struct lpaif_dmactl {
 	struct regmap_field *burst8;
 	struct regmap_field *burst16;
 	struct regmap_field *dynburst;
+	struct regmap_field *codec_enable;
+	struct regmap_field *codec_pack;
+	struct regmap_field *codec_intf;
+	struct regmap_field *codec_fs_sel;
+	struct regmap_field *codec_channel;
+	struct regmap_field *codec_fs_delay;
 };
 
 /* Both the CPU DAI and platform drivers will access this data */
@@ -64,6 +92,25 @@ struct lpass_data {
 	/* MI2S bit clock (derived from system clock by a divider */
 	struct clk *mi2s_bit_clk[LPASS_MAX_MI2S_PORTS];
 
+	struct clk *cdc_dma_clks[LPASS_MAX_CDC_CLKS];
+
+	struct clk *xo;
+	struct clk *gcc_cfg_noc_lpass;
+	struct clk *core_cc_core;
+	struct clk *aon_cc_audio_hm_h;
+	struct clk *core_cc_sysnoc_mport_core;
+	struct clk *audio_cc_ext_mclk0;
+	struct clk *audio_cc_ext_mclk1;
+	struct clk *core_cc_lpm_core;
+	struct clk *core_cc_lpm_mem0_core;
+	struct clk *audio_cc_codec_mem;
+	struct clk *audio_cc_codec_mem0;
+	struct clk *audio_cc_codec_mem1;
+	struct clk *audio_cc_codec_mem2;
+	struct clk *audio_cc_rx_mclk_2x;
+	struct clk *audio_cc_rx_mclk;
+	struct clk *aon_cc_va_mem0;
+
 	/* MI2S SD lines to use for playback/capture */
 	unsigned int mi2s_playback_sd_mode[LPASS_MAX_MI2S_PORTS];
 	unsigned int mi2s_capture_sd_mode[LPASS_MAX_MI2S_PORTS];
@@ -72,38 +119,61 @@ struct lpass_data {
 	bool mi2s_was_prepared[LPASS_MAX_MI2S_PORTS];
 
 	int hdmi_port_enable;
+	int codec_dma_enable;
 
 	/* low-power audio interface (LPAIF) registers */
 	void __iomem *lpaif;
 	void __iomem *hdmiif;
+	void __iomem *rxtx_lpaif;
+	void __iomem *va_lpaif;
+
+	u32 rxtx_cdc_dma_lpm_buf;
+	u32 va_cdc_dma_lpm_buf;
 
 	/* regmap backed by the low-power audio interface (LPAIF) registers */
 	struct regmap *lpaif_map;
 	struct regmap *hdmiif_map;
+	struct regmap *rxtx_lpaif_map;
+	struct regmap *va_lpaif_map;
 
 	/* interrupts from the low-power audio interface (LPAIF) */
 	int lpaif_irq;
 	int hdmiif_irq;
+	int rxtxif_irq;
+	int vaif_irq;
+
 	/* SOC specific variations in the LPASS IP integration */
 	struct lpass_variant *variant;
 
 	/* bit map to keep track of static channel allocations */
 	unsigned long dma_ch_bit_map;
 	unsigned long hdmi_dma_ch_bit_map;
+	unsigned long rxtx_dma_ch_bit_map;
+	unsigned long va_dma_ch_bit_map;
 
 	/* used it for handling interrupt per dma channel */
 	struct snd_pcm_substream *substream[LPASS_MAX_DMA_CHANNELS];
 	struct snd_pcm_substream *hdmi_substream[LPASS_MAX_HDMI_DMA_CHANNELS];
+	struct snd_pcm_substream *rxtx_substream[LPASS_MAX_CDC_DMA_CHANNELS];
+	struct snd_pcm_substream *va_substream[LPASS_MAX_CDC_DMA_CHANNELS];
 
 	/* SOC specific clock list */
 	struct clk_bulk_data *clks;
 	int num_clks;
+	struct clk_bulk_data *cdc_clks;
+	int cdc_num_clks;
 
 	/* Regmap fields of I2SCTL & DMACTL registers bitfields */
 	struct lpaif_i2sctl *i2sctl;
 	struct lpaif_dmactl *rd_dmactl;
 	struct lpaif_dmactl *wr_dmactl;
 	struct lpaif_dmactl *hdmi_rd_dmactl;
+
+	/* Regmap fields of CODEC DMA CTRL registers*/
+	struct lpaif_dmactl *rxtx_rd_dmactl;
+	struct lpaif_dmactl *rxtx_wr_dmactl;
+	struct lpaif_dmactl *va_wr_dmactl;
+
 	/* Regmap fields of HDMI_CTRL registers*/
 	struct regmap_field *hdmitx_legacy_en;
 	struct regmap_field *hdmitx_parity_calc_en;
@@ -130,6 +200,24 @@ struct lpass_variant {
 	u32	wrdma_reg_base;
 	u32	wrdma_reg_stride;
 	u32	wrdma_channels;
+	u32	rxtx_irq_reg_base;
+	u32	rxtx_irq_reg_stride;
+	u32	rxtx_irq_ports;
+	u32	rxtx_rdma_reg_base;
+	u32	rxtx_rdma_reg_stride;
+	u32	rxtx_rdma_channels;
+	u32	rxtx_wrdma_reg_base;
+	u32	rxtx_wrdma_reg_stride;
+	u32	rxtx_wrdma_channels;
+	u32	va_irq_reg_base;
+	u32	va_irq_reg_stride;
+	u32	va_irq_ports;
+	u32	va_rdma_reg_base;
+	u32	va_rdma_reg_stride;
+	u32	va_rdma_channels;
+	u32	va_wrdma_reg_base;
+	u32	va_wrdma_reg_stride;
+	u32	va_wrdma_channels;
 	u32	i2sctrl_reg_base;
 	u32	i2sctrl_reg_stride;
 	u32	i2s_ports;
@@ -233,12 +321,66 @@ struct lpass_variant {
 	struct reg_field wrdma_enable;
 	struct reg_field wrdma_dyncclk;
 
+	/*CDC RXTX RD_DMA */
+	struct reg_field rxtx_rdma_intf;
+	struct reg_field rxtx_rdma_bursten;
+	struct reg_field rxtx_rdma_wpscnt;
+	struct reg_field rxtx_rdma_fifowm;
+	struct reg_field rxtx_rdma_enable;
+	struct reg_field rxtx_rdma_dyncclk;
+	struct reg_field rxtx_rdma_burst8;
+	struct reg_field rxtx_rdma_burst16;
+	struct reg_field rxtx_rdma_dynburst;
+	struct reg_field rxtx_rdma_codec_enable;
+	struct reg_field rxtx_rdma_codec_pack;
+	struct reg_field rxtx_rdma_codec_intf;
+	struct reg_field rxtx_rdma_codec_fs_sel;
+	struct reg_field rxtx_rdma_codec_ch;
+	struct reg_field rxtx_rdma_codec_fs_delay;
+
+	/*CDC RXTX WR_DMA */
+	struct reg_field rxtx_wrdma_intf;
+	struct reg_field rxtx_wrdma_bursten;
+	struct reg_field rxtx_wrdma_wpscnt;
+	struct reg_field rxtx_wrdma_fifowm;
+	struct reg_field rxtx_wrdma_enable;
+	struct reg_field rxtx_wrdma_dyncclk;
+	struct reg_field rxtx_wrdma_burst8;
+	struct reg_field rxtx_wrdma_burst16;
+	struct reg_field rxtx_wrdma_dynburst;
+	struct reg_field rxtx_wrdma_codec_enable;
+	struct reg_field rxtx_wrdma_codec_pack;
+	struct reg_field rxtx_wrdma_codec_intf;
+	struct reg_field rxtx_wrdma_codec_fs_sel;
+	struct reg_field rxtx_wrdma_codec_ch;
+	struct reg_field rxtx_wrdma_codec_fs_delay;
+
+	/*CDC VA WR_DMA */
+	struct reg_field va_wrdma_intf;
+	struct reg_field va_wrdma_bursten;
+	struct reg_field va_wrdma_wpscnt;
+	struct reg_field va_wrdma_fifowm;
+	struct reg_field va_wrdma_enable;
+	struct reg_field va_wrdma_dyncclk;
+	struct reg_field va_wrdma_burst8;
+	struct reg_field va_wrdma_burst16;
+	struct reg_field va_wrdma_dynburst;
+	struct reg_field va_wrdma_codec_enable;
+	struct reg_field va_wrdma_codec_pack;
+	struct reg_field va_wrdma_codec_intf;
+	struct reg_field va_wrdma_codec_fs_sel;
+	struct reg_field va_wrdma_codec_ch;
+	struct reg_field va_wrdma_codec_fs_delay;
+
 	/**
 	 * on SOCs like APQ8016 the channel control bits start
 	 * at different offset to ipq806x
 	 **/
 	u32	dmactl_audif_start;
 	u32	wrdma_channel_start;
+	u32	rxtx_wrdma_channel_start;
+	u32	va_wrdma_channel_start;
+
 	/* SOC specific initialization like clocks */
 	int (*init)(struct platform_device *pdev);
 	int (*exit)(struct platform_device *pdev);
@@ -250,10 +392,12 @@ struct lpass_variant {
 	int num_dai;
 	const char * const *dai_osr_clk_names;
 	const char * const *dai_bit_clk_names;
+	const char * const *cdc_dma_clk_names;
 
 	/* SOC specific clocks configuration */
 	const char **clk_name;
 	int num_clks;
+	int cdc_dma_num_clks;
 };
 
 struct lpass_pcm_data {
-- 
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] 10+ messages in thread

* [PATCH v5 03/10] ASoC: qcom: Add register definition for codec rddma and wrdma
  2021-11-18 12:48 [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivasa Rao Mandadapu
  2021-11-18 12:48 ` [PATCH v5 01/10] ASoC: qcom: Move lpass_pcm_data structure to lpass header Srinivasa Rao Mandadapu
  2021-11-18 12:48 ` [PATCH v5 02/10] ASoC: qcom: lpass: Add dma fields for codec dma lpass interface Srinivasa Rao Mandadapu
@ 2021-11-18 12:48 ` Srinivasa Rao Mandadapu
  2021-11-18 12:48 ` [PATCH v5 04/10] ASoC: qcom: Add lpass CPU driver for codec dma control Srinivasa Rao Mandadapu
  2021-11-23 12:28 ` [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivas Kandagatla
  4 siblings, 0 replies; 10+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-11-18 12:48 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu, Venkata Prasad Potturu

This patch adds register definitions for codec read dma and write dma
lpass interface.

Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
---
 sound/soc/qcom/lpass-lpaif-reg.h | 103 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 97 insertions(+), 6 deletions(-)

diff --git a/sound/soc/qcom/lpass-lpaif-reg.h b/sound/soc/qcom/lpass-lpaif-reg.h
index 2eb03ad..697a11f 100644
--- a/sound/soc/qcom/lpass-lpaif-reg.h
+++ b/sound/soc/qcom/lpass-lpaif-reg.h
@@ -74,6 +74,16 @@
 #define LPAIF_IRQSTAT_REG(v, port)	LPAIF_IRQ_REG_ADDR(v, 0x4, (port))
 #define LPAIF_IRQCLEAR_REG(v, port)	LPAIF_IRQ_REG_ADDR(v, 0xC, (port))
 
+/* LPAIF RXTX IRQ */
+#define LPAIF_RXTX_IRQ_REG_ADDR(v, addr, port, dai_id) \
+		((dai_id == LPASS_CDC_DMA_RX0 || dai_id == LPASS_CDC_DMA_TX3) ? \
+		(v->rxtx_irq_reg_base + (addr) + v->rxtx_irq_reg_stride * (port)) : \
+		(v->va_irq_reg_base + (addr) + v->va_irq_reg_stride * (port)))
+
+#define LPAIF_RXTX_IRQEN_REG(v, port, dai_id) LPAIF_RXTX_IRQ_REG_ADDR(v, 0x0, port, dai_id)
+#define LPAIF_RXTX_IRQSTAT_REG(v, port, dai_id) LPAIF_RXTX_IRQ_REG_ADDR(v, 0x4, port, dai_id)
+#define LPAIF_RXTX_IRQCLEAR_REG(v, port, dai_id) LPAIF_RXTX_IRQ_REG_ADDR(v, 0xC, port, dai_id)
+
 
 #define LPASS_HDMITX_APP_IRQ_REG_ADDR(v, addr)  \
 	((v->hdmi_irq_reg_base) + (addr))
@@ -139,12 +149,93 @@
 		(LPAIF_INTFDMA_REG(v, chan, reg, dai_id)) : \
 		LPAIF_WRDMA##reg##_REG(v, chan))
 
-#define LPAIF_DMACTL_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, CTL, dai_id)
-#define LPAIF_DMABASE_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, BASE, dai_id)
-#define	LPAIF_DMABUFF_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, BUFF, dai_id)
-#define LPAIF_DMACURR_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, CURR, dai_id)
-#define	LPAIF_DMAPER_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, PER, dai_id)
-#define	LPAIF_DMAPERCNT_REG(v, chan, dir, dai_id) __LPAIF_DMA_REG(v, chan, dir, PERCNT, dai_id)
+#define LPAIF_DMACTL_REG(v, chan, dir, dai_id)  \
+	(((dai_id == LPASS_CDC_DMA_RX0) || \
+	(dai_id == LPASS_CDC_DMA_TX3) || \
+	(dai_id == LPASS_CDC_DMA_VA_TX0)) ? \
+	__LPAIF_CDC_DMA_REG(v, chan, dir, CTL, dai_id) : \
+	__LPAIF_DMA_REG(v, chan, dir, CTL, dai_id))
+#define LPAIF_DMABASE_REG(v, chan, dir, dai_id) \
+	((dai_id == LPASS_CDC_DMA_RX0 || \
+	dai_id == LPASS_CDC_DMA_TX3 || \
+	dai_id == LPASS_CDC_DMA_VA_TX0) ? \
+	__LPAIF_CDC_DMA_REG(v, chan, dir, BASE, dai_id) : \
+	__LPAIF_DMA_REG(v, chan, dir, BASE, dai_id))
+#define LPAIF_DMABUFF_REG(v, chan, dir, dai_id) \
+	((dai_id == LPASS_CDC_DMA_RX0 || \
+	dai_id == LPASS_CDC_DMA_TX3 || \
+	dai_id == LPASS_CDC_DMA_VA_TX0) ? \
+	__LPAIF_CDC_DMA_REG(v, chan, dir, BUFF, dai_id) : \
+	__LPAIF_DMA_REG(v, chan, dir, BUFF, dai_id))
+#define LPAIF_DMACURR_REG(v, chan, dir, dai_id) \
+	((dai_id == LPASS_CDC_DMA_RX0 || \
+	dai_id == LPASS_CDC_DMA_TX3 || \
+	dai_id == LPASS_CDC_DMA_VA_TX0) ? \
+	__LPAIF_CDC_DMA_REG(v, chan, dir, CURR, dai_id) : \
+	__LPAIF_DMA_REG(v, chan, dir, CURR, dai_id))
+#define LPAIF_DMAPER_REG(v, chan, dir, dai_id)  \
+	((dai_id == LPASS_CDC_DMA_RX0 || \
+	dai_id == LPASS_CDC_DMA_TX3 || \
+	dai_id == LPASS_CDC_DMA_VA_TX0) ? \
+	__LPAIF_CDC_DMA_REG(v, chan, dir, PER, dai_id) : \
+	__LPAIF_DMA_REG(v, chan, dir, PER, dai_id))
+#define LPAIF_DMAPERCNT_REG(v, chan, dir, dai_id) \
+	((dai_id == LPASS_CDC_DMA_RX0 || \
+	dai_id == LPASS_CDC_DMA_TX3 || \
+	dai_id == LPASS_CDC_DMA_VA_TX0) ? \
+	__LPAIF_CDC_DMA_REG(v, chan, dir, PERCNT, dai_id) : \
+	__LPAIF_DMA_REG(v, chan, dir, PERCNT, dai_id))
+
+#define LPAIF_CDC_RDMA_REG_ADDR(v, addr, chan, dai_id) \
+	((dai_id == LPASS_CDC_DMA_RX0 || dai_id == LPASS_CDC_DMA_TX3) ? \
+	(v->rxtx_rdma_reg_base + (addr) + v->rxtx_rdma_reg_stride * (chan)) : \
+	(v->va_rdma_reg_base + (addr) + v->va_rdma_reg_stride * (chan)))
+
+#define LPAIF_CDC_RDMACTL_REG(v, chan, dai_id) LPAIF_CDC_RDMA_REG_ADDR(v, 0x00, (chan), dai_id)
+#define LPAIF_CDC_RDMABASE_REG(v, chan, dai_id) LPAIF_CDC_RDMA_REG_ADDR(v, 0x04, (chan), dai_id)
+#define LPAIF_CDC_RDMABUFF_REG(v, chan, dai_id) LPAIF_CDC_RDMA_REG_ADDR(v, 0x08, (chan), dai_id)
+#define LPAIF_CDC_RDMACURR_REG(v, chan, dai_id) LPAIF_CDC_RDMA_REG_ADDR(v, 0x0C, (chan), dai_id)
+#define LPAIF_CDC_RDMAPER_REG(v, chan, dai_id) LPAIF_CDC_RDMA_REG_ADDR(v, 0x10, (chan), dai_id)
+
+#define LPAIF_CDC_RDMA_INTF_REG(v, chan, dai_id) \
+	LPAIF_CDC_RDMA_REG_ADDR(v, 0x50, (chan), dai_id)
+
+#define LPAIF_CDC_WRDMA_REG_ADDR(v, addr, chan, dai_id) \
+	((dai_id == LPASS_CDC_DMA_RX0 || dai_id == LPASS_CDC_DMA_TX3) ? \
+	(v->rxtx_wrdma_reg_base + (addr) + \
+	v->rxtx_wrdma_reg_stride * (chan - v->rxtx_wrdma_channel_start)) : \
+	(v->va_wrdma_reg_base + (addr) + \
+	v->va_wrdma_reg_stride * (chan - v->va_wrdma_channel_start)))
+
+#define LPAIF_CDC_WRDMACTL_REG(v, chan, dai_id) \
+	LPAIF_CDC_WRDMA_REG_ADDR(v, 0x00, (chan), dai_id)
+#define LPAIF_CDC_WRDMABASE_REG(v, chan, dai_id) \
+	LPAIF_CDC_WRDMA_REG_ADDR(v, 0x04, (chan), dai_id)
+#define LPAIF_CDC_WRDMABUFF_REG(v, chan, dai_id) \
+	LPAIF_CDC_WRDMA_REG_ADDR(v, 0x08, (chan), dai_id)
+#define LPAIF_CDC_WRDMACURR_REG(v, chan, dai_id) \
+	LPAIF_CDC_WRDMA_REG_ADDR(v, 0x0C, (chan), dai_id)
+#define LPAIF_CDC_WRDMAPER_REG(v, chan, dai_id) \
+	LPAIF_CDC_WRDMA_REG_ADDR(v, 0x10, (chan), dai_id)
+#define LPAIF_CDC_WRDMA_INTF_REG(v, chan, dai_id) \
+	LPAIF_CDC_WRDMA_REG_ADDR(v, 0x50, (chan), dai_id)
+
+#define __LPAIF_CDC_DMA_REG(v, chan, dir, reg, dai_id)  \
+		((dir ==  SNDRV_PCM_STREAM_PLAYBACK) ? \
+		(LPAIF_CDC_RDMA##reg##_REG(v, chan, dai_id)) : \
+		LPAIF_CDC_WRDMA##reg##_REG(v, chan, dai_id))
+
+#define LPAIF_CDC_INTF_REG(v, chan, dir, dai_id) \
+		((dir ==  SNDRV_PCM_STREAM_PLAYBACK) ? \
+		LPAIF_CDC_RDMA_INTF_REG(v, chan, dai_id) : \
+		LPAIF_CDC_WRDMA_INTF_REG(v, chan, dai_id))
+
+#define LPAIF_INTF_REG(v, chan, dir, dai_id) \
+		((dai_id == LPASS_CDC_DMA_RX0 || \
+		dai_id == LPASS_CDC_DMA_TX3 || \
+		dai_id == LPASS_CDC_DMA_VA_TX0) ? \
+		LPAIF_CDC_INTF_REG(v, chan, dir, dai_id) : \
+		LPAIF_DMACTL_REG(v, chan, dir, dai_id))
 
 #define LPAIF_DMACTL_BURSTEN_SINGLE	0
 #define LPAIF_DMACTL_BURSTEN_INCR4	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] 10+ messages in thread

* [PATCH v5 04/10] ASoC: qcom: Add lpass CPU driver for codec dma control
  2021-11-18 12:48 [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivasa Rao Mandadapu
                   ` (2 preceding siblings ...)
  2021-11-18 12:48 ` [PATCH v5 03/10] ASoC: qcom: Add register definition for codec rddma and wrdma Srinivasa Rao Mandadapu
@ 2021-11-18 12:48 ` Srinivasa Rao Mandadapu
  2021-11-23 12:27   ` Srinivas Kandagatla
  2021-11-23 12:28 ` [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivas Kandagatla
  4 siblings, 1 reply; 10+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-11-18 12:48 UTC (permalink / raw)
  To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
	bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao
  Cc: Srinivasa Rao Mandadapu, Venkata Prasad Potturu

Add lpass cpu driver to support audio over codec dma for
ADSP bypass usecase.

Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
---
 sound/soc/qcom/lpass-cdc-dma.c | 195 +++++++++++++++++++++++++++++++++++++++++
 sound/soc/qcom/lpass.h         |   1 +
 2 files changed, 196 insertions(+)
 create mode 100644 sound/soc/qcom/lpass-cdc-dma.c

diff --git a/sound/soc/qcom/lpass-cdc-dma.c b/sound/soc/qcom/lpass-cdc-dma.c
new file mode 100644
index 0000000..f5bfda9
--- /dev/null
+++ b/sound/soc/qcom/lpass-cdc-dma.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021 The Linux Foundation. All rights reserved.
+ *
+ * lpass-cdc-dma.c -- ALSA SoC WCD -CPU DAI driver for QTi LPASS WCD
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+
+#include "lpass-lpaif-reg.h"
+#include "lpass.h"
+
+static void __get_dmactl_handle(struct snd_pcm_substream *substream, struct snd_soc_dai *dai,
+					struct lpaif_dmactl **dmactl, int *id)
+{
+	struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
+	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+	struct snd_pcm_runtime *rt = substream->runtime;
+	struct lpass_pcm_data *pcm_data = rt->private_data;
+	struct lpass_variant *v = drvdata->variant;
+	int dir = substream->stream;
+	unsigned int dai_id = cpu_dai->driver->id;
+
+	if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
+		*dmactl = drvdata->rxtx_rd_dmactl;
+		*id = pcm_data->dma_ch;
+	} else {
+		if (dai_id == LPASS_CDC_DMA_TX3) {
+			*dmactl = drvdata->rxtx_wr_dmactl;
+			*id = pcm_data->dma_ch - v->rxtx_wrdma_channel_start;
+		} else if (dai_id == LPASS_CDC_DMA_VA_TX0) {
+			*dmactl = drvdata->va_wr_dmactl;
+			*id = pcm_data->dma_ch - v->va_wrdma_channel_start;
+		}
+	}
+}
+
+static int __lpass_platform_codec_intf_init(struct snd_soc_dai *dai,
+					 struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
+	struct lpaif_dmactl *dmactl;
+	int dir = substream->stream;
+	int ret, id, codec_intf;
+	unsigned int dai_id = cpu_dai->driver->id;
+
+	if (dir == SNDRV_PCM_STREAM_PLAYBACK)
+		codec_intf = LPASS_CDC_DMA_RX0_INTERFACE;
+	else
+		codec_intf = LPASS_CDC_DMA_INTERFACE(dai_id);
+
+	__get_dmactl_handle(substream, dai, &dmactl, &id);
+
+	ret = regmap_fields_write(dmactl->codec_intf, id, codec_intf);
+	if (ret) {
+		dev_err(soc_runtime->dev,
+			"error writing to dmactl codec_intf reg field: %d\n", ret);
+		return ret;
+	}
+	ret = regmap_fields_write(dmactl->codec_fs_sel, id, 0x0);
+	if (ret) {
+		dev_err(soc_runtime->dev,
+			"error writing to dmactl codec_fs_sel reg field: %d\n", ret);
+		return ret;
+	}
+	ret = regmap_fields_write(dmactl->codec_fs_delay, id, 0x0);
+	if (ret) {
+		dev_err(soc_runtime->dev,
+			"error writing to dmactl codec_fs_delay reg field: %d\n", ret);
+		return ret;
+	}
+	ret = regmap_fields_write(dmactl->codec_pack, id, 0x1);
+	if (ret) {
+		dev_err(soc_runtime->dev,
+			"error writing to dmactl codec_pack reg field: %d\n", ret);
+		return ret;
+	}
+	ret = regmap_fields_write(dmactl->codec_enable, id, LPAIF_DMACTL_ENABLE_ON);
+	if (ret) {
+		dev_err(soc_runtime->dev,
+			"error writing to dmactl codec_enable reg field: %d\n", ret);
+		return ret;
+	}
+	return 0;
+}
+
+static int lpass_wcd_daiops_startup(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+	int ret;
+
+	ret = clk_bulk_prepare_enable(drvdata->cdc_num_clks, drvdata->cdc_clks);
+	if (ret) {
+		dev_err(dai->dev, "error in enabling cdc dma clks: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void lpass_wcd_daiops_shutdown(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+
+	clk_bulk_disable_unprepare(drvdata->cdc_num_clks, drvdata->cdc_clks);
+}
+
+static int lpass_wcd_daiops_hw_params(struct snd_pcm_substream *substream,
+		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
+	struct lpaif_dmactl *dmactl;
+	unsigned int ret, regval;
+	unsigned int channels = params_channels(params);
+	int id;
+
+	__get_dmactl_handle(substream, dai, &dmactl, &id);
+
+	switch (channels) {
+	case 1:
+		regval = LPASS_CDC_DMA_INTF_ONE_CHANNEL;
+		break;
+	case 2:
+		regval = LPASS_CDC_DMA_INTF_TWO_CHANNEL;
+		break;
+	case 4:
+		regval = LPASS_CDC_DMA_INTF_FOUR_CHANNEL;
+		break;
+	case 6:
+		regval = LPASS_CDC_DMA_INTF_SIX_CHANNEL;
+		break;
+	case 8:
+		regval = LPASS_CDC_DMA_INTF_EIGHT_CHANNEL;
+		break;
+	default:
+		dev_err(soc_runtime->dev, "invalid PCM config\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_fields_write(dmactl->codec_channel, id, regval);
+	if (ret) {
+		dev_err(soc_runtime->dev,
+			"error writing to dmactl codec_channel reg field: %d\n", ret);
+		return ret;
+	}
+	return 0;
+}
+
+static int lpass_wcd_daiops_trigger(struct snd_pcm_substream *substream,
+		int cmd, struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
+	struct lpaif_dmactl *dmactl;
+	int ret = 0, id;
+
+	__get_dmactl_handle(substream, dai, &dmactl, &id);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		__lpass_platform_codec_intf_init(dai, substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		ret = regmap_fields_write(dmactl->codec_enable, id, LPAIF_DMACTL_ENABLE_OFF);
+		if (ret) {
+			dev_err(soc_runtime->dev,
+				"error writing to dmactl codec_enable reg: %d\n", ret);
+			return ret;
+		}
+
+		break;
+	}
+	return ret;
+}
+
+const struct snd_soc_dai_ops asoc_qcom_lpass_wcd_dai_ops = {
+	.startup	= lpass_wcd_daiops_startup,
+	.shutdown	= lpass_wcd_daiops_shutdown,
+	.hw_params	= lpass_wcd_daiops_hw_params,
+	.trigger	= lpass_wcd_daiops_trigger,
+};
+EXPORT_SYMBOL_GPL(asoc_qcom_lpass_wcd_dai_ops);
+
+MODULE_DESCRIPTION("QTi LPASS CDC DMA Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
index b068534..f912425 100644
--- a/sound/soc/qcom/lpass.h
+++ b/sound/soc/qcom/lpass.h
@@ -414,5 +414,6 @@ int asoc_qcom_lpass_cpu_dai_probe(struct snd_soc_dai *dai);
 extern const struct snd_soc_dai_ops asoc_qcom_lpass_cpu_dai_ops;
 int lpass_cpu_pcm_new(struct snd_soc_pcm_runtime *rtd,
 				struct snd_soc_dai *dai);
+extern const struct snd_soc_dai_ops asoc_qcom_lpass_wcd_dai_ops;
 
 #endif /* __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	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 01/10] ASoC: qcom: Move lpass_pcm_data structure to lpass header
  2021-11-18 12:48 ` [PATCH v5 01/10] ASoC: qcom: Move lpass_pcm_data structure to lpass header Srinivasa Rao Mandadapu
@ 2021-11-23 12:27   ` Srinivas Kandagatla
  0 siblings, 0 replies; 10+ messages in thread
From: Srinivas Kandagatla @ 2021-11-23 12:27 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, lgirdwood,
	broonie, robh+dt, plai, bgoswami, perex, tiwai, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao
  Cc: Venkata Prasad Potturu



On 18/11/2021 12:48, Srinivasa Rao Mandadapu wrote:
> Declare lpass_pcm_data structure in lpass header file instead of
> platform source file to make common use of it by other drivers
> 
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
> Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>

LGTM,

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

> ---
>   sound/soc/qcom/lpass-platform.c | 5 -----
>   sound/soc/qcom/lpass.h          | 5 +++++
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
> index a59e9d2..a44162c 100644
> --- a/sound/soc/qcom/lpass-platform.c
> +++ b/sound/soc/qcom/lpass-platform.c
> @@ -18,11 +18,6 @@
>   
>   #define DRV_NAME "lpass-platform"
>   
> -struct lpass_pcm_data {
> -	int dma_ch;
> -	int i2s_port;
> -};
> -
>   #define LPASS_PLATFORM_BUFFER_SIZE	(24 *  2 * 1024)
>   #define LPASS_PLATFORM_PERIODS		2
>   
> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
> index 67ef497..63aaa6f 100644
> --- a/sound/soc/qcom/lpass.h
> +++ b/sound/soc/qcom/lpass.h
> @@ -256,6 +256,11 @@ struct lpass_variant {
>   	int num_clks;
>   };
>   
> +struct lpass_pcm_data {
> +	int dma_ch;
> +	int i2s_port;
> +};
> +
>   /* register the platform driver from the CPU DAI driver */
>   int asoc_qcom_lpass_platform_register(struct platform_device *);
>   int asoc_qcom_lpass_cpu_platform_remove(struct platform_device *pdev);
> 

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

* Re: [PATCH v5 04/10] ASoC: qcom: Add lpass CPU driver for codec dma control
  2021-11-18 12:48 ` [PATCH v5 04/10] ASoC: qcom: Add lpass CPU driver for codec dma control Srinivasa Rao Mandadapu
@ 2021-11-23 12:27   ` Srinivas Kandagatla
  2021-11-26 11:40     ` Srinivasa Rao Mandadapu
  0 siblings, 1 reply; 10+ messages in thread
From: Srinivas Kandagatla @ 2021-11-23 12:27 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, lgirdwood,
	broonie, robh+dt, plai, bgoswami, perex, tiwai, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao
  Cc: Venkata Prasad Potturu



On 18/11/2021 12:48, Srinivasa Rao Mandadapu wrote:
> Add lpass cpu driver to support audio over codec dma for
> ADSP bypass usecase.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
> Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
> ---
>   sound/soc/qcom/lpass-cdc-dma.c | 195 +++++++++++++++++++++++++++++++++++++++++
>   sound/soc/qcom/lpass.h         |   1 +
>   2 files changed, 196 insertions(+)
>   create mode 100644 sound/soc/qcom/lpass-cdc-dma.c
> 
> diff --git a/sound/soc/qcom/lpass-cdc-dma.c b/sound/soc/qcom/lpass-cdc-dma.c
> new file mode 100644
> index 0000000..f5bfda9
> --- /dev/null
> +++ b/sound/soc/qcom/lpass-cdc-dma.c
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2021 The Linux Foundation. All rights reserved.
> + *
> + * lpass-cdc-dma.c -- ALSA SoC WCD -CPU DAI driver for QTi LPASS WCD
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <sound/soc.h>
> +#include <sound/soc-dai.h>
> +
> +#include "lpass-lpaif-reg.h"
> +#include "lpass.h"
> +
> +static void __get_dmactl_handle(struct snd_pcm_substream *substream, struct snd_soc_dai *dai,
> +					struct lpaif_dmactl **dmactl, int *id)

Please rename this with an lpass prefix, Also indentation looks bit off 
here.

> +{
> +	struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
> +	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
> +	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +	struct snd_pcm_runtime *rt = substream->runtime;
> +	struct lpass_pcm_data *pcm_data = rt->private_data;
> +	struct lpass_variant *v = drvdata->variant;
> +	int dir = substream->stream;
> +	unsigned int dai_id = cpu_dai->driver->id;
> +
> +	if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
> +		*dmactl = drvdata->rxtx_rd_dmactl;
> +		*id = pcm_data->dma_ch;
> +	} else {
> +		if (dai_id == LPASS_CDC_DMA_TX3) {
> +			*dmactl = drvdata->rxtx_wr_dmactl;
> +			*id = pcm_data->dma_ch - v->rxtx_wrdma_channel_start;
> +		} else if (dai_id == LPASS_CDC_DMA_VA_TX0) {
> +			*dmactl = drvdata->va_wr_dmactl;
> +			*id = pcm_data->dma_ch - v->va_wrdma_channel_start;
> +		}

use switch case here, also something like this makes more sense rather 
that just dealing with TX3 and VA TX0

switch (dai_id) {
case: LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX8:
	*dmactl = drvdata->rxtx_wr_dmactl;
	*id = pcm_data->dma_ch;
	break;
case: LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
	*dmactl = drvdata->rxtx_wr_dmactl;
	*id = pcm_data->dma_ch - v->rxtx_wrdma_channel_start;
	break;
case LPASS_CDC_DMA_VA_TX0 ...  LPASS_CDC_DMA_VA_TX8:
	*dmactl = drvdata->rxtx_wr_dmactl;
	*id = pcm_data->dma_ch - v->va_wrdma_channel_start;
	break;
default:
	dev_err(dev, "Invalid dai id for dma ctl\n");
	break;
}

> +	}
> +}
> +
> +static int __lpass_platform_codec_intf_init(struct snd_soc_dai *dai,
> +					 struct snd_pcm_substream *substream)

Indentation is off, I think most of the code has simillar issue.

> +{
> +	struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
> +	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
> +	struct lpaif_dmactl *dmactl;
> +	int dir = substream->stream;
> +	int ret, id, codec_intf;
> +	unsigned int dai_id = cpu_dai->driver->id;
> +
> +	if (dir == SNDRV_PCM_STREAM_PLAYBACK)
> +		codec_intf = LPASS_CDC_DMA_RX0_INTERFACE;

Why are we limiting this to just RX0, it should be trivial to add 
support to other RX and TX dais.

Add a helper to convert the dai ids to interface indexs

> +	else
> +		codec_intf = LPASS_CDC_DMA_INTERFACE(dai_id);
> +
> +	__get_dmactl_handle(substream, dai, &dmactl, &id);

We are not checking if dmactl is valid or not, best is to have a error 
returned from __get_dmactl_handle() and handle it properly here.
> +
> +	ret = regmap_fields_write(dmactl->codec_intf, id, codec_intf);
> +	if (ret) {
> +		dev_err(soc_runtime->dev,
Why not use a local declare a local dev pointer and use it here, it will 
save lines here.
> +			"error writing to dmactl codec_intf reg field: %d\n", ret);
> +		return ret;
> +	}
> +	ret = regmap_fields_write(dmactl->codec_fs_sel, id, 0x0);
> +	if (ret) {
> +		dev_err(soc_runtime->dev,
> +			"error writing to dmactl codec_fs_sel reg field: %d\n", ret);
> +		return ret;
> +	}
> +	ret = regmap_fields_write(dmactl->codec_fs_delay, id, 0x0);
> +	if (ret) {
> +		dev_err(soc_runtime->dev,
> +			"error writing to dmactl codec_fs_delay reg field: %d\n", ret);
> +		return ret;
> +	}
> +	ret = regmap_fields_write(dmactl->codec_pack, id, 0x1);
> +	if (ret) {
> +		dev_err(soc_runtime->dev,
> +			"error writing to dmactl codec_pack reg field: %d\n", ret);
> +		return ret;
> +	}
> +	ret = regmap_fields_write(dmactl->codec_enable, id, LPAIF_DMACTL_ENABLE_ON);
> +	if (ret) {
> +		dev_err(soc_runtime->dev,
> +			"error writing to dmactl codec_enable reg field: %d\n", ret);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static int lpass_wcd_daiops_startup(struct snd_pcm_substream *substream,
> +		struct snd_soc_dai *dai)
> +{
> +	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(drvdata->cdc_num_clks, drvdata->cdc_clks);
> +	if (ret) {
> +		dev_err(dai->dev, "error in enabling cdc dma clks: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;

may be just do
return clk_bulk_prepare_enable(drvdata->cdc_num_clks, drvdata->cdc_clks);

soc_dai_ret() should print an error if startup fails.



> +}
> +
> +static void lpass_wcd_daiops_shutdown(struct snd_pcm_substream *substream,
> +		struct snd_soc_dai *dai)
> +{
> +	struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +
> +	clk_bulk_disable_unprepare(drvdata->cdc_num_clks, drvdata->cdc_clks);
> +}
> +
> +static int lpass_wcd_daiops_hw_params(struct snd_pcm_substream *substream,
> +		struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
> +	struct lpaif_dmactl *dmactl;
> +	unsigned int ret, regval;
> +	unsigned int channels = params_channels(params);
> +	int id;
> +
> +	__get_dmactl_handle(substream, dai, &dmactl, &id);

may be move it after switch case and handle the errors.
> +
> +	switch (channels) {
> +	case 1:
> +		regval = LPASS_CDC_DMA_INTF_ONE_CHANNEL;
> +		break;
> +	case 2:
> +		regval = LPASS_CDC_DMA_INTF_TWO_CHANNEL;
> +		break;
> +	case 4:
> +		regval = LPASS_CDC_DMA_INTF_FOUR_CHANNEL;
> +		break;
> +	case 6:
> +		regval = LPASS_CDC_DMA_INTF_SIX_CHANNEL;
> +		break;
> +	case 8:
> +		regval = LPASS_CDC_DMA_INTF_EIGHT_CHANNEL;
> +		break;
> +	default:
> +		dev_err(soc_runtime->dev, "invalid PCM config\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_fields_write(dmactl->codec_channel, id, regval);
> +	if (ret) {
> +		dev_err(soc_runtime->dev,
> +			"error writing to dmactl codec_channel reg field: %d\n", ret);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static int lpass_wcd_daiops_trigger(struct snd_pcm_substream *substream,
> +		int cmd, struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream);
> +	struct lpaif_dmactl *dmactl;
> +	int ret = 0, id;
> +
> +	__get_dmactl_handle(substream, dai, &dmactl, &id);

move it inside the case where this is really used.

> +
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		__lpass_platform_codec_intf_init(dai, substream);
> +		break;
> +	case SNDRV_PCM_TRIGGER_STOP:
> +	case SNDRV_PCM_TRIGGER_SUSPEND:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +		ret = regmap_fields_write(dmactl->codec_enable, id, LPAIF_DMACTL_ENABLE_OFF);
> +		if (ret) {
> +			dev_err(soc_runtime->dev,
> +				"error writing to dmactl codec_enable reg: %d\n", ret);
> +			return ret;
> +		}
> +
> +		break;

missing default:

> +	}
> +	return ret;
> +}
> +
> +const struct snd_soc_dai_ops asoc_qcom_lpass_wcd_dai_ops = {
> +	.startup	= lpass_wcd_daiops_startup,
> +	.shutdown	= lpass_wcd_daiops_shutdown,
> +	.hw_params	= lpass_wcd_daiops_hw_params,
> +	.trigger	= lpass_wcd_daiops_trigger,
> +};
> +EXPORT_SYMBOL_GPL(asoc_qcom_lpass_wcd_dai_ops);
> +
> +MODULE_DESCRIPTION("QTi LPASS CDC DMA Driver");
> +MODULE_LICENSE("GPL v2");

use
MODULE_LICENSE("GPL");

more info in ./Documentation/process/license-rules.rst

--srini
> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
> index b068534..f912425 100644
> --- a/sound/soc/qcom/lpass.h
> +++ b/sound/soc/qcom/lpass.h
> @@ -414,5 +414,6 @@ int asoc_qcom_lpass_cpu_dai_probe(struct snd_soc_dai *dai);
>   extern const struct snd_soc_dai_ops asoc_qcom_lpass_cpu_dai_ops;
>   int lpass_cpu_pcm_new(struct snd_soc_pcm_runtime *rtd,
>   				struct snd_soc_dai *dai);
> +extern const struct snd_soc_dai_ops asoc_qcom_lpass_wcd_dai_ops;
>   
>   #endif /* __LPASS_H__ */
> 

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

* Re: [PATCH v5 00/10] Add support for audio on SC7280 based targets
  2021-11-18 12:48 [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivasa Rao Mandadapu
                   ` (3 preceding siblings ...)
  2021-11-18 12:48 ` [PATCH v5 04/10] ASoC: qcom: Add lpass CPU driver for codec dma control Srinivasa Rao Mandadapu
@ 2021-11-23 12:28 ` Srinivas Kandagatla
  2021-11-26  4:39   ` Srinivasa Rao Mandadapu
  4 siblings, 1 reply; 10+ messages in thread
From: Srinivas Kandagatla @ 2021-11-23 12:28 UTC (permalink / raw)
  To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, lgirdwood,
	broonie, robh+dt, plai, bgoswami, perex, tiwai, rohitkr,
	linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
	judyhsiao



On 18/11/2021 12:48, Srinivasa Rao Mandadapu wrote:
> This patch set is to add support for Audio over wcd codec,
> digital mics, through digital codecs and without ADSP.
> This patch set depends on:

> 	-- https://patchwork.kernel.org/project/alsa-devel/list/?series=570161

> 	-- https://patchwork.kernel.org/project/alsa-devel/list/?series=572615

Except this one, rest of the patches are already in sound-next.
and the only dependency I see here is the final patch which adds 
Kconfigs to the soundcard SND_SOC_SC7280.
Why not just move those two lines in the patchset that adds the soundcard?



--srini


> 	-- https://patchwork.kernel.org/project/alsa-devel/list/?series=559677

> 
> Changes Since V4:
>      -- Remove unused variable in lpass-sc7280 platform driver.
> Changes Since V3:
>      -- Remove redundant power domain controls. As power domains can be configured from dtsi.
> Changes Since V2:
>      -- Split lpass sc7280 cpu driver patch and create regmap config patch.
>      -- Create patches based on latest kernel tip.
>      -- Add helper function to get dma control and lpaif handle.
>      -- Remove unused variables.
> Changes Since V1:
>      -- Typo errors fix
>      -- CPU driver readable/writable apis optimization.
>      -- Add Missing config patch
>      -- Add Common api for repeated dmactl initialization.
> 
> Srinivasa Rao Mandadapu (10):
>    ASoC: qcom: Move lpass_pcm_data structure to lpass header
>    ASoC: qcom: lpass: Add dma fields for codec dma lpass interface
>    ASoC: qcom: Add register definition for codec rddma and wrdma
>    ASoC: qcom: Add lpass CPU driver for codec dma control
>    ASoC: qcom: Add helper function to get dma control and lpaif handle
>    ASoC: qcom: Add support for codec dma driver
>    ASoC: qcom: Add regmap config support for codec dma driver
>    ASoC: dt-bindings: Add SC7280 sound card bindings
>    ASoC: qcom: lpass-sc7280: Add platform driver for lpass audio
>    ASoC: qcom: SC7280: Update config for building codec dma drivers
> 
>   .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  |  69 ++-
>   sound/soc/qcom/Kconfig                             |  13 +
>   sound/soc/qcom/Makefile                            |   4 +
>   sound/soc/qcom/common.c                            |  39 ++
>   sound/soc/qcom/common.h                            |   1 +
>   sound/soc/qcom/lpass-cdc-dma.c                     | 195 ++++++++
>   sound/soc/qcom/lpass-cpu.c                         | 245 +++++++++-
>   sound/soc/qcom/lpass-lpaif-reg.h                   | 103 ++++-
>   sound/soc/qcom/lpass-platform.c                    | 513 ++++++++++++++++++---
>   sound/soc/qcom/lpass-sc7280.c                      | 416 +++++++++++++++++
>   sound/soc/qcom/lpass.h                             | 150 ++++++
>   11 files changed, 1669 insertions(+), 79 deletions(-)
>   create mode 100644 sound/soc/qcom/lpass-cdc-dma.c
>   create mode 100644 sound/soc/qcom/lpass-sc7280.c
> 

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

* Re: [PATCH v5 00/10] Add support for audio on SC7280 based targets
  2021-11-23 12:28 ` [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivas Kandagatla
@ 2021-11-26  4:39   ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 10+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-11-26  4:39 UTC (permalink / raw)
  To: Srinivas Kandagatla, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, swboyd, judyhsiao


On 11/23/2021 5:58 PM, Srinivas Kandagatla wrote:
Thanks for your time Srini!!!
>
> On 18/11/2021 12:48, Srinivasa Rao Mandadapu wrote:
>> This patch set is to add support for Audio over wcd codec,
>> digital mics, through digital codecs and without ADSP.
>> This patch set depends on:
>
>>     -- 
>> https://patchwork.kernel.org/project/alsa-devel/list/?series=570161
>
>>     -- 
>> https://patchwork.kernel.org/project/alsa-devel/list/?series=572615
>
> Except this one, rest of the patches are already in sound-next.
> and the only dependency I see here is the final patch which adds 
> Kconfigs to the soundcard SND_SOC_SC7280.
> Why not just move those two lines in the patchset that adds the 
> soundcard?
>
>
>
> --srini
>
Okay. will move this dependency to corresponding patch.
>
>>     -- 
>> https://patchwork.kernel.org/project/alsa-devel/list/?series=559677
>
>>
>> Changes Since V4:
>>      -- Remove unused variable in lpass-sc7280 platform driver.
>> Changes Since V3:
>>      -- Remove redundant power domain controls. As power domains can 
>> be configured from dtsi.
>> Changes Since V2:
>>      -- Split lpass sc7280 cpu driver patch and create regmap config 
>> patch.
>>      -- Create patches based on latest kernel tip.
>>      -- Add helper function to get dma control and lpaif handle.
>>      -- Remove unused variables.
>> Changes Since V1:
>>      -- Typo errors fix
>>      -- CPU driver readable/writable apis optimization.
>>      -- Add Missing config patch
>>      -- Add Common api for repeated dmactl initialization.
>>
>> Srinivasa Rao Mandadapu (10):
>>    ASoC: qcom: Move lpass_pcm_data structure to lpass header
>>    ASoC: qcom: lpass: Add dma fields for codec dma lpass interface
>>    ASoC: qcom: Add register definition for codec rddma and wrdma
>>    ASoC: qcom: Add lpass CPU driver for codec dma control
>>    ASoC: qcom: Add helper function to get dma control and lpaif handle
>>    ASoC: qcom: Add support for codec dma driver
>>    ASoC: qcom: Add regmap config support for codec dma driver
>>    ASoC: dt-bindings: Add SC7280 sound card bindings
>>    ASoC: qcom: lpass-sc7280: Add platform driver for lpass audio
>>    ASoC: qcom: SC7280: Update config for building codec dma drivers
>>
>>   .../devicetree/bindings/sound/qcom,lpass-cpu.yaml  |  69 ++-
>>   sound/soc/qcom/Kconfig                             |  13 +
>>   sound/soc/qcom/Makefile                            |   4 +
>>   sound/soc/qcom/common.c                            |  39 ++
>>   sound/soc/qcom/common.h                            |   1 +
>>   sound/soc/qcom/lpass-cdc-dma.c                     | 195 ++++++++
>>   sound/soc/qcom/lpass-cpu.c                         | 245 +++++++++-
>>   sound/soc/qcom/lpass-lpaif-reg.h                   | 103 ++++-
>>   sound/soc/qcom/lpass-platform.c                    | 513 
>> ++++++++++++++++++---
>>   sound/soc/qcom/lpass-sc7280.c                      | 416 
>> +++++++++++++++++
>>   sound/soc/qcom/lpass.h                             | 150 ++++++
>>   11 files changed, 1669 insertions(+), 79 deletions(-)
>>   create mode 100644 sound/soc/qcom/lpass-cdc-dma.c
>>   create mode 100644 sound/soc/qcom/lpass-sc7280.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] 10+ messages in thread

* Re: [PATCH v5 04/10] ASoC: qcom: Add lpass CPU driver for codec dma control
  2021-11-23 12:27   ` Srinivas Kandagatla
@ 2021-11-26 11:40     ` Srinivasa Rao Mandadapu
  0 siblings, 0 replies; 10+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-11-26 11:40 UTC (permalink / raw)
  To: Srinivas Kandagatla, agross, bjorn.andersson, lgirdwood, broonie,
	robh+dt, plai, bgoswami, perex, tiwai, rohitkr, linux-arm-msm,
	alsa-devel, devicetree, linux-kernel, swboyd, judyhsiao
  Cc: Venkata Prasad Potturu


On 11/23/2021 5:57 PM, Srinivas Kandagatla wrote:
Thanks for your time Srini!!
>
> On 18/11/2021 12:48, Srinivasa Rao Mandadapu wrote:
>> Add lpass cpu driver to support audio over codec dma for
>> ADSP bypass usecase.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>> Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
>> Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
>> ---
>>   sound/soc/qcom/lpass-cdc-dma.c | 195 
>> +++++++++++++++++++++++++++++++++++++++++
>>   sound/soc/qcom/lpass.h         |   1 +
>>   2 files changed, 196 insertions(+)
>>   create mode 100644 sound/soc/qcom/lpass-cdc-dma.c
>>
>> diff --git a/sound/soc/qcom/lpass-cdc-dma.c 
>> b/sound/soc/qcom/lpass-cdc-dma.c
>> new file mode 100644
>> index 0000000..f5bfda9
>> --- /dev/null
>> +++ b/sound/soc/qcom/lpass-cdc-dma.c
>> @@ -0,0 +1,195 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2021 The Linux Foundation. All rights reserved.
>> + *
>> + * lpass-cdc-dma.c -- ALSA SoC WCD -CPU DAI driver for QTi LPASS WCD
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/module.h>
>> +#include <sound/soc.h>
>> +#include <sound/soc-dai.h>
>> +
>> +#include "lpass-lpaif-reg.h"
>> +#include "lpass.h"
>> +
>> +static void __get_dmactl_handle(struct snd_pcm_substream *substream, 
>> struct snd_soc_dai *dai,
>> +                    struct lpaif_dmactl **dmactl, int *id)
>
> Please rename this with an lpass prefix, Also indentation looks bit 
> off here.
Okay.
>
>> +{
>> +    struct snd_soc_pcm_runtime *soc_runtime = 
>> asoc_substream_to_rtd(substream);
>> +    struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
>> +    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
>> +    struct snd_pcm_runtime *rt = substream->runtime;
>> +    struct lpass_pcm_data *pcm_data = rt->private_data;
>> +    struct lpass_variant *v = drvdata->variant;
>> +    int dir = substream->stream;
>> +    unsigned int dai_id = cpu_dai->driver->id;
>> +
>> +    if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
>> +        *dmactl = drvdata->rxtx_rd_dmactl;
>> +        *id = pcm_data->dma_ch;
>> +    } else {
>> +        if (dai_id == LPASS_CDC_DMA_TX3) {
>> +            *dmactl = drvdata->rxtx_wr_dmactl;
>> +            *id = pcm_data->dma_ch - v->rxtx_wrdma_channel_start;
>> +        } else if (dai_id == LPASS_CDC_DMA_VA_TX0) {
>> +            *dmactl = drvdata->va_wr_dmactl;
>> +            *id = pcm_data->dma_ch - v->va_wrdma_channel_start;
>> +        }
>
> use switch case here, also something like this makes more sense rather 
> that just dealing with TX3 and VA TX0
okay. will change accordingly and add other cdc dma ports.
>
> switch (dai_id) {
> case: LPASS_CDC_DMA_RX0 ... LPASS_CDC_DMA_RX8:
>     *dmactl = drvdata->rxtx_wr_dmactl;
>     *id = pcm_data->dma_ch;
>     break;
> case: LPASS_CDC_DMA_TX0 ... LPASS_CDC_DMA_TX8:
>     *dmactl = drvdata->rxtx_wr_dmactl;
>     *id = pcm_data->dma_ch - v->rxtx_wrdma_channel_start;
>     break;
> case LPASS_CDC_DMA_VA_TX0 ...  LPASS_CDC_DMA_VA_TX8:
>     *dmactl = drvdata->rxtx_wr_dmactl;
>     *id = pcm_data->dma_ch - v->va_wrdma_channel_start;
>     break;
> default:
>     dev_err(dev, "Invalid dai id for dma ctl\n");
>     break;
> }
>
>> +    }
>> +}
>> +
>> +static int __lpass_platform_codec_intf_init(struct snd_soc_dai *dai,
>> +                     struct snd_pcm_substream *substream)
>
> Indentation is off, I think most of the code has simillar issue.
Okay. will change in all places.
>
>> +{
>> +    struct snd_soc_pcm_runtime *soc_runtime = 
>> asoc_substream_to_rtd(substream);
>> +    struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0);
>> +    struct lpaif_dmactl *dmactl;
>> +    int dir = substream->stream;
>> +    int ret, id, codec_intf;
>> +    unsigned int dai_id = cpu_dai->driver->id;
>> +
>> +    if (dir == SNDRV_PCM_STREAM_PLAYBACK)
>> +        codec_intf = LPASS_CDC_DMA_RX0_INTERFACE;
>
> Why are we limiting this to just RX0, it should be trivial to add 
> support to other RX and TX dais.
>
> Add a helper to convert the dai ids to interface indexs
Okay. will change accordingly.
>
>> +    else
>> +        codec_intf = LPASS_CDC_DMA_INTERFACE(dai_id);
>> +
>> +    __get_dmactl_handle(substream, dai, &dmactl, &id);
>
> We are not checking if dmactl is valid or not, best is to have a error 
> returned from __get_dmactl_handle() and handle it properly here.
Okay. will add error handling.
>> +
>> +    ret = regmap_fields_write(dmactl->codec_intf, id, codec_intf);
>> +    if (ret) {
>> +        dev_err(soc_runtime->dev,
> Why not use a local declare a local dev pointer and use it here, it 
> will save lines here.
Okay.
>> +            "error writing to dmactl codec_intf reg field: %d\n", ret);
>> +        return ret;
>> +    }
>> +    ret = regmap_fields_write(dmactl->codec_fs_sel, id, 0x0);
>> +    if (ret) {
>> +        dev_err(soc_runtime->dev,
>> +            "error writing to dmactl codec_fs_sel reg field: %d\n", 
>> ret);
>> +        return ret;
>> +    }
>> +    ret = regmap_fields_write(dmactl->codec_fs_delay, id, 0x0);
>> +    if (ret) {
>> +        dev_err(soc_runtime->dev,
>> +            "error writing to dmactl codec_fs_delay reg field: 
>> %d\n", ret);
>> +        return ret;
>> +    }
>> +    ret = regmap_fields_write(dmactl->codec_pack, id, 0x1);
>> +    if (ret) {
>> +        dev_err(soc_runtime->dev,
>> +            "error writing to dmactl codec_pack reg field: %d\n", ret);
>> +        return ret;
>> +    }
>> +    ret = regmap_fields_write(dmactl->codec_enable, id, 
>> LPAIF_DMACTL_ENABLE_ON);
>> +    if (ret) {
>> +        dev_err(soc_runtime->dev,
>> +            "error writing to dmactl codec_enable reg field: %d\n", 
>> ret);
>> +        return ret;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static int lpass_wcd_daiops_startup(struct snd_pcm_substream 
>> *substream,
>> +        struct snd_soc_dai *dai)
>> +{
>> +    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
>> +    int ret;
>> +
>> +    ret = clk_bulk_prepare_enable(drvdata->cdc_num_clks, 
>> drvdata->cdc_clks);
>> +    if (ret) {
>> +        dev_err(dai->dev, "error in enabling cdc dma clks: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>
> may be just do
> return clk_bulk_prepare_enable(drvdata->cdc_num_clks, drvdata->cdc_clks);
>
> soc_dai_ret() should print an error if startup fails.
>
Okay.
>
>
>> +}
>> +
>> +static void lpass_wcd_daiops_shutdown(struct snd_pcm_substream 
>> *substream,
>> +        struct snd_soc_dai *dai)
>> +{
>> +    struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
>> +
>> +    clk_bulk_disable_unprepare(drvdata->cdc_num_clks, 
>> drvdata->cdc_clks);
>> +}
>> +
>> +static int lpass_wcd_daiops_hw_params(struct snd_pcm_substream 
>> *substream,
>> +        struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
>> +{
>> +    struct snd_soc_pcm_runtime *soc_runtime = 
>> asoc_substream_to_rtd(substream);
>> +    struct lpaif_dmactl *dmactl;
>> +    unsigned int ret, regval;
>> +    unsigned int channels = params_channels(params);
>> +    int id;
>> +
>> +    __get_dmactl_handle(substream, dai, &dmactl, &id);
>
> may be move it after switch case and handle the errors.
Okay. will move it below.
>> +
>> +    switch (channels) {
>> +    case 1:
>> +        regval = LPASS_CDC_DMA_INTF_ONE_CHANNEL;
>> +        break;
>> +    case 2:
>> +        regval = LPASS_CDC_DMA_INTF_TWO_CHANNEL;
>> +        break;
>> +    case 4:
>> +        regval = LPASS_CDC_DMA_INTF_FOUR_CHANNEL;
>> +        break;
>> +    case 6:
>> +        regval = LPASS_CDC_DMA_INTF_SIX_CHANNEL;
>> +        break;
>> +    case 8:
>> +        regval = LPASS_CDC_DMA_INTF_EIGHT_CHANNEL;
>> +        break;
>> +    default:
>> +        dev_err(soc_runtime->dev, "invalid PCM config\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    ret = regmap_fields_write(dmactl->codec_channel, id, regval);
>> +    if (ret) {
>> +        dev_err(soc_runtime->dev,
>> +            "error writing to dmactl codec_channel reg field: %d\n", 
>> ret);
>> +        return ret;
>> +    }
>> +    return 0;
>> +}
>> +
>> +static int lpass_wcd_daiops_trigger(struct snd_pcm_substream 
>> *substream,
>> +        int cmd, struct snd_soc_dai *dai)
>> +{
>> +    struct snd_soc_pcm_runtime *soc_runtime = 
>> asoc_substream_to_rtd(substream);
>> +    struct lpaif_dmactl *dmactl;
>> +    int ret = 0, id;
>> +
>> +    __get_dmactl_handle(substream, dai, &dmactl, &id);
>
> move it inside the case where this is really used.
Okay.
>
>> +
>> +    switch (cmd) {
>> +    case SNDRV_PCM_TRIGGER_START:
>> +    case SNDRV_PCM_TRIGGER_RESUME:
>> +    case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>> +        __lpass_platform_codec_intf_init(dai, substream);
>> +        break;
>> +    case SNDRV_PCM_TRIGGER_STOP:
>> +    case SNDRV_PCM_TRIGGER_SUSPEND:
>> +    case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
>> +        ret = regmap_fields_write(dmactl->codec_enable, id, 
>> LPAIF_DMACTL_ENABLE_OFF);
>> +        if (ret) {
>> +            dev_err(soc_runtime->dev,
>> +                "error writing to dmactl codec_enable reg: %d\n", ret);
>> +            return ret;
>> +        }
>> +
>> +        break;
>
> missing default:
Okay. will add it.
>
>> +    }
>> +    return ret;
>> +}
>> +
>> +const struct snd_soc_dai_ops asoc_qcom_lpass_wcd_dai_ops = {
>> +    .startup    = lpass_wcd_daiops_startup,
>> +    .shutdown    = lpass_wcd_daiops_shutdown,
>> +    .hw_params    = lpass_wcd_daiops_hw_params,
>> +    .trigger    = lpass_wcd_daiops_trigger,
>> +};
>> +EXPORT_SYMBOL_GPL(asoc_qcom_lpass_wcd_dai_ops);
>> +
>> +MODULE_DESCRIPTION("QTi LPASS CDC DMA Driver");
>> +MODULE_LICENSE("GPL v2");
>
> use
> MODULE_LICENSE("GPL");
>
> more info in ./Documentation/process/license-rules.rst
>
> --srini
Okay.
>> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
>> index b068534..f912425 100644
>> --- a/sound/soc/qcom/lpass.h
>> +++ b/sound/soc/qcom/lpass.h
>> @@ -414,5 +414,6 @@ int asoc_qcom_lpass_cpu_dai_probe(struct 
>> snd_soc_dai *dai);
>>   extern const struct snd_soc_dai_ops asoc_qcom_lpass_cpu_dai_ops;
>>   int lpass_cpu_pcm_new(struct snd_soc_pcm_runtime *rtd,
>>                   struct snd_soc_dai *dai);
>> +extern const struct snd_soc_dai_ops asoc_qcom_lpass_wcd_dai_ops;
>>     #endif /* __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	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-11-26 11:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18 12:48 [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivasa Rao Mandadapu
2021-11-18 12:48 ` [PATCH v5 01/10] ASoC: qcom: Move lpass_pcm_data structure to lpass header Srinivasa Rao Mandadapu
2021-11-23 12:27   ` Srinivas Kandagatla
2021-11-18 12:48 ` [PATCH v5 02/10] ASoC: qcom: lpass: Add dma fields for codec dma lpass interface Srinivasa Rao Mandadapu
2021-11-18 12:48 ` [PATCH v5 03/10] ASoC: qcom: Add register definition for codec rddma and wrdma Srinivasa Rao Mandadapu
2021-11-18 12:48 ` [PATCH v5 04/10] ASoC: qcom: Add lpass CPU driver for codec dma control Srinivasa Rao Mandadapu
2021-11-23 12:27   ` Srinivas Kandagatla
2021-11-26 11:40     ` Srinivasa Rao Mandadapu
2021-11-23 12:28 ` [PATCH v5 00/10] Add support for audio on SC7280 based targets Srinivas Kandagatla
2021-11-26  4:39   ` Srinivasa Rao Mandadapu

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).