Re: [PATCH v3 3/8] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxxxxx>

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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux