[PATCH 0/4] Add regmap_field helpers for simple bit operations

Li Chen posted 4 patches 3 years, 11 months ago
There is a newer version of this series
drivers/base/regmap/regmap.c          | 22 ++++++++
drivers/pinctrl/bcm/pinctrl-bcm6358.c |  2 +-
drivers/pinctrl/pinctrl-st.c          | 23 ++++----
include/linux/regmap.h                | 37 +++++++++++++
sound/soc/sunxi/sun4i-codec.c         | 78 +++++++++++----------------
5 files changed, 99 insertions(+), 63 deletions(-)
[PATCH 0/4] Add regmap_field helpers for simple bit operations
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

This series proposes to add simple bit operations for setting, clearing
and testing specific bits with regmap_field.

Li Chen (4):
  regmap: provide regmap_field helpers for simple bit operations
  ASoC: sunxi: Use {regmap/regmap_field}_{set/clear}_bits helpers
  pinctrl: bcm: Use regmap_field_{set/clear}_bits helpers
  pinctrl: st: Switch to use regmap_field_test_bits

 drivers/base/regmap/regmap.c          | 22 ++++++++
 drivers/pinctrl/bcm/pinctrl-bcm6358.c |  2 +-
 drivers/pinctrl/pinctrl-st.c          | 23 ++++----
 include/linux/regmap.h                | 37 +++++++++++++
 sound/soc/sunxi/sun4i-codec.c         | 78 +++++++++++----------------
 5 files changed, 99 insertions(+), 63 deletions(-)

-- 
2.36.1
[PATCH v3 0/4] Add regmap_field helpers for simple bit operations
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

This series proposes to add simple bit operations for setting, clearing
and testing specific bits with regmap_field.

Li Chen (4):
  regmap: provide regmap_field helpers for simple bit operations
  ASoC: sunxi: Use {regmap/regmap_field}_{set/clear}_bits helpers
  pinctrl: bcm: Use regmap_field_{set/clear}_bits helpers
  pinctrl: st: Switch to use regmap_field_test_bits

Changelogs:
v2: fix regmap_field_test_bits compile error in drivers/pinctrl/pinctrl-st.c
v3: fix regmap_field_clear_bits and regmap_field_set_bits implementation,
    reported by Samuel Holland

 drivers/base/regmap/regmap.c          | 22 ++++++++
 drivers/pinctrl/bcm/pinctrl-bcm6358.c |  2 +-
 drivers/pinctrl/pinctrl-st.c          | 23 ++++----
 include/linux/regmap.h                | 37 +++++++++++++
 sound/soc/sunxi/sun4i-codec.c         | 78 +++++++++++----------------
 5 files changed, 99 insertions(+), 63 deletions(-)

-- 
2.36.1
Re: [PATCH v3 0/4] Add regmap_field helpers for simple bit operations
Posted by Mark Brown 3 years, 10 months ago
On Sun, May 22, 2022 at 08:26:21PM -0700, Li Chen wrote:
> From: Li Chen <lchen@ambarella.com>
> 
> This series proposes to add simple bit operations for setting, clearing
> and testing specific bits with regmap_field.

The following changes since commit f2906aa863381afb0015a9eb7fefad885d4e5a56:

  Linux 5.19-rc1 (2022-06-05 17:18:54 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/regmap-field-bit-helpers

for you to fetch changes up to f67be8b7ee90c292948c3ec6395673963cccaee6:

  regmap: provide regmap_field helpers for simple bit operations (2022-06-15 11:17:45 +0100)

----------------------------------------------------------------
regmap: Add regmap_field helpers for simple bit operations

Add simple bit operations for setting, clearing and testing specific
bits with regmap_field.

----------------------------------------------------------------
Li Chen (1):
      regmap: provide regmap_field helpers for simple bit operations

 drivers/base/regmap/regmap.c | 22 ++++++++++++++++++++++
 include/linux/regmap.h       | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)
Re: (subset) [PATCH v3 0/4] Add regmap_field helpers for simple bit operations
Posted by Mark Brown 3 years, 10 months ago
On Sun, 22 May 2022 20:26:21 -0700, Li Chen wrote:
> From: Li Chen <lchen@ambarella.com>
> 
> This series proposes to add simple bit operations for setting, clearing
> and testing specific bits with regmap_field.
> 
> Li Chen (4):
>   regmap: provide regmap_field helpers for simple bit operations
>   ASoC: sunxi: Use {regmap/regmap_field}_{set/clear}_bits helpers
>   pinctrl: bcm: Use regmap_field_{set/clear}_bits helpers
>   pinctrl: st: Switch to use regmap_field_test_bits
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/4] regmap: provide regmap_field helpers for simple bit operations
      commit: f67be8b7ee90c292948c3ec6395673963cccaee6
[2/4] ASoC: sunxi: Use {regmap/regmap_field}_{set/clear}_bits helpers
      commit: b23662406b1b225847b964e4549a5718c45f20d6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
[PATCH v3 1/4] regmap: provide regmap_field helpers for simple bit operations
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

We have set/clear/test operations for regmap, but not for regmap_field yet.
So let's introduce regmap_field helpers too.

In many instances regmap_field_update_bits() is used for simple bit setting
and clearing. In these cases the last argument is redundant and we can
hide it with a static inline function.

This adds three new helpers for simple bit operations: set_bits,
clear_bits and test_bits (the last one defined as a regular function).

