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;
break; case 24:regval = LPAIF_I2SCTL_BITWIDTH_16;
regval |= LPAIF_I2SCTL_BITWIDTH_24;
break; case 32:regval = LPAIF_I2SCTL_BITWIDTH_24;
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];
elsemode = drvdata->mi2s_playback_sd_mode[id];
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,
elseLPAIF_I2SCTL_SPKMONO_STEREO);
regval |= LPAIF_I2SCTL_SPKMONO_MONO;
ret = regmap_fields_write(i2sctl->spkmono, id,
} else {LPAIF_I2SCTL_SPKMONO_MONO);
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,
elseLPAIF_I2SCTL_MICMONO_STEREO);
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),
if (ret) {regval);
dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
dev_err(dai->dev, "error writing to i2sctl channels mode: %d\n",
return ret; }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),
if (ret)mask, val);
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