linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/17] Non-const bitfield helper conversions
@ 2021-11-22 15:53 Geert Uytterhoeven
  2021-11-22 15:53 ` [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers Geert Uytterhoeven
                   ` (17 more replies)
  0 siblings, 18 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:53 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

	Hi all,

<linux/bitfield.h> contains various helpers for accessing bitfields, as
typically used in hardware registers for memory-mapped I/O blocks. These
helpers ensure type safety, and deduce automatically shift values from
mask values, avoiding mistakes due to inconsistent shifts and masks, and
leading to a reduction in source code size.

I have already submitted a few conversions to the FIELD_{GET,PREP}()
helpers that were fixes for real bugs:
  - [PATCH] mips: cm: Convert to bitfield API to fix out-of-bounds
    access
    https://lore.kernel.org/r/0471c545117c5fa05bd9c73005cda9b74608a61e.1635501373.git.geert+renesas@glider.be
  - [PATCH] drm/armada: Fix off-by-one error in
    armada_overlay_get_property()
    https://lore.kernel.org/r/5818c8b04834e6a9525441bc181580a230354b69.1635501237.git.geert+renesas@glider.be

Plus several patches for normal conversions:
  - [PATCH] ARM: ptrace: Use bitfield helpers
    https://lore.kernel.org/r/a1445d3abb45cfc95cb1b03180fd53caf122035b.1637593297.git.geert+renesas@glider.be
  - [PATCH] MIPS: CPC: Use bitfield helpers
    https://lore.kernel.org/r/35f0f17e3d987afaa9cd09cdcb8131d42a53c3e1.1637593297.git.geert+renesas@glider.be
  - [PATCH] MIPS: CPS: Use bitfield helpers
    https://lore.kernel.org/r/8bd8b1b9a3787e594285addcf2057754540d0a5f.1637593297.git.geert+renesas@glider.be
  - [PATCH] crypto: sa2ul - Use bitfield helpers
    https://lore.kernel.org/r/ca89d204ef2e40193479db2742eadf0d9cf3c0ff.1637593297.git.geert+renesas@glider.be
  - [PATCH] dmaengine: stm32-mdma: Use bitfield helpers
    https://lore.kernel.org/r/36ceab242a594233dc7dc6f1dddb4ac32d1e846f.1637593297.git.geert+renesas@glider.be
  - [PATCH] intel_th: Use bitfield helpers
    https://lore.kernel.org/r/b1e4f027aa88acfbdfaa771b0920bd1d977828ba.1637593297.git.geert+renesas@glider.be
  - [PATCH] Input: palmas-pwrbutton - use bitfield helpers
    https://lore.kernel.org/r/f8831b88346b36fc6e01e0910d0db6c94287d2b4.1637593297.git.geert+renesas@glider.be
  - [PATCH] irqchip/mips-gic: Use bitfield helpers
    https://lore.kernel.org/r/74f9d126961a90d3e311b92a54870eaac5b3ae57.1637593297.git.geert+renesas@glider.be
  - [PATCH] mfd: mc13xxx: Use bitfield helpers
    https://lore.kernel.org/r/afa46868cf8c1666e9cbbbec42767ca2294b024d.1637593297.git.geert+renesas@glider.be
  - [PATCH] regulator: lp873x: Use bitfield helpers
    https://lore.kernel.org/r/44d60384b640c8586b4ca7edbc9287a34ce21c5b.1637593297.git.geert+renesas@glider.be
  - [PATCH] regulator: lp87565: Use bitfield helpers
    https://lore.kernel.org/r/941c2dfd5b5b124b8950bcce42db4c343dfe9821.1637593297.git.geert+renesas@glider.be

The existing FIELD_{GET,PREP}() macros are limited to compile-time
constants.  However, it is very common to prepare or extract bitfield
elements where the bitfield mask is not a compile-time constant.
To avoid this limitation, the AT91 clock driver already has its own
field_{prep,get}() macros.

This patch series makes them available for general use, and converts
several drivers to the existing FIELD_{GET,PREP}() and the new
field_{get,prep}() helpers.

I can take the first two patches through the reneas-clk tree for v5.17,
but probably it is best for the remaining patches to be postponed to
v5.18.

Thanks for your comments!

Geert Uytterhoeven (17):
  bitfield: Add non-constant field_{prep,get}() helpers
  clk: renesas: Use bitfield helpers
  [RFC] soc: renesas: Use bitfield helpers
  [RFC] ARM: OMAP2+: Use bitfield helpers
  [RFC] bus: omap_l3_noc: Use bitfield helpers
  [RFC] clk: ti: Use bitfield helpers
  [RFC] iio: st_sensors: Use bitfield helpers
  [RFC] iio: humidity: hts221: Use bitfield helpers
  [RFC] iio: imu: st_lsm6dsx: Use bitfield helpers
  [RFC] media: ti-vpe: cal: Use bitfield helpers
  [RFC] mmc: sdhci-of-aspeed: Use bitfield helpers
  [RFC] pinctrl: aspeed: Use bitfield helpers
  [RFC] pinctl: ti: iodelay: Use bitfield helpers
  [RFC] regulator: ti-abb: Use bitfield helpers
  [RFC] thermal/ti-soc-thermal: Use bitfield helpers
  [RFC] ALSA: ice1724: Use bitfield helpers
  [RFC] rtw89: Use bitfield helpers

 arch/arm/mach-omap2/clkt2xxx_dpllcore.c       |  5 +-
 arch/arm/mach-omap2/cm2xxx.c                  | 11 ++-
 arch/arm/mach-omap2/cm2xxx_3xxx.h             |  9 +--
 arch/arm/mach-omap2/cm33xx.c                  |  9 +--
 arch/arm/mach-omap2/cm3xxx.c                  |  7 +-
 arch/arm/mach-omap2/cminst44xx.c              |  9 +--
 arch/arm/mach-omap2/powerdomains3xxx_data.c   |  3 +-
 arch/arm/mach-omap2/prm.h                     |  2 -
 arch/arm/mach-omap2/prm2xxx.c                 |  4 +-
 arch/arm/mach-omap2/prm2xxx_3xxx.c            |  7 +-
 arch/arm/mach-omap2/prm2xxx_3xxx.h            |  9 +--
 arch/arm/mach-omap2/prm33xx.c                 | 53 +++++-------
 arch/arm/mach-omap2/prm3xxx.c                 |  3 +-
 arch/arm/mach-omap2/prm44xx.c                 | 53 ++++--------
 arch/arm/mach-omap2/vc.c                      | 12 +--
 arch/arm/mach-omap2/vp.c                      | 11 +--
 drivers/bus/omap_l3_noc.c                     |  4 +-
 drivers/clk/at91/clk-peripheral.c             |  1 +
 drivers/clk/at91/pmc.h                        |  3 -
 drivers/clk/renesas/clk-div6.c                |  6 +-
 drivers/clk/renesas/r8a779a0-cpg-mssr.c       |  9 +--
 drivers/clk/renesas/rcar-gen3-cpg.c           | 15 ++--
 drivers/clk/ti/apll.c                         | 25 +++---
 drivers/clk/ti/dpll3xxx.c                     | 81 ++++++++-----------
 .../iio/common/st_sensors/st_sensors_core.c   |  5 +-
 drivers/iio/humidity/hts221_core.c            |  8 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  1 -
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  7 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 45 +++++------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  | 11 +--
 drivers/media/platform/ti-vpe/cal.h           |  4 +-
 drivers/mmc/host/sdhci-of-aspeed.c            |  5 +-
 drivers/net/wireless/realtek/rtw89/core.h     | 38 ++-------
 drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c    |  3 +-
 drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c    |  3 +-
 drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c    |  3 +-
 drivers/pinctrl/aspeed/pinctrl-aspeed.c       |  5 +-
 drivers/pinctrl/aspeed/pinmux-aspeed.c        |  6 +-
 drivers/pinctrl/ti/pinctrl-ti-iodelay.c       | 35 +++-----
 drivers/regulator/ti-abb-regulator.c          |  7 +-
 drivers/soc/renesas/renesas-soc.c             |  4 +-
 drivers/thermal/ti-soc-thermal/ti-bandgap.c   | 11 ++-
 include/linux/bitfield.h                      | 30 +++++++
 sound/pci/ice1712/wm8766.c                    | 14 ++--
 sound/pci/ice1712/wm8776.c                    | 14 ++--
 45 files changed, 263 insertions(+), 347 deletions(-)

-- 
2.25.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
@ 2021-11-22 15:53 ` Geert Uytterhoeven
  2021-11-22 16:32   ` Johannes Berg
  2021-11-22 15:53 ` [PATCH 02/17] clk: renesas: Use bitfield helpers Geert Uytterhoeven
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:53 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

The existing FIELD_{GET,PREP}() macros are limited to compile-time
constants.  However, it is very common to prepare or extract bitfield
elements where the bitfield mask is not a compile-time constant.

To avoid this limitation, the AT91 clock driver already has its own
field_{prep,get}() macros.  Make them available for general use by
moving them to <linux/bitfield.h>, and improve them slightly:
  1. Avoid evaluating macro parameters more than once,
  2. Replace "ffs() - 1" by "__ffs()", as the latter operates on
     "unsigned long", just like BIT() and GENMASK().

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Using __ffs() actually reduces code size (16 bytes for each of
drivers/clk/at91/clk-{generated,peripheral}.o), as __ffs() doesn't
need to verify that the value passed is non-zero.
---
 drivers/clk/at91/clk-peripheral.c |  1 +
 drivers/clk/at91/pmc.h            |  3 ---
 include/linux/bitfield.h          | 30 ++++++++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/at91/clk-peripheral.c b/drivers/clk/at91/clk-peripheral.c
index e14fa5ac734cead7..e2f33498139a1b8c 100644
--- a/drivers/clk/at91/clk-peripheral.c
+++ b/drivers/clk/at91/clk-peripheral.c
@@ -3,6 +3,7 @@
  *  Copyright (C) 2013 Boris BREZILLON <b.brezillon@overkiz.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/clk-provider.h>
 #include <linux/clkdev.h>
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 3a1bf6194c287d09..1256e1ab91526a25 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -114,9 +114,6 @@ struct at91_clk_pms {
 	unsigned int parent;
 };
 
-#define field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffs(_mask) - 1))
-#define field_prep(_mask, _val) (((_val) << (ffs(_mask) - 1)) & (_mask))
-
 #define ndck(a, s) (a[s - 1].id + 1)
 #define nck(a) (a[ARRAY_SIZE(a) - 1].id + 1)
 struct pmc_data *pmc_data_allocate(unsigned int ncore, unsigned int nsystem,
diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 4e035aca6f7e6000..f03b0712e4babec1 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -156,4 +156,34 @@ __MAKE_OP(64)
 #undef __MAKE_OP
 #undef ____MAKE_OP
 
+/**
+ * field_prep() - prepare a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_val:  value to put in the field
+ *
+ * field_prep() masks and shifts up the value.  The result should be
+ * combined with other fields of the bitfield using logical OR.
+ * Unlike FIELD_PREP(), @_mask is not limited to a compile-time constant.
+ */
+#define field_prep(_mask, _val)						\
+	({								\
+		typeof(_mask) ___mask = (_mask);			\
+		(((_val) << __ffs(___mask)) & (___mask));		\
+	})
+
+/**
+ * field_get() - extract a bitfield element
+ * @_mask: shifted mask defining the field's length and position
+ * @_reg:  value of entire bitfield
+ *
+ * field_get() extracts the field specified by @_mask from the
+ * bitfield passed in as @_reg by masking and shifting it down.
+ * Unlike FIELD_GET(), @_mask is not limited to a compile-time constant.
+ */
+#define field_get(_mask, _reg)						\
+	({								\
+		typeof(_mask) ___mask = _mask;				\
+		(((_reg) & (___mask)) >> __ffs(___mask));		\
+	})
+
 #endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH 02/17] clk: renesas: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
  2021-11-22 15:53 ` [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers Geert Uytterhoeven
@ 2021-11-22 15:53 ` Geert Uytterhoeven
  2021-11-22 15:53 ` [PATCH/RFC 03/17] soc: " Geert Uytterhoeven
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:53 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the FIELD_{GET,PREP}() and field_{get,prep}() helpers for const
respective non-const bitfields, instead of open-coding the same
operations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/renesas/clk-div6.c          |  6 +++---
 drivers/clk/renesas/r8a779a0-cpg-mssr.c |  9 +++------
 drivers/clk/renesas/rcar-gen3-cpg.c     | 15 +++++----------
 3 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/clk/renesas/clk-div6.c b/drivers/clk/renesas/clk-div6.c
index 3abd6e5400aded6a..f7b827b5e9b2dd32 100644
--- a/drivers/clk/renesas/clk-div6.c
+++ b/drivers/clk/renesas/clk-div6.c
@@ -7,6 +7,7 @@
  * Contact: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk-provider.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -171,8 +172,7 @@ static u8 cpg_div6_clock_get_parent(struct clk_hw *hw)
 	if (clock->src_mask == 0)
 		return 0;
 
-	hw_index = (readl(clock->reg) & clock->src_mask) >>
-		   __ffs(clock->src_mask);
+	hw_index = field_get(clock->src_mask, readl(clock->reg));
 	for (i = 0; i < clk_hw_get_num_parents(hw); i++) {
 		if (clock->parents[i] == hw_index)
 			return i;
@@ -191,7 +191,7 @@ static int cpg_div6_clock_set_parent(struct clk_hw *hw, u8 index)
 	if (index >= clk_hw_get_num_parents(hw))
 		return -EINVAL;
 
-	src = clock->parents[index] << __ffs(clock->src_mask);
+	src = field_prep(clock->src_mask, clock->parents[index]);
 	writel((readl(clock->reg) & ~clock->src_mask) | src, clock->reg);
 	return 0;
 }
diff --git a/drivers/clk/renesas/r8a779a0-cpg-mssr.c b/drivers/clk/renesas/r8a779a0-cpg-mssr.c
index 7df86743c5491292..f716e739d138b722 100644
--- a/drivers/clk/renesas/r8a779a0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a779a0-cpg-mssr.c
@@ -302,11 +302,7 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
 					   unsigned long parent_rate)
 {
 	struct cpg_z_clk *zclk = to_z_clk(hw);
-	unsigned int mult;
-	u32 val;
-
-	val = readl(zclk->reg) & zclk->mask;
-	mult = 32 - (val >> __ffs(zclk->mask));
+	unsigned int mult = 32 - field_get(zclk->mask, readl(zclk->reg));
 
 	return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult,
 				     32 * zclk->fixed_div);
@@ -357,7 +353,8 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
 		return -EBUSY;
 
-	cpg_reg_modify(zclk->reg, zclk->mask, (32 - mult) << __ffs(zclk->mask));
+	cpg_reg_modify(zclk->reg, zclk->mask,
+		       field_prep(zclk->mask, 32 - mult));
 
 	/*
 	 * Set KICK bit in FRQCRB to update hardware setting and wait for
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index a9816b1beabb2582..30bbe8418e018153 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -54,10 +54,8 @@ static unsigned long cpg_pll_clk_recalc_rate(struct clk_hw *hw,
 {
 	struct cpg_pll_clk *pll_clk = to_pll_clk(hw);
 	unsigned int mult;
-	u32 val;
 
-	val = readl(pll_clk->pllcr_reg) & CPG_PLLnCR_STC_MASK;
-	mult = (val >> __ffs(CPG_PLLnCR_STC_MASK)) + 1;
+	mult = FIELD_GET(CPG_PLLnCR_STC_MASK, readl(pll_clk->pllcr_reg)) + 1;
 
 	return parent_rate * mult * pll_clk->fixed_mult;
 }
@@ -94,7 +92,7 @@ static int cpg_pll_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 
 	val = readl(pll_clk->pllcr_reg);
 	val &= ~CPG_PLLnCR_STC_MASK;
-	val |= (mult - 1) << __ffs(CPG_PLLnCR_STC_MASK);
+	val |= FIELD_PREP(CPG_PLLnCR_STC_MASK, mult - 1);
 	writel(val, pll_clk->pllcr_reg);
 
 	for (i = 1000; i; i--) {
@@ -176,11 +174,7 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
 					   unsigned long parent_rate)
 {
 	struct cpg_z_clk *zclk = to_z_clk(hw);
-	unsigned int mult;
-	u32 val;
-
-	val = readl(zclk->reg) & zclk->mask;
-	mult = 32 - (val >> __ffs(zclk->mask));
+	unsigned int mult = 32 - field_get(zclk->mask, readl(zclk->reg));
 
 	return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult,
 				     32 * zclk->fixed_div);
@@ -231,7 +225,8 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
 		return -EBUSY;
 
-	cpg_reg_modify(zclk->reg, zclk->mask, (32 - mult) << __ffs(zclk->mask));
+	cpg_reg_modify(zclk->reg, zclk->mask,
+		       field_prep(zclk->mask, 32 - mult));
 
 	/*
 	 * Set KICK bit in FRQCRB to update hardware setting and wait for
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 03/17] soc: renesas: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
  2021-11-22 15:53 ` [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers Geert Uytterhoeven
  2021-11-22 15:53 ` [PATCH 02/17] clk: renesas: Use bitfield helpers Geert Uytterhoeven
@ 2021-11-22 15:53 ` Geert Uytterhoeven
  2021-11-22 15:53 ` [PATCH/RFC 04/17] ARM: OMAP2+: " Geert Uytterhoeven
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:53 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_get() helper, instead of open-coding the same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This depends on "[PATCH] soc: renesas: Consolidate product register
handling"
(https://lore.kernel.org/linux-renesas-soc/057721f46c7499de4133135488f0f3da7fb39265.1636570669.git.geert+renesas@glider.be)

Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/soc/renesas/renesas-soc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c
index 97957d5d7dafbe2a..33940258f37eef03 100644
--- a/drivers/soc/renesas/renesas-soc.c
+++ b/drivers/soc/renesas/renesas-soc.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2014-2016 Glider bvba
  */
 
+#include <linux/bitfield.h>
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
@@ -434,8 +435,7 @@ static int __init renesas_soc_init(void)
 			eslo = product & 0xf;
 		}
 
