public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* Re: [PATCH v4 1/4] bitfield: Drop underscores from macro parameters
       [not found] ` <792d176149bc4ffde2a7b78062388dc2466c23ca.1760696560.git.geert+renesas@glider.be>
@ 2025-10-17 16:37   ` Yury Norov
  2025-10-20 12:13     ` Geert Uytterhoeven
  0 siblings, 1 reply; 3+ messages in thread
From: Yury Norov @ 2025-10-17 16:37 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea, Giovanni Cabiddu, Herbert Xu, David Miller,
	Linus Walleij, Bartosz Golaszewski, Joel Stanley, Andrew Jeffery,
	Crt Mori, Jonathan Cameron, Lars-Peter Clausen, Jacky Huang,
	Shan-Chun Hung, Rasmus Villemoes, Jaroslav Kysela, Takashi Iwai,
	Johannes Berg, Jakub Kicinski, Alex Elder, David Laight,
	Vincent Mailhol, Jason Baron, Borislav Petkov, Tony Luck,
	Michael Hennerich, Kim Seer Paller, David Lechner, Nuno Sá,
	Andy Shevchenko, Richard Genoud, Cosmin Tanislav, Biju Das,
	Jianping Shen, linux-clk, linux-arm-kernel, linux-renesas-soc,
	linux-crypto, linux-edac, qat-linux, linux-gpio, linux-aspeed,
	linux-iio, linux-sound, linux-kernel

On Fri, Oct 17, 2025 at 12:54:09PM +0200, Geert Uytterhoeven wrote:
> There is no need to prefix macro parameters with underscores.
> Remove the underscores.
> 
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v4:
>   - Update recently introduced FIELD_MODIFY() macro,
> 
> v3:
>   - New.
> ---
>  include/linux/bitfield.h | 106 +++++++++++++++++++--------------------
>  1 file changed, 53 insertions(+), 53 deletions(-)
> 
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 5355f8f806a97974..7ff817bdae19b468 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -60,68 +60,68 @@
>  
>  #define __bf_cast_unsigned(type, x)	((__unsigned_scalar_typeof(type))(x))
>  
> -#define __BF_FIELD_CHECK(_mask, _reg, _val, _pfx)			\
> +#define __BF_FIELD_CHECK(mask, reg, val, pfx)				\
>  	({								\
> -		BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),		\
> -				 _pfx "mask is not constant");		\
> -		BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero");	\
> -		BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ?		\
> -				 ~((_mask) >> __bf_shf(_mask)) &	\
> -					(0 + (_val)) : 0,		\
> -				 _pfx "value too large for the field"); \
> -		BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >	\
> -				 __bf_cast_unsigned(_reg, ~0ull),	\
> -				 _pfx "type of reg too small for mask"); \
> -		__BUILD_BUG_ON_NOT_POWER_OF_2((_mask) +			\
> -					      (1ULL << __bf_shf(_mask))); \
> +		BUILD_BUG_ON_MSG(!__builtin_constant_p(mask),		\
> +				 pfx "mask is not constant");		\
> +		BUILD_BUG_ON_MSG((mask) == 0, pfx "mask is zero");	\
> +		BUILD_BUG_ON_MSG(__builtin_constant_p(val) ?		\
> +				 ~((mask) >> __bf_shf(mask)) &	\
> +					(0 + (val)) : 0,		\
> +				 pfx "value too large for the field"); \
> +		BUILD_BUG_ON_MSG(__bf_cast_unsigned(mask, mask) >	\
> +				 __bf_cast_unsigned(reg, ~0ull),	\
> +				 pfx "type of reg too small for mask"); \
> +		__BUILD_BUG_ON_NOT_POWER_OF_2((mask) +			\
> +					      (1ULL << __bf_shf(mask))); \
>  	})

Hi Geert,

Thanks for the series!

I agree that underscored parameters are excessive. But fixing them has
a side effect of wiping the history, which is a bad thing.

I would prefer to save a history over following a rule that seemingly
is not written down. Let's keep this untouched for now, and if there
will be a need to move the code, we can drop underscores as well.

Meanwhile you (and David) can propose a corresponding rule in
coding-style.rst and a checkpatch warning. That way we at least will
stop merging a new code of that style.

Thanks,
Yury


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

* Re: [PATCH v4 1/4] bitfield: Drop underscores from macro parameters
  2025-10-17 16:37   ` [PATCH v4 1/4] bitfield: Drop underscores from macro parameters Yury Norov
@ 2025-10-20 12:13     ` Geert Uytterhoeven
  2025-10-20 14:56       ` Yury Norov
  0 siblings, 1 reply; 3+ messages in thread
From: Geert Uytterhoeven @ 2025-10-20 12:13 UTC (permalink / raw)
  To: Yury Norov
  Cc: Michael Turquette, Stephen Boyd, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea, Giovanni Cabiddu, Herbert Xu, David Miller,
	Linus Walleij, Bartosz Golaszewski, Joel Stanley, Andrew Jeffery,
	Crt Mori, Jonathan Cameron, Lars-Peter Clausen, Jacky Huang,
	Shan-Chun Hung, Rasmus Villemoes, Jaroslav Kysela, Takashi Iwai,
	Johannes Berg, Jakub Kicinski, Alex Elder, David Laight,
	Vincent Mailhol, Jason Baron, Borislav Petkov, Tony Luck,
	Michael Hennerich, Kim Seer Paller, David Lechner, Nuno Sá,
	Andy Shevchenko, Richard Genoud, Cosmin Tanislav, Biju Das,
	Jianping Shen, linux-clk, linux-arm-kernel, linux-renesas-soc,
	linux-crypto, linux-edac, qat-linux, linux-gpio, linux-aspeed,
	linux-iio, linux-sound, linux-kernel

