From: Paul Cercueil <paul@crapouillou.net>
To: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
Cc: alsa-devel@alsa-project.org, lgirdwood@gmail.com,
linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
tiwai@suse.com, broonie@kernel.org
Subject: Re: [PATCH v4 04/11] ASoC: jz4740-i2s: Simplify using regmap fields
Date: Wed, 20 Jul 2022 12:50:42 +0100 [thread overview]
Message-ID: <I8IBFR.IRTZANT1TBO53@crapouillou.net> (raw)
In-Reply-To: <20220708160244.21933-5-aidanmacdonald.0x0@gmail.com>
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
>
next prev parent reply other threads:[~2022-07-20 11:51 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-08 16:02 [PATCH v4 00/11] ASoC: cleanups and improvements for jz4740-i2s Aidan MacDonald
2022-07-08 16:02 ` [PATCH v4 01/11] ASoC: jz4740-i2s: Handle independent FIFO flush bits Aidan MacDonald
2022-07-20 11:44 ` Paul Cercueil
2022-07-20 14:43 ` Aidan MacDonald
2022-07-21 10:08 ` Paul Cercueil
2022-10-22 15:43 ` Aidan MacDonald
2022-07-08 16:02 ` [PATCH v4 02/11] ASoC: jz4740-i2s: Remove unused 'mem' resource Aidan MacDonald
2022-07-20 11:44 ` Paul Cercueil
2022-07-08 16:02 ` [PATCH v4 03/11] ASoC: jz4740-i2s: Convert to regmap API Aidan MacDonald
2022-07-20 12:05 ` Paul Cercueil
2022-07-08 16:02 ` [PATCH v4 04/11] ASoC: jz4740-i2s: Simplify using regmap fields Aidan MacDonald
2022-07-20 11:50 ` Paul Cercueil [this message]
2022-07-08 16:02 ` [PATCH v4 05/11] ASoC: jz4740-i2s: Use FIELD_PREP() macros in hw_params callback Aidan MacDonald
2022-07-20 11:52 ` Paul Cercueil
2022-07-08 16:02 ` [PATCH v4 06/11] ASoC: jz4740-i2s: Align macro values and sort includes Aidan MacDonald
2022-07-08 16:02 ` [PATCH v4 07/11] ASoC: jz4740-i2s: Make the PLL clock name SoC-specific Aidan MacDonald
2022-07-13 14:33 ` Zhou Yanjie
2022-07-13 15:07 ` Paul Cercueil
2022-07-13 15:29 ` Zhou Yanjie
2022-10-22 17:15 ` Aidan MacDonald
2022-10-22 20:03 ` Paul Cercueil
2022-10-23 13:29 ` Aidan MacDonald
2022-10-24 13:06 ` Paul Cercueil
2022-10-25 9:20 ` Aidan MacDonald
2022-07-20 11:53 ` Paul Cercueil
2022-07-08 16:02 ` [PATCH v4 08/11] ASoC: jz4740-i2s: Support S20_LE and S24_LE sample formats Aidan MacDonald
2022-07-20 11:56 ` Paul Cercueil
2022-07-08 16:02 ` [PATCH v4 09/11] ASoC: jz4740-i2s: Support continuous sample rate Aidan MacDonald
2022-07-08 16:02 ` [PATCH v4 10/11] ASoC: jz4740-i2s: Move component functions near the component driver Aidan MacDonald
2022-07-20 11:58 ` Paul Cercueil
2022-07-08 16:02 ` [PATCH v4 11/11] ASoC: jz4740-i2s: Refactor DAI probe/remove ops as component ops Aidan MacDonald
2022-07-20 12:04 ` Paul Cercueil
2022-07-20 23:12 ` (subset) [PATCH v4 00/11] ASoC: cleanups and improvements for jz4740-i2s Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=I8IBFR.IRTZANT1TBO53@crapouillou.net \
--to=paul@crapouillou.net \
--cc=aidanmacdonald.0x0@gmail.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).