-		if (soc->id &&
-		    ((product & id->mask) >> __ffs(id->mask)) != soc->id) {
+		if (soc->id && field_get(id->mask, product) != soc->id) {
 			pr_warn("SoC mismatch (product = 0x%x)\n", product);
 			return -ENODEV;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 04/17] ARM: OMAP2+: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2021-11-22 15:53 ` [PATCH/RFC 03/17] soc: " Geert Uytterhoeven
@ 2021-11-22 15:53 ` Geert Uytterhoeven
  2021-11-22 15:53 ` [PATCH/RFC 05/17] bus: omap_l3_noc: " Geert Uytterhoeven
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:53 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the FIELD_{GET,PREP}() and field_{get,prep}() helpers for const
respective non-const bitfields, instead of open-coding the same
operations.

Remove the definitions of OMAP_POWERSTATEST_SHIFT and
OMAP_POWERSTATE_SHIFT, as they are no longer used.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 arch/arm/mach-omap2/clkt2xxx_dpllcore.c     |  5 +-
 arch/arm/mach-omap2/cm2xxx.c                | 11 ++---
 arch/arm/mach-omap2/cm2xxx_3xxx.h           |  9 +---
 arch/arm/mach-omap2/cm33xx.c                |  9 +---
 arch/arm/mach-omap2/cm3xxx.c                |  7 ++-
 arch/arm/mach-omap2/cminst44xx.c            |  9 +---
 arch/arm/mach-omap2/powerdomains3xxx_data.c |  3 +-
 arch/arm/mach-omap2/prm.h                   |  2 -
 arch/arm/mach-omap2/prm2xxx.c               |  4 +-
 arch/arm/mach-omap2/prm2xxx_3xxx.c          |  7 +--
 arch/arm/mach-omap2/prm2xxx_3xxx.h          |  9 +---
 arch/arm/mach-omap2/prm33xx.c               | 53 +++++++--------------
 arch/arm/mach-omap2/prm3xxx.c               |  3 +-
 arch/arm/mach-omap2/prm44xx.c               | 53 +++++++--------------
 arch/arm/mach-omap2/vc.c                    | 12 ++---
 arch/arm/mach-omap2/vp.c                    | 11 +++--
 16 files changed, 77 insertions(+), 130 deletions(-)

diff --git a/arch/arm/mach-omap2/clkt2xxx_dpllcore.c b/arch/arm/mach-omap2/clkt2xxx_dpllcore.c
index 8a9983cb4733dfb1..6a61dc932b24f387 100644
--- a/arch/arm/mach-omap2/clkt2xxx_dpllcore.c
+++ b/arch/arm/mach-omap2/clkt2xxx_dpllcore.c
@@ -17,6 +17,7 @@
  */
 #undef DEBUG
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/clk.h>
@@ -151,8 +152,8 @@ int omap2_reprogram_dpllcore(struct clk_hw *hw, unsigned long rate,
 			mult = (rate / 1000000);
 			done_rate = CORE_CLK_SRC_DPLL;
 		}
-		tmpset.cm_clksel1_pll |= (div << __ffs(dd->mult_mask));
-		tmpset.cm_clksel1_pll |= (mult << __ffs(dd->div1_mask));
+		tmpset.cm_clksel1_pll |= field_prep(dd->mult_mask, div);
+		tmpset.cm_clksel1_pll |= field_prep(dd->div1_mask, mult);
 
 		/* Worst case */
 		tmpset.base_sdrc_rfr = SDRC_RFR_CTRL_BYPASS;
diff --git a/arch/arm/mach-omap2/cm2xxx.c b/arch/arm/mach-omap2/cm2xxx.c
index 0827acb605843778..59d35d4d43bec533 100644
--- a/arch/arm/mach-omap2/cm2xxx.c
+++ b/arch/arm/mach-omap2/cm2xxx.c
@@ -8,6 +8,7 @@
  * Rajendra Nayak <rnayak@ti.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/delay.h>
@@ -46,7 +47,7 @@ static void _write_clktrctrl(u8 c, s16 module, u32 mask)
 
 	v = omap2_cm_read_mod_reg(module, OMAP2_CM_CLKSTCTRL);
 	v &= ~mask;
-	v |= c << __ffs(mask);
+	v |= field_prep(mask, c);
 	omap2_cm_write_mod_reg(v, module, OMAP2_CM_CLKSTCTRL);
 }
 
@@ -54,9 +55,7 @@ static bool omap2xxx_cm_is_clkdm_in_hwsup(s16 module, u32 mask)
 {
 	u32 v;
 
-	v = omap2_cm_read_mod_reg(module, OMAP2_CM_CLKSTCTRL);
-	v &= mask;
-	v >>= __ffs(mask);
+	v = field_get(mask, omap2_cm_read_mod_reg(module, OMAP2_CM_CLKSTCTRL));
 
 	return (v == OMAP24XX_CLKSTCTRL_ENABLE_AUTO) ? 1 : 0;
 }
@@ -81,7 +80,7 @@ static void _omap2xxx_set_dpll_autoidle(u8 m)
 
 	v = omap2_cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
 	v &= ~OMAP24XX_AUTO_DPLL_MASK;
-	v |= m << OMAP24XX_AUTO_DPLL_SHIFT;
+	v |= FIELD_PREP(OMAP24XX_AUTO_DPLL_MASK, m);
 	omap2_cm_write_mod_reg(v, PLL_MOD, CM_AUTOIDLE);
 }
 
@@ -105,7 +104,7 @@ static void _omap2xxx_set_apll_autoidle(u8 m, u32 mask)
 
 	v = omap2_cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
 	v &= ~mask;
-	v |= m << __ffs(mask);
+	v |= field_prep(mask, m);
 	omap2_cm_write_mod_reg(v, PLL_MOD, CM_AUTOIDLE);
 }
 
diff --git a/arch/arm/mach-omap2/cm2xxx_3xxx.h b/arch/arm/mach-omap2/cm2xxx_3xxx.h
index 70944b94cc098d7f..e3214491f1b556fd 100644
--- a/arch/arm/mach-omap2/cm2xxx_3xxx.h
+++ b/arch/arm/mach-omap2/cm2xxx_3xxx.h
@@ -45,6 +45,7 @@
 
 #ifndef __ASSEMBLER__
 
+#include <linux/bitfield.h>
 #include <linux/io.h>
 
 static inline u32 omap2_cm_read_mod_reg(s16 module, u16 idx)
@@ -74,13 +75,7 @@ static inline u32 omap2_cm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module,
 /* Read a CM register, AND it, and shift the result down to bit 0 */
 static inline u32 omap2_cm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask)
 {
-	u32 v;
-
-	v = omap2_cm_read_mod_reg(domain, idx);
-	v &= mask;
-	v >>= __ffs(mask);
-
-	return v;
+	return field_get(mask, omap2_cm_read_mod_reg(domain, idx));
 }
 
 static inline u32 omap2_cm_set_mod_reg_bits(u32 bits, s16 module, s16 idx)
diff --git a/arch/arm/mach-omap2/cm33xx.c b/arch/arm/mach-omap2/cm33xx.c
index ac4882ebdca33fdf..5479b9587d688885 100644
--- a/arch/arm/mach-omap2/cm33xx.c
+++ b/arch/arm/mach-omap2/cm33xx.c
@@ -16,6 +16,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/errno.h>
@@ -74,13 +75,7 @@ static inline u32 am33xx_cm_rmw_reg_bits(u32 mask, u32 bits, s16 inst, s16 idx)
 
 static inline u32 am33xx_cm_read_reg_bits(u16 inst, s16 idx, u32 mask)
 {
-	u32 v;
-
-	v = am33xx_cm_read_reg(inst, idx);
-	v &= mask;
-	v >>= __ffs(mask);
-
-	return v;
+	return field_get(mask, am33xx_cm_read_reg(inst, idx));
 }
 
 /**
diff --git a/arch/arm/mach-omap2/cm3xxx.c b/arch/arm/mach-omap2/cm3xxx.c
index b03b6123b8fcc8f2..951c6a9cee1e80ba 100644
--- a/arch/arm/mach-omap2/cm3xxx.c
+++ b/arch/arm/mach-omap2/cm3xxx.c
@@ -8,6 +8,7 @@
  * Rajendra Nayak <rnayak@ti.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/delay.h>
@@ -35,7 +36,7 @@ static void _write_clktrctrl(u8 c, s16 module, u32 mask)
 
 	v = omap2_cm_read_mod_reg(module, OMAP2_CM_CLKSTCTRL);
 	v &= ~mask;
-	v |= c << __ffs(mask);
+	v |= field_prep(mask, c);
 	omap2_cm_write_mod_reg(v, module, OMAP2_CM_CLKSTCTRL);
 }
 
@@ -43,9 +44,7 @@ static bool omap3xxx_cm_is_clkdm_in_hwsup(s16 module, u32 mask)
 {
 	u32 v;
 
-	v = omap2_cm_read_mod_reg(module, OMAP2_CM_CLKSTCTRL);
-	v &= mask;
-	v >>= __ffs(mask);
+	v = field_get(mask, omap2_cm_read_mod_reg(module, OMAP2_CM_CLKSTCTRL));
 
 	return (v == OMAP34XX_CLKSTCTRL_ENABLE_AUTO) ? 1 : 0;
 }
diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
index 46670521b27883ec..e9ca128b14349be2 100644
--- a/arch/arm/mach-omap2/cminst44xx.c
+++ b/arch/arm/mach-omap2/cminst44xx.c
@@ -12,6 +12,7 @@
  * the PRM hardware module.  What a mess...
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/errno.h>
@@ -154,13 +155,7 @@ static u32 omap4_cminst_clear_inst_reg_bits(u32 bits, u8 part, u16 inst,
 
 static u32 omap4_cminst_read_inst_reg_bits(u8 part, u16 inst, s16 idx, u32 mask)
 {
-	u32 v;
-
-	v = omap4_cminst_read_inst_reg(part, inst, idx);
-	v &= mask;
-	v >>= __ffs(mask);
-
-	return v;
+	return field_get(mask, omap4_cminst_read_inst_reg(part, inst, idx));
 }
 
 /*
diff --git a/arch/arm/mach-omap2/powerdomains3xxx_data.c b/arch/arm/mach-omap2/powerdomains3xxx_data.c
index 3564fade67e45061..08ae42ddb15b138e 100644
--- a/arch/arm/mach-omap2/powerdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/powerdomains3xxx_data.c
@@ -8,6 +8,7 @@
  * Paul Walmsley, Jouni Högander
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/bug.h>
@@ -513,7 +514,7 @@ static struct powerdomain *powerdomains_ti816x[] __initdata = {
 static int ti81xx_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
-				   (pwrst << OMAP_POWERSTATE_SHIFT),
+				   FIELD_PREP(OMAP_POWERSTATE_MASK, pwrst),
 				   pwrdm->prcm_offs, TI81XX_PM_PWSTCTRL);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/prm.h b/arch/arm/mach-omap2/prm.h
index 08df78810a5e39e9..c54991e7058e7cd7 100644
--- a/arch/arm/mach-omap2/prm.h
+++ b/arch/arm/mach-omap2/prm.h
@@ -67,7 +67,6 @@ int omap2_prcm_base_init(void);
  *	 PM_PWSTST_DSS, PM_PWSTST_CAM, PM_PWSTST_PER, PM_PWSTST_EMU,
  *	 PM_PWSTST_NEON
  */
-#define OMAP_POWERSTATEST_SHIFT				0
 #define OMAP_POWERSTATEST_MASK				(0x3 << 0)
 
 /*
@@ -80,7 +79,6 @@ int omap2_prcm_base_init(void);
  *	 PM_PWSTCTRL_GFX, PM_PWSTCTRL_DSS, PM_PWSTCTRL_CAM, PM_PWSTCTRL_PER,
  *	 PM_PWSTCTRL_NEON shared bits
  */
-#define OMAP_POWERSTATE_SHIFT				0
 #define OMAP_POWERSTATE_MASK				(0x3 << 0)
 
 /*
diff --git a/arch/arm/mach-omap2/prm2xxx.c b/arch/arm/mach-omap2/prm2xxx.c
index 35a58f54b528f335..e835e381a82b7685 100644
--- a/arch/arm/mach-omap2/prm2xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx.c
@@ -9,6 +9,7 @@
  * Rajendra Nayak <rnayak@ti.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/err.h>
@@ -165,7 +166,8 @@ static int omap2xxx_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 	}
 
 	omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
-				   (omap24xx_pwrst << OMAP_POWERSTATE_SHIFT),
+				   FIELD_PREP(OMAP_POWERSTATE_MASK,
+					      omap24xx_pwrst),
 				   pwrdm->prcm_offs, OMAP2_PM_PWSTCTRL);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
index d983efac6f4ff6f5..8a223ced2bcad25b 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
@@ -8,6 +8,7 @@
  * Paul Walmsley
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/err.h>
@@ -115,7 +116,7 @@ int omap2_pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank,
 
 	m = omap2_pwrdm_get_mem_bank_onstate_mask(bank);
 
-	omap2_prm_rmw_mod_reg_bits(m, (pwrst << __ffs(m)), pwrdm->prcm_offs,
+	omap2_prm_rmw_mod_reg_bits(m, field_prep(m, pwrst), pwrdm->prcm_offs,
 				   OMAP2_PM_PWSTCTRL);
 
 	return 0;
@@ -128,7 +129,7 @@ int omap2_pwrdm_set_mem_retst(struct powerdomain *pwrdm, u8 bank,
 
 	m = omap2_pwrdm_get_mem_bank_retst_mask(bank);
 
-	omap2_prm_rmw_mod_reg_bits(m, (pwrst << __ffs(m)), pwrdm->prcm_offs,
+	omap2_prm_rmw_mod_reg_bits(m, field_prep(m, pwrst), pwrdm->prcm_offs,
 				   OMAP2_PM_PWSTCTRL);
 
 	return 0;
@@ -158,7 +159,7 @@ int omap2_pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	u32 v;
 
-	v = pwrst << __ffs(OMAP_LOGICRETSTATE_MASK);
+	v = FIELD_PREP(OMAP_LOGICRETSTATE_MASK, pwrst);
 	omap2_prm_rmw_mod_reg_bits(OMAP_LOGICRETSTATE_MASK, v, pwrdm->prcm_offs,
 				   OMAP2_PM_PWSTCTRL);
 
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.h b/arch/arm/mach-omap2/prm2xxx_3xxx.h
index 3d803f7182b98f04..5f438cca9f8a3584 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.h
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.h
@@ -46,6 +46,7 @@
 
 #ifndef __ASSEMBLER__
 
+#include <linux/bitfield.h>
 #include <linux/io.h>
 #include "powerdomain.h"
 
@@ -77,13 +78,7 @@ static inline u32 omap2_prm_rmw_mod_reg_bits(u32 mask, u32 bits, s16 module,
 /* Read a PRM register, AND it, and shift the result down to bit 0 */
 static inline u32 omap2_prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask)
 {
-	u32 v;
-
-	v = omap2_prm_read_mod_reg(domain, idx);
-	v &= mask;
-	v >>= __ffs(mask);
-
-	return v;
+	return field_get(mask, omap2_prm_read_mod_reg(domain, idx));
 }
 
 static inline u32 omap2_prm_set_mod_reg_bits(u32 bits, s16 module, s16 idx)
diff --git a/arch/arm/mach-omap2/prm33xx.c b/arch/arm/mach-omap2/prm33xx.c
index 9144cc0479afe19c..864420e9c2102df5 100644
--- a/arch/arm/mach-omap2/prm33xx.c
+++ b/arch/arm/mach-omap2/prm33xx.c
@@ -13,6 +13,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/errno.h>
@@ -149,37 +150,29 @@ static int am33xx_prm_deassert_hardreset(u8 shift, u8 st_shift, u8 part,
 static int am33xx_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	am33xx_prm_rmw_reg_bits(OMAP_POWERSTATE_MASK,
-				(pwrst << OMAP_POWERSTATE_SHIFT),
+				FIELD_PREP(OMAP_POWERSTATE_MASK, pwrst),
 				pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
 	return 0;
 }
 
 static int am33xx_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 {
-	u32 v;
-
-	v = am33xx_prm_read_reg(pwrdm->prcm_offs,  pwrdm->pwrstctrl_offs);
-	v &= OMAP_POWERSTATE_MASK;
-	v >>= OMAP_POWERSTATE_SHIFT;
+	u32 v = am33xx_prm_read_reg(pwrdm->prcm_offs,  pwrdm->pwrstctrl_offs);
 
-	return v;
+	return FIELD_GET(OMAP_POWERSTATE_MASK, v);
 }
 
 static int am33xx_pwrdm_read_pwrst(struct powerdomain *pwrdm)
 {
-	u32 v;
+	u32 v = am33xx_prm_read_reg(pwrdm->prcm_offs, pwrdm->pwrstst_offs);
 
-	v = am33xx_prm_read_reg(pwrdm->prcm_offs, pwrdm->pwrstst_offs);
-	v &= OMAP_POWERSTATEST_MASK;
-	v >>= OMAP_POWERSTATEST_SHIFT;
-
-	return v;
+	return FIELD_GET(OMAP_POWERSTATEST_MASK, v);
 }
 
 static int am33xx_pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
 {
 	am33xx_prm_rmw_reg_bits(AM33XX_LOWPOWERSTATECHANGE_MASK,
-				(1 << AM33XX_LOWPOWERSTATECHANGE_SHIFT),
+				FIELD_PREP(AM33XX_LOWPOWERSTATECHANGE_MASK, 1),
 				pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
 	return 0;
 }
@@ -200,21 +193,17 @@ static int am33xx_pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 	if (!m)
 		return -EINVAL;
 
-	am33xx_prm_rmw_reg_bits(m, (pwrst << __ffs(m)),
-				pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
+	am33xx_prm_rmw_reg_bits(m, field_prep(m, pwrst), pwrdm->prcm_offs,
+				pwrdm->pwrstctrl_offs);
 
 	return 0;
 }
 
 static int am33xx_pwrdm_read_logic_pwrst(struct powerdomain *pwrdm)
 {
-	u32 v;
+	u32 v = am33xx_prm_read_reg(pwrdm->prcm_offs, pwrdm->pwrstst_offs);
 
-	v = am33xx_prm_read_reg(pwrdm->prcm_offs, pwrdm->pwrstst_offs);
-	v &= AM33XX_LOGICSTATEST_MASK;
-	v >>= AM33XX_LOGICSTATEST_SHIFT;
-
-	return v;
+	return FIELD_GET(AM33XX_LOGICSTATEST_MASK, v);
 }
 
 static int am33xx_pwrdm_read_logic_retst(struct powerdomain *pwrdm)
@@ -226,10 +215,8 @@ static int am33xx_pwrdm_read_logic_retst(struct powerdomain *pwrdm)
 		return -EINVAL;
 
 	v = am33xx_prm_read_reg(pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
-	v &= m;
-	v >>= __ffs(m);
 
-	return v;
+	return field_get(m, v);
 }
 
 static int am33xx_pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank,
@@ -241,8 +228,8 @@ static int am33xx_pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank,
 	if (!m)
 		return -EINVAL;
 
-	am33xx_prm_rmw_reg_bits(m, (pwrst << __ffs(m)),
-				pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
+	am33xx_prm_rmw_reg_bits(m, field_prep(m, pwrst), pwrdm->prcm_offs,
+				pwrdm->pwrstctrl_offs);
 
 	return 0;
 }
@@ -256,8 +243,8 @@ static int am33xx_pwrdm_set_mem_retst(struct powerdomain *pwrdm, u8 bank,
 	if (!m)
 		return -EINVAL;
 
-	am33xx_prm_rmw_reg_bits(m, (pwrst << __ffs(m)),
-				pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
+	am33xx_prm_rmw_reg_bits(m, field_prep(m, pwrst), pwrdm->prcm_offs,
+				pwrdm->pwrstctrl_offs);
 
 	return 0;
 }
@@ -271,10 +258,8 @@ static int am33xx_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
 		return -EINVAL;
 
 	v = am33xx_prm_read_reg(pwrdm->prcm_offs, pwrdm->pwrstst_offs);
-	v &= m;
-	v >>= __ffs(m);
 
-	return v;
+	return field_get(m, v);
 }
 
 static int am33xx_pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank)
@@ -286,10 +271,8 @@ static int am33xx_pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank)
 		return -EINVAL;
 
 	v = am33xx_prm_read_reg(pwrdm->prcm_offs, pwrdm->pwrstctrl_offs);
-	v &= m;
-	v >>= __ffs(m);
 
-	return v;
+	return field_get(m, v);
 }
 
 static int am33xx_pwrdm_wait_transition(struct powerdomain *pwrdm)
diff --git a/arch/arm/mach-omap2/prm3xxx.c b/arch/arm/mach-omap2/prm3xxx.c
index 1b442b1285693cd9..88c8963ff602589b 100644
--- a/arch/arm/mach-omap2/prm3xxx.c
+++ b/arch/arm/mach-omap2/prm3xxx.c
@@ -9,6 +9,7 @@
  * Rajendra Nayak <rnayak@ti.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/err.h>
@@ -536,7 +537,7 @@ void omap3_prm_save_scratchpad_contents(u32 *ptr)
 static int omap3_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	omap2_prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
-				   (pwrst << OMAP_POWERSTATE_SHIFT),
+				   FIELD_PREP(OMAP_POWERSTATE_MASK, pwrst),
 				   pwrdm->prcm_offs, OMAP2_PM_PWSTCTRL);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/prm44xx.c b/arch/arm/mach-omap2/prm44xx.c
index 25093c1e5b9ac927..629b0980d301e0a6 100644
--- a/arch/arm/mach-omap2/prm44xx.c
+++ b/arch/arm/mach-omap2/prm44xx.c
@@ -9,6 +9,7 @@
  * Rajendra Nayak <rnayak@ti.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/cpu_pm.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
@@ -316,10 +317,8 @@ static void omap44xx_prm_reconfigure_io_chain(void)
 				    inst,
 				    omap4_prcm_irq_setup.pm_ctrl);
 	omap_test_timeout(
-		(((omap4_prm_read_inst_reg(inst,
-					   omap4_prcm_irq_setup.pm_ctrl) &
-		   OMAP4430_WUCLK_STATUS_MASK) >>
-		  OMAP4430_WUCLK_STATUS_SHIFT) == 1),
+		(FIELD_GET(OMAP4430_WUCLK_STATUS_MASK,
+			   omap4_prm_read_inst_reg(inst, omap4_prcm_irq_setup.pm_ctrl)) == 1),
 		MAX_IOPAD_LATCH_TIME, i);
 	if (i == MAX_IOPAD_LATCH_TIME)
 		pr_warn("PRM: I/O chain clock line assertion timed out\n");
@@ -329,10 +328,8 @@ static void omap44xx_prm_reconfigure_io_chain(void)
 				    inst,
 				    omap4_prcm_irq_setup.pm_ctrl);
 	omap_test_timeout(
-		(((omap4_prm_read_inst_reg(inst,
-					   omap4_prcm_irq_setup.pm_ctrl) &
-		   OMAP4430_WUCLK_STATUS_MASK) >>
-		  OMAP4430_WUCLK_STATUS_SHIFT) == 0),
+		(FIELD_GET(OMAP4430_WUCLK_STATUS_MASK,
+			   omap4_prm_read_inst_reg(inst, omap4_prcm_irq_setup.pm_ctrl)) == 0),
 		MAX_IOPAD_LATCH_TIME, i);
 	if (i == MAX_IOPAD_LATCH_TIME)
 		pr_warn("PRM: I/O chain clock line deassertion timed out\n");
@@ -427,7 +424,7 @@ static void omap44xx_prm_clear_context_loss_flags_old(u8 part, s16 inst,
 static int omap4_pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	omap4_prminst_rmw_inst_reg_bits(OMAP_POWERSTATE_MASK,
-					(pwrst << OMAP_POWERSTATE_SHIFT),
+					FIELD_PREP(OMAP_POWERSTATE_MASK, pwrst),
 					pwrdm->prcm_partition,
 					pwrdm->prcm_offs, OMAP4_PM_PWSTCTRL);
 	return 0;
@@ -439,10 +436,8 @@ static int omap4_pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 
 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTCTRL);
-	v &= OMAP_POWERSTATE_MASK;
-	v >>= OMAP_POWERSTATE_SHIFT;
 
-	return v;
+	return FIELD_GET(OMAP_POWERSTATE_MASK, v);
 }
 
 static int omap4_pwrdm_read_pwrst(struct powerdomain *pwrdm)
@@ -451,10 +446,8 @@ static int omap4_pwrdm_read_pwrst(struct powerdomain *pwrdm)
 
 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTST);
-	v &= OMAP_POWERSTATEST_MASK;
-	v >>= OMAP_POWERSTATEST_SHIFT;
 
-	return v;
+	return FIELD_GET(OMAP_POWERSTATEST_MASK, v);
 }
 
 static int omap4_pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
@@ -463,16 +456,14 @@ static int omap4_pwrdm_read_prev_pwrst(struct powerdomain *pwrdm)
 
 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTST);
-	v &= OMAP4430_LASTPOWERSTATEENTERED_MASK;
-	v >>= OMAP4430_LASTPOWERSTATEENTERED_SHIFT;
 
-	return v;
+	return FIELD_GET(OMAP4430_LASTPOWERSTATEENTERED_MASK, v);
 }
 
 static int omap4_pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm)
 {
 	omap4_prminst_rmw_inst_reg_bits(OMAP4430_LOWPOWERSTATECHANGE_MASK,
-					(1 << OMAP4430_LOWPOWERSTATECHANGE_SHIFT),
+					FIELD_PREP(OMAP4430_LOWPOWERSTATECHANGE_MASK, 1),
 					pwrdm->prcm_partition,
 					pwrdm->prcm_offs, OMAP4_PM_PWSTCTRL);
 	return 0;
@@ -491,7 +482,7 @@ static int omap4_pwrdm_set_logic_retst(struct powerdomain *pwrdm, u8 pwrst)
 {
 	u32 v;
 
-	v = pwrst << __ffs(OMAP4430_LOGICRETSTATE_MASK);
+	v = FIELD_PREP(OMAP4430_LOGICRETSTATE_MASK, pwrst);
 	omap4_prminst_rmw_inst_reg_bits(OMAP4430_LOGICRETSTATE_MASK, v,
 					pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTCTRL);
@@ -506,7 +497,7 @@ static int omap4_pwrdm_set_mem_onst(struct powerdomain *pwrdm, u8 bank,
 
 	m = omap2_pwrdm_get_mem_bank_onstate_mask(bank);
 
-	omap4_prminst_rmw_inst_reg_bits(m, (pwrst << __ffs(m)),
+	omap4_prminst_rmw_inst_reg_bits(m, field_prep(m, pwrst),
 					pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTCTRL);
 
@@ -520,7 +511,7 @@ static int omap4_pwrdm_set_mem_retst(struct powerdomain *pwrdm, u8 bank,
 
 	m = omap2_pwrdm_get_mem_bank_retst_mask(bank);
 
-	omap4_prminst_rmw_inst_reg_bits(m, (pwrst << __ffs(m)),
+	omap4_prminst_rmw_inst_reg_bits(m, field_prep(m, pwrst),
 					pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTCTRL);
 
@@ -533,10 +524,8 @@ static int omap4_pwrdm_read_logic_pwrst(struct powerdomain *pwrdm)
 
 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTST);
-	v &= OMAP4430_LOGICSTATEST_MASK;
-	v >>= OMAP4430_LOGICSTATEST_SHIFT;
 
-	return v;
+	return FIELD_GET(OMAP4430_LOGICSTATEST_MASK, v);
 }
 
 static int omap4_pwrdm_read_logic_retst(struct powerdomain *pwrdm)
@@ -545,10 +534,8 @@ static int omap4_pwrdm_read_logic_retst(struct powerdomain *pwrdm)
 
 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTCTRL);
-	v &= OMAP4430_LOGICRETSTATE_MASK;
-	v >>= OMAP4430_LOGICRETSTATE_SHIFT;
 
-	return v;
+	return FIELD_GET(OMAP4430_LOGICRETSTATE_MASK, v);
 }
 
 /**
@@ -587,10 +574,7 @@ static int omap4_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
 
 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTST);
-	v &= m;
-	v >>= __ffs(m);
-
-	return v;
+	return field_get(m, v);
 }
 
 static int omap4_pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank)
@@ -601,10 +585,7 @@ static int omap4_pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank)
 
 	v = omap4_prminst_read_inst_reg(pwrdm->prcm_partition, pwrdm->prcm_offs,
 					OMAP4_PM_PWSTCTRL);
-	v &= m;
-	v >>= __ffs(m);
-
-	return v;
+	return field_get(m, v);
 }
 
 /**
diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index 86f1ac4c24125ae2..1a836627eee2eaec 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -7,6 +7,7 @@
  * License version 2. This program is licensed "as is" without any
  * warranty of any kind, whether express or implied.
  */
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
 #include <linux/init.h>
@@ -379,9 +380,8 @@ static void omap3_init_voltsetup1(struct voltagedomain *voltdm,
 
 	val = (voltdm->vc_param->on - idle) / voltdm->pmic->slew_rate;
 	val *= voltdm->sys_clk.rate / 8 / 1000000 + 1;
-	val <<= __ffs(voltdm->vfsm->voltsetup_mask);
 	c->voltsetup1 &= ~voltdm->vfsm->voltsetup_mask;
-	c->voltsetup1 |= val;
+	c->voltsetup1 |= field_prep(voltdm->vfsm->voltsetup_mask, val);
 }
 
 /**
@@ -772,7 +772,7 @@ static void __init omap_vc_i2c_init(struct voltagedomain *voltdm)
 	mcode = voltdm->pmic->i2c_mcode;
 	if (mcode)
 		voltdm->rmw(vc->common->i2c_mcode_mask,
-			    mcode << __ffs(vc->common->i2c_mcode_mask),
+			    field_prep(vc->common->i2c_mcode_mask, mcode),
 			    vc->common->i2c_cfg_reg);
 
 	if (cpu_is_omap44xx())
@@ -850,7 +850,7 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 
 	/* Configure the i2c slave address for this VC */
 	voltdm->rmw(vc->smps_sa_mask,
-		    vc->i2c_slave_addr << __ffs(vc->smps_sa_mask),
+		    field_prep(vc->smps_sa_mask, vc->i2c_slave_addr),
 		    vc->smps_sa_reg);
 	vc->cfg_channel |= vc_cfg_bits->sa;
 
@@ -858,13 +858,13 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	 * Configure the PMIC register addresses.
 	 */
 	voltdm->rmw(vc->smps_volra_mask,
-		    vc->volt_reg_addr << __ffs(vc->smps_volra_mask),
+		    field_prep(vc->smps_volra_mask, vc->volt_reg_addr),
 		    vc->smps_volra_reg);
 	vc->cfg_channel |= vc_cfg_bits->rav;
 
 	if (vc->cmd_reg_addr) {
 		voltdm->rmw(vc->smps_cmdra_mask,
-			    vc->cmd_reg_addr << __ffs(vc->smps_cmdra_mask),
+			    field_prep(vc->smps_cmdra_mask, vc->cmd_reg_addr),
 			    vc->smps_cmdra_reg);
 		vc->cfg_channel |= vc_cfg_bits->rac;
 	}
diff --git a/arch/arm/mach-omap2/vp.c b/arch/arm/mach-omap2/vp.c
index a709655b978cbcc9..3071426a5ec116d7 100644
--- a/arch/arm/mach-omap2/vp.c
+++ b/arch/arm/mach-omap2/vp.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 
@@ -22,7 +23,7 @@ static u32 _vp_set_init_voltage(struct voltagedomain *voltdm, u32 volt)
 	vpconfig &= ~(vp->common->vpconfig_initvoltage_mask |
 		      vp->common->vpconfig_forceupdate |
 		      vp->common->vpconfig_initvdd);
-	vpconfig |= vsel << __ffs(vp->common->vpconfig_initvoltage_mask);
+	vpconfig |= field_prep(vp->common->vpconfig_initvoltage_mask, vsel);
 	voltdm->write(vpconfig, vp->vpconfig);
 
 	/* Trigger initVDD value copy to voltage processor */
@@ -73,8 +74,8 @@ void __init omap_vp_init(struct voltagedomain *voltdm)
 	 * VP_CONFIG: error gain is not set here, it will be updated
 	 * on each scale, based on OPP.
 	 */
-	val = (voltdm->pmic->vp_erroroffset <<
-	       __ffs(voltdm->vp->common->vpconfig_erroroffset_mask)) |
+	val = field_prep(voltdm->vp->common->vpconfig_erroroffset_mask,
+			 voltdm->pmic->vp_erroroffset) |
 		vp->common->vpconfig_timeouten;
 	voltdm->write(val, vp->vpconfig);
 
@@ -110,8 +111,8 @@ int omap_vp_update_errorgain(struct voltagedomain *voltdm,
 
 	/* Setting vp errorgain based on the voltage */
 	voltdm->rmw(voltdm->vp->common->vpconfig_errorgain_mask,
-		    volt_data->vp_errgain <<
-		    __ffs(voltdm->vp->common->vpconfig_errorgain_mask),
+		    field_prep(voltdm->vp->common->vpconfig_errorgain_mask,
+			       volt_data->vp_errgain),
 		    voltdm->vp->vpconfig);
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 05/17] bus: omap_l3_noc: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (3 preceding siblings ...)
  2021-11-22 15:53 ` [PATCH/RFC 04/17] ARM: OMAP2+: " Geert Uytterhoeven
@ 2021-11-22 15:53 ` Geert Uytterhoeven
  2021-11-22 15:53 ` [PATCH/RFC 06/17] clk: ti: " Geert Uytterhoeven
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:53 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_get() bitfield helper, instead of open-coding the same
operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/bus/omap_l3_noc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/omap_l3_noc.c b/drivers/bus/omap_l3_noc.c
index dcfb32ee5cb60239..1a692c86d085f43e 100644
--- a/drivers/bus/omap_l3_noc.c
+++ b/drivers/bus/omap_l3_noc.c
@@ -14,6 +14,7 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  */
+#include <linux/bitfield.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -118,8 +119,7 @@ static int l3_handle_target(struct omap_l3 *l3, void __iomem *base,
 	}
 
 	/* STDERRLOG_MSTADDR Stores the NTTP master address. */
