Messages in this thread | | | Date | Wed, 20 Jul 2022 12:50:42 +0100 | From | Paul Cercueil <> | Subject | Re: [PATCH v4 04/11] ASoC: jz4740-i2s: Simplify using regmap fields |
| |
Hi Aidan,
Le ven., juil. 8 2022 at 17:02:37 +0100, Aidan MacDonald <aidanmacdonald.0x0@gmail.com> a écrit : > The differences between register fields on different SoC versions > can be abstracted away using the regmap field API. This is easier > to understand and extend than comparisons based on the version ID. > Since the version IDs are unused after this change, remove them at > the same time, and remove unused macros. > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
Reviewed-by: Paul Cercueil <paul@crapouillou.net>
Cheers, -Paul
> --- > sound/soc/jz4740/jz4740-i2s.c | 135 > +++++++++++++++++++--------------- > 1 file changed, 77 insertions(+), 58 deletions(-) > > diff --git a/sound/soc/jz4740/jz4740-i2s.c > b/sound/soc/jz4740/jz4740-i2s.c > index 1393b383a886..043f100a9cfa 100644 > --- a/sound/soc/jz4740/jz4740-i2s.c > +++ b/sound/soc/jz4740/jz4740-i2s.c > @@ -34,8 +34,6 @@ > #define JZ_REG_AIC_CLK_DIV 0x30 > #define JZ_REG_AIC_FIFO 0x34 > > -#define JZ_AIC_CONF_FIFO_RX_THRESHOLD_MASK (0xf << 12) > -#define JZ_AIC_CONF_FIFO_TX_THRESHOLD_MASK (0xf << 8) > #define JZ_AIC_CONF_OVERFLOW_PLAY_LAST BIT(6) > #define JZ_AIC_CONF_INTERNAL_CODEC BIT(5) > #define JZ_AIC_CONF_I2S BIT(4) > @@ -44,11 +42,6 @@ > #define JZ_AIC_CONF_SYNC_CLK_MASTER BIT(1) > #define JZ_AIC_CONF_ENABLE BIT(0) > > -#define JZ_AIC_CONF_FIFO_RX_THRESHOLD_OFFSET 12 > -#define JZ_AIC_CONF_FIFO_TX_THRESHOLD_OFFSET 8 > -#define JZ4760_AIC_CONF_FIFO_RX_THRESHOLD_OFFSET 24 > -#define JZ4760_AIC_CONF_FIFO_TX_THRESHOLD_OFFSET 16 > - > #define JZ_AIC_CTRL_OUTPUT_SAMPLE_SIZE_MASK (0x7 << 19) > #define JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_MASK (0x7 << 16) > #define JZ_AIC_CTRL_ENABLE_RX_DMA BIT(15) > @@ -78,29 +71,25 @@ > > #define JZ_AIC_I2S_STATUS_BUSY BIT(2) > > -#define JZ_AIC_CLK_DIV_MASK 0xf > -#define I2SDIV_DV_SHIFT 0 > -#define I2SDIV_DV_MASK (0xf << I2SDIV_DV_SHIFT) > -#define I2SDIV_IDV_SHIFT 8 > -#define I2SDIV_IDV_MASK (0xf << I2SDIV_IDV_SHIFT) > - > -enum jz47xx_i2s_version { > - JZ_I2S_JZ4740, > - JZ_I2S_JZ4760, > - JZ_I2S_JZ4770, > - JZ_I2S_JZ4780, > -}; > - > struct i2s_soc_info { > - enum jz47xx_i2s_version version; > struct snd_soc_dai_driver *dai; > > + struct reg_field field_rx_fifo_thresh; > + struct reg_field field_tx_fifo_thresh; > + struct reg_field field_i2sdiv_capture; > + struct reg_field field_i2sdiv_playback; > + > bool shared_fifo_flush; > }; > > struct jz4740_i2s { > struct regmap *regmap; > > + struct regmap_field *field_rx_fifo_thresh; > + struct regmap_field *field_tx_fifo_thresh; > + struct regmap_field *field_i2sdiv_capture; > + struct regmap_field *field_i2sdiv_playback; > + > struct clk *clk_aic; > struct clk *clk_i2s; > > @@ -238,12 +227,12 @@ static int jz4740_i2s_hw_params(struct > snd_pcm_substream *substream, > struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) > { > struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); > + struct regmap_field *div_field; > unsigned int sample_size; > - uint32_t ctrl, div_reg; > + uint32_t ctrl; > int div; > > regmap_read(i2s->regmap, JZ_REG_AIC_CTRL, &ctrl); > - regmap_read(i2s->regmap, JZ_REG_AIC_CLK_DIV, &div_reg); > > div = clk_get_rate(i2s->clk_i2s) / (64 * params_rate(params)); > > @@ -266,23 +255,16 @@ static int jz4740_i2s_hw_params(struct > snd_pcm_substream *substream, > else > ctrl &= ~JZ_AIC_CTRL_MONO_TO_STEREO; > > - div_reg &= ~I2SDIV_DV_MASK; > - div_reg |= (div - 1) << I2SDIV_DV_SHIFT; > + div_field = i2s->field_i2sdiv_playback; > } else { > ctrl &= ~JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_MASK; > ctrl |= sample_size << JZ_AIC_CTRL_INPUT_SAMPLE_SIZE_OFFSET; > > - if (i2s->soc_info->version >= JZ_I2S_JZ4770) { > - div_reg &= ~I2SDIV_IDV_MASK; > - div_reg |= (div - 1) << I2SDIV_IDV_SHIFT; > - } else { > - div_reg &= ~I2SDIV_DV_MASK; > - div_reg |= (div - 1) << I2SDIV_DV_SHIFT; > - } > + div_field = i2s->field_i2sdiv_capture; > } > > regmap_write(i2s->regmap, JZ_REG_AIC_CTRL, ctrl); > - regmap_write(i2s->regmap, JZ_REG_AIC_CLK_DIV, div_reg); > + regmap_field_write(div_field, div - 1); > > return 0; > } > @@ -355,7 +337,6 @@ static int jz4740_i2s_resume(struct > snd_soc_component *component) > static int jz4740_i2s_dai_probe(struct snd_soc_dai *dai) > { > struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); > - uint32_t conf; > int ret; > > ret = clk_prepare_enable(i2s->clk_aic); > @@ -365,22 +346,14 @@ static int jz4740_i2s_dai_probe(struct > snd_soc_dai *dai) > snd_soc_dai_init_dma_data(dai, &i2s->playback_dma_data, > &i2s->capture_dma_data); > > - if (i2s->soc_info->version >= JZ_I2S_JZ4760) { > - conf = (7 << JZ4760_AIC_CONF_FIFO_RX_THRESHOLD_OFFSET) | > - (8 << JZ4760_AIC_CONF_FIFO_TX_THRESHOLD_OFFSET) | > - JZ_AIC_CONF_OVERFLOW_PLAY_LAST | > - JZ_AIC_CONF_I2S | > - JZ_AIC_CONF_INTERNAL_CODEC; > - } else { > - conf = (7 << JZ_AIC_CONF_FIFO_RX_THRESHOLD_OFFSET) | > - (8 << JZ_AIC_CONF_FIFO_TX_THRESHOLD_OFFSET) | > - JZ_AIC_CONF_OVERFLOW_PLAY_LAST | > - JZ_AIC_CONF_I2S | > - JZ_AIC_CONF_INTERNAL_CODEC; > - } > - > regmap_write(i2s->regmap, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); > - regmap_write(i2s->regmap, JZ_REG_AIC_CONF, conf); > + > + regmap_write(i2s->regmap, JZ_REG_AIC_CONF, > + JZ_AIC_CONF_OVERFLOW_PLAY_LAST | > + JZ_AIC_CONF_I2S | JZ_AIC_CONF_INTERNAL_CODEC); > + > + regmap_field_write(i2s->field_rx_fifo_thresh, 7); > + regmap_field_write(i2s->field_tx_fifo_thresh, 8); > > return 0; > } > @@ -425,14 +398,20 @@ static struct snd_soc_dai_driver jz4740_i2s_dai > = { > }; > > static const struct i2s_soc_info jz4740_i2s_soc_info = { > - .version = JZ_I2S_JZ4740, > - .dai = &jz4740_i2s_dai, > - .shared_fifo_flush = true, > + .dai = &jz4740_i2s_dai, > + .field_rx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 12, 15), > + .field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 8, 11), > + .field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3), > + .field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3), > + .shared_fifo_flush = true, > }; > > static const struct i2s_soc_info jz4760_i2s_soc_info = { > - .version = JZ_I2S_JZ4760, > - .dai = &jz4740_i2s_dai, > + .dai = &jz4740_i2s_dai, > + .field_rx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 24, 27), > + .field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20), > + .field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3), > + .field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3), > }; > > static struct snd_soc_dai_driver jz4770_i2s_dai = { > @@ -454,13 +433,19 @@ static struct snd_soc_dai_driver jz4770_i2s_dai > = { > }; > > static const struct i2s_soc_info jz4770_i2s_soc_info = { > - .version = JZ_I2S_JZ4770, > - .dai = &jz4770_i2s_dai, > + .dai = &jz4770_i2s_dai, > + .field_rx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 24, 27), > + .field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20), > + .field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 8, 11), > + .field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3), > }; > > static const struct i2s_soc_info jz4780_i2s_soc_info = { > - .version = JZ_I2S_JZ4780, > - .dai = &jz4770_i2s_dai, > + .dai = &jz4770_i2s_dai, > + .field_rx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 24, 27), > + .field_tx_fifo_thresh = REG_FIELD(JZ_REG_AIC_CONF, 16, 20), > + .field_i2sdiv_capture = REG_FIELD(JZ_REG_AIC_CLK_DIV, 8, 11), > + .field_i2sdiv_playback = REG_FIELD(JZ_REG_AIC_CLK_DIV, 0, 3), > }; > > static const struct snd_soc_component_driver jz4740_i2s_component = { > @@ -479,6 +464,36 @@ static const struct of_device_id > jz4740_of_matches[] = { > }; > MODULE_DEVICE_TABLE(of, jz4740_of_matches); > > +static int jz4740_i2s_init_regmap_fields(struct device *dev, > + struct jz4740_i2s *i2s) > +{ > + i2s->field_rx_fifo_thresh = > + devm_regmap_field_alloc(dev, i2s->regmap, > + i2s->soc_info->field_rx_fifo_thresh); > + if (IS_ERR(i2s->field_rx_fifo_thresh)) > + return PTR_ERR(i2s->field_rx_fifo_thresh); > + > + i2s->field_tx_fifo_thresh = > + devm_regmap_field_alloc(dev, i2s->regmap, > + i2s->soc_info->field_tx_fifo_thresh); > + if (IS_ERR(i2s->field_tx_fifo_thresh)) > + return PTR_ERR(i2s->field_tx_fifo_thresh); > + > + i2s->field_i2sdiv_capture = > + devm_regmap_field_alloc(dev, i2s->regmap, > + i2s->soc_info->field_i2sdiv_capture); > + if (IS_ERR(i2s->field_i2sdiv_capture)) > + return PTR_ERR(i2s->field_i2sdiv_capture); > + > + i2s->field_i2sdiv_playback = > + devm_regmap_field_alloc(dev, i2s->regmap, > + i2s->soc_info->field_i2sdiv_playback); > + if (IS_ERR(i2s->field_i2sdiv_playback)) > + return PTR_ERR(i2s->field_i2sdiv_playback); > + > + return 0; > +} > + > static const struct regmap_config jz4740_i2s_regmap_config = { > .reg_bits = 32, > .reg_stride = 4, > @@ -523,6 +538,10 @@ static int jz4740_i2s_dev_probe(struct > platform_device *pdev) > if (IS_ERR(i2s->regmap)) > return PTR_ERR(i2s->regmap); > > + ret = jz4740_i2s_init_regmap_fields(dev, i2s); > + if (ret) > + return ret; > + > platform_set_drvdata(pdev, i2s); > > ret = devm_snd_soc_register_component(dev, &jz4740_i2s_component, > -- > 2.35.1 >
| |