Signed-off-by: Li Chen <lchen@ambarella.com>
---
 drivers/base/regmap/regmap.c | 22 +++++++++++++++++++++
 include/linux/regmap.h       | 37 ++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 5e12f7cb5147..a37d6041b7bd 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2208,6 +2208,28 @@ int regmap_field_update_bits_base(struct regmap_field *field,
 }
 EXPORT_SYMBOL_GPL(regmap_field_update_bits_base);
 
+/**
+ * regmap_field_test_bits() - Check if all specified bits are set in a
+ *                            register field.
+ *
+ * @field: Register field to operate on
+ * @bits: Bits to test
+ *
+ * Returns -1 if the underlying regmap_field_read() fails, 0 if at least one of the
+ * tested bits is not set and 1 if all tested bits are set.
+ */
+int regmap_field_test_bits(struct regmap_field *field, unsigned int bits)
+{
+	unsigned int val, ret;
+
+	ret = regmap_field_read(field, &val);
+	if (ret)
+		return ret;
+
+	return (val & bits) == bits;
+}
+EXPORT_SYMBOL_GPL(regmap_field_test_bits);
+
 /**
  * regmap_fields_update_bits_base() - Perform a read/modify/write cycle a
  *                                    register field with port ID
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index de81a94d7b30..5f317403c520 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1324,6 +1324,22 @@ static inline int regmap_field_update_bits(struct regmap_field *field,
 					     NULL, false, false);
 }
 
+static inline int regmap_field_set_bits(struct regmap_field *field,
+					unsigned int bits)
+{
+	return regmap_field_update_bits_base(field, bits, bits, NULL, false,
+					     false);
+}
+
+static inline int regmap_field_clear_bits(struct regmap_field *field,
+					  unsigned int bits)
+{
+	return regmap_field_update_bits_base(field, bits, 0, NULL, false,
+					     false);
+}
+
+int regmap_field_test_bits(struct regmap_field *field, unsigned int bits);
+
 static inline int
 regmap_field_force_update_bits(struct regmap_field *field,
 			       unsigned int mask, unsigned int val)
@@ -1757,6 +1773,27 @@ regmap_field_force_update_bits(struct regmap_field *field,
 	return -EINVAL;
 }
 
+static inline int regmap_field_set_bits(struct regmap_field *field,
+					unsigned int bits)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
+static inline int regmap_field_clear_bits(struct regmap_field *field,
+					  unsigned int bits)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
+static inline int regmap_field_test_bits(struct regmap_field *field,
+					 unsigned int bits)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
 static inline int regmap_fields_write(struct regmap_field *field,
 				      unsigned int id, unsigned int val)
 {
-- 
2.36.1
[PATCH v3 2/4] ASoC: sunxi: Use {regmap/regmap_field}_{set/clear}_bits helpers
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

Appropriately change calls to {regmap/regmap_field}_update_bits()
with {regmap/regmap_field}_set_bits()
and {regmap/regmap_field}_clear_bits() for improved readability.

Signed-off-by: Li Chen <lchen@ambarella.com>
---
 sound/soc/sunxi/sun4i-codec.c | 78 ++++++++++++++---------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 60712f24ade5..53e3f43816cc 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -250,37 +250,33 @@ struct sun4i_codec {
 static void sun4i_codec_start_playback(struct sun4i_codec *scodec)
 {
 	/* Flush TX FIFO */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH),
-			   BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH));
+	regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+			BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH));
 
 	/* Enable DAC DRQ */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN),
-			   BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN));
+	regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+			BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN));
 }
 
 static void sun4i_codec_stop_playback(struct sun4i_codec *scodec)
 {
 	/* Disable DAC DRQ */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN),
-			   0);
+	regmap_clear_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+			  BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN));
 }
 
 static void sun4i_codec_start_capture(struct sun4i_codec *scodec)
 {
 	/* Enable ADC DRQ */
-	regmap_field_update_bits(scodec->reg_adc_fifoc,
-				 BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN),
-				 BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN));
+	regmap_field_set_bits(scodec->reg_adc_fifoc,
+			      BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN));
 }
 
 static void sun4i_codec_stop_capture(struct sun4i_codec *scodec)
 {
 	/* Disable ADC DRQ */
-	regmap_field_update_bits(scodec->reg_adc_fifoc,
-				 BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN), 0);
+	regmap_field_clear_bits(scodec->reg_adc_fifoc,
+				 BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN));
 }
 
 static int sun4i_codec_trigger(struct snd_pcm_substream *substream, int cmd,
@@ -323,8 +319,7 @@ static int sun4i_codec_prepare_capture(struct snd_pcm_substream *substream,
 
 
 	/* Flush RX FIFO */
-	regmap_field_update_bits(scodec->reg_adc_fifoc,
-				 BIT(SUN4I_CODEC_ADC_FIFOC_FIFO_FLUSH),
+	regmap_field_set_bits(scodec->reg_adc_fifoc,
 				 BIT(SUN4I_CODEC_ADC_FIFOC_FIFO_FLUSH));
 
 
@@ -365,8 +360,7 @@ static int sun4i_codec_prepare_playback(struct snd_pcm_substream *substream,
 	u32 val;
 
 	/* Flush the TX FIFO */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH),
+	regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
 			   BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH));
 
 	/* Set TX FIFO Empty Trigger Level */
@@ -386,9 +380,8 @@ static int sun4i_codec_prepare_playback(struct snd_pcm_substream *substream,
 			   val);
 
 	/* Send zeros when we have an underrun */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_SEND_LASAT),