-	masterid = (readl_relaxed(l3_targ_mstaddr) &
-		    l3->mst_addr_mask) >> __ffs(l3->mst_addr_mask);
+	masterid = field_get(l3->mst_addr_mask, readl_relaxed(l3_targ_mstaddr));
 
 	for (k = 0, master = l3->l3_masters; k < l3->num_masters;
 	     k++, master++) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 06/17] clk: ti: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (4 preceding siblings ...)
  2021-11-22 15:53 ` [PATCH/RFC 05/17] bus: omap_l3_noc: " Geert Uytterhoeven
@ 2021-11-22 15:53 ` Geert Uytterhoeven
  2021-11-22 15:54 ` [PATCH/RFC 07/17] iio: st_sensors: " Geert Uytterhoeven
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:53 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_{get,prep}() helpers, instead of open-coding the same
operations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/clk/ti/apll.c     | 25 +++++-------
 drivers/clk/ti/dpll3xxx.c | 81 +++++++++++++++++----------------------
 2 files changed, 44 insertions(+), 62 deletions(-)

diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index ac5bc8857a51456e..145a42ff050f076b 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -15,6 +15,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/module.h>
@@ -62,7 +63,7 @@ static int dra7_apll_enable(struct clk_hw *hw)
 
 	v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
 	v &= ~ad->enable_mask;
-	v |= APLL_FORCE_LOCK << __ffs(ad->enable_mask);
+	v |= field_prep(ad->enable_mask, APLL_FORCE_LOCK);
 	ti_clk_ll_ops->clk_writel(v, &ad->control_reg);
 
 	state <<= __ffs(ad->idlest_mask);
@@ -101,7 +102,7 @@ static void dra7_apll_disable(struct clk_hw *hw)
 
 	v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
 	v &= ~ad->enable_mask;
-	v |= APLL_AUTO_IDLE << __ffs(ad->enable_mask);
+	v |= field_prep(ad->enable_mask, APLL_AUTO_IDLE);
 	ti_clk_ll_ops->clk_writel(v, &ad->control_reg);
 }
 
@@ -114,11 +115,8 @@ static int dra7_apll_is_enabled(struct clk_hw *hw)
 	ad = clk->dpll_data;
 
 	v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
-	v &= ad->enable_mask;
 
-	v >>= __ffs(ad->enable_mask);
-
-	return v == APLL_AUTO_IDLE ? 0 : 1;
+	return field_get(ad->enable_mask, v) == APLL_AUTO_IDLE ? 0 : 1;
 }
 
 static u8 dra7_init_apll_parent(struct clk_hw *hw)
@@ -242,14 +240,9 @@ static int omap2_apll_is_enabled(struct clk_hw *hw)
 {
 	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
 	struct dpll_data *ad = clk->dpll_data;
-	u32 v;
-
-	v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
-	v &= ad->enable_mask;
-
-	v >>= __ffs(ad->enable_mask);
+	u32 v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
 
-	return v == OMAP2_EN_APLL_LOCKED ? 1 : 0;
+	return field_get(ad->enable_mask, v) == OMAP2_EN_APLL_LOCKED ? 1 : 0;
 }
 
 static unsigned long omap2_apll_recalc(struct clk_hw *hw,
@@ -272,7 +265,7 @@ static int omap2_apll_enable(struct clk_hw *hw)
 
 	v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
 	v &= ~ad->enable_mask;
-	v |= OMAP2_EN_APLL_LOCKED << __ffs(ad->enable_mask);
+	v |= field_prep(ad->enable_mask, OMAP2_EN_APLL_LOCKED);
 	ti_clk_ll_ops->clk_writel(v, &ad->control_reg);
 
 	while (1) {
@@ -302,7 +295,7 @@ static void omap2_apll_disable(struct clk_hw *hw)
 
 	v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
 	v &= ~ad->enable_mask;
-	v |= OMAP2_EN_APLL_STOPPED << __ffs(ad->enable_mask);
+	v |= field_prep(ad->enable_mask, OMAP2_EN_APLL_STOPPED);
 	ti_clk_ll_ops->clk_writel(v, &ad->control_reg);
 }
 
@@ -320,7 +313,7 @@ static void omap2_apll_set_autoidle(struct clk_hw_omap *clk, u32 val)
 
 	v = ti_clk_ll_ops->clk_readl(&ad->autoidle_reg);
 	v &= ~ad->autoidle_mask;
-	v |= val << __ffs(ad->autoidle_mask);
+	v |= field_prep(ad->autoidle_mask, val);
 	ti_clk_ll_ops->clk_writel(v, &ad->control_reg);
 }
 
diff --git a/drivers/clk/ti/dpll3xxx.c b/drivers/clk/ti/dpll3xxx.c
index e32b3515f9e76b67..21c94d758eed92b7 100644
--- a/drivers/clk/ti/dpll3xxx.c
+++ b/drivers/clk/ti/dpll3xxx.c
@@ -15,6 +15,7 @@
  * Richard Woodruff, Tony Lindgren, Tuukka Tikkanen, Karthik Dasu
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/device.h>
 #include <linux/list.h>
@@ -53,7 +54,7 @@ static void _omap3_dpll_write_clken(struct clk_hw_omap *clk, u8 clken_bits)
 
 	v = ti_clk_ll_ops->clk_readl(&dd->control_reg);
 	v &= ~dd->enable_mask;
-	v |= clken_bits << __ffs(dd->enable_mask);
+	v |= field_prep(dd->enable_mask, clken_bits);
 	ti_clk_ll_ops->clk_writel(v, &dd->control_reg);
 }
 
@@ -333,8 +334,8 @@ static void omap3_noncore_dpll_ssc_program(struct clk_hw_omap *clk)
 
 		v = ti_clk_ll_ops->clk_readl(&dd->ssc_modfreq_reg);
 		v &= ~(dd->ssc_modfreq_mant_mask | dd->ssc_modfreq_exp_mask);
-		v |= mantissa << __ffs(dd->ssc_modfreq_mant_mask);
-		v |= exponent << __ffs(dd->ssc_modfreq_exp_mask);
+		v |= field_prep(dd->ssc_modfreq_mant_mask, mantissa);
+		v |= field_prep(dd->ssc_modfreq_exp_mask, exponent);
 		ti_clk_ll_ops->clk_writel(v, &dd->ssc_modfreq_reg);
 
 		deltam_step = dd->last_rounded_m * dd->ssc_deltam;
@@ -348,8 +349,7 @@ static void omap3_noncore_dpll_ssc_program(struct clk_hw_omap *clk)
 		if (deltam_step > 0xFFFFF)
 			deltam_step = 0xFFFFF;
 
-		deltam_ceil = (deltam_step & dd->ssc_deltam_int_mask) >>
-		    __ffs(dd->ssc_deltam_int_mask);
+		deltam_ceil = field_get(dd->ssc_deltam_int_mask, deltam_step);
 		if (deltam_step & dd->ssc_deltam_frac_mask)
 			deltam_ceil++;
 
@@ -363,8 +363,8 @@ static void omap3_noncore_dpll_ssc_program(struct clk_hw_omap *clk)
 
 		v = ti_clk_ll_ops->clk_readl(&dd->ssc_deltam_reg);
 		v &= ~(dd->ssc_deltam_int_mask | dd->ssc_deltam_frac_mask);
-		v |= deltam_step << __ffs(dd->ssc_deltam_int_mask |
-					  dd->ssc_deltam_frac_mask);
+		v |= field_prep(dd->ssc_deltam_int_mask | dd->ssc_deltam_frac_mask,
+				deltam_step);
 		ti_clk_ll_ops->clk_writel(v, &dd->ssc_deltam_reg);
 	} else {
 		ctrl &= ~dd->ssc_enable_mask;
@@ -398,7 +398,7 @@ static int omap3_noncore_dpll_program(struct clk_hw_omap *clk, u16 freqsel)
 	if (ti_clk_get_features()->flags & TI_CLK_DPLL_HAS_FREQSEL) {
 		v = ti_clk_ll_ops->clk_readl(&dd->control_reg);
 		v &= ~dd->freqsel_mask;
-		v |= freqsel << __ffs(dd->freqsel_mask);
+		v |= field_prep(dd->freqsel_mask, freqsel);
 		ti_clk_ll_ops->clk_writel(v, &dd->control_reg);
 	}
 
@@ -414,20 +414,20 @@ static int omap3_noncore_dpll_program(struct clk_hw_omap *clk, u16 freqsel)
 	}
 
 	v &= ~(dd->mult_mask | dd->div1_mask);
-	v |= dd->last_rounded_m << __ffs(dd->mult_mask);
-	v |= (dd->last_rounded_n - 1) << __ffs(dd->div1_mask);
+	v |= field_prep(dd->mult_mask, dd->last_rounded_m);
+	v |= field_prep(dd->div1_mask, dd->last_rounded_n - 1);
 
 	/* Configure dco and sd_div for dplls that have these fields */
 	if (dd->dco_mask) {
 		_lookup_dco(clk, &dco, dd->last_rounded_m, dd->last_rounded_n);
 		v &= ~(dd->dco_mask);
-		v |= dco << __ffs(dd->dco_mask);
+		v |= field_prep(dd->dco_mask, dco);
 	}
 	if (dd->sddiv_mask) {
 		_lookup_sddiv(clk, &sd_div, dd->last_rounded_m,
 			      dd->last_rounded_n);
 		v &= ~(dd->sddiv_mask);
-		v |= sd_div << __ffs(dd->sddiv_mask);
+		v |= field_prep(dd->sddiv_mask, sd_div);
 	}
 
 	/*
@@ -728,7 +728,6 @@ int omap3_noncore_dpll_set_rate_and_parent(struct clk_hw *hw,
 static u32 omap3_dpll_autoidle_read(struct clk_hw_omap *clk)
 {
 	const struct dpll_data *dd;
-	u32 v;
 
 	if (!clk || !clk->dpll_data)
 		return -EINVAL;
@@ -738,11 +737,8 @@ static u32 omap3_dpll_autoidle_read(struct clk_hw_omap *clk)
 	if (!dd->autoidle_mask)
 		return -EINVAL;
 
-	v = ti_clk_ll_ops->clk_readl(&dd->autoidle_reg);
-	v &= dd->autoidle_mask;
-	v >>= __ffs(dd->autoidle_mask);
-
-	return v;
+	return field_get(dd->autoidle_mask,
+			 ti_clk_ll_ops->clk_readl(&dd->autoidle_reg));
 }
 
 /**
@@ -774,7 +770,7 @@ static void omap3_dpll_allow_idle(struct clk_hw_omap *clk)
 	 */
 	v = ti_clk_ll_ops->clk_readl(&dd->autoidle_reg);
 	v &= ~dd->autoidle_mask;
-	v |= DPLL_AUTOIDLE_LOW_POWER_STOP << __ffs(dd->autoidle_mask);
+	v |= field_prep(dd->autoidle_mask, DPLL_AUTOIDLE_LOW_POWER_STOP);
 	ti_clk_ll_ops->clk_writel(v, &dd->autoidle_reg);
 }
 
@@ -799,7 +795,7 @@ static void omap3_dpll_deny_idle(struct clk_hw_omap *clk)
 
 	v = ti_clk_ll_ops->clk_readl(&dd->autoidle_reg);
 	v &= ~dd->autoidle_mask;
-	v |= DPLL_AUTOIDLE_DISABLE << __ffs(dd->autoidle_mask);
+	v |= field_prep(dd->autoidle_mask, DPLL_AUTOIDLE_DISABLE);
 	ti_clk_ll_ops->clk_writel(v, &dd->autoidle_reg);
 }
 
@@ -857,8 +853,8 @@ unsigned long omap3_clkoutx2_recalc(struct clk_hw *hw,
 
 	WARN_ON(!dd->enable_mask);
 
-	v = ti_clk_ll_ops->clk_readl(&dd->control_reg) & dd->enable_mask;
-	v >>= __ffs(dd->enable_mask);
+	v = field_get(dd->enable_mask,
+		      ti_clk_ll_ops->clk_readl(&dd->control_reg));
 	if ((v != OMAP3XXX_EN_DPLL_LOCKED) || (dd->flags & DPLL_J_TYPE))
 		rate = parent_rate;
 	else
@@ -877,19 +873,17 @@ int omap3_core_dpll_save_context(struct clk_hw *hw)
 {
 	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
 	struct dpll_data *dd;
-	u32 v;
 
 	dd = clk->dpll_data;
 
-	v = ti_clk_ll_ops->clk_readl(&dd->control_reg);
-	clk->context = (v & dd->enable_mask) >> __ffs(dd->enable_mask);
+	clk->context = field_get(dd->enable_mask,
+				 ti_clk_ll_ops->clk_readl(&dd->control_reg));
 
 	if (clk->context == DPLL_LOCKED) {
-		v = ti_clk_ll_ops->clk_readl(&dd->mult_div1_reg);
-		dd->last_rounded_m = (v & dd->mult_mask) >>
-						__ffs(dd->mult_mask);
-		dd->last_rounded_n = ((v & dd->div1_mask) >>
-						__ffs(dd->div1_mask)) + 1;
+		u32 v = ti_clk_ll_ops->clk_readl(&dd->mult_div1_reg);
+
+		dd->last_rounded_m = field_get(dd->mult_mask, v);
+		dd->last_rounded_n = field_get(dd->div1_mask, v) + 1;
 	}
 
 	return 0;
@@ -916,8 +910,8 @@ void omap3_core_dpll_restore_context(struct clk_hw *hw)
 
 		v = ti_clk_ll_ops->clk_readl(&dd->mult_div1_reg);
 		v &= ~(dd->mult_mask | dd->div1_mask);
-		v |= dd->last_rounded_m << __ffs(dd->mult_mask);
-		v |= (dd->last_rounded_n - 1) << __ffs(dd->div1_mask);
+		v |= field_prep(dd->mult_mask, dd->last_rounded_m);
+		v |= field_prep(dd->div1_mask, dd->last_rounded_n - 1);
 		ti_clk_ll_ops->clk_writel(v, &dd->mult_div1_reg);
 
 		_omap3_dpll_write_clken(clk, DPLL_LOCKED);
@@ -938,19 +932,17 @@ int omap3_noncore_dpll_save_context(struct clk_hw *hw)
 {
 	struct clk_hw_omap *clk = to_clk_hw_omap(hw);
 	struct dpll_data *dd;
-	u32 v;
 
 	dd = clk->dpll_data;
 
-	v = ti_clk_ll_ops->clk_readl(&dd->control_reg);
-	clk->context = (v & dd->enable_mask) >> __ffs(dd->enable_mask);
+	clk->context = field_get(dd->enable_mask,
+				 ti_clk_ll_ops->clk_readl(&dd->control_reg));
 
 	if (clk->context == DPLL_LOCKED) {
-		v = ti_clk_ll_ops->clk_readl(&dd->mult_div1_reg);
-		dd->last_rounded_m = (v & dd->mult_mask) >>
-						__ffs(dd->mult_mask);
-		dd->last_rounded_n = ((v & dd->div1_mask) >>
-						__ffs(dd->div1_mask)) + 1;
+		u32 v = ti_clk_ll_ops->clk_readl(&dd->mult_div1_reg);
+
+		dd->last_rounded_m = field_get(dd->mult_mask, v);
+		dd->last_rounded_n = field_get(dd->div1_mask, v) + 1;
 	}
 
 	return 0;
@@ -974,12 +966,9 @@ void omap3_noncore_dpll_restore_context(struct clk_hw *hw)
 	ctrl = ti_clk_ll_ops->clk_readl(&dd->control_reg);
 	mult_div1 = ti_clk_ll_ops->clk_readl(&dd->mult_div1_reg);
 
-	if (clk->context == ((ctrl & dd->enable_mask) >>
-			     __ffs(dd->enable_mask)) &&
-	    dd->last_rounded_m == ((mult_div1 & dd->mult_mask) >>
-				   __ffs(dd->mult_mask)) &&
-	    dd->last_rounded_n == ((mult_div1 & dd->div1_mask) >>
-				   __ffs(dd->div1_mask)) + 1) {
+	if (clk->context == field_get(dd->enable_mask, ctrl) &&
+	    dd->last_rounded_m == field_get(dd->mult_mask, mult_div1) &&
+	    dd->last_rounded_n == field_get(dd->div1_mask, mult_div1) + 1) {
 		/* nothing to be done */
 		return;
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 07/17] iio: st_sensors: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (5 preceding siblings ...)
  2021-11-22 15:53 ` [PATCH/RFC 06/17] clk: ti: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-23 23:35   ` Linus Walleij
  2021-11-22 15:54 ` [PATCH/RFC 08/17] iio: humidity: hts221: " Geert Uytterhoeven
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_prep() helper, instead of open-coding the same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/iio/common/st_sensors/st_sensors_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 1de395bda03eb6d3..b11c3f474d299b96 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -7,6 +7,7 @@
  * Denis Ciocca <denis.ciocca@st.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -26,8 +27,8 @@ int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
 {
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
 
-	return regmap_update_bits(sdata->regmap,
-				  reg_addr, mask, data << __ffs(mask));
+	return regmap_update_bits(sdata->regmap, reg_addr, mask,
+				  field_prep(mask, data));
 }
 
 int st_sensors_debugfs_reg_access(struct iio_dev *indio_dev,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 08/17] iio: humidity: hts221: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (6 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 07/17] iio: st_sensors: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-24 15:21   ` Jonathan Cameron
  2021-11-22 15:54 ` [PATCH/RFC 09/17] iio: imu: st_lsm6dsx: " Geert Uytterhoeven
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_prep() helper, instead of open-coding the same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/iio/humidity/hts221_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index 6a39615b696114cd..749aedc469ede5c1 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -7,6 +7,7 @@
  * Lorenzo Bianconi <lorenzo.bianconi@st.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/device.h>
