Quoting Michal Wajdeczko (2018-06-27 16:51:42) > On Wed, 27 Jun 2018 16:41:13 +0200, Jani Nikula <jani.nikula@xxxxxxxxx> > wrote: > > > There's already some BIT() usage here and there, embrace it. > > > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 476118f46cf3..64b9c270045d 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -65,9 +65,10 @@ > > * but do note that the macros may be needed to read as well as write > > the > > * register contents. > > * > > - * Define bits using ``(1 << N)`` instead of ``BIT(N)``. We may change > > this in > > - * the future, but this is the prevailing style. Do **not** add > > ``_BIT`` suffix > > - * to the name. > > + * Define bits using ``BIT(N)`` instead of ``(1 << N)``. Do **not** add > > ``_BIT`` > > + * suffix to the name. Exception to ``BIT()`` usage: Value 1 for a bit > > field > > + * should be defined using ``(1 << N)`` to be in line with other values > > such as > > + * ``(2 << N)`` for the same field. > > * > > * Group the register and its contents together without blank lines, > > separate > > * from other registers and their contents with one blank line. > > @@ -105,7 +106,7 @@ > > * #define _FOO_A 0xf000 > > * #define _FOO_B 0xf001 > > * #define FOO(pipe) _MMIO_PIPE(pipe, _FOO_A, _FOO_B) > > - * #define FOO_ENABLE (1 << 31) > > + * #define FOO_ENABLE BIT(31) > > hmm, this breaks nice consistency between one- and multi-bit fields .. > > > * #define FOO_MODE_MASK (0xf << 16) > > .. but if you want to use macro for single bit, then maybe you should > also consider other existing macro for the mask definition: > > #define FOO_MODE_MASK GENMASK(19, 16) > > > * #define FOO_MODE_SHIFT 16 > > * #define FOO_MODE_BAR (0 << 16) > > .. but we still don't have any macro for defining multi-bit values > so I'm not sure if this change will make code really easier to read #include <linux/bitfield.h> I'm not sure if I'm ready to embrace that yet, but it does seem to be the direction we should be heading in. Primarily to check the invalid range checking & usage. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