-			   0);
+	regmap_clear_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+			   BIT(SUN4I_CODEC_DAC_FIFOC_SEND_LASAT));
 
 	return 0;
 };
@@ -485,33 +478,27 @@ static int sun4i_codec_hw_params_capture(struct sun4i_codec *scodec,
 
 	/* Set the number of channels we want to use */
 	if (params_channels(params) == 1)
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-					 BIT(SUN4I_CODEC_ADC_FIFOC_MONO_EN),
+		regmap_field_set_bits(scodec->reg_adc_fifoc,
 					 BIT(SUN4I_CODEC_ADC_FIFOC_MONO_EN));
 	else
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-					 BIT(SUN4I_CODEC_ADC_FIFOC_MONO_EN),
-					 0);
+		regmap_field_clear_bits(scodec->reg_adc_fifoc,
+					 BIT(SUN4I_CODEC_ADC_FIFOC_MONO_EN));
 
 	/* Set the number of sample bits to either 16 or 24 bits */
 	if (hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS)->min == 32) {
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_SAMPLE_BITS),
+		regmap_field_set_bits(scodec->reg_adc_fifoc,
 				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_SAMPLE_BITS));
 
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_FIFO_MODE),
-				   0);
+		regmap_field_clear_bits(scodec->reg_adc_fifoc,
+				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_FIFO_MODE));
 
 		scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	} else {
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_SAMPLE_BITS),
-				   0);
+		regmap_field_clear_bits(scodec->reg_adc_fifoc,
+				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_SAMPLE_BITS));
 
 		/* Fill most significant bits with valid data MSB */
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_FIFO_MODE),
+		regmap_field_set_bits(scodec->reg_adc_fifoc,
 				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_FIFO_MODE));
 
 		scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -543,24 +530,20 @@ static int sun4i_codec_hw_params_playback(struct sun4i_codec *scodec,
 
 	/* Set the number of sample bits to either 16 or 24 bits */
 	if (hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS)->min == 32) {
-		regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_SAMPLE_BITS),
+		regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
 				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_SAMPLE_BITS));
 
 		/* Set TX FIFO mode to padding the LSBs with 0 */
-		regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_FIFO_MODE),
-				   0);
+		regmap_clear_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_FIFO_MODE));
 
 		scodec->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	} else {
-		regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_SAMPLE_BITS),
-				   0);
+		regmap_clear_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_SAMPLE_BITS));
 
 		/* Set TX FIFO mode to repeat the MSB */
-		regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_FIFO_MODE),
+		regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
 				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_FIFO_MODE));
 
 		scodec->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -624,8 +607,7 @@ static int sun4i_codec_startup(struct snd_pcm_substream *substream,
 	 * Stop issuing DRQ when we have room for less than 16 samples
 	 * in our TX FIFO
 	 */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   3 << SUN4I_CODEC_DAC_FIFOC_DRQ_CLR_CNT,
+	regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
 			   3 << SUN4I_CODEC_DAC_FIFOC_DRQ_CLR_CNT);
 
 	return clk_prepare_enable(scodec->clk_module);
-- 
2.36.1
[PATCH v3 3/4] pinctrl: bcm: Use regmap_field_{set/clear}_bits helpers
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

Appropriately change calls to regmap_field_update_bits()
with regmap_field_clear_bits() for improved readability.