@@ -171,7 +172,7 @@ static int hts221_update_avg(struct hts221_hw *hw,
 			     u16 val)
 {
 	const struct hts221_avg *avg = &hts221_avg_list[type];
-	int i, err, data;
+	int i, err;
 
 	for (i = 0; i < HTS221_AVG_DEPTH; i++)
 		if (avg->avg_avl[i] == val)
@@ -180,9 +181,8 @@ static int hts221_update_avg(struct hts221_hw *hw,
 	if (i == HTS221_AVG_DEPTH)
 		return -EINVAL;
 
-	data = ((i << __ffs(avg->mask)) & avg->mask);
-	err = regmap_update_bits(hw->regmap, avg->addr,
-				 avg->mask, data);
+	err = regmap_update_bits(hw->regmap, avg->addr, avg->mask,
+				 field_prep(avg->mask, i));
 	if (err < 0)
 		return err;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 09/17] iio: imu: st_lsm6dsx: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (7 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 08/17] iio: humidity: hts221: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-22 15:54 ` [PATCH/RFC 10/17] media: ti-vpe: cal: " Geert Uytterhoeven
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_prep() helper, instead of defining a custom macro, or
open-coding the same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  1 -
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  7 +--
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 45 +++++++++----------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  | 11 ++---
 4 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 6ac4eac36458aa23..5282bdb0942e8d71 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -62,7 +62,6 @@ enum st_lsm6dsx_hw_id {
 					 ST_LSM6DSX_SAMPLE_SIZE)
 #define ST_LSM6DSX_MAX_TAGGED_WORD_LEN	((32 / ST_LSM6DSX_TAGGED_SAMPLE_SIZE) \
 					 * ST_LSM6DSX_TAGGED_SAMPLE_SIZE)
-#define ST_LSM6DSX_SHIFT_VAL(val, mask)	(((val) << __ffs(mask)) & (mask))
 
 #define ST_LSM6DSX_CHANNEL_ACC(chan_type, addr, mod, scan_idx)		\
 {									\
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 16730a7809643695..80c763b837bfde01 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -29,6 +29,7 @@
  * Lorenzo Bianconi <lorenzo.bianconi@st.com>
  * Denis Ciocca <denis.ciocca@st.com>
  */
+#include <linux/bitfield.h>
 #include <linux/module.h>
 #include <linux/iio/kfifo_buf.h>
 #include <linux/iio/iio.h>
@@ -155,7 +156,7 @@ static int st_lsm6dsx_update_decimators(struct st_lsm6dsx_hw *hw)
 
 		dec_reg = &hw->settings->decimator[sensor->id];
 		if (dec_reg->addr) {
-			int val = ST_LSM6DSX_SHIFT_VAL(data, dec_reg->mask);
+			int val = field_prep(dec_reg->mask, data);
 
 			err = st_lsm6dsx_update_bits_locked(hw, dec_reg->addr,
 							    dec_reg->mask,
@@ -177,7 +178,7 @@ static int st_lsm6dsx_update_decimators(struct st_lsm6dsx_hw *hw)
 	if (ts_dec_reg->addr) {
 		int val, ts_dec = !!hw->ts_sip;
 
-		val = ST_LSM6DSX_SHIFT_VAL(ts_dec, ts_dec_reg->mask);
+		val = field_prep(ts_dec_reg->mask, ts_dec);
 		err = st_lsm6dsx_update_bits_locked(hw, ts_dec_reg->addr,
 						    ts_dec_reg->mask, val);
 	}
@@ -215,7 +216,7 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
 		} else {
 			data = 0;
 		}
-		val = ST_LSM6DSX_SHIFT_VAL(data, batch_reg->mask);
+		val = field_prep(batch_reg->mask, data);
 		return st_lsm6dsx_update_bits_locked(hw, batch_reg->addr,
 						     batch_reg->mask, val);
 	} else {
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index f2cbbc756459b1cb..f7f6783f8842ed98 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -46,6 +46,7 @@
  * Denis Ciocca <denis.ciocca@st.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/delay.h>
@@ -1159,7 +1160,7 @@ int st_lsm6dsx_set_page(struct st_lsm6dsx_hw *hw, bool enable)
 	int err;
 
 	hub_settings = &hw->settings->shub_settings;
-	data = ST_LSM6DSX_SHIFT_VAL(enable, hub_settings->page_mux.mask);
+	data = field_prep(hub_settings->page_mux.mask, enable);
 	err = regmap_update_bits(hw->regmap, hub_settings->page_mux.addr,
 				 hub_settings->page_mux.mask, data);
 	usleep_range(100, 150);
@@ -1220,8 +1221,7 @@ static int st_lsm6dsx_set_full_scale(struct st_lsm6dsx_sensor *sensor,
 	if (i == fs_table->fs_len)
 		return -EINVAL;
 
-	data = ST_LSM6DSX_SHIFT_VAL(fs_table->fs_avl[i].val,
-				    fs_table->reg.mask);
+	data = field_prep(fs_table->reg.mask, fs_table->fs_avl[i].val);
 	err = st_lsm6dsx_update_bits_locked(sensor->hw, fs_table->reg.addr,
 					    fs_table->reg.mask, data);
 	if (err < 0)
@@ -1319,7 +1319,7 @@ st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u32 req_odr)
 	}
 
 	reg = &hw->settings->odr_table[ref_sensor->id].reg;
-	data = ST_LSM6DSX_SHIFT_VAL(val, reg->mask);
+	data = field_prep(reg->mask, val);
 	return st_lsm6dsx_update_bits_locked(hw, reg->addr, reg->mask, data);
 }
 
@@ -1473,7 +1473,7 @@ static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, int state)
 
 	reg = &hw->settings->event_settings.enable_reg;
 	if (reg->addr) {
-		data = ST_LSM6DSX_SHIFT_VAL(state, reg->mask);
+		data = field_prep(reg->mask, state);
 		err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
 						    reg->mask, data);
 		if (err < 0)
@@ -1481,7 +1481,7 @@ static int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, int state)
 	}
 
 	/* Enable wakeup interrupt */
-	data = ST_LSM6DSX_SHIFT_VAL(state, hw->irq_routing->mask);
+	data = field_prep(hw->irq_routing->mask, state);
 	return st_lsm6dsx_update_bits_locked(hw, hw->irq_routing->addr,
 					     hw->irq_routing->mask, data);
 }
@@ -1526,7 +1526,7 @@ st_lsm6dsx_write_event(struct iio_dev *iio_dev,
 		return -EINVAL;
 
 	reg = &hw->settings->event_settings.wakeup_reg;
-	data = ST_LSM6DSX_SHIFT_VAL(val, reg->mask);
+	data = field_prep(reg->mask, val);
 	err = st_lsm6dsx_update_bits_locked(hw, reg->addr,
 					    reg->mask, data);
 	if (err < 0)
@@ -1786,7 +1786,7 @@ static int st_lsm6dsx_init_shub(struct st_lsm6dsx_hw *hw)
 				return err;
 		}
 
-		data = ST_LSM6DSX_SHIFT_VAL(1, hub_settings->pullup_en.mask);
+		data = field_prep(hub_settings->pullup_en.mask, 1);
 		err = regmap_update_bits(hw->regmap,
 					 hub_settings->pullup_en.addr,
 					 hub_settings->pullup_en.mask, data);
@@ -1804,7 +1804,7 @@ static int st_lsm6dsx_init_shub(struct st_lsm6dsx_hw *hw)
 		if (err < 0)
 			return err;
 
-		data = ST_LSM6DSX_SHIFT_VAL(3, hub_settings->aux_sens.mask);
+		data = field_prep(hub_settings->aux_sens.mask, 3);
 		err = regmap_update_bits(hw->regmap,
 					 hub_settings->aux_sens.addr,
 					 hub_settings->aux_sens.mask, data);
@@ -1816,7 +1816,7 @@ static int st_lsm6dsx_init_shub(struct st_lsm6dsx_hw *hw)
 	}
 
 	if (hub_settings->emb_func.addr) {
-		data = ST_LSM6DSX_SHIFT_VAL(1, hub_settings->emb_func.mask);
+		data = field_prep(hub_settings->emb_func.mask, 1);
 		err = regmap_update_bits(hw->regmap,
 					 hub_settings->emb_func.addr,
 					 hub_settings->emb_func.mask, data);
@@ -1833,7 +1833,7 @@ static int st_lsm6dsx_init_hw_timer(struct st_lsm6dsx_hw *hw)
 	ts_settings = &hw->settings->ts_settings;
 	/* enable hw timestamp generation if necessary */
 	if (ts_settings->timer_en.addr) {
-		val = ST_LSM6DSX_SHIFT_VAL(1, ts_settings->timer_en.mask);
+		val = field_prep(ts_settings->timer_en.mask, 1);
 		err = regmap_update_bits(hw->regmap,
 					 ts_settings->timer_en.addr,
 					 ts_settings->timer_en.mask, val);
@@ -1843,7 +1843,7 @@ static int st_lsm6dsx_init_hw_timer(struct st_lsm6dsx_hw *hw)
 
 	/* enable high resolution for hw ts timer if necessary */
 	if (ts_settings->hr_timer.addr) {
-		val = ST_LSM6DSX_SHIFT_VAL(1, ts_settings->hr_timer.mask);
+		val = field_prep(ts_settings->hr_timer.mask, 1);
 		err = regmap_update_bits(hw->regmap,
 					 ts_settings->hr_timer.addr,
 					 ts_settings->hr_timer.mask, val);
@@ -1853,7 +1853,7 @@ static int st_lsm6dsx_init_hw_timer(struct st_lsm6dsx_hw *hw)
 
 	/* enable ts queueing in FIFO if necessary */
 	if (ts_settings->fifo_en.addr) {
-		val = ST_LSM6DSX_SHIFT_VAL(1, ts_settings->fifo_en.mask);
+		val = field_prep(ts_settings->fifo_en.mask, 1);
 		err = regmap_update_bits(hw->regmap,
 					 ts_settings->fifo_en.addr,
 					 ts_settings->fifo_en.mask, val);
@@ -1899,7 +1899,7 @@ static int st_lsm6dsx_reset_device(struct st_lsm6dsx_hw *hw)
 	/* device sw reset */
 	reg = &hw->settings->reset;
 	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
-				 ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
+				 field_prep(reg->mask, 1));
 	if (err < 0)
 		return err;
 
@@ -1908,7 +1908,7 @@ static int st_lsm6dsx_reset_device(struct st_lsm6dsx_hw *hw)
 	/* reload trimming parameter */
 	reg = &hw->settings->boot;
 	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
-				 ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
+				 field_prep(reg->mask, 1));
 	if (err < 0)
 		return err;
 
@@ -1929,7 +1929,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
 	/* enable Block Data Update */
 	reg = &hw->settings->bdu;
 	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
-				 ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
+				 field_prep(reg->mask, 1));
 	if (err < 0)
 		return err;
 
@@ -1939,7 +1939,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
 		return err;
 
 	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
-				 ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
+				 field_prep(reg->mask, 1));
 	if (err < 0)
 		return err;
 
@@ -1947,7 +1947,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
 	if (hw->settings->irq_config.lir.addr) {
 		reg = &hw->settings->irq_config.lir;
 		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
-					 ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
+					 field_prep(reg->mask, 1));
 		if (err < 0)
 			return err;
 
@@ -1956,7 +1956,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
 			reg = &hw->settings->irq_config.clear_on_read;
 			err = regmap_update_bits(hw->regmap,
 					reg->addr, reg->mask,
-					ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
+					field_prep(reg->mask, 1));
 			if (err < 0)
 				return err;
 		}
@@ -1966,7 +1966,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
 	if (hw->settings->drdy_mask.addr) {
 		reg = &hw->settings->drdy_mask;
 		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
-					 ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
+					 field_prep(reg->mask, 1));
 		if (err < 0)
 			return err;
 	}
@@ -2131,8 +2131,7 @@ static int st_lsm6dsx_irq_setup(struct st_lsm6dsx_hw *hw)
 
 	reg = &hw->settings->irq_config.hla;
 	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
-				 ST_LSM6DSX_SHIFT_VAL(irq_active_low,
-						      reg->mask));
+				 field_prep(reg->mask, irq_active_low));
 	if (err < 0)
 		return err;
 
@@ -2141,7 +2140,7 @@ static int st_lsm6dsx_irq_setup(struct st_lsm6dsx_hw *hw)
 	    (pdata && pdata->open_drain)) {
 		reg = &hw->settings->irq_config.od;
 		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
-					 ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
+					 field_prep(reg->mask, 1));
 		if (err < 0)
 			return err;
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
index 99562ba85ee43098..ea0e568c042b4b43 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
@@ -22,6 +22,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  *
  */
+#include <linux/bitfield.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/iio/iio.h>
@@ -263,7 +264,7 @@ static int st_lsm6dsx_shub_master_enable(struct st_lsm6dsx_sensor *sensor,
 			goto out;
 	}
 
-	data = ST_LSM6DSX_SHIFT_VAL(enable, hub_settings->master_en.mask);
+	data = field_prep(hub_settings->master_en.mask, enable);
 	err = regmap_update_bits(hw->regmap, hub_settings->master_en.addr,
 				 hub_settings->master_en.mask, data);
 
@@ -296,7 +297,7 @@ st_lsm6dsx_shub_read(struct st_lsm6dsx_sensor *sensor, u8 addr,
 	aux_sens = &hw->settings->shub_settings.aux_sens;
 	/* do not overwrite aux_sens */
 	if (slv_addr + 2 == aux_sens->addr)
-		slv_config = ST_LSM6DSX_SHIFT_VAL(3, aux_sens->mask);
+		slv_config = field_prep(aux_sens->mask, 3);
 
 	config[0] = (sensor->ext_info.addr << 1) | 1;
 	config[1] = addr;
@@ -346,7 +347,7 @@ st_lsm6dsx_shub_write(struct st_lsm6dsx_sensor *sensor, u8 addr,
 	if (hub_settings->wr_once.addr) {
 		unsigned int data;
 
-		data = ST_LSM6DSX_SHIFT_VAL(1, hub_settings->wr_once.mask);
+		data = field_prep(hub_settings->wr_once.mask, 1);
 		err = st_lsm6dsx_shub_write_reg_with_mask(hw,
 			hub_settings->wr_once.addr,
 			hub_settings->wr_once.mask,
@@ -395,7 +396,7 @@ st_lsm6dsx_shub_write_with_mask(struct st_lsm6dsx_sensor *sensor,
 	if (err < 0)
 		return err;
 
-	data = ((data & ~mask) | (val << __ffs(mask) & mask));
+	data = (data & ~mask) | field_prep(mask, val);
 
 	return st_lsm6dsx_shub_write(sensor, addr, &data, sizeof(data));
 }
@@ -835,7 +836,7 @@ st_lsm6dsx_shub_check_wai(struct st_lsm6dsx_hw *hw, u8 *i2c_addr,
 	slv_addr = ST_LSM6DSX_SLV_ADDR(0, hub_settings->slv0_addr);
 	/* do not overwrite aux_sens */
 	if (slv_addr + 2 == aux_sens->addr)
-		slv_config = ST_LSM6DSX_SHIFT_VAL(3, aux_sens->mask);
+		slv_config = field_prep(aux_sens->mask, 3);
 
 	for (i = 0; i < ARRAY_SIZE(settings->i2c_addr); i++) {
 		if (!settings->i2c_addr[i])
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 10/17] media: ti-vpe: cal: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (8 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 09/17] iio: imu: st_lsm6dsx: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-22 15:54 ` [PATCH/RFC 11/17] mmc: sdhci-of-aspeed: " Geert Uytterhoeven
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_prep() helper, instead of open-coding the same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/media/platform/ti-vpe/cal.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
index 527e22d022f300b7..5fcf1b55ff2879ac 100644
--- a/drivers/media/platform/ti-vpe/cal.h
+++ b/drivers/media/platform/ti-vpe/cal.h
@@ -303,7 +303,7 @@ static inline void cal_write_field(struct cal_dev *cal, u32 offset, u32 value,
 	u32 val = cal_read(cal, offset);
 
 	val &= ~mask;
-	val |= (value << __ffs(mask)) & mask;
+	val |= field_prep(mask, value);
 	cal_write(cal, offset, val);
 }
 
@@ -312,7 +312,7 @@ static inline void cal_set_field(u32 *valp, u32 field, u32 mask)
 	u32 val = *valp;
 
 	val &= ~mask;
-	val |= (field << __ffs(mask)) & mask;
+	val |= field_prep(mask, field);
 	*valp = val;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 11/17] mmc: sdhci-of-aspeed: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (9 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 10/17] media: ti-vpe: cal: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-22 15:54 ` [PATCH/RFC 12/17] pinctrl: aspeed: " Geert Uytterhoeven
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_prep() helper, instead open-coding the same operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/mmc/host/sdhci-of-aspeed.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index 6e4e132903a6346b..26ac73aafb2ed55d 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -2,6 +2,7 @@
 /* Copyright (C) 2019 ASPEED Technology Inc. */
 /* Copyright (C) 2019 IBM Corp. */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -131,8 +132,8 @@ aspeed_sdc_set_phase_tap(const struct aspeed_sdhci_tap_desc *desc,
 {
 	reg &= ~(desc->enable_mask | desc->tap_mask);
 	if (enable) {
-		reg |= tap << __ffs(desc->tap_mask);
-		reg |= desc->enable_value << __ffs(desc->enable_mask);
+		reg |= field_prep(desc->tap_mask, tap);
+		reg |= field_prep(desc->enable_mask, desc->enable_value);
 	}
 
 	return reg;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 12/17] pinctrl: aspeed: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (10 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 11/17] mmc: sdhci-of-aspeed: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-22 15:54 ` [PATCH/RFC 13/17] pinctl: ti: iodelay: " Geert Uytterhoeven
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_{get,prep}() helpers, instead of open-coding the same
operations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c | 3 ++-
 drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 3 ++-
 drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 3 ++-
 drivers/pinctrl/aspeed/pinctrl-aspeed.c    | 5 +++--
 drivers/pinctrl/aspeed/pinmux-aspeed.c     | 6 ++++--
 5 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