Hi Yury,

On Fri, 17 Oct 2025 at 18:37, Yury Norov <yury.norov@gmail.com> wrote:
> On Fri, Oct 17, 2025 at 12:54:09PM +0200, Geert Uytterhoeven wrote:
> > There is no need to prefix macro parameters with underscores.
> > Remove the underscores.
> >
> > Suggested-by: David Laight <david.laight.linux@gmail.com>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > v4:
> >   - Update recently introduced FIELD_MODIFY() macro,

> > --- a/include/linux/bitfield.h
> > +++ b/include/linux/bitfield.h
> > @@ -60,68 +60,68 @@
> >
> >  #define __bf_cast_unsigned(type, x)  ((__unsigned_scalar_typeof(type))(x))
> >
> > -#define __BF_FIELD_CHECK(_mask, _reg, _val, _pfx)                    \
> > +#define __BF_FIELD_CHECK(mask, reg, val, pfx)                                \
> >       ({                                                              \
> > -             BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),          \
> > -                              _pfx "mask is not constant");          \
> > -             BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero");    \
> > -             BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ?           \
> > -                              ~((_mask) >> __bf_shf(_mask)) &        \
> > -                                     (0 + (_val)) : 0,               \
> > -                              _pfx "value too large for the field"); \
> > -             BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >     \
> > -                              __bf_cast_unsigned(_reg, ~0ull),       \
> > -                              _pfx "type of reg too small for mask"); \
> > -             __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) +                 \
> > -                                           (1ULL << __bf_shf(_mask))); \
> > +             BUILD_BUG_ON_MSG(!__builtin_constant_p(mask),           \
> > +                              pfx "mask is not constant");           \
> > +             BUILD_BUG_ON_MSG((mask) == 0, pfx "mask is zero");      \
> > +             BUILD_BUG_ON_MSG(__builtin_constant_p(val) ?            \
> > +                              ~((mask) >> __bf_shf(mask)) &  \
> > +                                     (0 + (val)) : 0,                \
> > +                              pfx "value too large for the field"); \
> > +             BUILD_BUG_ON_MSG(__bf_cast_unsigned(mask, mask) >       \
> > +                              __bf_cast_unsigned(reg, ~0ull),        \
> > +                              pfx "type of reg too small for mask"); \
> > +             __BUILD_BUG_ON_NOT_POWER_OF_2((mask) +                  \
> > +                                           (1ULL << __bf_shf(mask))); \
> >       })
>
> I agree that underscored parameters are excessive. But fixing them has
> a side effect of wiping the history, which is a bad thing.
>
> I would prefer to save a history over following a rule that seemingly
> is not written down. Let's keep this untouched for now, and if there
> will be a need to move the code, we can drop underscores as well.

Fair enough.
So I assume you are fine with not having underscored parameters in
new code, like in [PATCH v4 2/4]?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds


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

* Re: [PATCH v4 1/4] bitfield: Drop underscores from macro parameters
  2025-10-20 12:13     ` Geert Uytterhoeven
@ 2025-10-20 14:56       ` Yury Norov
  0 siblings, 0 replies; 3+ messages in thread
From: Yury Norov @ 2025-10-20 14:56 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, Nicolas Ferre, Alexandre Belloni,
	Claudiu Beznea, Giovanni Cabiddu, Herbert Xu, David Miller,
	Linus Walleij, Bartosz Golaszewski, Joel Stanley, Andrew Jeffery,
	Crt Mori, Jonathan Cameron, Lars-Peter Clausen, Jacky Huang,
	Shan-Chun Hung, Rasmus Villemoes, Jaroslav Kysela, Takashi Iwai,
	Johannes Berg, Jakub Kicinski, Alex Elder, David Laight,
	Vincent Mailhol, Jason Baron, Borislav Petkov, Tony Luck,
	Michael Hennerich, Kim Seer Paller, David Lechner, Nuno Sá,
	Andy Shevchenko, Richard Genoud, Cosmin Tanislav, Biju Das,
	Jianping Shen, linux-clk, linux-arm-kernel, linux-renesas-soc,
	linux-crypto, linux-edac, qat-linux, linux-gpio, linux-aspeed,
	linux-iio, linux-sound, linux-kernel

> > I agree that underscored parameters are excessive. But fixing them has
> > a side effect of wiping the history, which is a bad thing.
> >
> > I would prefer to save a history over following a rule that seemingly
> > is not written down. Let's keep this untouched for now, and if there
> > will be a need to move the code, we can drop underscores as well.
> 
> Fair enough.
> So I assume you are fine with not having underscored parameters in
> new code, like in [PATCH v4 2/4]?

Yes, sure.


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

end of thread, other threads:[~2025-10-20 14:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1760696560.git.geert+renesas@glider.be>
     [not found] ` <792d176149bc4ffde2a7b78062388dc2466c23ca.1760696560.git.geert+renesas@glider.be>
2025-10-17 16:37   ` [PATCH v4 1/4] bitfield: Drop underscores from macro parameters Yury Norov
2025-10-20 12:13     ` Geert Uytterhoeven
2025-10-20 14:56       ` Yury Norov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox