On Sat, Oct 25, 2025 at 12:40:10PM -0400, 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.
This doesn't seem to line up with the actual change, you're not altering
FIELD_PREP_WM16() at all but rather altering things that use it.
As mentioned in submitting-patches.rst when submitting a patch series
you should supply a cover letter for that patch series which describes
the overall content of the series. This helps people understand what
they are looking at and how things fit together.
> ---
> 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(-)
It doesn't help to send changes to unrelated subsystems in one patch
either...
> --- 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)))
This is adjusting the implementation of something that takes in h, l to
use l, h which if anything seems likely to introduce confusion, and
definitely feels well into bikeshed territory. I don't *super* care but
I'm having a hard time seeing the benefit.
signature.asc
Description: PGP signature
