Re: [PATCH v2 4/7] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers

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

 





On 14/05/2020 17:38, Ajit Pandey wrote:
I2SCTL and DMACTL registers has different bits alignment for newer
LPASS variants of SC7180 soc. Instead of adding extra overhead for
calculating masks and shifts for newer variants registers layout we
changed the approach to use regmap_field_write() API to update bit.
Such API's will internally do the required bit shift and mask based
on reg_field struct defined for bit fields. We'll define REG_FIELD()
macros with bit layout for both lpass variants and use such macros
to initialize register fields in variant specific driver callbacks.
Also added new bitfieds values for I2SCTL and DMACTL registers and
removed shifts and mask macros for such registers from header file.

Signed-off-by: Ajit Pandey <ajitp@xxxxxxxxxxxxxx>
---
  sound/soc/qcom/lpass-apq8016.c   |  61 ++++++++++++
  sound/soc/qcom/lpass-cpu.c       | 114 +++++++++++++---------
  sound/soc/qcom/lpass-lpaif-reg.h | 203 ++++++++++++++++++++++++---------------
  sound/soc/qcom/lpass-platform.c  |  86 +++++++++++------
  sound/soc/qcom/lpass.h           |  30 ++++++
  5 files changed, 340 insertions(+), 154 deletions(-)


Thanks for moving this to regmap fields, looks clean!
However this patch just removed support to lpass-ipq806x.c variant, which should to be taken care of while doing patches that apply to all variants.


diff --git a/sound/soc/qcom/lpass-apq8016.c b/sound/soc/qcom/lpass-apq8016.c
index 8210e37..3149645 100644
--- a/sound/soc/qcom/lpass-apq8016.c
+++ b/sound/soc/qcom/lpass-apq8016.c
@@ -124,6 +124,32 @@
  	},
  };
+static int apq8016_init_dmactl_bitfields(struct lpaif_dmactl *dmactl,
+					 struct regmap *map,
+					 unsigned int reg)
+{
+	struct reg_field bursten = DMACTL_BURSTEN_FLD(reg);
+	struct reg_field wpscnt = DMACTL_WPSCNT_FLD(reg);
+	struct reg_field fifowm = DMACTL_FIFOWM_FLD(reg);
+	struct reg_field intf = DMACTL_AUDINTF_FLD(reg);
+	struct reg_field enable = DMACTL_ENABLE_FLD(reg);
+	struct reg_field dyncclk = DMACTL_DYNCLK_FLD(reg);
+
+	dmactl->bursten = regmap_field_alloc(map, bursten);
+	dmactl->wpscnt = regmap_field_alloc(map, wpscnt);
+	dmactl->fifowm = regmap_field_alloc(map, fifowm);
+	dmactl->intf = regmap_field_alloc(map, intf);
+	dmactl->enable = regmap_field_alloc(map, enable);
+	dmactl->dyncclk = regmap_field_alloc(map, dyncclk);

My idea was to move this all regmap fields to variant structure and common code will do the regmap_filed_alloc rather than each variant duplicating the same code for each variant, also am guessing some of the members in the lpass_variant structure tp become redundant due to regmap field which can be removed as well.

ex :

struct lpass_variant {
	...
	struct reg_field bursten
	...
};

in lpass-apq8016.c

we do
static struct lpass_variant apq8016_data = {

	.bursten = REG_FIELD(reg, 11, 11),
	...
}

in lpass-cpu.c we can do the real regmap_field_alloc	
asoc_qcom_lpass_cpu_platform_probe



+
+	if (IS_ERR(dmactl->bursten) || IS_ERR(dmactl->wpscnt) ||
+	    IS_ERR(dmactl->fifowm) || IS_ERR(dmactl->intf) ||
+	    IS_ERR(dmactl->enable) || IS_ERR(dmactl->dyncclk))
+		return -EINVAL;
+
+	return 0;
+}
+
  static int apq8016_lpass_alloc_dma_channel(struct lpass_data *drvdata,
  					   int direction)
  {
@@ -158,6 +184,39 @@ static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
  	return 0;
  }
+static int sc7180_init_i2sctl_bitfields(struct lpaif_i2sctl *i2sctl,
+					struct regmap *map, unsigned int reg)
+{
Should this be apq8016_init_i2sctl_bitfields

Please make sure that you compile the code before sending it out!

--srini





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux