Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 4/7] ASoC: qcom: lpass: Use regmap_field for i2sctl and dmactl registers | From | Srinivas Kandagatla <> | Date | Mon, 18 May 2020 10:49:36 +0100 |
| |
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@codeaurora.org> > --- > 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
>
| |