Signed-off-by: Li Chen <lchen@ambarella.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm6358.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm6358.c b/drivers/pinctrl/bcm/pinctrl-bcm6358.c
index 9f6cd7447887..b03dfcb171d1 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm6358.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm6358.c
@@ -300,7 +300,7 @@ static int bcm6358_gpio_request_enable(struct pinctrl_dev *pctldev,
 		return 0;
 
 	/* disable all functions using this pin */
-	return regmap_field_update_bits(priv->overlays, mask, 0);
+	return regmap_field_clear_bits(priv->overlays, mask);
 }
 
 static const struct pinctrl_ops bcm6358_pctl_ops = {
-- 
2.36.1
[PATCH v3 4/4] pinctrl: st: Switch to use regmap_field_test_bits
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

Appropriately change calls to regmap_field_read() with
regmap_field_test_bits() for improved readability.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Li Chen <lchen@ambarella.com>
---
 drivers/pinctrl/pinctrl-st.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 0fea71fd9a00..198b4a9d263b 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -573,23 +573,18 @@ static void st_pinconf_set_retime_dedicated(struct st_pinctrl *info,
 static void st_pinconf_get_direction(struct st_pio_control *pc,
 	int pin, unsigned long *config)
 {
-	unsigned int oe_value, pu_value, od_value;
-
 	if (pc->oe) {
-		regmap_field_read(pc->oe, &oe_value);
-		if (oe_value & BIT(pin))
+		if (regmap_field_test_bits(pc->oe, BIT(pin)))
 			ST_PINCONF_PACK_OE(*config);
 	}
 
 	if (pc->pu) {
-		regmap_field_read(pc->pu, &pu_value);
-		if (pu_value & BIT(pin))
+		if (regmap_field_test_bits(pc->pu, BIT(pin)))
 			ST_PINCONF_PACK_PU(*config);
 	}
 
 	if (pc->od) {
-		regmap_field_read(pc->od, &od_value);
-		if (od_value & BIT(pin))
+		if (regmap_field_test_bits(pc->od, BIT(pin)))
 			ST_PINCONF_PACK_OD(*config);
 	}
 }
@@ -599,22 +594,22 @@ static int st_pinconf_get_retime_packed(struct st_pinctrl *info,
 {
 	const struct st_pctl_data *data = info->data;
 	struct st_retime_packed *rt_p = &pc->rt.rt_p;
-	unsigned int delay_bits, delay, delay0, delay1, val;
+	unsigned int delay_bits, delay, delay0, delay1;
 	int output = ST_PINCONF_UNPACK_OE(*config);
 
-	if (!regmap_field_read(rt_p->retime, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->retime, BIT(pin)))
 		ST_PINCONF_PACK_RT(*config);
 
-	if (!regmap_field_read(rt_p->clk1notclk0, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->clk1notclk0, BIT(pin)))
 		ST_PINCONF_PACK_RT_CLK(*config, 1);
 
-	if (!regmap_field_read(rt_p->clknotdata, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->clknotdata, BIT(pin)))
 		ST_PINCONF_PACK_RT_CLKNOTDATA(*config);
 
-	if (!regmap_field_read(rt_p->double_edge, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->double_edge, BIT(pin)))
 		ST_PINCONF_PACK_RT_DOUBLE_EDGE(*config);
 
-	if (!regmap_field_read(rt_p->invertclk, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->invertclk, BIT(pin)))
 		ST_PINCONF_PACK_RT_INVERTCLK(*config);
 
 	regmap_field_read(rt_p->delay_0, &delay0);
-- 
2.36.1
[PATCH v2 0/4] Add regmap_field helpers for simple bit operations
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

This series proposes to add simple bit operations for setting, clearing
and testing specific bits with regmap_field.

Li Chen (4):
  regmap: provide regmap_field helpers for simple bit operations
  ASoC: sunxi: Use {regmap/regmap_field}_{set/clear}_bits helpers
  pinctrl: bcm: Use regmap_field_{set/clear}_bits helpers
  pinctrl: st: Switch to use regmap_field_test_bits

Changelogs:
v2: fix regmap_field_test_bits compile error in drivers/pinctrl/pinctrl-st.c

 drivers/base/regmap/regmap.c          | 22 ++++++++
 drivers/pinctrl/bcm/pinctrl-bcm6358.c |  2 +-
 drivers/pinctrl/pinctrl-st.c          | 23 ++++----
 include/linux/regmap.h                | 37 +++++++++++++
 sound/soc/sunxi/sun4i-codec.c         | 78 +++++++++++----------------
 5 files changed, 99 insertions(+), 63 deletions(-)

-- 
2.36.1
Re: [PATCH v2 0/4] Add regmap_field helpers for simple bit operations
Posted by Mark Brown 3 years, 11 months ago
On Sun, May 22, 2022 at 07:22:37PM -0700, Li Chen wrote:
> From: Li Chen <lchen@ambarella.com>
> 
> This series proposes to add simple bit operations for setting, clearing
> and testing specific bits with regmap_field.

Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.
Re: [PATCH v2 0/4] Add regmap_field helpers for simple bit operations
Posted by Li Chen 3 years, 11 months ago
Hi Mark

 ---- On Mon, 23 May 2022 04:39:46 -0700 Mark Brown <broonie@kernel.org> wrote ----
 > On Sun, May 22, 2022 at 07:22:37PM -0700, Li Chen wrote:
 > > From: Li Chen <lchen@ambarella.com>
 > > 
 > > This series proposes to add simple bit operations for setting, clearing
 > > and testing specific bits with regmap_field.
 > 
 > Please don't send new patches in reply to old patches or serieses, this
 > makes it harder for both people and tools to understand what is going
 > on - it can bury things in mailboxes and make it difficult to keep track
 > of what current patches are, both for the new patches and the old ones.
 > 

Thanks for letting me know, I won't do this again.

Regards,
Li
[PATCH v2 1/4] regmap: provide regmap_field helpers for simple bit operations
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

We have set/clear/test operations for regmap, but not for regmap_field yet.
So let's introduce regmap_field helpers too.

In many instances regmap_field_update_bits() is used for simple bit setting
and clearing. In these cases the last argument is redundant and we can
hide it with a static inline function.

This adds three new helpers for simple bit operations: set_bits,
clear_bits and test_bits (the last one defined as a regular function).

Signed-off-by: Li Chen <lchen@ambarella.com>
---
 drivers/base/regmap/regmap.c | 22 +++++++++++++++++++++
 include/linux/regmap.h       | 37 ++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 5e12f7cb5147..a37d6041b7bd 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2208,6 +2208,28 @@ int regmap_field_update_bits_base(struct regmap_field *field,
 }
 EXPORT_SYMBOL_GPL(regmap_field_update_bits_base);
 
+/**
+ * regmap_field_test_bits() - Check if all specified bits are set in a
+ *                            register field.
+ *
+ * @field: Register field to operate on
+ * @bits: Bits to test
+ *
+ * Returns -1 if the underlying regmap_field_read() fails, 0 if at least one of the
+ * tested bits is not set and 1 if all tested bits are set.
+ */
+int regmap_field_test_bits(struct regmap_field *field, unsigned int bits)
+{
+	unsigned int val, ret;
+
+	ret = regmap_field_read(field, &val);
+	if (ret)
+		return ret;
+
+	return (val & bits) == bits;
+}
+EXPORT_SYMBOL_GPL(regmap_field_test_bits);
+
 /**
  * regmap_fields_update_bits_base() - Perform a read/modify/write cycle a
  *                                    register field with port ID
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index de81a94d7b30..10b410734d9e 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1324,6 +1324,22 @@ static inline int regmap_field_update_bits(struct regmap_field *field,
 					     NULL, false, false);
 }
 
+static inline int regmap_field_set_bits(struct regmap_field *field,
+					unsigned int bits)
+{
+	return regmap_field_update_bits_base(field, bits, 0, NULL, false,
+					     false);
+}
+
+static inline int regmap_field_clear_bits(struct regmap_field *field,
+					  unsigned int bits)
+{
+	return regmap_field_update_bits_base(field, bits, bits, NULL, false,
+					     false);
+}
+
+int regmap_field_test_bits(struct regmap_field *field, unsigned int bits);
+
 static inline int
 regmap_field_force_update_bits(struct regmap_field *field,
 			       unsigned int mask, unsigned int val)
@@ -1757,6 +1773,27 @@ regmap_field_force_update_bits(struct regmap_field *field,
 	return -EINVAL;
 }
 
+static inline int regmap_field_set_bits(struct regmap_field *field,
+					unsigned int bits)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
+static inline int regmap_field_clear_bits(struct regmap_field *field,
+					  unsigned int bits)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
+static inline int regmap_field_test_bits(struct regmap_field *field,
+					 unsigned int bits)
+{
+	WARN_ONCE(1, "regmap API is disabled");
+	return -EINVAL;
+}
+
 static inline int regmap_fields_write(struct regmap_field *field,
 				      unsigned int id, unsigned int val)
 {
-- 
2.36.1
Re: [PATCH v2 1/4] regmap: provide regmap_field helpers for simple bit operations
Posted by Samuel Holland 3 years, 11 months ago
Hi,

On 5/22/22 9:23 PM, Li Chen wrote:
> From: Li Chen <lchen@ambarella.com>
> 
> We have set/clear/test operations for regmap, but not for regmap_field yet.
> So let's introduce regmap_field helpers too.
> 
> In many instances regmap_field_update_bits() is used for simple bit setting
> and clearing. In these cases the last argument is redundant and we can
> hide it with a static inline function.
> 
> This adds three new helpers for simple bit operations: set_bits,
> clear_bits and test_bits (the last one defined as a regular function).
> 
> Signed-off-by: Li Chen <lchen@ambarella.com>
> ---
>  drivers/base/regmap/regmap.c | 22 +++++++++++++++++++++
>  include/linux/regmap.h       | 37 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 5e12f7cb5147..a37d6041b7bd 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -2208,6 +2208,28 @@ int regmap_field_update_bits_base(struct regmap_field *field,
>  }
>  EXPORT_SYMBOL_GPL(regmap_field_update_bits_base);
>  
> +/**
> + * regmap_field_test_bits() - Check if all specified bits are set in a
> + *                            register field.
> + *
> + * @field: Register field to operate on
> + * @bits: Bits to test
> + *
> + * Returns -1 if the underlying regmap_field_read() fails, 0 if at least one of the
> + * tested bits is not set and 1 if all tested bits are set.
> + */
> +int regmap_field_test_bits(struct regmap_field *field, unsigned int bits)
> +{
> +	unsigned int val, ret;
> +
> +	ret = regmap_field_read(field, &val);
> +	if (ret)
> +		return ret;
> +
> +	return (val & bits) == bits;
> +}
> +EXPORT_SYMBOL_GPL(regmap_field_test_bits);
> +
>  /**
>   * regmap_fields_update_bits_base() - Perform a read/modify/write cycle a
>   *                                    register field with port ID
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index de81a94d7b30..10b410734d9e 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -1324,6 +1324,22 @@ static inline int regmap_field_update_bits(struct regmap_field *field,
>  					     NULL, false, false);
>  }
>  
> +static inline int regmap_field_set_bits(struct regmap_field *field,
> +					unsigned int bits)
> +{
> +	return regmap_field_update_bits_base(field, bits, 0, NULL, false,
> +					     false);
> +}
> +
> +static inline int regmap_field_clear_bits(struct regmap_field *field,
> +					  unsigned int bits)
> +{
> +	return regmap_field_update_bits_base(field, bits, bits, NULL, false,
> +					     false);

The contents of these two functions are swapped when compared to their names.

Regards,
Samuel

> +}
> +
> +int regmap_field_test_bits(struct regmap_field *field, unsigned int bits);
> +
>  static inline int
>  regmap_field_force_update_bits(struct regmap_field *field,
>  			       unsigned int mask, unsigned int val)
> @@ -1757,6 +1773,27 @@ regmap_field_force_update_bits(struct regmap_field *field,
>  	return -EINVAL;
>  }
>  
> +static inline int regmap_field_set_bits(struct regmap_field *field,
> +					unsigned int bits)
> +{
> +	WARN_ONCE(1, "regmap API is disabled");
> +	return -EINVAL;
> +}
> +
> +static inline int regmap_field_clear_bits(struct regmap_field *field,
> +					  unsigned int bits)
> +{
> +	WARN_ONCE(1, "regmap API is disabled");
> +	return -EINVAL;
> +}
> +
> +static inline int regmap_field_test_bits(struct regmap_field *field,
> +					 unsigned int bits)
> +{
> +	WARN_ONCE(1, "regmap API is disabled");
> +	return -EINVAL;
> +}
> +
>  static inline int regmap_fields_write(struct regmap_field *field,
>  				      unsigned int id, unsigned int val)
>  {
>
Re: [PATCH v2 1/4] regmap: provide regmap_field helpers for simple bit operations
Posted by Li Chen 3 years, 11 months ago
Hi Samuel,

 ---- On Sun, 22 May 2022 20:09:55 -0700 Samuel Holland <samuel@sholland.org> wrote ----
 > Hi,
 > 
 > On 5/22/22 9:23 PM, Li Chen wrote:
 > > From: Li Chen <lchen@ambarella.com>
 > > 
 > > We have set/clear/test operations for regmap, but not for regmap_field yet.
 > > So let's introduce regmap_field helpers too.
 > > 
 > > In many instances regmap_field_update_bits() is used for simple bit setting
 > > and clearing. In these cases the last argument is redundant and we can
 > > hide it with a static inline function.
 > > 
 > > This adds three new helpers for simple bit operations: set_bits,
 > > clear_bits and test_bits (the last one defined as a regular function).
 > > 
 > > Signed-off-by: Li Chen <lchen@ambarella.com>
 > > ---
 > >  drivers/base/regmap/regmap.c | 22 +++++++++++++++++++++
 > >  include/linux/regmap.h       | 37 ++++++++++++++++++++++++++++++++++++
 > >  2 files changed, 59 insertions(+)
 > > 
 > > diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
 > > index 5e12f7cb5147..a37d6041b7bd 100644
 > > --- a/drivers/base/regmap/regmap.c
 > > +++ b/drivers/base/regmap/regmap.c
 > > @@ -2208,6 +2208,28 @@ int regmap_field_update_bits_base(struct regmap_field *field,
 > >  }
 > >  EXPORT_SYMBOL_GPL(regmap_field_update_bits_base);
 > >  
 > > +/**
 > > + * regmap_field_test_bits() - Check if all specified bits are set in a
 > > + *                            register field.
 > > + *
 > > + * @field: Register field to operate on
 > > + * @bits: Bits to test
 > > + *
 > > + * Returns -1 if the underlying regmap_field_read() fails, 0 if at least one of the
 > > + * tested bits is not set and 1 if all tested bits are set.
 > > + */
 > > +int regmap_field_test_bits(struct regmap_field *field, unsigned int bits)
 > > +{
 > > +    unsigned int val, ret;
 > > +
 > > +    ret = regmap_field_read(field, &val);
 > > +    if (ret)
 > > +        return ret;
 > > +
 > > +    return (val & bits) == bits;
 > > +}
 > > +EXPORT_SYMBOL_GPL(regmap_field_test_bits);
 > > +
 > >  /**
 > >   * regmap_fields_update_bits_base() - Perform a read/modify/write cycle a
 > >   *                                    register field with port ID
 > > diff --git a/include/linux/regmap.h b/include/linux/regmap.h
 > > index de81a94d7b30..10b410734d9e 100644
 > > --- a/include/linux/regmap.h
 > > +++ b/include/linux/regmap.h
 > > @@ -1324,6 +1324,22 @@ static inline int regmap_field_update_bits(struct regmap_field *field,
 > >                           NULL, false, false);
 > >  }
 > >  
 > > +static inline int regmap_field_set_bits(struct regmap_field *field,
 > > +                    unsigned int bits)
 > > +{
 > > +    return regmap_field_update_bits_base(field, bits, 0, NULL, false,
 > > +                         false);
 > > +}
 > > +
 > > +static inline int regmap_field_clear_bits(struct regmap_field *field,
 > > +                      unsigned int bits)
 > > +{
 > > +    return regmap_field_update_bits_base(field, bits, bits, NULL, false,
 > > +                         false);
 > 
 > The contents of these two functions are swapped when compared to their names.

Thanks for spotting this out!
Fixed in v3.

Regards,
Li
 > 
 > Regards,
 > Samuel
 > 
 > > +}
 > > +
 > > +int regmap_field_test_bits(struct regmap_field *field, unsigned int bits);
 > > +
 > >  static inline int
 > >  regmap_field_force_update_bits(struct regmap_field *field,
 > >                     unsigned int mask, unsigned int val)
 > > @@ -1757,6 +1773,27 @@ regmap_field_force_update_bits(struct regmap_field *field,
 > >      return -EINVAL;
 > >  }
 > >  
 > > +static inline int regmap_field_set_bits(struct regmap_field *field,
 > > +                    unsigned int bits)
 > > +{
 > > +    WARN_ONCE(1, "regmap API is disabled");
 > > +    return -EINVAL;
 > > +}
 > > +
 > > +static inline int regmap_field_clear_bits(struct regmap_field *field,
 > > +                      unsigned int bits)
 > > +{
 > > +    WARN_ONCE(1, "regmap API is disabled");
 > > +    return -EINVAL;
 > > +}
 > > +
 > > +static inline int regmap_field_test_bits(struct regmap_field *field,
 > > +                     unsigned int bits)
 > > +{
 > > +    WARN_ONCE(1, "regmap API is disabled");
 > > +    return -EINVAL;
 > > +}
 > > +
 > >  static inline int regmap_fields_write(struct regmap_field *field,
 > >                        unsigned int id, unsigned int val)
 > >  {
 > > 
 > 
 >
[PATCH v2 2/4] ASoC: sunxi: Use {regmap/regmap_field}_{set/clear}_bits helpers
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

Appropriately change calls to {regmap/regmap_field}_update_bits()
with {regmap/regmap_field}_set_bits()
and {regmap/regmap_field}_clear_bits() for improved readability.

Signed-off-by: Li Chen <lchen@ambarella.com>
---
 sound/soc/sunxi/sun4i-codec.c | 78 ++++++++++++++---------------------
 1 file changed, 30 insertions(+), 48 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 60712f24ade5..53e3f43816cc 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -250,37 +250,33 @@ struct sun4i_codec {
 static void sun4i_codec_start_playback(struct sun4i_codec *scodec)
 {
 	/* Flush TX FIFO */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH),
-			   BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH));
+	regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+			BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH));
 
 	/* Enable DAC DRQ */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN),
