linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 06/21] mfd: prepare to generalize BITS() macro
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
@ 2025-10-25 16:28 ` Yury Norov (NVIDIA)
  2025-11-06 16:48   ` Lee Jones
  2025-10-25 16:32 ` [PATCH 08/21] mfd: drop local " Yury Norov (NVIDIA)
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:28 UTC (permalink / raw)
  To: Linus Torvalds, Linus Walleij, Nicolas Frattaroli, Lee Jones,
	linux-arm-kernel, linux-kernel
  Cc: Yury Norov (NVIDIA), Rasmus Villemoes

In preparation for adding generic BITS() macro, add an #undef directive
for the existing BITS(). The following patches will drop it entirely.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 drivers/mfd/db8500-prcmu-regs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/db8500-prcmu-regs.h b/drivers/mfd/db8500-prcmu-regs.h
index 75fd1069372c..25d2d5966211 100644
--- a/drivers/mfd/db8500-prcmu-regs.h
+++ b/drivers/mfd/db8500-prcmu-regs.h
@@ -12,6 +12,7 @@
 #ifndef __DB8500_PRCMU_REGS_H
 #define __DB8500_PRCMU_REGS_H
 
+#undef BITS
 #define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))
 
 #define PRCM_ACLK_MGT		(0x004)
-- 
2.43.0



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

* [PATCH 08/21] mfd: drop local BITS() macro
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
  2025-10-25 16:28 ` [PATCH 06/21] mfd: prepare to generalize BITS() macro Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-27 22:21   ` Linus Walleij
  2025-10-25 16:32 ` [PATCH 09/21] bits: generalize BITMAP_{FIRST,LAST}_WORD_MASK Yury Norov (NVIDIA)
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel
  Cc: Yury Norov (NVIDIA)

Now that generic BITS() is introduced, drop the local one.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 drivers/mfd/db8500-prcmu-regs.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mfd/db8500-prcmu-regs.h b/drivers/mfd/db8500-prcmu-regs.h
index 25d2d5966211..c2bbf325efb9 100644
--- a/drivers/mfd/db8500-prcmu-regs.h
+++ b/drivers/mfd/db8500-prcmu-regs.h
@@ -12,9 +12,6 @@
 #ifndef __DB8500_PRCMU_REGS_H
 #define __DB8500_PRCMU_REGS_H
 
-#undef BITS
-#define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))
-
 #define PRCM_ACLK_MGT		(0x004)
 #define PRCM_SVAMMCSPCLK_MGT	(0x008)
 #define PRCM_SIAMMDSPCLK_MGT	(0x00C)
-- 
2.43.0



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

* [PATCH 09/21] bits: generalize BITMAP_{FIRST,LAST}_WORD_MASK
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
  2025-10-25 16:28 ` [PATCH 06/21] mfd: prepare to generalize BITS() macro Yury Norov (NVIDIA)
  2025-10-25 16:32 ` [PATCH 08/21] mfd: drop local " Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-25 16:32 ` [PATCH 10/21] i2c: nomadik: don't use GENMASK() Yury Norov (NVIDIA)
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Yury Norov, Rasmus Villemoes