index bfed0e2746437b4a..bfb2a7b229915a68 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (C) 2016 IBM Corp.
  */
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -2551,7 +2552,7 @@ static int aspeed_g4_sig_expr_set(struct aspeed_pinmux_data *ctx,
 	for (i = 0; i < expr->ndescs; i++) {
 		const struct aspeed_sig_desc *desc = &expr->descs[i];
 		u32 pattern = enable ? desc->enable : desc->disable;
-		u32 val = (pattern << __ffs(desc->mask));
+		u32 val = field_prep(desc->mask, pattern);
 
 		if (!ctx->maps[desc->ip])
 			return -ENODEV;
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
index 4c0d26606b6cc7d6..8cc6d9c1f1c78296 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (C) 2016 IBM Corp.
  */
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -2724,7 +2725,7 @@ static int aspeed_g5_sig_expr_set(struct aspeed_pinmux_data *ctx,
 	for (i = 0; i < expr->ndescs; i++) {
 		const struct aspeed_sig_desc *desc = &expr->descs[i];
 		u32 pattern = enable ? desc->enable : desc->disable;
-		u32 val = (pattern << __ffs(desc->mask));
+		u32 val = field_prep(desc->mask, pattern);
 		struct regmap *map;
 
 		map = aspeed_g5_acquire_regmap(ctx, desc->ip);
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
index a3fa03bcd9a30577..00f7b69a74e9e743 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /* Copyright (C) 2019 IBM Corp. */
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -2649,7 +2650,7 @@ static int aspeed_g6_sig_expr_set(struct aspeed_pinmux_data *ctx,
 	for (i = 0; i < expr->ndescs; i++) {
 		const struct aspeed_sig_desc *desc = &expr->descs[i];
 		u32 pattern = enable ? desc->enable : desc->disable;
-		u32 val = (pattern << __ffs(desc->mask));
+		u32 val = field_prep(desc->mask, pattern);
 		bool is_strap;
 
 		if (!ctx->maps[desc->ip])
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index c94e24aadf922d2a..839ac48f75836352 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2016 IBM Corp.
  */
 
+#include <linux/bitfield.h>
 #include <linux/mfd/syscon.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -547,7 +548,7 @@ int aspeed_pin_config_get(struct pinctrl_dev *pctldev, unsigned int offset,
 		return rc;
 
 	pmap = find_pinconf_map(pdata, param, MAP_TYPE_VAL,
-			(val & pconf->mask) >> __ffs(pconf->mask));
+				field_get(pconf->mask, val));
 
 	if (!pmap)
 		return -EINVAL;
@@ -595,7 +596,7 @@ int aspeed_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset,
 		if (WARN_ON(!pmap))
 			return -EINVAL;
 
-		val = pmap->val << __ffs(pconf->mask);
+		val = field_prep(pconf->mask, pmap->val);
 
 		rc = regmap_update_bits(pdata->scu, pconf->reg,
 					pconf->mask, val);
diff --git a/drivers/pinctrl/aspeed/pinmux-aspeed.c b/drivers/pinctrl/aspeed/pinmux-aspeed.c
index 4aa46383c2c533f0..61ddd550439325ee 100644
--- a/drivers/pinctrl/aspeed/pinmux-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinmux-aspeed.c
@@ -3,6 +3,8 @@
 
 /* Pieces to enable drivers to implement the .set callback */
 
+#include <linux/bitfield.h>
+
 #include "pinmux-aspeed.h"
 
 static const char *const aspeed_pinmux_ips[] = {
@@ -17,7 +19,7 @@ static inline void aspeed_sig_desc_print_val(
 	pr_debug("Want %s%X[0x%08X]=0x%X, got 0x%X from 0x%08X\n",
 			aspeed_pinmux_ips[desc->ip], desc->reg,
 			desc->mask, enable ? desc->enable : desc->disable,
-			(rv & desc->mask) >> __ffs(desc->mask), rv);
+			field_get(desc->mask, rv), rv);
 }
 
 /**
@@ -55,7 +57,7 @@ int aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
 	aspeed_sig_desc_print_val(desc, enabled, raw);
 	want = enabled ? desc->enable : desc->disable;
 
-	return ((raw & desc->mask) >> __ffs(desc->mask)) == want;
+	return field_get(desc->mask, raw) == want;
 }
 
 /**
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 13/17] pinctl: ti: iodelay: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (11 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 12/17] pinctrl: aspeed: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-22 17:52   ` Alexandre Belloni
  2021-11-22 15:54 ` [PATCH/RFC 14/17] regulator: ti-abb: " Geert Uytterhoeven
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_{get,prep}() helpers, instead of defining a custom
function, or open-coding the same operations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 35 +++++++------------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
index 4e2382778d38f557..b220dcd9215520db 100644
--- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
+++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
@@ -9,6 +9,7 @@
  * warranty of any kind, whether express or implied.
  */
 
+#include <linux/bitfield.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -155,18 +156,6 @@ struct ti_iodelay_device {
 	struct ti_iodelay_reg_values reg_init_conf_values;
 };
 
-/**
- * ti_iodelay_extract() - extract bits for a field
- * @val: Register value
- * @mask: Mask
- *
- * Return: extracted value which is appropriately shifted
- */
-static inline u32 ti_iodelay_extract(u32 val, u32 mask)
-{
-	return (val & mask) >> __ffs(mask);
-}
-
 /**
  * ti_iodelay_compute_dpe() - Compute equation for delay parameter
  * @period: Period to use
@@ -233,10 +222,10 @@ static int ti_iodelay_pinconf_set(struct ti_iodelay_device *iod,
 	}
 
 	reg_mask = reg->signature_mask;
-	reg_val = reg->signature_value << __ffs(reg->signature_mask);
+	reg_val = field_prep(reg->signature_mask, reg->signature_value);
 
 	reg_mask |= reg->binary_data_coarse_mask;
-	tmp_val = c_elements << __ffs(reg->binary_data_coarse_mask);
+	tmp_val = field_prep(reg->binary_data_coarse_mask, c_elements);
 	if (tmp_val & ~reg->binary_data_coarse_mask) {
 		dev_err(dev, "Masking overflow of coarse elements %08x\n",
 			tmp_val);
@@ -245,7 +234,7 @@ static int ti_iodelay_pinconf_set(struct ti_iodelay_device *iod,
 	reg_val |= tmp_val;
 
 	reg_mask |= reg->binary_data_fine_mask;
-	tmp_val = f_elements << __ffs(reg->binary_data_fine_mask);
+	tmp_val = field_prep(reg->binary_data_fine_mask, f_elements);
 	if (tmp_val & ~reg->binary_data_fine_mask) {
 		dev_err(dev, "Masking overflow of fine elements %08x\n",
 			tmp_val);
@@ -260,7 +249,7 @@ static int ti_iodelay_pinconf_set(struct ti_iodelay_device *iod,
 	 * impacting iodelay configuration. Use with care!
 	 */
 	reg_mask |= reg->lock_mask;
-	reg_val |= reg->unlock_val << __ffs(reg->lock_mask);
+	reg_val |= field_prep(reg->lock_mask, reg->unlock_val);
 	r = regmap_update_bits(iod->regmap, cfg->offset, reg_mask, reg_val);
 
 	dev_dbg(dev, "Set reg 0x%x Delay(a: %d g: %d), Elements(C=%d F=%d)0x%x\n",
@@ -296,16 +285,14 @@ static int ti_iodelay_pinconf_init_dev(struct ti_iodelay_device *iod)
 	r = regmap_read(iod->regmap, reg->reg_refclk_offset, &val);
 	if (r)
 		return r;
-	ival->ref_clk_period = ti_iodelay_extract(val, reg->refclk_period_mask);
+	ival->ref_clk_period = field_get(reg->refclk_period_mask, val);
 	dev_dbg(dev, "refclk_period=0x%04x\n", ival->ref_clk_period);
 
 	r = regmap_read(iod->regmap, reg->reg_coarse_offset, &val);
 	if (r)
 		return r;
-	ival->coarse_ref_count =
-	    ti_iodelay_extract(val, reg->coarse_ref_count_mask);
-	ival->coarse_delay_count =
-	    ti_iodelay_extract(val, reg->coarse_delay_count_mask);
+	ival->coarse_ref_count = field_get(reg->coarse_ref_count_mask, val);
+	ival->coarse_delay_count = field_get(reg->coarse_delay_count_mask, val);
 	if (!ival->coarse_delay_count) {
 		dev_err(dev, "Invalid Coarse delay count (0) (reg=0x%08x)\n",
 			val);
@@ -326,10 +313,8 @@ static int ti_iodelay_pinconf_init_dev(struct ti_iodelay_device *iod)
 	r = regmap_read(iod->regmap, reg->reg_fine_offset, &val);
 	if (r)
 		return r;
-	ival->fine_ref_count =
-	    ti_iodelay_extract(val, reg->fine_ref_count_mask);
-	ival->fine_delay_count =
-	    ti_iodelay_extract(val, reg->fine_delay_count_mask);
+	ival->fine_ref_count = field_get(reg->fine_ref_count_mask, val);
+	ival->fine_delay_count = field_get(reg->fine_delay_count_mask, val);
 	if (!ival->fine_delay_count) {
 		dev_err(dev, "Invalid Fine delay count (0) (reg=0x%08x)\n",
 			val);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 14/17] regulator: ti-abb: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (12 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 13/17] pinctl: ti: iodelay: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-22 16:31   ` Mark Brown
  2021-11-22 15:54 ` [PATCH/RFC 15/17] thermal/ti-soc-thermal: " Geert Uytterhoeven
                   ` (3 subsequent siblings)
  17 siblings, 1 reply; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_{get,prep}() helpers, instead of open-coding the same
operations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/regulator/ti-abb-regulator.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
index 2931a0b89bffbf7a..3bc6ca5c382a4273 100644
--- a/drivers/regulator/ti-abb-regulator.c
+++ b/drivers/regulator/ti-abb-regulator.c
@@ -17,6 +17,7 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  */
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
@@ -132,7 +133,7 @@ static inline u32 ti_abb_rmw(u32 mask, u32 value, void __iomem *reg)
 
 	val = readl(reg);
 	val &= ~mask;
-	val |= (value << __ffs(mask)) & mask;
+	val |= field_prep(mask, value);
 	writel(val, reg);
 
 	return val;
@@ -229,7 +230,7 @@ static void ti_abb_program_ldovbb(struct device *dev, const struct ti_abb *abb,
 	case TI_ABB_SLOW_OPP:
 	case TI_ABB_FAST_OPP:
 		val |= abb->ldovbb_override_mask;
-		val |= info->vset << __ffs(abb->ldovbb_vset_mask);
+		val |= field_prep(abb->ldovbb_vset_mask, info->vset);
 		break;
 	}
 
@@ -606,7 +607,7 @@ static int ti_abb_init_table(struct device *dev, struct ti_abb *abb,
 					pname, *volt_table, vset_mask);
 			continue;
 		}
-		info->vset = (efuse_val & vset_mask) >> __ffs(vset_mask);
+		info->vset = field_get(vset_mask, efuse_val);
 		dev_dbg(dev, "[%d]v=%d vset=%x\n", i, *volt_table, info->vset);
 check_abb:
 		switch (info->opp_sel) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 15/17] thermal/ti-soc-thermal: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (13 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 14/17] regulator: ti-abb: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-22 15:54 ` [PATCH/RFC 16/17] ALSA: ice1724: " Geert Uytterhoeven
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_{get,prep}() helpers, instead of open-coding the same
operations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/thermal/ti-soc-thermal/ti-bandgap.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index ea0603b59309f5f0..83a34d698414b177 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -9,6 +9,7 @@
  *   Eduardo Valentin <eduardo.valentin@ti.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/cpu_pm.h>
 #include <linux/device.h>
@@ -80,10 +81,10 @@ do {								\
 	struct temp_sensor_registers *t;			\
 	u32 r;							\
 								\
-	t = bgp->conf->sensors[(id)].registers;		\
+	t = bgp->conf->sensors[(id)].registers;			\
 	r = ti_bandgap_readl(bgp, t->reg);			\
 	r &= ~t->mask;						\
-	r |= (val) << __ffs(t->mask);				\
+	r |= field_prep(t->mask, val);				\
 	ti_bandgap_writel(bgp, r, t->reg);			\
 } while (0)
 
@@ -342,8 +343,7 @@ static void ti_bandgap_read_counter(struct ti_bandgap *bgp, int id,
 
 	tsr = bgp->conf->sensors[id].registers;
 	time = ti_bandgap_readl(bgp, tsr->bgap_counter);
-	time = (time & tsr->counter_mask) >>
-					__ffs(tsr->counter_mask);
+	time = field_get(tsr->counter_mask, time);
 	time = time * 1000 / bgp->clk_rate;
 	*interval = time;
 }
@@ -363,8 +363,7 @@ static void ti_bandgap_read_counter_delay(struct ti_bandgap *bgp, int id,
 	tsr = bgp->conf->sensors[id].registers;
 
 	reg_val = ti_bandgap_readl(bgp, tsr->bgap_mask_ctrl);
-	reg_val = (reg_val & tsr->mask_counter_delay_mask) >>
-				__ffs(tsr->mask_counter_delay_mask);
+	reg_val = field_get(tsr->mask_counter_delay_mask, reg_val);
 	switch (reg_val) {
 	case 0:
 		*interval = 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 16/17] ALSA: ice1724: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (14 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 15/17] thermal/ti-soc-thermal: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-23 11:24   ` Takashi Iwai
  2021-11-22 15:54 ` [PATCH/RFC 17/17] rtw89: " Geert Uytterhoeven
  2021-11-22 17:50 ` [PATCH 00/17] Non-const bitfield helper conversions Alexandre Belloni
  17 siblings, 1 reply; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_{get,prep}() helpers, instead of open-coding the same
operations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 sound/pci/ice1712/wm8766.c | 14 +++++++-------
 sound/pci/ice1712/wm8776.c | 14 +++++++-------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/sound/pci/ice1712/wm8766.c b/sound/pci/ice1712/wm8766.c
index fe3e243b38549035..3e4d7f8f692785b0 100644
--- a/sound/pci/ice1712/wm8766.c
+++ b/sound/pci/ice1712/wm8766.c
@@ -7,6 +7,7 @@
  *	Copyright (c) 2012 Ondrej Zary <linux@rainbow-software.org>
  */
 
+#include <linux/bitfield.h>
 #include <linux/delay.h>
 #include <sound/core.h>
 #include <sound/control.h>
@@ -212,11 +213,10 @@ static int snd_wm8766_ctl_get(struct snd_kcontrol *kcontrol,
 	if (wm->ctl[n].get)
 		wm->ctl[n].get(wm, &val1, &val2);
 	else {
-		val1 = wm->regs[wm->ctl[n].reg1] & wm->ctl[n].mask1;
-		val1 >>= __ffs(wm->ctl[n].mask1);
+		val1 = field_get(wm->ctl[n].mask1, wm->regs[wm->ctl[n].reg1]);
 		if (wm->ctl[n].flags & WM8766_FLAG_STEREO) {
-			val2 = wm->regs[wm->ctl[n].reg2] & wm->ctl[n].mask2;
-			val2 >>= __ffs(wm->ctl[n].mask2);
+			val2 = field_get(wm->ctl[n].mask2,
+					 wm->regs[wm->ctl[n].reg2]);
 			if (wm->ctl[n].flags & WM8766_FLAG_VOL_UPDATE)
 				val2 &= ~WM8766_VOL_UPDATE;
 		}
@@ -251,19 +251,19 @@ static int snd_wm8766_ctl_put(struct snd_kcontrol *kcontrol,
 		wm->ctl[n].set(wm, regval1, regval2);
 	else {
 		val = wm->regs[wm->ctl[n].reg1] & ~wm->ctl[n].mask1;
-		val |= regval1 << __ffs(wm->ctl[n].mask1);
+		val |= field_prep(wm->ctl[n].mask1, regval1);
 		/* both stereo controls in one register */
 		if (wm->ctl[n].flags & WM8766_FLAG_STEREO &&
 				wm->ctl[n].reg1 == wm->ctl[n].reg2) {
 			val &= ~wm->ctl[n].mask2;
-			val |= regval2 << __ffs(wm->ctl[n].mask2);
+			val |= field_prep(wm->ctl[n].mask2, regval2);
 		}
 		snd_wm8766_write(wm, wm->ctl[n].reg1, val);
 		/* stereo controls in different registers */
 		if (wm->ctl[n].flags & WM8766_FLAG_STEREO &&
 				wm->ctl[n].reg1 != wm->ctl[n].reg2) {
 			val = wm->regs[wm->ctl[n].reg2] & ~wm->ctl[n].mask2;
-			val |= regval2 << __ffs(wm->ctl[n].mask2);
+			val |= field_prep(wm->ctl[n].mask2, regval2);
 			if (wm->ctl[n].flags & WM8766_FLAG_VOL_UPDATE)
 				val |= WM8766_VOL_UPDATE;
 			snd_wm8766_write(wm, wm->ctl[n].reg2, val);
diff --git a/sound/pci/ice1712/wm8776.c b/sound/pci/ice1712/wm8776.c
index 6eda86119dff60b3..b4edf8a03c342351 100644
--- a/sound/pci/ice1712/wm8776.c
+++ b/sound/pci/ice1712/wm8776.c
@@ -7,6 +7,7 @@
  *	Copyright (c) 2012 Ondrej Zary <linux@rainbow-software.org>
  */
 
+#include <linux/bitfield.h>
 #include <linux/delay.h>
 #include <sound/core.h>
 #include <sound/control.h>
@@ -486,11 +487,10 @@ static int snd_wm8776_ctl_get(struct snd_kcontrol *kcontrol,
 	if (wm->ctl[n].get)
 		wm->ctl[n].get(wm, &val1, &val2);
 	else {
-		val1 = wm->regs[wm->ctl[n].reg1] & wm->ctl[n].mask1;
-		val1 >>= __ffs(wm->ctl[n].mask1);
+		val1 = field_get(wm->ctl[n].mask1, wm->regs[wm->ctl[n].reg1]);
 		if (wm->ctl[n].flags & WM8776_FLAG_STEREO) {
-			val2 = wm->regs[wm->ctl[n].reg2] & wm->ctl[n].mask2;
-			val2 >>= __ffs(wm->ctl[n].mask2);
+			val2 = field_get(wm->ctl[n].mask2,
+					 wm->regs[wm->ctl[n].reg2]);
 			if (wm->ctl[n].flags & WM8776_FLAG_VOL_UPDATE)
 				val2 &= ~WM8776_VOL_UPDATE;
 		}
@@ -525,19 +525,19 @@ static int snd_wm8776_ctl_put(struct snd_kcontrol *kcontrol,
 		wm->ctl[n].set(wm, regval1, regval2);
 	else {
 		val = wm->regs[wm->ctl[n].reg1] & ~wm->ctl[n].mask1;
-		val |= regval1 << __ffs(wm->ctl[n].mask1);
+		val |= field_prep(wm->ctl[n].mask1, regval1);
 		/* both stereo controls in one register */
 		if (wm->ctl[n].flags & WM8776_FLAG_STEREO &&
 				wm->ctl[n].reg1 == wm->ctl[n].reg2) {
 			val &= ~wm->ctl[n].mask2;
-			val |= regval2 << __ffs(wm->ctl[n].mask2);
+			val |= field_prep(wm->ctl[n].mask2, regval2);
 		}
 		snd_wm8776_write(wm, wm->ctl[n].reg1, val);
 		/* stereo controls in different registers */
 		if (wm->ctl[n].flags & WM8776_FLAG_STEREO &&
 				wm->ctl[n].reg1 != wm->ctl[n].reg2) {
 			val = wm->regs[wm->ctl[n].reg2] & ~wm->ctl[n].mask2;
-			val |= regval2 << __ffs(wm->ctl[n].mask2);
+			val |= field_prep(wm->ctl[n].mask2, regval2);
 			if (wm->ctl[n].flags & WM8776_FLAG_VOL_UPDATE)
 				val |= WM8776_VOL_UPDATE;
 			snd_wm8776_write(wm, wm->ctl[n].reg2, val);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [PATCH/RFC 17/17] rtw89: Use bitfield helpers
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (15 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 16/17] ALSA: ice1724: " Geert Uytterhoeven
@ 2021-11-22 15:54 ` Geert Uytterhoeven
  2021-11-22 18:38   ` Larry Finger
  2021-11-22 17:50 ` [PATCH 00/17] Non-const bitfield helper conversions Alexandre Belloni
  17 siblings, 1 reply; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-22 15:54 UTC (permalink / raw)
  To: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel, Geert Uytterhoeven

Use the field_{get,prep}() helpers, instead of open-coding the same
operations.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.
Marked RFC, as this depends on [PATCH 01/17], but follows a different
path to upstream.
---
 drivers/net/wireless/realtek/rtw89/core.h | 38 ++++-------------------
 1 file changed, 6 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index c2885e4dd882f045..f9c0300ec373aaf2 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -2994,81 +2994,55 @@ rtw89_write32_clr(struct rtw89_dev *rtwdev, u32 addr, u32 bit)
 static inline u32
 rtw89_read32_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
 {
-	u32 shift = __ffs(mask);
-	u32 orig;
-	u32 ret;
-
-	orig = rtw89_read32(rtwdev, addr);
-	ret = (orig & mask) >> shift;
-
-	return ret;
+	return field_get(mask, rtw89_read32(rtwdev, addr));
 }
 
 static inline u16
 rtw89_read16_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
 {
-	u32 shift = __ffs(mask);
-	u32 orig;
-	u32 ret;
-
-	orig = rtw89_read16(rtwdev, addr);
-	ret = (orig & mask) >> shift;
-
-	return ret;
+	return field_get(mask, rtw89_read16(rtwdev, addr));
 }
 
 static inline u8
 rtw89_read8_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
 {
-	u32 shift = __ffs(mask);
-	u32 orig;
-	u32 ret;
-
-	orig = rtw89_read8(rtwdev, addr);
-	ret = (orig & mask) >> shift;
-
-	return ret;
+	return field_get(mask, rtw89_read8(rtwdev, addr));
 }
 
 static inline void
 rtw89_write32_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask, u32 data)
 {
-	u32 shift = __ffs(mask);
 	u32 orig;
 	u32 set;
 
 	WARN(addr & 0x3, "should be 4-byte aligned, addr = 0x%08x\n", addr);
 
 	orig = rtw89_read32(rtwdev, addr);
-	set = (orig & ~mask) | ((data << shift) & mask);
+	set = (orig & ~mask) | field_prep(mask, data);
 	rtw89_write32(rtwdev, addr, set);
 }
 
 static inline void
 rtw89_write16_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask, u16 data)
 {
-	u32 shift;
 	u16 orig, set;
 
 	mask &= 0xffff;
-	shift = __ffs(mask);
 
 	orig = rtw89_read16(rtwdev, addr);
-	set = (orig & ~mask) | ((data << shift) & mask);
+	set = (orig & ~mask) | field_prep(mask, data);
 	rtw89_write16(rtwdev, addr, set);
 }
 
 static inline void
 rtw89_write8_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask, u8 data)
 {
-	u32 shift;
 	u8 orig, set;
 
 	mask &= 0xff;
-	shift = __ffs(mask);
 
 	orig = rtw89_read8(rtwdev, addr);
-	set = (orig & ~mask) | ((data << shift) & mask);
+	set = (orig & ~mask) | field_prep(mask, data);
 	rtw89_write8(rtwdev, addr, set);
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [PATCH/RFC 14/17] regulator: ti-abb: Use bitfield helpers
  2021-11-22 15:54 ` [PATCH/RFC 14/17] regulator: ti-abb: " Geert Uytterhoeven
@ 2021-11-22 16:31   ` Mark Brown
  0 siblings, 0 replies; 41+ messages in thread
From: Mark Brown @ 2021-11-22 16:31 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

[-- Attachment #1: Type: text/plain, Size: 195 bytes --]

On Mon, Nov 22, 2021 at 04:54:07PM +0100, Geert Uytterhoeven wrote:
> Use the field_{get,prep}() helpers, instead of open-coding the same
> operations.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-22 15:53 ` [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers Geert Uytterhoeven
@ 2021-11-22 16:32   ` Johannes Berg
  2021-11-23  1:17     ` Jakub Kicinski
                       ` (2 more replies)
  0 siblings, 3 replies; 41+ messages in thread
From: Johannes Berg @ 2021-11-22 16:32 UTC (permalink / raw)
  To: Geert Uytterhoeven, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel

On Mon, 2021-11-22 at 16:53 +0100, Geert Uytterhoeven wrote:
> The existing FIELD_{GET,PREP}() macros are limited to compile-time
> constants.  However, it is very common to prepare or extract bitfield
> elements where the bitfield mask is not a compile-time constant.
> 

I'm not sure it's really a good idea to add a third API here?

We have the upper-case (constant) versions, and already
{u32,...}_get_bits()/etc.

Also, you're using __ffs(), which doesn't work for 64-bit on 32-bit
architectures (afaict), so that seems a bit awkward.

Maybe we can make {u32,...}_get_bits() be doing compile-time only checks
if it is indeed a constant? The __field_overflow() usage is already only
done if __builtin_constant_p(v), so I guess we can do the same with
__bad_mask()?

johannes

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 00/17] Non-const bitfield helper conversions
  2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
                   ` (16 preceding siblings ...)
  2021-11-22 15:54 ` [PATCH/RFC 17/17] rtw89: " Geert Uytterhoeven
@ 2021-11-22 17:50 ` Alexandre Belloni
  2021-11-23  8:20   ` Geert Uytterhoeven
  17 siblings, 1 reply; 41+ messages in thread
From: Alexandre Belloni @ 2021-11-22 17:50 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Ludovic Desroches, Tero Kristo, Jonathan Cameron,
	Lars-Peter Clausen, Lorenzo Bianconi, Benoit Parrot,
	Mauro Carvalho Chehab, Adrian Hunter, Andrew Jeffery,
	Ulf Hansson, Joel Stanley, Ping-Ke Shih, Kalle Valo,
	David S . Miller, Jakub Kicinski, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On 22/11/2021 16:53:53+0100, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> <linux/bitfield.h> contains various helpers for accessing bitfields, as
> typically used in hardware registers for memory-mapped I/O blocks. These
> helpers ensure type safety, and deduce automatically shift values from
> mask values, avoiding mistakes due to inconsistent shifts and masks, and
> leading to a reduction in source code size.
> 
> I have already submitted a few conversions to the FIELD_{GET,PREP}()
> helpers that were fixes for real bugs:
>   - [PATCH] mips: cm: Convert to bitfield API to fix out-of-bounds
>     access
>     https://lore.kernel.org/r/0471c545117c5fa05bd9c73005cda9b74608a61e.1635501373.git.geert+renesas@glider.be
>   - [PATCH] drm/armada: Fix off-by-one error in
>     armada_overlay_get_property()
>     https://lore.kernel.org/r/5818c8b04834e6a9525441bc181580a230354b69.1635501237.git.geert+renesas@glider.be
> 
> Plus several patches for normal conversions:
>   - [PATCH] ARM: ptrace: Use bitfield helpers
>     https://lore.kernel.org/r/a1445d3abb45cfc95cb1b03180fd53caf122035b.1637593297.git.geert+renesas@glider.be
>   - [PATCH] MIPS: CPC: Use bitfield helpers
>     https://lore.kernel.org/r/35f0f17e3d987afaa9cd09cdcb8131d42a53c3e1.1637593297.git.geert+renesas@glider.be
>   - [PATCH] MIPS: CPS: Use bitfield helpers
>     https://lore.kernel.org/r/8bd8b1b9a3787e594285addcf2057754540d0a5f.1637593297.git.geert+renesas@glider.be
>   - [PATCH] crypto: sa2ul - Use bitfield helpers
>     https://lore.kernel.org/r/ca89d204ef2e40193479db2742eadf0d9cf3c0ff.1637593297.git.geert+renesas@glider.be
>   - [PATCH] dmaengine: stm32-mdma: Use bitfield helpers
>     https://lore.kernel.org/r/36ceab242a594233dc7dc6f1dddb4ac32d1e846f.1637593297.git.geert+renesas@glider.be
>   - [PATCH] intel_th: Use bitfield helpers
>     https://lore.kernel.org/r/b1e4f027aa88acfbdfaa771b0920bd1d977828ba.1637593297.git.geert+renesas@glider.be
>   - [PATCH] Input: palmas-pwrbutton - use bitfield helpers
>     https://lore.kernel.org/r/f8831b88346b36fc6e01e0910d0db6c94287d2b4.1637593297.git.geert+renesas@glider.be
>   - [PATCH] irqchip/mips-gic: Use bitfield helpers
>     https://lore.kernel.org/r/74f9d126961a90d3e311b92a54870eaac5b3ae57.1637593297.git.geert+renesas@glider.be
>   - [PATCH] mfd: mc13xxx: Use bitfield helpers
>     https://lore.kernel.org/r/afa46868cf8c1666e9cbbbec42767ca2294b024d.1637593297.git.geert+renesas@glider.be
>   - [PATCH] regulator: lp873x: Use bitfield helpers
>     https://lore.kernel.org/r/44d60384b640c8586b4ca7edbc9287a34ce21c5b.1637593297.git.geert+renesas@glider.be
>   - [PATCH] regulator: lp87565: Use bitfield helpers
>     https://lore.kernel.org/r/941c2dfd5b5b124b8950bcce42db4c343dfe9821.1637593297.git.geert+renesas@glider.be
> 
> The existing FIELD_{GET,PREP}() macros are limited to compile-time
> constants.  However, it is very common to prepare or extract bitfield
> elements where the bitfield mask is not a compile-time constant.
> To avoid this limitation, the AT91 clock driver already has its own
> field_{prep,get}() macros.
> 

My understanding was that this (being compile time only) was actually
done on purpose. Did I misunderstand?

> This patch series makes them available for general use, and converts
> several drivers to the existing FIELD_{GET,PREP}() and the new
> field_{get,prep}() helpers.
> 
> I can take the first two patches through the reneas-clk tree for v5.17,
> but probably it is best for the remaining patches to be postponed to
> v5.18.
> 
> Thanks for your comments!
> 
> Geert Uytterhoeven (17):
>   bitfield: Add non-constant field_{prep,get}() helpers
>   clk: renesas: Use bitfield helpers
>   [RFC] soc: renesas: Use bitfield helpers
>   [RFC] ARM: OMAP2+: Use bitfield helpers
>   [RFC] bus: omap_l3_noc: Use bitfield helpers
>   [RFC] clk: ti: Use bitfield helpers
>   [RFC] iio: st_sensors: Use bitfield helpers
>   [RFC] iio: humidity: hts221: Use bitfield helpers
>   [RFC] iio: imu: st_lsm6dsx: Use bitfield helpers
>   [RFC] media: ti-vpe: cal: Use bitfield helpers
>   [RFC] mmc: sdhci-of-aspeed: Use bitfield helpers
>   [RFC] pinctrl: aspeed: Use bitfield helpers
>   [RFC] pinctl: ti: iodelay: Use bitfield helpers
>   [RFC] regulator: ti-abb: Use bitfield helpers
>   [RFC] thermal/ti-soc-thermal: Use bitfield helpers
>   [RFC] ALSA: ice1724: Use bitfield helpers
>   [RFC] rtw89: Use bitfield helpers
> 
>  arch/arm/mach-omap2/clkt2xxx_dpllcore.c       |  5 +-
>  arch/arm/mach-omap2/cm2xxx.c                  | 11 ++-
>  arch/arm/mach-omap2/cm2xxx_3xxx.h             |  9 +--
>  arch/arm/mach-omap2/cm33xx.c                  |  9 +--
>  arch/arm/mach-omap2/cm3xxx.c                  |  7 +-
>  arch/arm/mach-omap2/cminst44xx.c              |  9 +--
>  arch/arm/mach-omap2/powerdomains3xxx_data.c   |  3 +-
>  arch/arm/mach-omap2/prm.h                     |  2 -
>  arch/arm/mach-omap2/prm2xxx.c                 |  4 +-
>  arch/arm/mach-omap2/prm2xxx_3xxx.c            |  7 +-
>  arch/arm/mach-omap2/prm2xxx_3xxx.h            |  9 +--
>  arch/arm/mach-omap2/prm33xx.c                 | 53 +++++-------
>  arch/arm/mach-omap2/prm3xxx.c                 |  3 +-
>  arch/arm/mach-omap2/prm44xx.c                 | 53 ++++--------
>  arch/arm/mach-omap2/vc.c                      | 12 +--
>  arch/arm/mach-omap2/vp.c                      | 11 +--
>  drivers/bus/omap_l3_noc.c                     |  4 +-
>  drivers/clk/at91/clk-peripheral.c             |  1 +
>  drivers/clk/at91/pmc.h                        |  3 -
>  drivers/clk/renesas/clk-div6.c                |  6 +-
>  drivers/clk/renesas/r8a779a0-cpg-mssr.c       |  9 +--
>  drivers/clk/renesas/rcar-gen3-cpg.c           | 15 ++--
>  drivers/clk/ti/apll.c                         | 25 +++---
>  drivers/clk/ti/dpll3xxx.c                     | 81 ++++++++-----------
>  .../iio/common/st_sensors/st_sensors_core.c   |  5 +-
>  drivers/iio/humidity/hts221_core.c            |  8 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  1 -
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  7 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 45 +++++------
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  | 11 +--
>  drivers/media/platform/ti-vpe/cal.h           |  4 +-
>  drivers/mmc/host/sdhci-of-aspeed.c            |  5 +-
>  drivers/net/wireless/realtek/rtw89/core.h     | 38 ++-------
>  drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c    |  3 +-
>  drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c    |  3 +-
>  drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c    |  3 +-
>  drivers/pinctrl/aspeed/pinctrl-aspeed.c       |  5 +-
>  drivers/pinctrl/aspeed/pinmux-aspeed.c        |  6 +-
>  drivers/pinctrl/ti/pinctrl-ti-iodelay.c       | 35 +++-----
>  drivers/regulator/ti-abb-regulator.c          |  7 +-
>  drivers/soc/renesas/renesas-soc.c             |  4 +-
>  drivers/thermal/ti-soc-thermal/ti-bandgap.c   | 11 ++-
>  include/linux/bitfield.h                      | 30 +++++++
>  sound/pci/ice1712/wm8766.c                    | 14 ++--
>  sound/pci/ice1712/wm8776.c                    | 14 ++--
>  45 files changed, 263 insertions(+), 347 deletions(-)
> 
> -- 
> 2.25.1
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> 							    -- Linus Torvalds

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH/RFC 13/17] pinctl: ti: iodelay: Use bitfield helpers
  2021-11-22 15:54 ` [PATCH/RFC 13/17] pinctl: ti: iodelay: " Geert Uytterhoeven
@ 2021-11-22 17:52   ` Alexandre Belloni
  0 siblings, 0 replies; 41+ messages in thread
From: Alexandre Belloni @ 2021-11-22 17:52 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Ludovic Desroches, Tero Kristo, Jonathan Cameron,
	Lars-Peter Clausen, Lorenzo Bianconi, Benoit Parrot,
	Mauro Carvalho Chehab, Adrian Hunter, Andrew Jeffery,
	Ulf Hansson, Joel Stanley, Ping-Ke Shih, Kalle Valo,
	David S . Miller, Jakub Kicinski, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

Hi Geert,

There is a typo in pinctrl in the subject

On 22/11/2021 16:54:06+0100, Geert Uytterhoeven wrote:
> Use the field_{get,prep}() helpers, instead of defining a custom
> function, or open-coding the same operations.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Compile-tested only.
> Marked RFC, as this depends on [PATCH 01/17], but follows a different
> path to upstream.
> ---
>  drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 35 +++++++------------------
>  1 file changed, 10 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> index 4e2382778d38f557..b220dcd9215520db 100644
> --- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> +++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> @@ -9,6 +9,7 @@
>   * warranty of any kind, whether express or implied.
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/err.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
> @@ -155,18 +156,6 @@ struct ti_iodelay_device {
>  	struct ti_iodelay_reg_values reg_init_conf_values;
>  };
>  
> -/**
> - * ti_iodelay_extract() - extract bits for a field
> - * @val: Register value
> - * @mask: Mask
> - *
> - * Return: extracted value which is appropriately shifted
> - */
> -static inline u32 ti_iodelay_extract(u32 val, u32 mask)
> -{
> -	return (val & mask) >> __ffs(mask);
> -}
> -
>  /**
>   * ti_iodelay_compute_dpe() - Compute equation for delay parameter
>   * @period: Period to use
> @@ -233,10 +222,10 @@ static int ti_iodelay_pinconf_set(struct ti_iodelay_device *iod,
>  	}
>  
>  	reg_mask = reg->signature_mask;
> -	reg_val = reg->signature_value << __ffs(reg->signature_mask);
> +	reg_val = field_prep(reg->signature_mask, reg->signature_value);
>  
>  	reg_mask |= reg->binary_data_coarse_mask;
> -	tmp_val = c_elements << __ffs(reg->binary_data_coarse_mask);
> +	tmp_val = field_prep(reg->binary_data_coarse_mask, c_elements);
>  	if (tmp_val & ~reg->binary_data_coarse_mask) {
>  		dev_err(dev, "Masking overflow of coarse elements %08x\n",
>  			tmp_val);
> @@ -245,7 +234,7 @@ static int ti_iodelay_pinconf_set(struct ti_iodelay_device *iod,
>  	reg_val |= tmp_val;
>  
>  	reg_mask |= reg->binary_data_fine_mask;
> -	tmp_val = f_elements << __ffs(reg->binary_data_fine_mask);
> +	tmp_val = field_prep(reg->binary_data_fine_mask, f_elements);
>  	if (tmp_val & ~reg->binary_data_fine_mask) {
>  		dev_err(dev, "Masking overflow of fine elements %08x\n",
>  			tmp_val);
> @@ -260,7 +249,7 @@ static int ti_iodelay_pinconf_set(struct ti_iodelay_device *iod,
>  	 * impacting iodelay configuration. Use with care!
>  	 */
>  	reg_mask |= reg->lock_mask;
> -	reg_val |= reg->unlock_val << __ffs(reg->lock_mask);
> +	reg_val |= field_prep(reg->lock_mask, reg->unlock_val);
>  	r = regmap_update_bits(iod->regmap, cfg->offset, reg_mask, reg_val);
>  
>  	dev_dbg(dev, "Set reg 0x%x Delay(a: %d g: %d), Elements(C=%d F=%d)0x%x\n",
> @@ -296,16 +285,14 @@ static int ti_iodelay_pinconf_init_dev(struct ti_iodelay_device *iod)
>  	r = regmap_read(iod->regmap, reg->reg_refclk_offset, &val);
>  	if (r)
>  		return r;
> -	ival->ref_clk_period = ti_iodelay_extract(val, reg->refclk_period_mask);
> +	ival->ref_clk_period = field_get(reg->refclk_period_mask, val);
>  	dev_dbg(dev, "refclk_period=0x%04x\n", ival->ref_clk_period);
>  
>  	r = regmap_read(iod->regmap, reg->reg_coarse_offset, &val);
>  	if (r)
>  		return r;
> -	ival->coarse_ref_count =
> -	    ti_iodelay_extract(val, reg->coarse_ref_count_mask);
> -	ival->coarse_delay_count =
> -	    ti_iodelay_extract(val, reg->coarse_delay_count_mask);
> +	ival->coarse_ref_count = field_get(reg->coarse_ref_count_mask, val);
> +	ival->coarse_delay_count = field_get(reg->coarse_delay_count_mask, val);
>  	if (!ival->coarse_delay_count) {
>  		dev_err(dev, "Invalid Coarse delay count (0) (reg=0x%08x)\n",
>  			val);
> @@ -326,10 +313,8 @@ static int ti_iodelay_pinconf_init_dev(struct ti_iodelay_device *iod)
>  	r = regmap_read(iod->regmap, reg->reg_fine_offset, &val);
>  	if (r)
>  		return r;
> -	ival->fine_ref_count =
> -	    ti_iodelay_extract(val, reg->fine_ref_count_mask);
> -	ival->fine_delay_count =
> -	    ti_iodelay_extract(val, reg->fine_delay_count_mask);
> +	ival->fine_ref_count = field_get(reg->fine_ref_count_mask, val);
> +	ival->fine_delay_count = field_get(reg->fine_delay_count_mask, val);
>  	if (!ival->fine_delay_count) {
>  		dev_err(dev, "Invalid Fine delay count (0) (reg=0x%08x)\n",
>  			val);
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH/RFC 17/17] rtw89: Use bitfield helpers
  2021-11-22 15:54 ` [PATCH/RFC 17/17] rtw89: " Geert Uytterhoeven
@ 2021-11-22 18:38   ` Larry Finger
  0 siblings, 0 replies; 41+ messages in thread
From: Larry Finger @ 2021-11-22 18:38 UTC (permalink / raw)
  To: Geert Uytterhoeven, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel

On 11/22/21 09:54, Geert Uytterhoeven wrote:
> Use the field_{get,prep}() helpers, instead of open-coding the same
> operations.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Compile-tested only.
> Marked RFC, as this depends on [PATCH 01/17], but follows a different
> path to upstream.
> ---
>   drivers/net/wireless/realtek/rtw89/core.h | 38 ++++-------------------
>   1 file changed, 6 insertions(+), 32 deletions(-)

Tested-by: Larry Finger <Larry,Finger@lwfinger.net>

Larry

> 
> diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
> index c2885e4dd882f045..f9c0300ec373aaf2 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.h
> +++ b/drivers/net/wireless/realtek/rtw89/core.h
> @@ -2994,81 +2994,55 @@ rtw89_write32_clr(struct rtw89_dev *rtwdev, u32 addr, u32 bit)
>   static inline u32
>   rtw89_read32_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
>   {
> -	u32 shift = __ffs(mask);
> -	u32 orig;
> -	u32 ret;
> -
> -	orig = rtw89_read32(rtwdev, addr);
> -	ret = (orig & mask) >> shift;
> -
> -	return ret;
> +	return field_get(mask, rtw89_read32(rtwdev, addr));
>   }
>   
>   static inline u16
>   rtw89_read16_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
>   {
> -	u32 shift = __ffs(mask);
> -	u32 orig;
> -	u32 ret;
> -
> -	orig = rtw89_read16(rtwdev, addr);
> -	ret = (orig & mask) >> shift;
> -
> -	return ret;
> +	return field_get(mask, rtw89_read16(rtwdev, addr));
>   }
>   
>   static inline u8
>   rtw89_read8_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask)
>   {
> -	u32 shift = __ffs(mask);
> -	u32 orig;
> -	u32 ret;
> -
> -	orig = rtw89_read8(rtwdev, addr);
> -	ret = (orig & mask) >> shift;
> -
> -	return ret;
> +	return field_get(mask, rtw89_read8(rtwdev, addr));
>   }
>   
>   static inline void
>   rtw89_write32_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask, u32 data)
>   {
> -	u32 shift = __ffs(mask);
>   	u32 orig;
>   	u32 set;
>   
>   	WARN(addr & 0x3, "should be 4-byte aligned, addr = 0x%08x\n", addr);
>   
>   	orig = rtw89_read32(rtwdev, addr);
> -	set = (orig & ~mask) | ((data << shift) & mask);
> +	set = (orig & ~mask) | field_prep(mask, data);
>   	rtw89_write32(rtwdev, addr, set);
>   }
>   
>   static inline void
>   rtw89_write16_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask, u16 data)
>   {
> -	u32 shift;
>   	u16 orig, set;
>   
>   	mask &= 0xffff;
> -	shift = __ffs(mask);
>   
>   	orig = rtw89_read16(rtwdev, addr);
> -	set = (orig & ~mask) | ((data << shift) & mask);
> +	set = (orig & ~mask) | field_prep(mask, data);
>   	rtw89_write16(rtwdev, addr, set);
>   }
>   
>   static inline void
>   rtw89_write8_mask(struct rtw89_dev *rtwdev, u32 addr, u32 mask, u8 data)
>   {
> -	u32 shift;
>   	u8 orig, set;
>   
>   	mask &= 0xff;
> -	shift = __ffs(mask);
>   
>   	orig = rtw89_read8(rtwdev, addr);
> -	set = (orig & ~mask) | ((data << shift) & mask);
> +	set = (orig & ~mask) | field_prep(mask, data);
>   	rtw89_write8(rtwdev, addr, set);
>   }
>   
> 


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-22 16:32   ` Johannes Berg
@ 2021-11-23  1:17     ` Jakub Kicinski
  2021-11-23  8:36       ` Geert Uytterhoeven
  2021-11-23  1:52     ` Alex Elder
  2021-11-23  8:30     ` Geert Uytterhoeven
  2 siblings, 1 reply; 41+ messages in thread
From: Jakub Kicinski @ 2021-11-23  1:17 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Geert Uytterhoeven, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On Mon, 22 Nov 2021 17:32:43 +0100 Johannes Berg wrote:
> On Mon, 2021-11-22 at 16:53 +0100, Geert Uytterhoeven wrote:
> > The existing FIELD_{GET,PREP}() macros are limited to compile-time
> > constants.  However, it is very common to prepare or extract bitfield
> > elements where the bitfield mask is not a compile-time constant.
> 
> I'm not sure it's really a good idea to add a third API here?

+1

> We have the upper-case (constant) versions, and already
> {u32,...}_get_bits()/etc.
> 
> Also, you're using __ffs(), which doesn't work for 64-bit on 32-bit
> architectures (afaict), so that seems a bit awkward.
> 
> Maybe we can make {u32,...}_get_bits() be doing compile-time only checks
> if it is indeed a constant? The __field_overflow() usage is already only
> done if __builtin_constant_p(v), so I guess we can do the same with
> __bad_mask()?

Either that or add decomposition macros. Are compilers still really bad
at passing small structs by value?

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-22 16:32   ` Johannes Berg
  2021-11-23  1:17     ` Jakub Kicinski
@ 2021-11-23  1:52     ` Alex Elder
  2021-11-23  8:38       ` Geert Uytterhoeven
  2021-11-23  8:30     ` Geert Uytterhoeven
  2 siblings, 1 reply; 41+ messages in thread
From: Alex Elder @ 2021-11-23  1:52 UTC (permalink / raw)
  To: Johannes Berg, Geert Uytterhoeven, Tony Lindgren, Russell King,
	Rajendra Nayak, Paul Walmsley, Michael Turquette, Stephen Boyd,
	Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai
  Cc: linux-arm-kernel, linux-omap, linux-kernel, linux-clk,
	linux-renesas-soc, linux-iio, linux-media, linux-mmc,
	linux-aspeed, openbmc, linux-wireless, netdev, linux-gpio,
	linux-pm, alsa-devel

On 11/22/21 10:32 AM, Johannes Berg wrote:
> On Mon, 2021-11-22 at 16:53 +0100, Geert Uytterhoeven wrote:
>> The existing FIELD_{GET,PREP}() macros are limited to compile-time
>> constants.  However, it is very common to prepare or extract bitfield
>> elements where the bitfield mask is not a compile-time constant.
>>
> 
> I'm not sure it's really a good idea to add a third API here?
> 
> We have the upper-case (constant) versions, and already
> {u32,...}_get_bits()/etc.

I've used these a lot (and personally prefer the lower-case ones).

Your new macros don't do anything to ensure the field mask is
of the right form, which is basically:  (2 ^ width - 1) << shift

I really like the property that the field mask must be constant.

That being said, I've had to use some strange coding patterns
in order to adhere to the "const only" rule in a few cases.
So if you can come up with a satisfactory naming scheme I'm
all for it.

					-Alex



> Also, you're using __ffs(), which doesn't work for 64-bit on 32-bit
> architectures (afaict), so that seems a bit awkward.
> 
> Maybe we can make {u32,...}_get_bits() be doing compile-time only checks
> if it is indeed a constant? The __field_overflow() usage is already only
> done if __builtin_constant_p(v), so I guess we can do the same with
> __bad_mask()?
> 
> johannes
> 


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 00/17] Non-const bitfield helper conversions
  2021-11-22 17:50 ` [PATCH 00/17] Non-const bitfield helper conversions Alexandre Belloni
@ 2021-11-23  8:20   ` Geert Uytterhoeven
  0 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-23  8:20 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Ludovic Desroches, Tero Kristo, Jonathan Cameron,
	Lars-Peter Clausen, Lorenzo Bianconi, Benoit Parrot,
	Mauro Carvalho Chehab, Adrian Hunter, Andrew Jeffery,
	Ulf Hansson, Joel Stanley, Ping-Ke Shih, Kalle Valo,
	David S . Miller, Jakub Kicinski, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

Hi Alexandre,

On Mon, Nov 22, 2021 at 6:50 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 22/11/2021 16:53:53+0100, Geert Uytterhoeven wrote:
> > The existing FIELD_{GET,PREP}() macros are limited to compile-time
> > constants.  However, it is very common to prepare or extract bitfield
> > elements where the bitfield mask is not a compile-time constant.
> > To avoid this limitation, the AT91 clock driver already has its own
> > field_{prep,get}() macros.
>
> My understanding was that this (being compile time only) was actually
> done on purpose. Did I misunderstand?

Yes, it was done on purpose, to maximize type safety.

However, this imposes a severe limitation: we cannot use them in case
the mask is non-const (i.e. stored in some data structure).  This
is a quite common use-case, given the examples I found and converted,
and given you added field_{get,prep}() to the AT91 clock driver.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-22 16:32   ` Johannes Berg
  2021-11-23  1:17     ` Jakub Kicinski
  2021-11-23  1:52     ` Alex Elder
@ 2021-11-23  8:30     ` Geert Uytterhoeven
  2021-11-23 16:21       ` Johannes Berg
  2021-11-24  8:24       ` Kalle Valo
  2 siblings, 2 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-23  8:30 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai, linux-arm-kernel,
	linux-omap, linux-kernel, linux-clk, linux-renesas-soc,
	linux-iio, linux-media, linux-mmc, linux-aspeed, openbmc,
	linux-wireless, netdev, linux-gpio, linux-pm, alsa-devel

Hi Johannes,

On Mon, Nov 22, 2021 at 5:33 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2021-11-22 at 16:53 +0100, Geert Uytterhoeven wrote:
> > The existing FIELD_{GET,PREP}() macros are limited to compile-time
> > constants.  However, it is very common to prepare or extract bitfield
> > elements where the bitfield mask is not a compile-time constant.
> >
>
> I'm not sure it's really a good idea to add a third API here?
>
> We have the upper-case (constant) versions, and already
> {u32,...}_get_bits()/etc.

These don't work for non-const masks.

> Also, you're using __ffs(), which doesn't work for 64-bit on 32-bit
> architectures (afaict), so that seems a bit awkward.

That's a valid comment. Can be fixed by using a wrapper macro
that checks if typeof(mask) == u64, and uses an __ffs64() version when
needed.

> Maybe we can make {u32,...}_get_bits() be doing compile-time only checks
> if it is indeed a constant? The __field_overflow() usage is already only
> done if __builtin_constant_p(v), so I guess we can do the same with
> __bad_mask()?

Are all compilers smart enough to replace the division by
field_multiplier(field) by a shift?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23  1:17     ` Jakub Kicinski
@ 2021-11-23  8:36       ` Geert Uytterhoeven
  2021-11-23 16:24         ` Johannes Berg
  2021-11-23 23:39         ` Jakub Kicinski
  0 siblings, 2 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-23  8:36 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Johannes Berg, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

Hi Jakub,

On Tue, Nov 23, 2021 at 2:17 AM Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 22 Nov 2021 17:32:43 +0100 Johannes Berg wrote:
> > On Mon, 2021-11-22 at 16:53 +0100, Geert Uytterhoeven wrote:
> > > The existing FIELD_{GET,PREP}() macros are limited to compile-time
> > > constants.  However, it is very common to prepare or extract bitfield
> > > elements where the bitfield mask is not a compile-time constant.
> >
> > I'm not sure it's really a good idea to add a third API here?
>
> +1

Yeah, a smaller API is better.

> > We have the upper-case (constant) versions, and already
> > {u32,...}_get_bits()/etc.

TBH, I don't like the *_get_bits() API: in general, u32_get_bits() does
the same as FIELD_GET(), but the order of the parameters is different?
(*_replace_bits() seems to be useful, though)

That's why I picked field_{get,prep}().

> > Also, you're using __ffs(), which doesn't work for 64-bit on 32-bit
> > architectures (afaict), so that seems a bit awkward.
> >
> > Maybe we can make {u32,...}_get_bits() be doing compile-time only checks
> > if it is indeed a constant? The __field_overflow() usage is already only
> > done if __builtin_constant_p(v), so I guess we can do the same with
> > __bad_mask()?
>
> Either that or add decomposition macros. Are compilers still really bad
> at passing small structs by value?

Sorry, I don't get what you mean by adding decomposition macros.
Can you please elaborate?
Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23  1:52     ` Alex Elder
@ 2021-11-23  8:38       ` Geert Uytterhoeven
  0 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-23  8:38 UTC (permalink / raw)
  To: Alex Elder
  Cc: Johannes Berg, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai, linux-arm-kernel,
	linux-omap, linux-kernel, linux-clk, linux-renesas-soc,
	linux-iio, linux-media, linux-mmc, linux-aspeed, openbmc,
	linux-wireless, netdev, linux-gpio, linux-pm, alsa-devel

Hi Alex,

On Tue, Nov 23, 2021 at 2:52 AM Alex Elder <elder@ieee.org> wrote:
> On 11/22/21 10:32 AM, Johannes Berg wrote:
> > On Mon, 2021-11-22 at 16:53 +0100, Geert Uytterhoeven wrote:
> >> The existing FIELD_{GET,PREP}() macros are limited to compile-time
> >> constants.  However, it is very common to prepare or extract bitfield
> >> elements where the bitfield mask is not a compile-time constant.
> >
> > I'm not sure it's really a good idea to add a third API here?
> >
> > We have the upper-case (constant) versions, and already
> > {u32,...}_get_bits()/etc.
>
> I've used these a lot (and personally prefer the lower-case ones).
>
> Your new macros don't do anything to ensure the field mask is
> of the right form, which is basically:  (2 ^ width - 1) << shift

> I really like the property that the field mask must be constant.

That's correct. How to enforce that in the non-const case?
BUG()/WARN() is not an option ;-)

> That being said, I've had to use some strange coding patterns
> in order to adhere to the "const only" rule in a few cases.
> So if you can come up with a satisfactory naming scheme I'm
> all for it.

There are plenty of drivers that handle masks stored in a data
structure, so it would be good if they can use a suitable helper,
as open-coding is prone to errors.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH/RFC 16/17] ALSA: ice1724: Use bitfield helpers
  2021-11-22 15:54 ` [PATCH/RFC 16/17] ALSA: ice1724: " Geert Uytterhoeven
@ 2021-11-23 11:24   ` Takashi Iwai
  0 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2021-11-23 11:24 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai, linux-arm-kernel,
	linux-omap, linux-kernel, linux-clk, linux-renesas-soc,
	linux-iio, linux-media, linux-mmc, linux-aspeed, openbmc,
	linux-wireless, netdev, linux-gpio, linux-pm, alsa-devel