-			   BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN));
+	regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+			BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN));
 }
 
 static void sun4i_codec_stop_playback(struct sun4i_codec *scodec)
 {
 	/* Disable DAC DRQ */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN),
-			   0);
+	regmap_clear_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+			  BIT(SUN4I_CODEC_DAC_FIFOC_DAC_DRQ_EN));
 }
 
 static void sun4i_codec_start_capture(struct sun4i_codec *scodec)
 {
 	/* Enable ADC DRQ */
-	regmap_field_update_bits(scodec->reg_adc_fifoc,
-				 BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN),
-				 BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN));
+	regmap_field_set_bits(scodec->reg_adc_fifoc,
+			      BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN));
 }
 
 static void sun4i_codec_stop_capture(struct sun4i_codec *scodec)
 {
 	/* Disable ADC DRQ */
-	regmap_field_update_bits(scodec->reg_adc_fifoc,
-				 BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN), 0);
+	regmap_field_clear_bits(scodec->reg_adc_fifoc,
+				 BIT(SUN4I_CODEC_ADC_FIFOC_ADC_DRQ_EN));
 }
 
 static int sun4i_codec_trigger(struct snd_pcm_substream *substream, int cmd,
@@ -323,8 +319,7 @@ static int sun4i_codec_prepare_capture(struct snd_pcm_substream *substream,
 
 
 	/* Flush RX FIFO */
-	regmap_field_update_bits(scodec->reg_adc_fifoc,
-				 BIT(SUN4I_CODEC_ADC_FIFOC_FIFO_FLUSH),
+	regmap_field_set_bits(scodec->reg_adc_fifoc,
 				 BIT(SUN4I_CODEC_ADC_FIFOC_FIFO_FLUSH));
 
 
@@ -365,8 +360,7 @@ static int sun4i_codec_prepare_playback(struct snd_pcm_substream *substream,
 	u32 val;
 
 	/* Flush the TX FIFO */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH),
+	regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
 			   BIT(SUN4I_CODEC_DAC_FIFOC_FIFO_FLUSH));
 
 	/* Set TX FIFO Empty Trigger Level */
@@ -386,9 +380,8 @@ static int sun4i_codec_prepare_playback(struct snd_pcm_substream *substream,
 			   val);
 
 	/* Send zeros when we have an underrun */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   BIT(SUN4I_CODEC_DAC_FIFOC_SEND_LASAT),
-			   0);
+	regmap_clear_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+			   BIT(SUN4I_CODEC_DAC_FIFOC_SEND_LASAT));
 
 	return 0;
 };
@@ -485,33 +478,27 @@ static int sun4i_codec_hw_params_capture(struct sun4i_codec *scodec,
 
 	/* Set the number of channels we want to use */
 	if (params_channels(params) == 1)
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-					 BIT(SUN4I_CODEC_ADC_FIFOC_MONO_EN),
+		regmap_field_set_bits(scodec->reg_adc_fifoc,
 					 BIT(SUN4I_CODEC_ADC_FIFOC_MONO_EN));
 	else
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-					 BIT(SUN4I_CODEC_ADC_FIFOC_MONO_EN),
-					 0);
+		regmap_field_clear_bits(scodec->reg_adc_fifoc,
+					 BIT(SUN4I_CODEC_ADC_FIFOC_MONO_EN));
 
 	/* Set the number of sample bits to either 16 or 24 bits */
 	if (hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS)->min == 32) {
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_SAMPLE_BITS),
+		regmap_field_set_bits(scodec->reg_adc_fifoc,
 				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_SAMPLE_BITS));
 
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_FIFO_MODE),
-				   0);
+		regmap_field_clear_bits(scodec->reg_adc_fifoc,
+				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_FIFO_MODE));
 
 		scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	} else {
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_SAMPLE_BITS),
-				   0);
+		regmap_field_clear_bits(scodec->reg_adc_fifoc,
+				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_SAMPLE_BITS));
 
 		/* Fill most significant bits with valid data MSB */
