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