| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2824034.mvXUDI8C0e@workhorse>
Date: Mon, 27 Oct 2025 13:21:15 +0100
From: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
Linus Walleij <linus.walleij@...aro.org>, Sandy Huang <hjc@...k-chips.com>,
Heiko Stübner <heiko@...ech.de>,
Andy Yan <andy.yan@...k-chips.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Shreeya Patel <shreeya.patel@...labora.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Jaehoon Chung <jh80.chung@...sung.com>, Ulf Hansson <ulf.hansson@...aro.org>,
Nicolas Frattaroli <frattaroli.nicolas@...il.com>,
Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
"Yury Norov (NVIDIA)" <yury.norov@...il.com>,
dri-devel@...ts.freedesktop.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, kernel@...labora.com, linux-mmc@...r.kernel.org,
linux-sound@...r.kernel.org, "Yury Norov (NVIDIA)" <yury.norov@...il.com>
Cc: Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH 11/21] drivers: don't use GENMASK() in FIELD_PREP_WM16()
On Saturday, 25 October 2025 18:40:10 Central European Standard Time Yury Norov (NVIDIA) wrote:
> 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@...ux-foundation.org>
> Link: https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ@mail.gmail.com/
> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@...il.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)
>
Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
I made sure there were no accidental changes introduced by the
swapping of values, so in terms of correctness, this appears
all good to me.
Kind regards,
Nicolas Frattaroli
Powered by blists - more mailing lists