On Mon, 22 Nov 2021 16:54:09 +0100,
Geert Uytterhoeven wrote:
> 
> Use the field_{get,prep}() helpers, instead of open-coding the same
> operations.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Compile-tested only.
> Marked RFC, as this depends on [PATCH 01/17], but follows a different
> path to upstream.

Acked-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23  8:30     ` Geert Uytterhoeven
@ 2021-11-23 16:21       ` Johannes Berg
  2021-11-23 16:31         ` Geert Uytterhoeven
  2021-11-24  8:24       ` Kalle Valo
  1 sibling, 1 reply; 41+ messages in thread
From: Johannes Berg @ 2021-11-23 16:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai, linux-arm-kernel,
	linux-omap, linux-kernel, linux-clk, linux-renesas-soc,
	linux-iio, linux-media, linux-mmc, linux-aspeed, openbmc,
	linux-wireless, netdev, linux-gpio, linux-pm, alsa-devel

On Tue, 2021-11-23 at 09:30 +0100, Geert Uytterhoeven wrote:
> > We have the upper-case (constant) versions, and already
> > {u32,...}_get_bits()/etc.
> 
> These don't work for non-const masks.

Obviously, I know that. Still, just saying.

I'm actually in the opposite camp to you I guess - I much prefer the
typed versions (u32_get_bits() and friends) over the FIELD_GET() macros
that are more magic.