The macros are helpful in many non-bitmap places too. Move them from
bitmap.h to bits.h.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 include/linux/bitmap.h | 7 +++++--
 include/linux/bits.h   | 8 ++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 595217b7a6e7..fbe2d12bceab 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -223,8 +223,11 @@ void bitmap_onto(unsigned long *dst, const unsigned long *orig,
 void bitmap_fold(unsigned long *dst, const unsigned long *orig,
 		unsigned int sz, unsigned int nbits);
 
-#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
-#define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
+#define BITMAP_FIRST_WORD_MASK(start) LAST_BITS(start)
+#define BITMAP_LAST_WORD_MASK(nbits) FIRST_BITS(nbits)
+
+#define BITMAP_FIRST_WORD_MASK_ULL(start) LAST_BITS(start)
+#define BITMAP_LAST_WORD_MASK_ULL(nbits) FIRST_BITS(nbits)
 
 #define bitmap_size(nbits)	(ALIGN(nbits, BITS_PER_LONG) / BITS_PER_BYTE)
 
diff --git a/include/linux/bits.h b/include/linux/bits.h
index c7c587e90e2d..0d2950b80a3b 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -5,6 +5,14 @@
 #include <vdso/bits.h>
 #include <uapi/linux/bits.h>
 
+/* Mask with first nbist set */
+#define FIRST_BITS(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
+#define FIRST_BITS_ULL(nbits) (~0ULL >> (-(nbits) & (BITS_PER_LONG_LONG - 1)))
+
+/* Mask with all bits before start unset */
+#define LAST_BITS(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
+#define LAST_BITS_ULL(start) (~0ULL << ((start) & (BITS_PER_LONG_LONG - 1)))
+
 #define BIT_MASK(nr)		(UL(1) << ((nr) % BITS_PER_LONG))
 #define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
 #define BIT_ULL_MASK(nr)	(ULL(1) << ((nr) % BITS_PER_LONG_LONG))
-- 
2.43.0



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

* [PATCH 10/21] i2c: nomadik: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (2 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 09/21] bits: generalize BITMAP_{FIRST,LAST}_WORD_MASK Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-27 22:22   ` Linus Walleij
  2025-10-25 16:32 ` [PATCH 11/21] drivers: don't use GENMASK() in FIELD_PREP_WM16() Yury Norov (NVIDIA)
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Andi Shyti, linux-i2c
  Cc: Yury Norov (NVIDIA), Linus Torvalds

GENMASK(high, low) notation is confusing. Switch to BITS() or FIRST_BITS()
where appropriate.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ@mail.gmail.com/
Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 44 ++++++++++++++++----------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 19b648fc094d..4c79ada5e1d4 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -53,9 +53,9 @@
 
 /* Control registers */
 #define I2C_CR_PE		BIT(0)		/* Peripheral Enable */
-#define I2C_CR_OM		GENMASK(2, 1)	/* Operating mode */
+#define I2C_CR_OM		BITS(1, 2)	/* Operating mode */
 #define I2C_CR_SAM		BIT(3)		/* Slave addressing mode */
-#define I2C_CR_SM		GENMASK(5, 4)	/* Speed mode */
+#define I2C_CR_SM		BITS(4, 5)	/* Speed mode */
 #define I2C_CR_SGCM		BIT(6)		/* Slave general call mode */
 #define I2C_CR_FTX		BIT(7)		/* Flush Transmit */
 #define I2C_CR_FRX		BIT(8)		/* Flush Receive */
@@ -63,31 +63,31 @@
 #define I2C_CR_DMA_RX_EN	BIT(10)		/* DMA Rx Enable */
 #define I2C_CR_DMA_SLE		BIT(11)		/* DMA sync. logic enable */
 #define I2C_CR_LM		BIT(12)		/* Loopback mode */
-#define I2C_CR_FON		GENMASK(14, 13)	/* Filtering on */
-#define I2C_CR_FS		GENMASK(16, 15)	/* Force stop enable */
+#define I2C_CR_FON		BITS(13, 14)	/* Filtering on */
+#define I2C_CR_FS		BITS(15, 16)	/* Force stop enable */
 
 /* Slave control register (SCR) */
-#define I2C_SCR_SLSU		GENMASK(31, 16)	/* Slave data setup time */
+#define I2C_SCR_SLSU		BITS(16, 31)	/* Slave data setup time */
 
 /* Master controller (MCR) register */
 #define I2C_MCR_OP		BIT(0)		/* Operation */
-#define I2C_MCR_A7		GENMASK(7, 1)	/* 7-bit address */
-#define I2C_MCR_EA10		GENMASK(10, 8)	/* 10-bit Extended address */
+#define I2C_MCR_A7		BITS(1, 7)	/* 7-bit address */
+#define I2C_MCR_EA10		BITS(8, 10)	/* 10-bit Extended address */
 #define I2C_MCR_SB		BIT(11)		/* Extended address */
-#define I2C_MCR_AM		GENMASK(13, 12)	/* Address type */
+#define I2C_MCR_AM		BITS(12, 13)	/* Address type */
 #define I2C_MCR_STOP		BIT(14)		/* Stop condition */
-#define I2C_MCR_LENGTH		GENMASK(25, 15)	/* Transaction length */
+#define I2C_MCR_LENGTH		BITS(15, 25)	/* Transaction length */
 
 /* Status register (SR) */
-#define I2C_SR_OP		GENMASK(1, 0)	/* Operation */
-#define I2C_SR_STATUS		GENMASK(3, 2)	/* controller status */
-#define I2C_SR_CAUSE		GENMASK(6, 4)	/* Abort cause */
-#define I2C_SR_TYPE		GENMASK(8, 7)	/* Receive type */
-#define I2C_SR_LENGTH		GENMASK(19, 9)	/* Transfer length */
+#define I2C_SR_OP		BITS(0, 1)	/* Operation */
+#define I2C_SR_STATUS		BITS(2, 3)	/* controller status */
+#define I2C_SR_CAUSE		BITS(4, 6)	/* Abort cause */
+#define I2C_SR_TYPE		BITS(7, 8)	/* Receive type */
+#define I2C_SR_LENGTH		BITS(9, 19)	/* Transfer length */
 
 /* Baud-rate counter register (BRCR) */
-#define I2C_BRCR_BRCNT1		GENMASK(31, 16)	/* Baud-rate counter 1 */
-#define I2C_BRCR_BRCNT2		GENMASK(15, 0)	/* Baud-rate counter 2 */
+#define I2C_BRCR_BRCNT2		FIRST_BITS(16)	/* Baud-rate counter 2 */
+#define I2C_BRCR_BRCNT1		BITS(16, 31)	/* Baud-rate counter 1 */
 
 /* Interrupt mask set/clear (IMSCR) bits */
 #define I2C_IT_TXFE		BIT(0)
@@ -339,7 +339,7 @@ static int init_hw(struct nmk_i2c_dev *priv)
 #define DEFAULT_I2C_REG_CR	(FIELD_PREP(I2C_CR_OM, I2C_OM_MASTER) | I2C_CR_PE)
 
 /* grab top three bits from extended I2C addresses */
-#define ADR_3MSB_BITS		GENMASK(9, 7)
+#define ADR_3MSB_BITS		BITS(7, 9)
 
 /**
  * load_i2c_mcr_reg() - load the MCR register
@@ -1028,11 +1028,11 @@ static void nmk_i2c_of_probe(struct device_node *np,
 }
 
 static const unsigned int nmk_i2c_eyeq5_masks[] = {
-	GENMASK(5, 4),
-	GENMASK(7, 6),
-	GENMASK(9, 8),
-	GENMASK(11, 10),
-	GENMASK(13, 12),
+	BITS(4, 5),
+	BITS(6, 7),
+	BITS(8, 9),
+	BITS(10, 11),
+	BITS(12, 13),
 };
 
 static int nmk_i2c_eyeq5_probe(struct nmk_i2c_dev *priv)
-- 
2.43.0



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

* [PATCH 11/21] drivers: don't use GENMASK() in FIELD_PREP_WM16()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (3 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 10/21] i2c: nomadik: don't use GENMASK() Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-25 16:32 ` [PATCH 12/21] bitmap: don't use GENMASK() Yury Norov (NVIDIA)
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Sandy Huang, Heiko Stübner, Andy Yan, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Shreeya Patel, Mauro Carvalho Chehab, Jaehoon Chung, Ulf Hansson,
	Nicolas Frattaroli, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Yury Norov (NVIDIA), dri-devel, linux-rockchip,
	linux-media, kernel, linux-mmc, linux-sound
  Cc: Linus Torvalds

Recently added FIELD_PREP_WM16() in a few places uses GENMASK. It's
confusing and may mislead readers. Switch to BITS() or FIRST_BITS()
as appropriate.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Link: https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ@mail.gmail.com/
Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 drivers/gpu/drm/rockchip/rockchip_lvds.h             | 2 +-
 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c         | 4 ++--
 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h | 4 ++--
 drivers/mmc/host/dw_mmc-rockchip.c                   | 4 ++--
 drivers/soc/rockchip/grf.c                           | 4 ++--
 sound/soc/rockchip/rockchip_i2s_tdm.h                | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
index 2d92447d819b..e79e6031be59 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
@@ -115,7 +115,7 @@
 #define   PX30_LVDS_INVERT_DCLK(val)		FIELD_PREP_WM16(BIT(5), (val))
 
 #define PX30_LVDS_GRF_PD_VO_CON1		0x438
-#define   PX30_LVDS_FORMAT(val)			FIELD_PREP_WM16(GENMASK(14, 13), (val))
+#define   PX30_LVDS_FORMAT(val)			FIELD_PREP_WM16(BITS(13, 14), (val))
 #define   PX30_LVDS_MODE_EN(val)		FIELD_PREP_WM16(BIT(12), (val))
 #define   PX30_LVDS_MSBSEL(val)			FIELD_PREP_WM16(BIT(11), (val))
 #define   PX30_LVDS_P2S_EN(val)			FIELD_PREP_WM16(BIT(6), (val))
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
index 38c49030c7ab..438fea5f6f6d 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
@@ -1698,7 +1698,7 @@ static unsigned long rk3588_set_intf_mux(struct vop2_video_port *vp, int id, u32
 		val = rk3588_get_hdmi_pol(polflags);
 		regmap_write(vop2->vop_grf, RK3588_GRF_VOP_CON2, FIELD_PREP_WM16(BIT(1), 1));
 		regmap_write(vop2->vo1_grf, RK3588_GRF_VO1_CON0,
-			     FIELD_PREP_WM16(GENMASK(6, 5), val));
+			     FIELD_PREP_WM16(BITS(5, 6), val));
 		break;
 	case ROCKCHIP_VOP2_EP_HDMI1:
 		div &= ~RK3588_DSP_IF_EDP_HDMI1_DCLK_DIV;
@@ -1711,7 +1711,7 @@ static unsigned long rk3588_set_intf_mux(struct vop2_video_port *vp, int id, u32
 		val = rk3588_get_hdmi_pol(polflags);
 		regmap_write(vop2->vop_grf, RK3588_GRF_VOP_CON2, FIELD_PREP_WM16(BIT(4), 1));
 		regmap_write(vop2->vo1_grf, RK3588_GRF_VO1_CON0,
-			     FIELD_PREP_WM16(GENMASK(8, 7), val));
+			     FIELD_PREP_WM16(BITS(7, 8), val));
 		break;
 	case ROCKCHIP_VOP2_EP_EDP0:
 		div &= ~RK3588_DSP_IF_EDP_HDMI0_DCLK_DIV;
diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
index b13f58e31944..14df3f53ff8f 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
@@ -12,8 +12,8 @@
 #include <linux/bitops.h>
 #include <linux/hw_bitfield.h>
 
-#define UPDATE(x, h, l)		FIELD_PREP(GENMASK((h), (l)), (x))
-#define HIWORD_UPDATE(v, h, l)	FIELD_PREP_WM16(GENMASK((h), (l)), (v))
+#define UPDATE(x, h, l)		FIELD_PREP(BITS((l), (h)), (x))
+#define HIWORD_UPDATE(v, h, l)	FIELD_PREP_WM16(BITS((l), (h)), (v))
 
 /* SYS_GRF */
 #define SYS_GRF_SOC_CON1			0x0304
diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index 82dd906bb002..7fac1a7281bf 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -148,10 +148,10 @@ static int rockchip_mmc_set_internal_phase(struct dw_mci *host, bool sample, int
 
 	if (sample)
 		mci_writel(host, TIMING_CON1,
-			   FIELD_PREP_WM16(GENMASK(11, 1), raw_value));
+			   FIELD_PREP_WM16(BITS(1, 11), raw_value));
 	else
 		mci_writel(host, TIMING_CON0,
-			   FIELD_PREP_WM16(GENMASK(11, 1), raw_value));
+			   FIELD_PREP_WM16(BITS(1, 11), raw_value));
 
 	dev_dbg(host->dev, "set %s_phase(%d) delay_nums=%u actual_degrees=%d\n",
 		sample ? "sample" : "drv", degrees, delay_num,
diff --git a/drivers/soc/rockchip/grf.c b/drivers/soc/rockchip/grf.c
index 344870da7675..89fd4a4c69eb 100644
--- a/drivers/soc/rockchip/grf.c
+++ b/drivers/soc/rockchip/grf.c
@@ -125,8 +125,8 @@ static const struct rockchip_grf_info rk3566_pipegrf __initconst = {
 #define RK3576_SYSGRF_SOC_CON1		0x0004
 
 static const struct rockchip_grf_value rk3576_defaults_sys_grf[] __initconst = {
-	{ "i3c0 weakpull", RK3576_SYSGRF_SOC_CON1, FIELD_PREP_WM16_CONST(GENMASK(7, 6), 3) },
-	{ "i3c1 weakpull", RK3576_SYSGRF_SOC_CON1, FIELD_PREP_WM16_CONST(GENMASK(9, 8), 3) },
+	{ "i3c0 weakpull", RK3576_SYSGRF_SOC_CON1, FIELD_PREP_WM16_CONST(BITS(6, 7), 3) },
+	{ "i3c1 weakpull", RK3576_SYSGRF_SOC_CON1, FIELD_PREP_WM16_CONST(BITS(8, 9), 3) },
 };
 
 static const struct rockchip_grf_info rk3576_sysgrf __initconst = {
diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.h b/sound/soc/rockchip/rockchip_i2s_tdm.h
index 0171e05ee886..eee6db372ee7 100644
--- a/sound/soc/rockchip/rockchip_i2s_tdm.h
+++ b/sound/soc/rockchip/rockchip_i2s_tdm.h
@@ -287,7 +287,7 @@ enum {
 #define I2S_TDM_RXCR	(0x0034)
 #define I2S_CLKDIV	(0x0038)
 
-#define HIWORD_UPDATE(v, h, l)	(FIELD_PREP_WM16_CONST(GENMASK((h), (l)), (v)))
+#define HIWORD_UPDATE(v, h, l)	(FIELD_PREP_WM16_CONST(BITS((l), (h)), (v)))
 
 /* PX30 GRF CONFIGS */
 #define PX30_I2S0_CLK_IN_SRC_FROM_TX		HIWORD_UPDATE(1, 13, 12)
-- 
2.43.0



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

* [PATCH 12/21] bitmap: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (4 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 11/21] drivers: don't use GENMASK() in FIELD_PREP_WM16() Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-25 16:32 ` [PATCH 13/21] trace: " Yury Norov (NVIDIA)
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Yury Norov, Rasmus Villemoes, Andrew Morton

GENMASK(high, low) notation is confusing. Switch to a more natural
BITS(low, high) mask generator, or FIRST/LAST_BITS() as appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 include/linux/bitmap.h |  4 ++--
 include/linux/find.h   | 34 +++++++++++++++++-----------------
 lib/bitmap.c           |  2 +-
 lib/test_bitmap.c      | 14 +++++++-------
 4 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index fbe2d12bceab..3bed29f1780b 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -473,7 +473,7 @@ void bitmap_set(unsigned long *map, unsigned int start, unsigned int nbits)
 	if (__builtin_constant_p(nbits) && nbits == 1)
 		__set_bit(start, map);
 	else if (small_const_nbits(start + nbits))
-		*map |= GENMASK(start + nbits - 1, start);
+		*map |= BITS(start, start + nbits - 1);
 	else if (__builtin_constant_p(start & BITMAP_MEM_MASK) &&
 		 IS_ALIGNED(start, BITMAP_MEM_ALIGNMENT) &&
 		 __builtin_constant_p(nbits & BITMAP_MEM_MASK) &&
@@ -489,7 +489,7 @@ void bitmap_clear(unsigned long *map, unsigned int start, unsigned int nbits)
 	if (__builtin_constant_p(nbits) && nbits == 1)
 		__clear_bit(start, map);
 	else if (small_const_nbits(start + nbits))
-		*map &= ~GENMASK(start + nbits - 1, start);
+		*map &= ~BITS(start, start + nbits - 1);
 	else if (__builtin_constant_p(start & BITMAP_MEM_MASK) &&
 		 IS_ALIGNED(start, BITMAP_MEM_ALIGNMENT) &&
 		 __builtin_constant_p(nbits & BITMAP_MEM_MASK) &&
diff --git a/include/linux/find.h b/include/linux/find.h
index 9d720ad92bc1..24d7266fd02b 100644
--- a/include/linux/find.h
+++ b/include/linux/find.h
@@ -66,7 +66,7 @@ unsigned long find_next_bit(const unsigned long *addr, unsigned long size,
 		if (unlikely(offset >= size))
 			return size;
 
-		val = *addr & GENMASK(size - 1, offset);
+		val = *addr & BITS(offset, size - 1);
 		return val ? __ffs(val) : size;
 	}
 
@@ -96,7 +96,7 @@ unsigned long find_next_and_bit(const unsigned long *addr1,
 		if (unlikely(offset >= size))
 			return size;
 
-		val = *addr1 & *addr2 & GENMASK(size - 1, offset);
+		val = *addr1 & *addr2 & BITS(offset, size - 1);
 		return val ? __ffs(val) : size;
 	}
 
@@ -127,7 +127,7 @@ unsigned long find_next_andnot_bit(const unsigned long *addr1,
 		if (unlikely(offset >= size))
 			return size;
 
-		val = *addr1 & ~*addr2 & GENMASK(size - 1, offset);
+		val = *addr1 & ~*addr2 & BITS(offset, size - 1);
 		return val ? __ffs(val) : size;
 	}
 
@@ -157,7 +157,7 @@ unsigned long find_next_or_bit(const unsigned long *addr1,
 		if (unlikely(offset >= size))
 			return size;
 
-		val = (*addr1 | *addr2) & GENMASK(size - 1, offset);
+		val = (*addr1 | *addr2) & BITS(offset, size - 1);
 		return val ? __ffs(val) : size;
 	}
 
@@ -185,7 +185,7 @@ unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
 		if (unlikely(offset >= size))
 			return size;
 
-		val = *addr | ~GENMASK(size - 1, offset);
+		val = *addr | ~BITS(offset, size - 1);
 		return val == ~0UL ? size : ffz(val);
 	}
 
@@ -206,7 +206,7 @@ static __always_inline
 unsigned long find_first_bit(const unsigned long *addr, unsigned long size)
 {
 	if (small_const_nbits(size)) {
-		unsigned long val = *addr & GENMASK(size - 1, 0);
+		unsigned long val = *addr & FIRST_BITS(size);
 
 		return val ? __ffs(val) : size;
 	}
@@ -235,7 +235,7 @@ unsigned long find_nth_bit(const unsigned long *addr, unsigned long size, unsign
 		return size;
 
 	if (small_const_nbits(size)) {
-		unsigned long val =  *addr & GENMASK(size - 1, 0);
+		unsigned long val =  *addr & FIRST_BITS(size);
 
 		return val ? fns(val, n) : size;
 	}
@@ -261,7 +261,7 @@ unsigned long find_nth_and_bit(const unsigned long *addr1, const unsigned long *
 		return size;
 
 	if (small_const_nbits(size)) {
-		unsigned long val =  *addr1 & *addr2 & GENMASK(size - 1, 0);
+		unsigned long val =  *addr1 & *addr2 & FIRST_BITS(size);
 
 		return val ? fns(val, n) : size;
 	}
@@ -291,7 +291,7 @@ unsigned long find_nth_and_andnot_bit(const unsigned long *addr1,
 		return size;
 
 	if (small_const_nbits(size)) {
-		unsigned long val =  *addr1 & *addr2 & (~*addr3) & GENMASK(size - 1, 0);
+		unsigned long val =  *addr1 & *addr2 & (~*addr3) & FIRST_BITS(size);
 
 		return val ? fns(val, n) : size;
 	}
@@ -315,7 +315,7 @@ unsigned long find_first_and_bit(const unsigned long *addr1,
 				 unsigned long size)
 {
 	if (small_const_nbits(size)) {
-		unsigned long val = *addr1 & *addr2 & GENMASK(size - 1, 0);
+		unsigned long val = *addr1 & *addr2 & FIRST_BITS(size);
 
 		return val ? __ffs(val) : size;
 	}
@@ -339,7 +339,7 @@ unsigned long find_first_andnot_bit(const unsigned long *addr1,
 				 unsigned long size)
 {
 	if (small_const_nbits(size)) {
-		unsigned long val = *addr1 & (~*addr2) & GENMASK(size - 1, 0);
+		unsigned long val = *addr1 & (~*addr2) & FIRST_BITS(size);
 
 		return val ? __ffs(val) : size;
 	}
@@ -364,7 +364,7 @@ unsigned long find_first_and_and_bit(const unsigned long *addr1,
 				     unsigned long size)
 {
 	if (small_const_nbits(size)) {
-		unsigned long val = *addr1 & *addr2 & *addr3 & GENMASK(size - 1, 0);
+		unsigned long val = *addr1 & *addr2 & *addr3 & FIRST_BITS(size);
 
 		return val ? __ffs(val) : size;
 	}
@@ -385,7 +385,7 @@ static __always_inline
 unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long size)
 {
 	if (small_const_nbits(size)) {
-		unsigned long val = *addr | ~GENMASK(size - 1, 0);
+		unsigned long val = *addr | ~FIRST_BITS(size);
 
 		return val == ~0UL ? size : ffz(val);
 	}
@@ -406,7 +406,7 @@ static __always_inline
 unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
 {
 	if (small_const_nbits(size)) {
-		unsigned long val = *addr & GENMASK(size - 1, 0);
+		unsigned long val = *addr & FIRST_BITS(size);
 
 		return val ? __fls(val) : size;
 	}
@@ -537,7 +537,7 @@ unsigned long find_next_zero_bit_le(const void *addr, unsigned
 		if (unlikely(offset >= size))
 			return size;
 
-		val = swab(val) | ~GENMASK(size - 1, offset);
+		val = swab(val) | ~BITS(offset, size - 1);
 		return val == ~0UL ? size : ffz(val);
 	}
 
@@ -550,7 +550,7 @@ static __always_inline
 unsigned long find_first_zero_bit_le(const void *addr, unsigned long size)
 {
 	if (small_const_nbits(size)) {
-		unsigned long val = swab(*(const unsigned long *)addr) | ~GENMASK(size - 1, 0);
+		unsigned long val = swab(*(const unsigned long *)addr) | ~FIRST_BITS(size);
 
 		return val == ~0UL ? size : ffz(val);
 	}
@@ -570,7 +570,7 @@ unsigned long find_next_bit_le(const void *addr, unsigned
 		if (unlikely(offset >= size))
 			return size;
 
-		val = swab(val) & GENMASK(size - 1, offset);
+		val = swab(val) & BITS(offset, size - 1);
 		return val ? __ffs(val) : size;
 	}
 
diff --git a/lib/bitmap.c b/lib/bitmap.c
index b97692854966..ec11cc36624e 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -876,7 +876,7 @@ void bitmap_to_arr64(u64 *buf, const unsigned long *bitmap, unsigned int nbits)
 
 	/* Clear tail bits in the last element of array beyond nbits. */
 	if (nbits % 64)
-		buf[-1] &= GENMASK_ULL((nbits - 1) % 64, 0);
+		buf[-1] &= FIRST_BITS_ULL(nbits);
 }
 EXPORT_SYMBOL(bitmap_to_arr64);
 #endif
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index c83829ef557f..c198fc7a66d2 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -692,10 +692,10 @@ static void __init test_bitmap_arr64(void)
 		}
 
 		if ((nbits % 64) &&
-		    (arr[(nbits - 1) / 64] & ~GENMASK_ULL((nbits - 1) % 64, 0))) {
+		    (arr[(nbits - 1) / 64] & ~FIRST_BITS_ULL(nbits))) {
 			pr_err("bitmap_to_arr64(nbits == %d): tail is not safely cleared: 0x%016llx (must be 0x%016llx)\n",
 			       nbits, arr[(nbits - 1) / 64],
-			       GENMASK_ULL((nbits - 1) % 64, 0));
+			       FIRST_BITS_ULL(nbits));
 			failed_tests++;
 		}
 
@@ -1217,7 +1217,7 @@ static void __init test_bitmap_const_eval(void)
 	 * in runtime.
 	 */
 
-	/* Equals to `unsigned long bitmap[1] = { GENMASK(6, 5), }` */
+	/* Equals to `unsigned long bitmap[1] = { BITS(5, 6), }` */
 	bitmap_clear(bitmap, 0, BITS_PER_LONG);
 	if (!test_bit(7, bitmap))
 		bitmap_set(bitmap, 5, 2);
@@ -1229,9 +1229,9 @@ static void __init test_bitmap_const_eval(void)
 	/* Equals to `unsigned long var = BIT(25)` */
 	var |= BIT(25);
 	if (var & BIT(0))
-		var ^= GENMASK(9, 6);
+		var ^= BITS(6, 9);
 
-	/* __const_hweight<32|64>(GENMASK(6, 5)) == 2 */
+	/* __const_hweight<32|64>(BITS(5, 6)) == 2 */
 	res = bitmap_weight(bitmap, 20);
 	BUILD_BUG_ON(!__builtin_constant_p(res));
 	BUILD_BUG_ON(res != 2);
@@ -1241,8 +1241,8 @@ static void __init test_bitmap_const_eval(void)
 	BUILD_BUG_ON(!__builtin_constant_p(res));
 	BUILD_BUG_ON(!res);
 
-	/* BIT(2) & GENMASK(14, 8) == 0 */
-	res = initvar & GENMASK(14, 8);
+	/* BIT(2) & BITS(8, 14) == 0 */
+	res = initvar & BITS(8, 14);
 	BUILD_BUG_ON(!__builtin_constant_p(res));
 	BUILD_BUG_ON(res);
 
-- 
2.43.0



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

* [PATCH 13/21] trace: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (5 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 12/21] bitmap: don't use GENMASK() Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-25 19:29   ` Steven Rostedt
  2025-10-25 16:32 ` [PATCH 14/21] lib: 842: don't use GENMASK_ULL() Yury Norov (NVIDIA)
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
	linux-trace-kernel
  Cc: Yury Norov (NVIDIA)

GENMASK(high, low) notation is confusing. FIRST_BITS() is more
appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 kernel/trace/fgraph.c      | 10 +++++-----
 kernel/trace/trace_probe.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 484ad7a18463..4f21bd837055 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -106,10 +106,10 @@
  *     (RESERVED or BITMAP)
  */
 #define FGRAPH_FRAME_OFFSET_BITS	10
-#define FGRAPH_FRAME_OFFSET_MASK	GENMASK(FGRAPH_FRAME_OFFSET_BITS - 1, 0)
+#define FGRAPH_FRAME_OFFSET_MASK	FIRST_BITS(FGRAPH_FRAME_OFFSET_BITS)
 
 #define FGRAPH_TYPE_BITS	2
-#define FGRAPH_TYPE_MASK	GENMASK(FGRAPH_TYPE_BITS - 1, 0)
+#define FGRAPH_TYPE_MASK	FIRST_BITS(FGRAPH_TYPE_BITS)
 #define FGRAPH_TYPE_SHIFT	FGRAPH_FRAME_OFFSET_BITS
 
 enum {
@@ -123,7 +123,7 @@ enum {
  *   FGRAPH_INDEX (12-27) bits holding the gops index wanting return callback called
  */
 #define FGRAPH_INDEX_BITS	16
-#define FGRAPH_INDEX_MASK	GENMASK(FGRAPH_INDEX_BITS - 1, 0)
+#define FGRAPH_INDEX_MASK	FIRST_BITS(FGRAPH_INDEX_BITS)
 #define FGRAPH_INDEX_SHIFT	(FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_BITS)
 
 /*
@@ -135,12 +135,12 @@ enum {
  *  data_size == 0 means 1 word, and 31 (=2^5 - 1) means 32 words.
  */
 #define FGRAPH_DATA_BITS	5
-#define FGRAPH_DATA_MASK	GENMASK(FGRAPH_DATA_BITS - 1, 0)
+#define FGRAPH_DATA_MASK	FIRST_BITS(FGRAPH_DATA_BITS)
 #define FGRAPH_DATA_SHIFT	(FGRAPH_TYPE_SHIFT + FGRAPH_TYPE_BITS)
 #define FGRAPH_MAX_DATA_SIZE (sizeof(long) * (1 << FGRAPH_DATA_BITS))
 
 #define FGRAPH_DATA_INDEX_BITS	4
-#define FGRAPH_DATA_INDEX_MASK	GENMASK(FGRAPH_DATA_INDEX_BITS - 1, 0)
+#define FGRAPH_DATA_INDEX_MASK	FIRST_BITS(FGRAPH_DATA_INDEX_BITS)
 #define FGRAPH_DATA_INDEX_SHIFT	(FGRAPH_DATA_SHIFT + FGRAPH_DATA_BITS)
 
 #define FGRAPH_MAX_INDEX	\
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 08b5bda24da2..88de129dcde0 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -401,7 +401,7 @@ static inline int traceprobe_get_entry_data_size(struct trace_probe *tp)
 #define TPARG_FL_USER   BIT(4)
 #define TPARG_FL_FPROBE BIT(5)
 #define TPARG_FL_TPOINT BIT(6)
-#define TPARG_FL_LOC_MASK	GENMASK(4, 0)
+#define TPARG_FL_LOC_MASK	FIRST_BITS(5)
 
 static inline bool tparg_is_function_entry(unsigned int flags)
 {
-- 
2.43.0



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

* [PATCH 14/21] lib: 842: don't use GENMASK_ULL()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (6 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 13/21] trace: " Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-25 16:32 ` [PATCH 15/21] bpf: don't use GENMASK() Yury Norov (NVIDIA)
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Haren Myneni
  Cc: Yury Norov (NVIDIA)

GENMASK_ULL(high, low) notation is confusing. FIRST_BITS_ULL() is more
appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 lib/842/842_compress.c   | 2 +-
 lib/842/842_decompress.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/842/842_compress.c b/lib/842/842_compress.c
index 055356508d97..83b68c85904f 100644
--- a/lib/842/842_compress.c
+++ b/lib/842/842_compress.c
@@ -161,7 +161,7 @@ static int __split_add_bits(struct sw842_param *p, u64 d, u8 n, u8 s)
 	ret = add_bits(p, d >> s, n - s);
 	if (ret)
 		return ret;
-	return add_bits(p, d & GENMASK_ULL(s - 1, 0), s);
+	return add_bits(p, d & FIRST_BITS_ULL(s), s);
 }
 
 static int add_bits(struct sw842_param *p, u64 d, u8 n)
diff --git a/lib/842/842_decompress.c b/lib/842/842_decompress.c
index 582085ef8b49..0520f20f4121 100644
--- a/lib/842/842_decompress.c
+++ b/lib/842/842_decompress.c
@@ -115,7 +115,7 @@ static int next_bits(struct sw842_param *p, u64 *d, u8 n)
 	else
 		*d = be64_to_cpu(get_unaligned((__be64 *)in)) >> (64 - bits);
 
-	*d &= GENMASK_ULL(n - 1, 0);
+	*d &= FIRST_BITS_ULL(n);
 
 	p->bit += n;
 
-- 
2.43.0



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

* [PATCH 15/21] bpf: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (7 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 14/21] lib: 842: don't use GENMASK_ULL() Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-25 16:32 ` [PATCH 16/21] kcsan: " Yury Norov (NVIDIA)
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	bpf
  Cc: Yury Norov (NVIDIA)

GENMASK(high, low) notation is confusing. BITS(low, high) is more
appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 kernel/bpf/verifier.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ff40e5e65c43..a9d690d3a507 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -17676,7 +17676,7 @@ static void mark_fastcall_pattern_for_call(struct bpf_verifier_env *env,
 	 * - includes R1-R5 if corresponding parameter has is described
 	 *   in the function prototype.
 	 */
-	clobbered_regs_mask = GENMASK(cs.num_params, cs.is_void ? 1 : 0);
+	clobbered_regs_mask = BITS(cs.is_void ? 1 : 0, cs.num_params);
 	/* e.g. if helper call clobbers r{0,1}, expect r{2,3,4,5} in the pattern */
 	expected_regs_mask = ~clobbered_regs_mask & ALL_CALLER_SAVED_REGS;
 
@@ -24210,7 +24210,7 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env,
 			def = ALL_CALLER_SAVED_REGS;
 			use = def & ~BIT(BPF_REG_0);
 			if (get_call_summary(env, insn, &cs))
-				use = GENMASK(cs.num_params, 1);
+				use = BITS(1, cs.num_params);
 			break;
 		default:
 			def = 0;
-- 
2.43.0



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

* [PATCH 16/21] kcsan: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (8 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 15/21] bpf: don't use GENMASK() Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-25 16:32 ` [PATCH 17/21] mm: " Yury Norov (NVIDIA)
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Marco Elver, Dmitry Vyukov, kasan-dev
  Cc: Yury Norov (NVIDIA)

GENMASK(high, low) notation is confusing. Use BITS(low, high) and
FIRST_BITS() where appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 kernel/kcsan/encoding.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/kcsan/encoding.h b/kernel/kcsan/encoding.h
index 170a2bb22f53..3a4cb7b354e3 100644
--- a/kernel/kcsan/encoding.h
+++ b/kernel/kcsan/encoding.h
@@ -44,8 +44,8 @@
 
 /* Bitmasks for the encoded watchpoint access information. */
 #define WATCHPOINT_WRITE_MASK	BIT(BITS_PER_LONG-1)
-#define WATCHPOINT_SIZE_MASK	GENMASK(BITS_PER_LONG-2, WATCHPOINT_ADDR_BITS)
-#define WATCHPOINT_ADDR_MASK	GENMASK(WATCHPOINT_ADDR_BITS-1, 0)
+#define WATCHPOINT_ADDR_MASK	FIRST_BITS(WATCHPOINT_ADDR_BITS)
+#define WATCHPOINT_SIZE_MASK	BITS(WATCHPOINT_ADDR_BITS, BITS_PER_LONG-2)
 static_assert(WATCHPOINT_ADDR_MASK == (1UL << WATCHPOINT_ADDR_BITS) - 1);
 static_assert((WATCHPOINT_WRITE_MASK ^ WATCHPOINT_SIZE_MASK ^ WATCHPOINT_ADDR_MASK) == ~0UL);
 
-- 
2.43.0



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

* [PATCH 17/21] mm: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (9 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 16/21] kcsan: " Yury Norov (NVIDIA)
@ 2025-10-25 16:32 ` Yury Norov (NVIDIA)
  2025-10-25 16:33 ` [PATCH 18/21] fprobe: " Yury Norov (NVIDIA)
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:32 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Andrew Morton, linux-mm
  Cc: Yury Norov (NVIDIA)

GENMASK(high, low) notation is confusing. FIRST_BITS() is more
appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 mm/debug_vm_pgtable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 830107b6dd08..b722dc3c091b 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -40,7 +40,7 @@
  * expectations that are being validated here. All future changes in here
  * or the documentation need to be in sync.
  */
-#define RANDOM_NZVALUE	GENMASK(7, 0)
+#define RANDOM_NZVALUE	FIRST_BITS(8)
 
 struct pgtable_debug_args {
 	struct mm_struct	*mm;
-- 
2.43.0



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

* [PATCH 18/21] fprobe: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (10 preceding siblings ...)
  2025-10-25 16:32 ` [PATCH 17/21] mm: " Yury Norov (NVIDIA)
@ 2025-10-25 16:33 ` Yury Norov (NVIDIA)
  2025-10-25 16:33 ` [PATCH 19/21] fs: " Yury Norov (NVIDIA)
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:33 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Arnd Bergmann, linux-arch
  Cc: Yury Norov (NVIDIA)

GENMASK(high, low) notation is confusing. FIRST/LAST_BITS() are
more appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 include/asm-generic/fprobe.h | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/asm-generic/fprobe.h b/include/asm-generic/fprobe.h
index 8659a4dc6eb6..bf2523078661 100644
--- a/include/asm-generic/fprobe.h
+++ b/include/asm-generic/fprobe.h
@@ -16,17 +16,14 @@
 #define ARCH_DEFINE_ENCODE_FPROBE_HEADER
 
 #define FPROBE_HEADER_MSB_SIZE_SHIFT (BITS_PER_LONG - FPROBE_DATA_SIZE_BITS)
-#define FPROBE_HEADER_MSB_MASK					\
-	GENMASK(FPROBE_HEADER_MSB_SIZE_SHIFT - 1, 0)
+#define FPROBE_HEADER_MSB_MASK	FIRST_BITS(FPROBE_HEADER_MSB_SIZE_SHIFT)
 
 /*
  * By default, this expects the MSBs in the address of kprobe is 0xf.
  * If any arch needs another fixed pattern (e.g. s390 is zero filled),
  * override this.
  */
-#define FPROBE_HEADER_MSB_PATTERN				\
-	GENMASK(BITS_PER_LONG - 1, FPROBE_HEADER_MSB_SIZE_SHIFT)
-
+#define FPROBE_HEADER_MSB_PATTERN	LAST_BITS(FPROBE_HEADER_MSB_SIZE_SHIFT)
 #define arch_fprobe_header_encodable(fp)			\
 	(((unsigned long)(fp) & ~FPROBE_HEADER_MSB_MASK) ==	\
 	 FPROBE_HEADER_MSB_PATTERN)
-- 
2.43.0



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

* [PATCH 19/21] fs: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (11 preceding siblings ...)
  2025-10-25 16:33 ` [PATCH 18/21] fprobe: " Yury Norov (NVIDIA)
@ 2025-10-25 16:33 ` Yury Norov (NVIDIA)
  2025-10-25 16:33 ` [PATCH 20/21] fortify-string: " Yury Norov (NVIDIA)
  2025-10-25 16:33 ` [PATCH 21/21] Docs: add Functions parameters order section Yury Norov (NVIDIA)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:33 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu, Sandeep Dhavale, Hongbo Li,
	Jaegeuk Kim, Tony Luck, Reinette Chatre, Dave Martin, James Morse,
	Babu Moger, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
	Vlastimil Babka, Suren Baghdasaryan, Jinjiang Tu, Baolin Wang,
	Ryan Roberts, Andrei Vagin, linux-erofs, linux-f2fs-devel,
	linux-fsdevel
  Cc: Yury Norov (NVIDIA)

GENMASK(high, low) notation is confusing. FIRST/LAST_BITS() are
more appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 fs/erofs/internal.h      | 2 +-
 fs/f2fs/data.c           | 2 +-
 fs/f2fs/inode.c          | 2 +-
 fs/f2fs/segment.c        | 2 +-
 fs/f2fs/super.c          | 2 +-
 fs/proc/task_mmu.c       | 2 +-
 fs/resctrl/pseudo_lock.c | 2 +-
 include/linux/f2fs_fs.h  | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index f7f622836198..6e0f03092c52 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -250,7 +250,7 @@ static inline u64 erofs_nid_to_ino64(struct erofs_sb_info *sbi, erofs_nid_t nid)
 	 * Note: on-disk NIDs remain unchanged as they are primarily used for
 	 * compatibility with non-LFS 32-bit applications.
 	 */
-	return ((nid << 1) & GENMASK_ULL(63, 32)) | (nid & GENMASK(30, 0)) |
+	return ((nid << 1) & LAST_BITS_ULL(32)) | (nid & FIRST_BITS(31)) |
 		((nid >> EROFS_DIRENT_NID_METABOX_BIT) << 31);
 }
 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 775aa4f63aa3..ef08464e003f 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -416,7 +416,7 @@ int f2fs_target_device_index(struct f2fs_sb_info *sbi, block_t blkaddr)
 
 static blk_opf_t f2fs_io_flags(struct f2fs_io_info *fio)
 {
-	unsigned int temp_mask = GENMASK(NR_TEMP_TYPE - 1, 0);
+	unsigned int temp_mask = FIRST_BITS(NR_TEMP_TYPE);
 	unsigned int fua_flag, meta_flag, io_flag;
 	blk_opf_t op_flags = 0;
 
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 8c4eafe9ffac..42a43f558136 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -524,7 +524,7 @@ static int do_read_inode(struct inode *inode)
 			fi->i_compress_level = compress_flag >>
 						COMPRESS_LEVEL_OFFSET;
 			fi->i_compress_flag = compress_flag &
-					GENMASK(COMPRESS_LEVEL_OFFSET - 1, 0);
+						FIRST_BITS(COMPRESS_LEVEL_OFFSET);
 			fi->i_cluster_size = BIT(fi->i_log_cluster_size);
 			set_inode_flag(inode, FI_COMPRESSED_FILE);
 		}
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b45eace879d7..64433d3b67d4 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -5425,7 +5425,7 @@ static int do_fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type)
 		wp_block = zbd->start_blk + (zone.wp >> log_sectors_per_block);
 		wp_segno = GET_SEGNO(sbi, wp_block);
 		wp_blkoff = wp_block - START_BLOCK(sbi, wp_segno);
-		wp_sector_off = zone.wp & GENMASK(log_sectors_per_block - 1, 0);
+		wp_sector_off = zone.wp & FIRST_BITS(log_sectors_per_block);
 
 		if (cs->segno == wp_segno && cs->next_blkoff == wp_blkoff &&
 				wp_sector_off == 0)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index db7afb806411..96621fd45cdc 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4501,7 +4501,7 @@ static void save_stop_reason(struct f2fs_sb_info *sbi, unsigned char reason)
 	unsigned long flags;
 
 	spin_lock_irqsave(&sbi->error_lock, flags);
-	if (sbi->stop_reason[reason] < GENMASK(BITS_PER_BYTE - 1, 0))
+	if (sbi->stop_reason[reason] < FIRST_BITS(BITS_PER_BYTE))
 		sbi->stop_reason[reason]++;
 	spin_unlock_irqrestore(&sbi->error_lock, flags);
 }
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index fc35a0543f01..71de487b244c 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1845,7 +1845,7 @@ struct pagemapread {
 
 #define PM_ENTRY_BYTES		sizeof(pagemap_entry_t)
 #define PM_PFRAME_BITS		55
-#define PM_PFRAME_MASK		GENMASK_ULL(PM_PFRAME_BITS - 1, 0)
+#define PM_PFRAME_MASK		FIRST_BITS_ULL(PM_PFRAME_BITS)
 #define PM_SOFT_DIRTY		BIT_ULL(55)
 #define PM_MMAP_EXCLUSIVE	BIT_ULL(56)
 #define PM_UFFD_WP		BIT_ULL(57)
diff --git a/fs/resctrl/pseudo_lock.c b/fs/resctrl/pseudo_lock.c
index 87bbc2605de1..45703bbd3bca 100644
--- a/fs/resctrl/pseudo_lock.c
+++ b/fs/resctrl/pseudo_lock.c
@@ -30,7 +30,7 @@
  */
 static unsigned int pseudo_lock_major;
 
-static unsigned long pseudo_lock_minor_avail = GENMASK(MINORBITS, 0);
+static unsigned long pseudo_lock_minor_avail = FIRST_BITS(MINORBITS + 1);
 
 static char *pseudo_lock_devnode(const struct device *dev, umode_t *mode)
 {
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 6afb4a13b81d..9996356b79e0 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -356,7 +356,7 @@ enum {
 	OFFSET_BIT_SHIFT
 };
 
-#define OFFSET_BIT_MASK		GENMASK(OFFSET_BIT_SHIFT - 1, 0)
+#define OFFSET_BIT_MASK		FIRST_BITS(OFFSET_BIT_SHIFT)
 
 struct f2fs_node {
 	/* can be one of three types: inode, direct, and indirect types */
-- 
2.43.0



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

* [PATCH 20/21] fortify-string: don't use GENMASK()
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (12 preceding siblings ...)
  2025-10-25 16:33 ` [PATCH 19/21] fs: " Yury Norov (NVIDIA)
@ 2025-10-25 16:33 ` Yury Norov (NVIDIA)
  2025-10-25 16:33 ` [PATCH 21/21] Docs: add Functions parameters order section Yury Norov (NVIDIA)
  14 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:33 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Kees Cook, linux-hardening
  Cc: Yury Norov (NVIDIA)

GENMASK(high, low) notation is confusing. BITS(low, high) is more
appropriate.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 include/linux/fortify-string.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index b3b53f8c1b28..0c95cdcca736 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -11,9 +11,9 @@
 #define __RENAME(x) __asm__(#x)
 
 #define FORTIFY_REASON_DIR(r)		FIELD_GET(BIT(0), r)
-#define FORTIFY_REASON_FUNC(r)		FIELD_GET(GENMASK(7, 1), r)
+#define FORTIFY_REASON_FUNC(r)		FIELD_GET(BITS(1, 7), r)
 #define FORTIFY_REASON(func, write)	(FIELD_PREP(BIT(0), write) | \
-					 FIELD_PREP(GENMASK(7, 1), func))
+					 FIELD_PREP(BITS(1, 7), func))
 
 /* Overridden by KUnit tests. */
 #ifndef fortify_panic
-- 
2.43.0



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

* [PATCH 21/21] Docs: add Functions parameters order section
       [not found] <20251025162858.305236-1-yury.norov@gmail.com>
                   ` (13 preceding siblings ...)
  2025-10-25 16:33 ` [PATCH 20/21] fortify-string: " Yury Norov (NVIDIA)
@ 2025-10-25 16:33 ` Yury Norov (NVIDIA)
  2025-10-27  9:02   ` Jani Nikula
  14 siblings, 1 reply; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:33 UTC (permalink / raw)
  To: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Jonathan Corbet, workflows, linux-doc
  Cc: Yury Norov (NVIDIA)

Standardize parameters ordering in some typical cases to minimize
confusion.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 Documentation/process/coding-style.rst | 48 ++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
index d1a8e5465ed9..dde24148305c 100644
--- a/Documentation/process/coding-style.rst
+++ b/Documentation/process/coding-style.rst
@@ -523,6 +523,54 @@ below, compared to the **declaration** example above)::
 	...
  }
 
+6.2) Function parameters order
+------------------------------
+
+The order of parameters is important both for code generation and readability.
+Passing parameters in an unusual order is a common source of bugs. Listing
+them in standard widely adopted order helps to avoid confusion.
+
+Many ABIs put first function parameter and return value in R0. If your
+function returns one of its parameters, passing it at the very beginning
+would lead to a better code generation. For example::
+
+        void *memset64(uint64_t *s, uint64_t v, size_t count);
+        void *memcpy(void *dest, const void *src, size_t count);
+
+If your function doesn't propagate a parameter, but has a meaning of copying
+and/or processing data, the best practice is following the traditional order:
+destination, source, options, flags.
+
+for_each()-like iterators should take an enumerator the first. For example::
+
+        for_each_set_bit(bit, mask, nbits);
+                do_something(bit);
+
+        list_for_each_entry(pos, head, member);
+                do_something(pos);
+
+If function operates on a range or ranges of data, corresponding parameters
+may be described as ``start - end`` or ``start - size`` pairs. In both cases,
+the parameters should follow each other. For example::
+
+        int
+        check_range(unsigned long vstart, unsigned long vend,
+                    unsigned long kstart, unsigned long kend);
+
+        static inline void flush_icache_range(unsigned long start, unsigned long end);
+
+        static inline void flush_icache_user_page(struct vm_area_struct *vma,
+                                            struct page *page,
+                                            unsigned long addr, int len);
+
+Both ``start`` and ``end`` of the interval are inclusive.
+
+Describing intervals in order ``end - start`` is unfavorable. One notable
+example is the ``GENMASK(high, low)`` macro. While such a notation is popular
+in hardware context, particularly to describe registers structure, in context
+of software development it looks counter intuitive and confusing. Please switch
+to an equivalent ``BITS(low, high)`` version.
+
 7) Centralized exiting of functions
 -----------------------------------
 
-- 
2.43.0



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

* [PATCH 06/21] mfd: prepare to generalize BITS() macro
       [not found] <20251025164023.308884-1-yury.norov@gmail.com>
@ 2025-10-25 16:40 ` Yury Norov (NVIDIA)
  0 siblings, 0 replies; 25+ messages in thread
From: Yury Norov (NVIDIA) @ 2025-10-25 16:40 UTC (permalink / raw)
  To: Linus Torvalds, Linus Walleij, Nicolas Frattaroli, Lee Jones,
	linux-arm-kernel, linux-kernel
  Cc: Yury Norov (NVIDIA), Rasmus Villemoes

In preparation for adding generic BITS() macro, add an #undef directive
for the existing BITS(). The following patches will drop it entirely.

Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
---
 drivers/mfd/db8500-prcmu-regs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/db8500-prcmu-regs.h b/drivers/mfd/db8500-prcmu-regs.h
index 75fd1069372c..25d2d5966211 100644
--- a/drivers/mfd/db8500-prcmu-regs.h
+++ b/drivers/mfd/db8500-prcmu-regs.h
@@ -12,6 +12,7 @@
 #ifndef __DB8500_PRCMU_REGS_H
 #define __DB8500_PRCMU_REGS_H
 
+#undef BITS
 #define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))
 
 #define PRCM_ACLK_MGT		(0x004)
-- 
2.43.0



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

* Re: [PATCH 13/21] trace: don't use GENMASK()
  2025-10-25 16:32 ` [PATCH 13/21] trace: " Yury Norov (NVIDIA)
@ 2025-10-25 19:29   ` Steven Rostedt
  2025-10-25 19:36     ` Steven Rostedt
  0 siblings, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2025-10-25 19:29 UTC (permalink / raw)
  To: Yury Norov (NVIDIA)
  Cc: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
	linux-trace-kernel

On Sat, 25 Oct 2025 12:32:55 -0400
"Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:

> GENMASK(high, low) notation is confusing. FIRST_BITS() is more
> appropriate.
> 

I'm fine with this change as a clean up, but...

> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
> ---
>  kernel/trace/fgraph.c      | 10 +++++-----
>  kernel/trace/trace_probe.h |  2 +-

Make this two different patches. trace_probe and fgraph go through
different topic branches.

Thanks,

-- Steve


>  2 files changed, 6 insertions(+), 6 deletions(-)


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

* Re: [PATCH 13/21] trace: don't use GENMASK()
  2025-10-25 19:29   ` Steven Rostedt
@ 2025-10-25 19:36     ` Steven Rostedt
  0 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2025-10-25 19:36 UTC (permalink / raw)
  To: Yury Norov (NVIDIA)
  Cc: Linus Walleij, Lee Jones, linux-arm-kernel, linux-kernel,
	Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
	linux-trace-kernel

On Sat, 25 Oct 2025 15:29:54 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Sat, 25 Oct 2025 12:32:55 -0400
> "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
> 
> > GENMASK(high, low) notation is confusing. FIRST_BITS() is more
> > appropriate.
> >   
> 
> I'm fine with this change as a clean up, but...
> 
> > Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
> > ---
> >  kernel/trace/fgraph.c      | 10 +++++-----
> >  kernel/trace/trace_probe.h |  2 +-  
> 
> Make this two different patches. trace_probe and fgraph go through
> different topic branches.

OK, I see this is part of a patch series that adds FIRST_BITS().

Please add that first, and the rest of the patches can go though their
individual trees. Something like this change I would like to make sure
doesn't accidentally cause a regression. That means it must go through
my tests before it gets applied.

-- Steve


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

* Re: [PATCH 21/21] Docs: add Functions parameters order section
  2025-10-25 16:33 ` [PATCH 21/21] Docs: add Functions parameters order section Yury Norov (NVIDIA)
@ 2025-10-27  9:02   ` Jani Nikula
  2025-10-27 16:08     ` Jeff Johnson
                       ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Jani Nikula @ 2025-10-27  9:02 UTC (permalink / raw)
  To: Yury Norov (NVIDIA), Linus Walleij, Lee Jones, linux-arm-kernel,
	linux-kernel, Jonathan Corbet, workflows, linux-doc
  Cc: Yury Norov (NVIDIA)

On Sat, 25 Oct 2025, "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
> Standardize parameters ordering in some typical cases to minimize
> confusion.
>
> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
> ---
>  Documentation/process/coding-style.rst | 48 ++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> index d1a8e5465ed9..dde24148305c 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -523,6 +523,54 @@ below, compared to the **declaration** example above)::
>  	...
>   }
>  
> +6.2) Function parameters order
> +------------------------------
> +
> +The order of parameters is important both for code generation and readability.
> +Passing parameters in an unusual order is a common source of bugs. Listing
> +them in standard widely adopted order helps to avoid confusion.
> +
> +Many ABIs put first function parameter and return value in R0. If your
> +function returns one of its parameters, passing it at the very beginning
> +would lead to a better code generation. For example::
> +
> +        void *memset64(uint64_t *s, uint64_t v, size_t count);
> +        void *memcpy(void *dest, const void *src, size_t count);
> +
> +If your function doesn't propagate a parameter, but has a meaning of copying
> +and/or processing data, the best practice is following the traditional order:
> +destination, source, options, flags.
> +
> +for_each()-like iterators should take an enumerator the first. For example::
> +
> +        for_each_set_bit(bit, mask, nbits);
> +                do_something(bit);
> +
> +        list_for_each_entry(pos, head, member);
> +                do_something(pos);
> +
> +If function operates on a range or ranges of data, corresponding parameters
> +may be described as ``start - end`` or ``start - size`` pairs. In both cases,
> +the parameters should follow each other. For example::
> +
> +        int
> +        check_range(unsigned long vstart, unsigned long vend,
> +                    unsigned long kstart, unsigned long kend);
> +
> +        static inline void flush_icache_range(unsigned long start, unsigned long end);
> +
> +        static inline void flush_icache_user_page(struct vm_area_struct *vma,
> +                                            struct page *page,
> +                                            unsigned long addr, int len);
> +
> +Both ``start`` and ``end`` of the interval are inclusive.
> +
> +Describing intervals in order ``end - start`` is unfavorable. One notable
> +example is the ``GENMASK(high, low)`` macro. While such a notation is popular
> +in hardware context, particularly to describe registers structure, in context
> +of software development it looks counter intuitive and confusing. Please switch
> +to an equivalent ``BITS(low, high)`` version.
> +

GENMASK when used for defining hardware registers is completely fine,
and *much* easier to deal with when you cross check against the specs
that almost invariably define high:low.

Which other parts of coding style take on specific interfaces and tell
you to switch? Weird. I for one don't want to encourage an influx of
trivial patches doing GENMASK to BITS conversions, and then keep
rejecting them. It's just a huge collective waste of time.

Anyway, that's a lot of text on "function parameter order" to justify
BITS(), but completely skips more important principles such as "context
parameter first", or "destination first".


BR,
Jani.


>  7) Centralized exiting of functions
>  -----------------------------------

-- 
Jani Nikula, Intel


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

* Re: [PATCH 21/21] Docs: add Functions parameters order section
  2025-10-27  9:02   ` Jani Nikula
@ 2025-10-27 16:08     ` Jeff Johnson
  2025-10-27 18:22     ` Andi Shyti
  2025-10-27 18:43     ` Randy Dunlap
  2 siblings, 0 replies; 25+ messages in thread
From: Jeff Johnson @ 2025-10-27 16:08 UTC (permalink / raw)
  To: Jani Nikula, Yury Norov (NVIDIA), Linus Walleij, Lee Jones,
	linux-arm-kernel, linux-kernel, Jonathan Corbet, workflows,
	linux-doc

On 10/27/2025 2:02 AM, Jani Nikula wrote:
> On Sat, 25 Oct 2025, "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
>> Standardize parameters ordering in some typical cases to minimize
>> confusion.
>>
>> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
>> ---
>>  Documentation/process/coding-style.rst | 48 ++++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
>> index d1a8e5465ed9..dde24148305c 100644
>> --- a/Documentation/process/coding-style.rst
>> +++ b/Documentation/process/coding-style.rst
>> @@ -523,6 +523,54 @@ below, compared to the **declaration** example above)::
>>  	...
>>   }
>>  
>> +6.2) Function parameters order
>> +------------------------------
>> +
>> +The order of parameters is important both for code generation and readability.
>> +Passing parameters in an unusual order is a common source of bugs. Listing
>> +them in standard widely adopted order helps to avoid confusion.
>> +
>> +Many ABIs put first function parameter and return value in R0. If your
>> +function returns one of its parameters, passing it at the very beginning
>> +would lead to a better code generation. For example::
>> +
>> +        void *memset64(uint64_t *s, uint64_t v, size_t count);
>> +        void *memcpy(void *dest, const void *src, size_t count);
>> +
>> +If your function doesn't propagate a parameter, but has a meaning of copying
>> +and/or processing data, the best practice is following the traditional order:
>> +destination, source, options, flags.
>> +
>> +for_each()-like iterators should take an enumerator the first. For example::
>> +
>> +        for_each_set_bit(bit, mask, nbits);
>> +                do_something(bit);
>> +
>> +        list_for_each_entry(pos, head, member);
>> +                do_something(pos);
>> +
>> +If function operates on a range or ranges of data, corresponding parameters
>> +may be described as ``start - end`` or ``start - size`` pairs. In both cases,
>> +the parameters should follow each other. For example::
>> +
>> +        int
>> +        check_range(unsigned long vstart, unsigned long vend,
>> +                    unsigned long kstart, unsigned long kend);
>> +
>> +        static inline void flush_icache_range(unsigned long start, unsigned long end);
>> +
>> +        static inline void flush_icache_user_page(struct vm_area_struct *vma,
>> +                                            struct page *page,
>> +                                            unsigned long addr, int len);
>> +
>> +Both ``start`` and ``end`` of the interval are inclusive.
>> +
>> +Describing intervals in order ``end - start`` is unfavorable. One notable
>> +example is the ``GENMASK(high, low)`` macro. While such a notation is popular
>> +in hardware context, particularly to describe registers structure, in context
>> +of software development it looks counter intuitive and confusing. Please switch
>> +to an equivalent ``BITS(low, high)`` version.
>> +
> 
> GENMASK when used for defining hardware registers is completely fine,
> and *much* easier to deal with when you cross check against the specs
> that almost invariably define high:low.

Not only that, there is no common definition of BITS

Defined in 7 files as a macro:
arch/arc/include/asm/disasm.h, line 32 (as a macro)
drivers/mfd/db8500-prcmu-regs.h, line 15 (as a macro)
drivers/net/wireless/intel/iwlwifi/fw/api/coex.h, line 14 (as a macro)
fs/select.c, line 415 (as a macro)
lib/zlib_inflate/inflate.c, line 232 (as a macro)
sound/core/oss/rate.c, line 28 (as a macro)
tools/perf/dlfilters/dlfilter-show-cycles.c, line 22 (as a macro)

Most of these do NOT have a (low, high) signature.

And GENMASK will throw a compile error if you swap the high and low:
#define GENMASK_INPUT_CHECK(h, l) BUILD_BUG_ON_ZERO(const_true((l) > (h)))

IMO the real confusion with GENMASK(), which would be the same with the
proposed BITS(), is that without knowledge of the implementation, when looking
at an instance of usage you can't tell if the parameters are two bit numbers
or a start bit and number of bits.

/jeff


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

* Re: [PATCH 21/21] Docs: add Functions parameters order section
  2025-10-27  9:02   ` Jani Nikula
  2025-10-27 16:08     ` Jeff Johnson
@ 2025-10-27 18:22     ` Andi Shyti
  2025-10-27 18:43     ` Randy Dunlap
  2 siblings, 0 replies; 25+ messages in thread
From: Andi Shyti @ 2025-10-27 18:22 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Yury Norov (NVIDIA), Linus Walleij, Lee Jones, linux-arm-kernel,
	linux-kernel, Jonathan Corbet, workflows, linux-doc

Hi,

On Mon, Oct 27, 2025 at 11:02:48AM +0200, Jani Nikula wrote:
> On Sat, 25 Oct 2025, "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
> > Standardize parameters ordering in some typical cases to minimize
> > confusion.
> >
> > Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
> > ---
> >  Documentation/process/coding-style.rst | 48 ++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >
> > diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> > index d1a8e5465ed9..dde24148305c 100644
> > --- a/Documentation/process/coding-style.rst
> > +++ b/Documentation/process/coding-style.rst
> > @@ -523,6 +523,54 @@ below, compared to the **declaration** example above)::
> >  	...
> >   }
> >  
> > +6.2) Function parameters order
> > +------------------------------
> > +
> > +The order of parameters is important both for code generation and readability.
> > +Passing parameters in an unusual order is a common source of bugs. Listing
> > +them in standard widely adopted order helps to avoid confusion.
> > +
> > +Many ABIs put first function parameter and return value in R0. If your
> > +function returns one of its parameters, passing it at the very beginning
> > +would lead to a better code generation. For example::
> > +
> > +        void *memset64(uint64_t *s, uint64_t v, size_t count);
> > +        void *memcpy(void *dest, const void *src, size_t count);
> > +
> > +If your function doesn't propagate a parameter, but has a meaning of copying
> > +and/or processing data, the best practice is following the traditional order:
> > +destination, source, options, flags.
> > +
> > +for_each()-like iterators should take an enumerator the first. For example::
> > +
> > +        for_each_set_bit(bit, mask, nbits);
> > +                do_something(bit);
> > +
> > +        list_for_each_entry(pos, head, member);
> > +                do_something(pos);
> > +
> > +If function operates on a range or ranges of data, corresponding parameters
> > +may be described as ``start - end`` or ``start - size`` pairs. In both cases,
> > +the parameters should follow each other. For example::
> > +
> > +        int
> > +        check_range(unsigned long vstart, unsigned long vend,
> > +                    unsigned long kstart, unsigned long kend);
> > +
> > +        static inline void flush_icache_range(unsigned long start, unsigned long end);
> > +
> > +        static inline void flush_icache_user_page(struct vm_area_struct *vma,
> > +                                            struct page *page,
> > +                                            unsigned long addr, int len);
> > +
> > +Both ``start`` and ``end`` of the interval are inclusive.
> > +
> > +Describing intervals in order ``end - start`` is unfavorable. One notable
> > +example is the ``GENMASK(high, low)`` macro. While such a notation is popular
> > +in hardware context, particularly to describe registers structure, in context
> > +of software development it looks counter intuitive and confusing. Please switch
> > +to an equivalent ``BITS(low, high)`` version.
> > +
> 
> GENMASK when used for defining hardware registers is completely fine,
> and *much* easier to deal with when you cross check against the specs
> that almost invariably define high:low.

I fully agree with Jani here! When coming into describing
registers my brain is hardwired to read values from left to
right, high-low.

Linus suggested also BITS(start_bit, n_bits) which, in my
opinion, complements what we already have.

We leave GENMASK to register mask descriptions and BITS to the
rest.

Andi


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

* Re: [PATCH 21/21] Docs: add Functions parameters order section
  2025-10-27  9:02   ` Jani Nikula
  2025-10-27 16:08     ` Jeff Johnson
  2025-10-27 18:22     ` Andi Shyti
@ 2025-10-27 18:43     ` Randy Dunlap
  2 siblings, 0 replies; 25+ messages in thread
From: Randy Dunlap @ 2025-10-27 18:43 UTC (permalink / raw)
  To: Jani Nikula, Yury Norov (NVIDIA), Linus Walleij, Lee Jones,
	linux-arm-kernel, linux-kernel, Jonathan Corbet, workflows,
	linux-doc



On 10/27/25 2:02 AM, Jani Nikula wrote:
> On Sat, 25 Oct 2025, "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
>> Standardize parameters ordering in some typical cases to minimize
>> confusion.
>>
>> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
>> ---
>>  Documentation/process/coding-style.rst | 48 ++++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
>> index d1a8e5465ed9..dde24148305c 100644
>> --- a/Documentation/process/coding-style.rst
>> +++ b/Documentation/process/coding-style.rst
>> @@ -523,6 +523,54 @@ below, compared to the **declaration** example above)::
>>  	...
>>   }
>>  
>> +6.2) Function parameters order
>> +------------------------------
>> +
>> +The order of parameters is important both for code generation and readability.
>> +Passing parameters in an unusual order is a common source of bugs. Listing
>> +them in standard widely adopted order helps to avoid confusion.
>> +
>> +Many ABIs put first function parameter and return value in R0. If your
>> +function returns one of its parameters, passing it at the very beginning
>> +would lead to a better code generation. For example::
>> +
>> +        void *memset64(uint64_t *s, uint64_t v, size_t count);
>> +        void *memcpy(void *dest, const void *src, size_t count);
>> +
>> +If your function doesn't propagate a parameter, but has a meaning of copying
>> +and/or processing data, the best practice is following the traditional order:
>> +destination, source, options, flags.
>> +
>> +for_each()-like iterators should take an enumerator the first. For example::
>> +
>> +        for_each_set_bit(bit, mask, nbits);
>> +                do_something(bit);
>> +
>> +        list_for_each_entry(pos, head, member);
>> +                do_something(pos);
>> +
>> +If function operates on a range or ranges of data, corresponding parameters
>> +may be described as ``start - end`` or ``start - size`` pairs. In both cases,
>> +the parameters should follow each other. For example::
>> +
>> +        int
>> +        check_range(unsigned long vstart, unsigned long vend,
>> +                    unsigned long kstart, unsigned long kend);
>> +
>> +        static inline void flush_icache_range(unsigned long start, unsigned long end);
>> +
>> +        static inline void flush_icache_user_page(struct vm_area_struct *vma,
>> +                                            struct page *page,
>> +                                            unsigned long addr, int len);
>> +
>> +Both ``start`` and ``end`` of the interval are inclusive.
>> +
>> +Describing intervals in order ``end - start`` is unfavorable. One notable
>> +example is the ``GENMASK(high, low)`` macro. While such a notation is popular
>> +in hardware context, particularly to describe registers structure, in context
>> +of software development it looks counter intuitive and confusing. Please switch
>> +to an equivalent ``BITS(low, high)`` version.
>> +
> 
> GENMASK when used for defining hardware registers is completely fine,
> and *much* easier to deal with when you cross check against the specs
> that almost invariably define high:low.
> 
> Which other parts of coding style take on specific interfaces and tell
> you to switch? Weird. I for one don't want to encourage an influx of
> trivial patches doing GENMASK to BITS conversions, and then keep
> rejecting them. It's just a huge collective waste of time.
> 
> Anyway, that's a lot of text on "function parameter order" to justify
> BITS(), but completely skips more important principles such as "context
> parameter first", or "destination first".

and usually flags or gfp_t last (if they are used).

There are several exceptions to these, but consistency helps and
lack of it has caused some argument problems in the past.

-- 
~Randy



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

* Re: [PATCH 08/21] mfd: drop local BITS() macro
  2025-10-25 16:32 ` [PATCH 08/21] mfd: drop local " Yury Norov (NVIDIA)
@ 2025-10-27 22:21   ` Linus Walleij
  0 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2025-10-27 22:21 UTC (permalink / raw)
  To: Yury Norov (NVIDIA); +Cc: Lee Jones, linux-arm-kernel, linux-kernel

On Sat, Oct 25, 2025 at 6:33 PM Yury Norov (NVIDIA)
<yury.norov@gmail.com> wrote:

> Now that generic BITS() is introduced, drop the local one.
>
> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>

Nice work Yury!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij


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

* Re: [PATCH 10/21] i2c: nomadik: don't use GENMASK()
  2025-10-25 16:32 ` [PATCH 10/21] i2c: nomadik: don't use GENMASK() Yury Norov (NVIDIA)
@ 2025-10-27 22:22   ` Linus Walleij
  0 siblings, 0 replies; 25+ messages in thread
From: Linus Walleij @ 2025-10-27 22:22 UTC (permalink / raw)
  To: Yury Norov (NVIDIA)
  Cc: Lee Jones, linux-arm-kernel, linux-kernel, Andi Shyti, linux-i2c,
	Linus Torvalds

On Sat, Oct 25, 2025 at 6:33 PM Yury Norov (NVIDIA)
<yury.norov@gmail.com> wrote:

> GENMASK(high, low) notation is confusing. Switch to BITS() or FIRST_BITS()
> where appropriate.
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Link: https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ@mail.gmail.com/
> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>

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

Yours,
Linus Walleij


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

* Re: [PATCH 06/21] mfd: prepare to generalize BITS() macro
  2025-10-25 16:28 ` [PATCH 06/21] mfd: prepare to generalize BITS() macro Yury Norov (NVIDIA)
@ 2025-11-06 16:48   ` Lee Jones
  0 siblings, 0 replies; 25+ messages in thread
From: Lee Jones @ 2025-11-06 16:48 UTC (permalink / raw)
  To: Yury Norov (NVIDIA)
  Cc: Linus Torvalds, Linus Walleij, Nicolas Frattaroli,
	linux-arm-kernel, linux-kernel, Rasmus Villemoes

On Sat, 25 Oct 2025, Yury Norov (NVIDIA) wrote:

> In preparation for adding generic BITS() macro, add an #undef directive
> for the existing BITS(). The following patches will drop it entirely.
> 
> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
> ---
>  drivers/mfd/db8500-prcmu-regs.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mfd/db8500-prcmu-regs.h b/drivers/mfd/db8500-prcmu-regs.h
> index 75fd1069372c..25d2d5966211 100644
> --- a/drivers/mfd/db8500-prcmu-regs.h
> +++ b/drivers/mfd/db8500-prcmu-regs.h
> @@ -12,6 +12,7 @@
>  #ifndef __DB8500_PRCMU_REGS_H
>  #define __DB8500_PRCMU_REGS_H
>  
> +#undef BITS

Doesn't this get removed in 2 patches time?

>  #define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))
>  
>  #define PRCM_ACLK_MGT		(0x004)
> -- 
> 2.43.0
> 

-- 
Lee Jones [李琼斯]


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

end of thread, other threads:[~2025-11-06 16:48 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20251025162858.305236-1-yury.norov@gmail.com>
2025-10-25 16:28 ` [PATCH 06/21] mfd: prepare to generalize BITS() macro Yury Norov (NVIDIA)
2025-11-06 16:48   ` Lee Jones
2025-10-25 16:32 ` [PATCH 08/21] mfd: drop local " Yury Norov (NVIDIA)
2025-10-27 22:21   ` Linus Walleij
2025-10-25 16:32 ` [PATCH 09/21] bits: generalize BITMAP_{FIRST,LAST}_WORD_MASK Yury Norov (NVIDIA)
2025-10-25 16:32 ` [PATCH 10/21] i2c: nomadik: don't use GENMASK() Yury Norov (NVIDIA)
2025-10-27 22:22   ` Linus Walleij
2025-10-25 16:32 ` [PATCH 11/21] drivers: don't use GENMASK() in FIELD_PREP_WM16() Yury Norov (NVIDIA)
2025-10-25 16:32 ` [PATCH 12/21] bitmap: don't use GENMASK() Yury Norov (NVIDIA)
2025-10-25 16:32 ` [PATCH 13/21] trace: " Yury Norov (NVIDIA)
2025-10-25 19:29   ` Steven Rostedt
2025-10-25 19:36     ` Steven Rostedt
2025-10-25 16:32 ` [PATCH 14/21] lib: 842: don't use GENMASK_ULL() Yury Norov (NVIDIA)
2025-10-25 16:32 ` [PATCH 15/21] bpf: don't use GENMASK() Yury Norov (NVIDIA)
2025-10-25 16:32 ` [PATCH 16/21] kcsan: " Yury Norov (NVIDIA)
2025-10-25 16:32 ` [PATCH 17/21] mm: " Yury Norov (NVIDIA)
2025-10-25 16:33 ` [PATCH 18/21] fprobe: " Yury Norov (NVIDIA)
2025-10-25 16:33 ` [PATCH 19/21] fs: " Yury Norov (NVIDIA)
2025-10-25 16:33 ` [PATCH 20/21] fortify-string: " Yury Norov (NVIDIA)
2025-10-25 16:33 ` [PATCH 21/21] Docs: add Functions parameters order section Yury Norov (NVIDIA)
2025-10-27  9:02   ` Jani Nikula
2025-10-27 16:08     ` Jeff Johnson
2025-10-27 18:22     ` Andi Shyti
2025-10-27 18:43     ` Randy Dunlap
     [not found] <20251025164023.308884-1-yury.norov@gmail.com>
2025-10-25 16:40 ` [PATCH 06/21] mfd: prepare to generalize BITS() macro Yury Norov (NVIDIA)

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