| 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: <6e82c656573d436b6c53d242a0ed45007e5849ad@intel.com>
Date: Mon, 27 Oct 2025 10:49:56 +0200
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: "Yury Norov (NVIDIA)" <yury.norov@...il.com>, Linus Torvalds
<torvalds@...ux-foundation.org>, Linus Walleij <linus.walleij@...aro.org>,
Nicolas Frattaroli <nicolas.frattaroli@...labora.com>, 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
Cc: Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH 11/21] drivers: don't use GENMASK() in FIELD_PREP_WM16()
On Sat, 25 Oct 2025, "Yury Norov (NVIDIA)" <yury.norov@...il.com> 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.
GENMASK argument order matches how most specs I've ever looked at define
bits. It's high:low. I, for one, think it's silly to add another set of
macros purely to swap the argument order.
> 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))
IMO this change would make more sense in defining register contents:
+#define PX30_LVDS_FORMAT_MASK GENMASK(14, 13)
-#define PX30_LVDS_FORMAT(val) FIELD_PREP_WM16(GENMASK(14, 13), (val))
+#define PX30_LVDS_FORMAT(val) FIELD_PREP_WM16(PX30_LVDS_FORMAT_MASK, (val))
BR,
Jani.
> #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)
--
Jani Nikula, Intel
Powered by blists - more mailing lists