Mostly though that's because the typed ones also have le32_/be32_/...
variants, which are tremendously useful, and so I prefer to use them all
across. In fact, I have considered in the past to just remove the upper-
case macros entirely but ... no time I guess.

> > Also, you're using __ffs(), which doesn't work for 64-bit on 32-bit
> > architectures (afaict), so that seems a bit awkward.
> 
> That's a valid comment. Can be fixed by using a wrapper macro
> that checks if typeof(mask) == u64, and uses an __ffs64() version when
> needed.

You can't really do a typeof()==something, but you can check the size,
so yeah, that could be done.

> > Maybe we can make {u32,...}_get_bits() be doing compile-time only checks
> > if it is indeed a constant? The __field_overflow() usage is already only
> > done if __builtin_constant_p(v), so I guess we can do the same with
> > __bad_mask()?
> 
> Are all compilers smart enough to replace the division by
> field_multiplier(field) by a shift?

In the constant case they are, but you'd have to replace
field_multiplier() with the __ffs(), including the size check discussed
above. Then it's no longer a constant, and then I'm not so sure it would
actually be able to translate it, even if it's "1<<__ffs64(...)". I
guess you can check, or just change it to not use the division and
multiplication, but shifts/masks instead manually?

IOW - I would much prefer to make the type_get_bits() and friends work
for non-constant masks.