-		regmap_field_update_bits(scodec->reg_adc_fifoc,
-				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_FIFO_MODE),
+		regmap_field_set_bits(scodec->reg_adc_fifoc,
 				   BIT(SUN4I_CODEC_ADC_FIFOC_RX_FIFO_MODE));
 
 		scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -543,24 +530,20 @@ static int sun4i_codec_hw_params_playback(struct sun4i_codec *scodec,
 
 	/* Set the number of sample bits to either 16 or 24 bits */
 	if (hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS)->min == 32) {
-		regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_SAMPLE_BITS),
+		regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
 				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_SAMPLE_BITS));
 
 		/* Set TX FIFO mode to padding the LSBs with 0 */
-		regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_FIFO_MODE),
-				   0);
+		regmap_clear_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_FIFO_MODE));
 
 		scodec->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	} else {
-		regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_SAMPLE_BITS),
-				   0);
+		regmap_clear_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
+				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_SAMPLE_BITS));
 
 		/* Set TX FIFO mode to repeat the MSB */
-		regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_FIFO_MODE),
+		regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
 				   BIT(SUN4I_CODEC_DAC_FIFOC_TX_FIFO_MODE));
 
 		scodec->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -624,8 +607,7 @@ static int sun4i_codec_startup(struct snd_pcm_substream *substream,
 	 * Stop issuing DRQ when we have room for less than 16 samples
 	 * in our TX FIFO
 	 */
-	regmap_update_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
-			   3 << SUN4I_CODEC_DAC_FIFOC_DRQ_CLR_CNT,
+	regmap_set_bits(scodec->regmap, SUN4I_CODEC_DAC_FIFOC,
 			   3 << SUN4I_CODEC_DAC_FIFOC_DRQ_CLR_CNT);
 
 	return clk_prepare_enable(scodec->clk_module);
-- 
2.36.1
[PATCH v2 3/4] pinctrl: bcm: Use regmap_field_{set/clear}_bits helpers
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

Appropriately change calls to regmap_field_update_bits()
with regmap_field_clear_bits() for improved readability.

Signed-off-by: Li Chen <lchen@ambarella.com>
---
 drivers/pinctrl/bcm/pinctrl-bcm6358.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/bcm/pinctrl-bcm6358.c b/drivers/pinctrl/bcm/pinctrl-bcm6358.c
index 9f6cd7447887..b03dfcb171d1 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm6358.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm6358.c
@@ -300,7 +300,7 @@ static int bcm6358_gpio_request_enable(struct pinctrl_dev *pctldev,
 		return 0;
 
 	/* disable all functions using this pin */
-	return regmap_field_update_bits(priv->overlays, mask, 0);
+	return regmap_field_clear_bits(priv->overlays, mask);
 }
 
 static const struct pinctrl_ops bcm6358_pctl_ops = {
-- 
2.36.1
[PATCH v2 4/4] pinctrl: st: Switch to use regmap_field_test_bits
Posted by Li Chen 3 years, 11 months ago
From: Li Chen <lchen@ambarella.com>

Appropriately change calls to regmap_field_read() with
regmap_field_test_bits() for improved readability.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Li Chen <lchen@ambarella.com>
---
 drivers/pinctrl/pinctrl-st.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 0fea71fd9a00..198b4a9d263b 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -573,23 +573,18 @@ static void st_pinconf_set_retime_dedicated(struct st_pinctrl *info,
 static void st_pinconf_get_direction(struct st_pio_control *pc,
 	int pin, unsigned long *config)
 {
-	unsigned int oe_value, pu_value, od_value;
-
 	if (pc->oe) {
-		regmap_field_read(pc->oe, &oe_value);
-		if (oe_value & BIT(pin))
+		if (regmap_field_test_bits(pc->oe, BIT(pin)))
 			ST_PINCONF_PACK_OE(*config);
 	}
 
 	if (pc->pu) {
-		regmap_field_read(pc->pu, &pu_value);
-		if (pu_value & BIT(pin))
+		if (regmap_field_test_bits(pc->pu, BIT(pin)))
 			ST_PINCONF_PACK_PU(*config);
 	}
 
 	if (pc->od) {
-		regmap_field_read(pc->od, &od_value);
-		if (od_value & BIT(pin))
+		if (regmap_field_test_bits(pc->od, BIT(pin)))
 			ST_PINCONF_PACK_OD(*config);
 	}
 }
@@ -599,22 +594,22 @@ static int st_pinconf_get_retime_packed(struct st_pinctrl *info,
 {
 	const struct st_pctl_data *data = info->data;
 	struct st_retime_packed *rt_p = &pc->rt.rt_p;
-	unsigned int delay_bits, delay, delay0, delay1, val;
+	unsigned int delay_bits, delay, delay0, delay1;
 	int output = ST_PINCONF_UNPACK_OE(*config);
 
-	if (!regmap_field_read(rt_p->retime, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->retime, BIT(pin)))
 		ST_PINCONF_PACK_RT(*config);
 
-	if (!regmap_field_read(rt_p->clk1notclk0, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->clk1notclk0, BIT(pin)))
 		ST_PINCONF_PACK_RT_CLK(*config, 1);
 
-	if (!regmap_field_read(rt_p->clknotdata, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->clknotdata, BIT(pin)))
 		ST_PINCONF_PACK_RT_CLKNOTDATA(*config);
 
-	if (!regmap_field_read(rt_p->double_edge, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->double_edge, BIT(pin)))
 		ST_PINCONF_PACK_RT_DOUBLE_EDGE(*config);
 
-	if (!regmap_field_read(rt_p->invertclk, &val) && (val & BIT(pin)))
+	if (!regmap_field_test_bits(rt_p->invertclk, BIT(pin)))
 		ST_PINCONF_PACK_RT_INVERTCLK(*config);
 
 	regmap_field_read(rt_p->delay_0, &delay0);
-- 
2.36.1