In fact, you have e.g. code in drivers/usb/chipidea/udc.c that does
things like cpu_to_le32(mul << __ffs(...)) - though in those cases it's
actually constant today, so you could already write it as
le32_encode_bits(...).

johannes

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23  8:36       ` Geert Uytterhoeven
@ 2021-11-23 16:24         ` Johannes Berg
  2021-11-23 23:49           ` Jakub Kicinski
  2021-11-23 23:39         ` Jakub Kicinski
  1 sibling, 1 reply; 41+ messages in thread
From: Johannes Berg @ 2021-11-23 16:24 UTC (permalink / raw)
  To: Geert Uytterhoeven, Jakub Kicinski
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On Tue, 2021-11-23 at 09:36 +0100, Geert Uytterhoeven wrote:


Ah, here's your comment wrt. which one is nicer :)

> > > We have the upper-case (constant) versions, and already
> > > {u32,...}_get_bits()/etc.
> 
> TBH, I don't like the *_get_bits() API: in general, u32_get_bits() does
> the same as FIELD_GET(), but the order of the parameters is different?

I don't really see how "the order of parameters is different" is a
downside? Yeah it means if you're used to FIELD_GET() then you'll
retrain, but ...?

> (*_replace_bits() seems to be useful, though)

Indeed.

Also as I said in my other mail, the le32/be32/... variants are
tremendously useful, and they fundamentally cannot be expressed with the
FIELD_GET() or field_get() macros. IMHO this is a clear advantage to the
typed versions, and if you ask me we should get rid of the FIELD_GETand
FIELD_PREP entirely - difficult now, but at least let's not propagate
that?

johannes

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23 16:21       ` Johannes Berg
@ 2021-11-23 16:31         ` Geert Uytterhoeven
  0 siblings, 0 replies; 41+ messages in thread
From: Geert Uytterhoeven @ 2021-11-23 16:31 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai, Linux ARM,
	open list:TI ETHERNET SWITCH DRIVER (CPSW),
	Linux Kernel Mailing List, linux-clk, Linux-Renesas, linux-iio,
	Linux Media Mailing List, Linux MMC List, linux-aspeed, openbmc,
	linux-wireless, netdev, open list:GPIO SUBSYSTEM, Linux PM list,
	ALSA Development Mailing List

Hi Johannes,

On Tue, Nov 23, 2021 at 5:21 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2021-11-23 at 09:30 +0100, Geert Uytterhoeven wrote:
> > > We have the upper-case (constant) versions, and already
> > > {u32,...}_get_bits()/etc.
> >
> > These don't work for non-const masks.
>
> Obviously, I know that. Still, just saying.
>
> I'm actually in the opposite camp to you I guess - I much prefer the
> typed versions (u32_get_bits() and friends) over the FIELD_GET() macros
> that are more magic.
>
> Mostly though that's because the typed ones also have le32_/be32_/...
> variants, which are tremendously useful, and so I prefer to use them all
> across. In fact, I have considered in the past to just remove the upper-
> case macros entirely but ... no time I guess.

OK, I have to think a bit about this.
FTR, initially I didn't like the FIELD_{GET,PREP}() macros neither ;-)

> In fact, you have e.g. code in drivers/usb/chipidea/udc.c that does
> things like cpu_to_le32(mul << __ffs(...)) - though in those cases it's
> actually constant today, so you could already write it as
> le32_encode_bits(...).

Yeah, there are lots of opportunities for improvement for
drivers/usb/chipidea/.  I didn't include a conversion patch for that
driver, as it led me too deep into the rabbit hole, and I wanted to
get something posted rather sooner than later...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH/RFC 07/17] iio: st_sensors: Use bitfield helpers
  2021-11-22 15:54 ` [PATCH/RFC 07/17] iio: st_sensors: " Geert Uytterhoeven
@ 2021-11-23 23:35   ` Linus Walleij
  0 siblings, 0 replies; 41+ messages in thread
From: Linus Walleij @ 2021-11-23 23:35 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On Mon, Nov 22, 2021 at 4:55 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> Use the field_prep() helper, instead of open-coding the same operation.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Clever!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23  8:36       ` Geert Uytterhoeven
  2021-11-23 16:24         ` Johannes Berg
@ 2021-11-23 23:39         ` Jakub Kicinski
  1 sibling, 0 replies; 41+ messages in thread
From: Jakub Kicinski @ 2021-11-23 23:39 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Johannes Berg, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On Tue, 23 Nov 2021 09:36:22 +0100 Geert Uytterhoeven wrote:
> > > Also, you're using __ffs(), which doesn't work for 64-bit on 32-bit
> > > architectures (afaict), so that seems a bit awkward.
> > >
> > > Maybe we can make {u32,...}_get_bits() be doing compile-time only checks
> > > if it is indeed a constant? The __field_overflow() usage is already only
> > > done if __builtin_constant_p(v), so I guess we can do the same with
> > > __bad_mask()?  
> >
> > Either that or add decomposition macros. Are compilers still really bad
> > at passing small structs by value?  
> 
> Sorry, I don't get what you mean by adding decomposition macros.
> Can you please elaborate?

#define DECOMPOSE(_mask) \
  (struct bf){ .mask = _mask, .shf = __bf_shf(_mask), }

Then drivers can save or pass around the mask and shift params 
broken apart as a small struct.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23 16:24         ` Johannes Berg
@ 2021-11-23 23:49           ` Jakub Kicinski
  2021-11-24  8:03             ` Johannes Berg
  0 siblings, 1 reply; 41+ messages in thread
From: Jakub Kicinski @ 2021-11-23 23:49 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Geert Uytterhoeven, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On Tue, 23 Nov 2021 17:24:15 +0100 Johannes Berg wrote:
> > (*_replace_bits() seems to be useful, though)  
> 
> Indeed.
> 
> Also as I said in my other mail, the le32/be32/... variants are
> tremendously useful, and they fundamentally cannot be expressed with the
> FIELD_GET() or field_get() macros. IMHO this is a clear advantage to the

Can you elaborate?

> typed versions, and if you ask me we should get rid of the FIELD_GETand
> FIELD_PREP entirely - difficult now, but at least let's not propagate
> that?

I don't see why.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23 23:49           ` Jakub Kicinski
@ 2021-11-24  8:03             ` Johannes Berg
  2021-11-24 13:59               ` Jakub Kicinski
  0 siblings, 1 reply; 41+ messages in thread
From: Johannes Berg @ 2021-11-24  8:03 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Geert Uytterhoeven, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On Tue, 2021-11-23 at 15:49 -0800, Jakub Kicinski wrote:
> On Tue, 23 Nov 2021 17:24:15 +0100 Johannes Berg wrote:
> > > (*_replace_bits() seems to be useful, though)  
> > 
> > Indeed.
> > 
> > Also as I said in my other mail, the le32/be32/... variants are
> > tremendously useful, and they fundamentally cannot be expressed with the
> > FIELD_GET() or field_get() macros. IMHO this is a clear advantage to the
> 
> Can you elaborate?

Well, the way I see it, the only advantage of FIELD_GET() is that it
will auto-determine the type (based on the mask type.) This cannot work
if you need be/le conversions, because the be/le type annotations are
invisible to the compiler.

So obviously you could write a BE32_FIELD_GET(), but then really that's
equivalent to be32_get_bits() - note you you have to actually specify
the type in the macro name. I guess in theory you could make macros
where the type is an argument (like FIELD_GET_TYPE(be32, ...)), but I
don't see how that gains anything.

> > typed versions, and if you ask me we should get rid of the FIELD_GETand
> > FIELD_PREP entirely - difficult now, but at least let's not propagate
> > that?
> 
> I don't see why.

Just for being more regular, in the spirit of "there's exactly one
correct way of doing it" :)

johannes

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-23  8:30     ` Geert Uytterhoeven
  2021-11-23 16:21       ` Johannes Berg
@ 2021-11-24  8:24       ` Kalle Valo
  1 sibling, 0 replies; 41+ messages in thread
From: Kalle Valo @ 2021-11-24  8:24 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Johannes Berg, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	David S . Miller, Jakub Kicinski, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Johannes,
>
> On Mon, Nov 22, 2021 at 5:33 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Mon, 2021-11-22 at 16:53 +0100, Geert Uytterhoeven wrote:
>> > The existing FIELD_{GET,PREP}() macros are limited to compile-time
>> > constants.  However, it is very common to prepare or extract bitfield
>> > elements where the bitfield mask is not a compile-time constant.
>> >
>>
>> I'm not sure it's really a good idea to add a third API here?
>>
>> We have the upper-case (constant) versions, and already
>> {u32,...}_get_bits()/etc.
>
> These don't work for non-const masks.
>
>> Also, you're using __ffs(), which doesn't work for 64-bit on 32-bit
>> architectures (afaict), so that seems a bit awkward.
>
> That's a valid comment. Can be fixed by using a wrapper macro
> that checks if typeof(mask) == u64, and uses an __ffs64() version when
> needed.
>
>> Maybe we can make {u32,...}_get_bits() be doing compile-time only checks
>> if it is indeed a constant? The __field_overflow() usage is already only
>> done if __builtin_constant_p(v), so I guess we can do the same with
>> __bad_mask()?
>
> Are all compilers smart enough to replace the division by
> field_multiplier(field) by a shift?

It looks like the answer is no as few weeks back I received a comment
internally that a team is seeing a slow down with u32_get_bits():

"Time taken for executing both the macros/inline function (in terms of microseconds)
(out of 3 Trails)
FIELD_GET	: 32, 31, 32
u32_get_bits	: 6379, 6664, 6558"

Sadly I didn't realise to ask what compiler they were using. But I still
prefer {u32,...}_get_bits() over FIELD_GET(), they are just so much
cleaner to use.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-24  8:03             ` Johannes Berg
@ 2021-11-24 13:59               ` Jakub Kicinski
  2021-11-24 14:07                 ` Johannes Berg
  0 siblings, 1 reply; 41+ messages in thread
From: Jakub Kicinski @ 2021-11-24 13:59 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Geert Uytterhoeven, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On Wed, 24 Nov 2021 09:03:24 +0100 Johannes Berg wrote:
> On Tue, 2021-11-23 at 15:49 -0800, Jakub Kicinski wrote:
> > > Indeed.
> > > 
> > > Also as I said in my other mail, the le32/be32/... variants are
> > > tremendously useful, and they fundamentally cannot be expressed with the
> > > FIELD_GET() or field_get() macros. IMHO this is a clear advantage to the  
> > 
> > Can you elaborate?  
> 
> Well, the way I see it, the only advantage of FIELD_GET() is that it
> will auto-determine the type (based on the mask type.) This cannot work
> if you need be/le conversions, because the be/le type annotations are
> invisible to the compiler.
> 
> So obviously you could write a BE32_FIELD_GET(), but then really that's
> equivalent to be32_get_bits() - note you you have to actually specify
> the type in the macro name. I guess in theory you could make macros
> where the type is an argument (like FIELD_GET_TYPE(be32, ...)), but I
> don't see how that gains anything.

Ah, that's what you meant! Thanks for spelling it out.

FWIW I never found the be/le versions useful. Most of the time the data
comes from bus accessors which swap or is unaligned so you have to do
be/le_get_unaligned, which swaps. Plus if you access/set multiple
fields you'd swap them one by one which seems wasteful.

> > > typed versions, and if you ask me we should get rid of the FIELD_GETand
> > > FIELD_PREP entirely - difficult now, but at least let's not propagate
> > > that?  
> > 
> > I don't see why.  
> 
> Just for being more regular, in the spirit of "there's exactly one
> correct way of doing it" :)

Right now it seems the uppercase macros are more prevalent.

Could just be because of the way the "swapping ones" are defined.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers
  2021-11-24 13:59               ` Jakub Kicinski
@ 2021-11-24 14:07                 ` Johannes Berg
  0 siblings, 0 replies; 41+ messages in thread
From: Johannes Berg @ 2021-11-24 14:07 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Geert Uytterhoeven, Tony Lindgren, Russell King, Rajendra Nayak,
	Paul Walmsley, Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Linus Walleij, Liam Girdwood,
	Mark Brown, Magnus Damm, Eduardo Valentin, Keerthy,
	Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria, Zhang Rui,
	Jaroslav Kysela, Takashi Iwai, linux-arm-kernel, linux-omap,
	linux-kernel, linux-clk, linux-renesas-soc, linux-iio,
	linux-media, linux-mmc, linux-aspeed, openbmc, linux-wireless,
	netdev, linux-gpio, linux-pm, alsa-devel

On Wed, 2021-11-24 at 05:59 -0800, Jakub Kicinski wrote:
> 
> FWIW I never found the be/le versions useful. Most of the time the data
> comes from bus accessors which swap or is unaligned so you have to do
> be/le_get_unaligned, which swaps. Plus if you access/set multiple
> fields you'd swap them one by one which seems wasteful.

Oh, we use them all the time in wifi!

I'm not sure I'm too concerned about wasteful - actually in wifi most of
the time it's little endian to start with, which matches the CPU for all
practical uses of wifi (**), and often we just access one field or so.
And anyway if we extract more than a single bit we need to swap anyway,
and I hope if it's just a single bit the compiler will optimize since
the one side is a constant? But whatever ...

(**) I had a fight with big-endian ARM a few years ago just to get wifi
tested on big-endian ...


> Right now it seems the uppercase macros are more prevalent.
> 

Not in my world ;-)

$ git grep FIELD_GET -- ... | wc -l
20
$ git grep le32_get_bits -- ... | wc -l
44
$ git grep le16_get_bits -- ... | wc -l
12
$ git grep u8_get_bits -- ... | wc -l
17

:-)

johannes

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH/RFC 08/17] iio: humidity: hts221: Use bitfield helpers
  2021-11-22 15:54 ` [PATCH/RFC 08/17] iio: humidity: hts221: " Geert Uytterhoeven
@ 2021-11-24 15:21   ` Jonathan Cameron
  0 siblings, 0 replies; 41+ messages in thread
From: Jonathan Cameron @ 2021-11-24 15:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Tony Lindgren, Russell King, Rajendra Nayak, Paul Walmsley,
	Michael Turquette, Stephen Boyd, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches, Tero Kristo,
	Jonathan Cameron, Lars-Peter Clausen, Lorenzo Bianconi,
	Benoit Parrot, Mauro Carvalho Chehab, Adrian Hunter,
	Andrew Jeffery, Ulf Hansson, Joel Stanley, Ping-Ke Shih,
	Kalle Valo, David S . Miller, Jakub Kicinski, Linus Walleij,
	Liam Girdwood, Mark Brown, Magnus Damm, Eduardo Valentin,
	Keerthy, Rafael J . Wysocki, Daniel Lezcano, Amit Kucheria,
	Zhang Rui, Jaroslav Kysela, Takashi Iwai, linux-arm-kernel,
	linux-omap, linux-kernel, linux-clk, linux-renesas-soc,
	linux-iio, linux-media, linux-mmc, linux-aspeed, openbmc,
	linux-wireless, netdev, linux-gpio, linux-pm, alsa-devel

On Mon, 22 Nov 2021 16:54:01 +0100
Geert Uytterhoeven <geert+renesas@glider.be> wrote:

> Use the field_prep() helper, instead of open-coding the same operation.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Hi Geert,

If this should got forwards, looks like a nice cleanup for the two IIO
ones, so I'll be happy to pick them up once infrastructure in place
(ideally have the infrastructure an immutable branch to save having
to revisit in 3+ months time!)

Jonathan

> ---
> Compile-tested only.
> Marked RFC, as this depends on [PATCH 01/17], but follows a different
> path to upstream.
> ---
>  drivers/iio/humidity/hts221_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index 6a39615b696114cd..749aedc469ede5c1 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -7,6 +7,7 @@
>   * Lorenzo Bianconi <lorenzo.bianconi@st.com>
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/device.h>
> @@ -171,7 +172,7 @@ static int hts221_update_avg(struct hts221_hw *hw,
>  			     u16 val)
>  {
>  	const struct hts221_avg *avg = &hts221_avg_list[type];
> -	int i, err, data;
> +	int i, err;
>  
>  	for (i = 0; i < HTS221_AVG_DEPTH; i++)
>  		if (avg->avg_avl[i] == val)
> @@ -180,9 +181,8 @@ static int hts221_update_avg(struct hts221_hw *hw,
>  	if (i == HTS221_AVG_DEPTH)
>  		return -EINVAL;
>  
> -	data = ((i << __ffs(avg->mask)) & avg->mask);
> -	err = regmap_update_bits(hw->regmap, avg->addr,
> -				 avg->mask, data);
> +	err = regmap_update_bits(hw->regmap, avg->addr, avg->mask,
> +				 field_prep(avg->mask, i));
>  	if (err < 0)
>  		return err;
>  


^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2021-11-24 15:21 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22 15:53 [PATCH 00/17] Non-const bitfield helper conversions Geert Uytterhoeven
2021-11-22 15:53 ` [PATCH 01/17] bitfield: Add non-constant field_{prep,get}() helpers Geert Uytterhoeven
2021-11-22 16:32   ` Johannes Berg
2021-11-23  1:17     ` Jakub Kicinski
2021-11-23  8:36       ` Geert Uytterhoeven
2021-11-23 16:24         ` Johannes Berg
2021-11-23 23:49           ` Jakub Kicinski
2021-11-24  8:03             ` Johannes Berg
2021-11-24 13:59               ` Jakub Kicinski
2021-11-24 14:07                 ` Johannes Berg
2021-11-23 23:39         ` Jakub Kicinski
2021-11-23  1:52     ` Alex Elder
2021-11-23  8:38       ` Geert Uytterhoeven
2021-11-23  8:30     ` Geert Uytterhoeven
2021-11-23 16:21       ` Johannes Berg
2021-11-23 16:31         ` Geert Uytterhoeven
2021-11-24  8:24       ` Kalle Valo
2021-11-22 15:53 ` [PATCH 02/17] clk: renesas: Use bitfield helpers Geert Uytterhoeven
2021-11-22 15:53 ` [PATCH/RFC 03/17] soc: " Geert Uytterhoeven
2021-11-22 15:53 ` [PATCH/RFC 04/17] ARM: OMAP2+: " Geert Uytterhoeven
2021-11-22 15:53 ` [PATCH/RFC 05/17] bus: omap_l3_noc: " Geert Uytterhoeven
2021-11-22 15:53 ` [PATCH/RFC 06/17] clk: ti: " Geert Uytterhoeven
2021-11-22 15:54 ` [PATCH/RFC 07/17] iio: st_sensors: " Geert Uytterhoeven
2021-11-23 23:35   ` Linus Walleij
2021-11-22 15:54 ` [PATCH/RFC 08/17] iio: humidity: hts221: " Geert Uytterhoeven
2021-11-24 15:21   ` Jonathan Cameron
2021-11-22 15:54 ` [PATCH/RFC 09/17] iio: imu: st_lsm6dsx: " Geert Uytterhoeven
2021-11-22 15:54 ` [PATCH/RFC 10/17] media: ti-vpe: cal: " Geert Uytterhoeven
2021-11-22 15:54 ` [PATCH/RFC 11/17] mmc: sdhci-of-aspeed: " Geert Uytterhoeven
2021-11-22 15:54 ` [PATCH/RFC 12/17] pinctrl: aspeed: " Geert Uytterhoeven
2021-11-22 15:54 ` [PATCH/RFC 13/17] pinctl: ti: iodelay: " Geert Uytterhoeven
2021-11-22 17:52   ` Alexandre Belloni
2021-11-22 15:54 ` [PATCH/RFC 14/17] regulator: ti-abb: " Geert Uytterhoeven
2021-11-22 16:31   ` Mark Brown
2021-11-22 15:54 ` [PATCH/RFC 15/17] thermal/ti-soc-thermal: " Geert Uytterhoeven
2021-11-22 15:54 ` [PATCH/RFC 16/17] ALSA: ice1724: " Geert Uytterhoeven
2021-11-23 11:24   ` Takashi Iwai
2021-11-22 15:54 ` [PATCH/RFC 17/17] rtw89: " Geert Uytterhoeven
2021-11-22 18:38   ` Larry Finger
2021-11-22 17:50 ` [PATCH 00/17] Non-const bitfield helper conversions Alexandre Belloni
2021-11-23  8:20   ` Geert Uytterhoeven

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).