On Wed, 2025-08-27 at 13:57 +0300, Jani Nikula wrote: > On Wed, 27 Aug 2025, Luca Coelho <luciano.coe...@intel.com> wrote: > > Add the unsigned suffix to FW_BLC_SELF_* macro definitions to avoid > > potentially typing them as 'int'. > > > > For example, this happens when we pass them to _MASKED_BIT_ENABLE(), > > because of the typeof() construct there. When we pass 1 << 15 (the > > FW_BLC_SELF_EN macro), we get typeof(1 << 15), which is 'int'. Then > > the value becomes negative (-2147450880) and we try to assign it to a > > 'u32'. > > > > In practice this is not a problem though, because when we try to > > assign -2147450880 to the u32, that becomes 0x80008000, which was the > > intended result. > > > > Signed-off-by: Luca Coelho <luciano.coe...@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index b283b25d8368..8c8e32b6c662 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -412,9 +412,9 @@ > > #define FW_BLC _MMIO(0x20d8) > > #define FW_BLC2 _MMIO(0x20dc) > > #define FW_BLC_SELF _MMIO(0x20e0) /* 915+ only */ > > -#define FW_BLC_SELF_EN_MASK (1 << 31) > > -#define FW_BLC_SELF_FIFO_MASK (1 << 16) /* 945 only */ > > -#define FW_BLC_SELF_EN (1 << 15) /* 945 only */ > > +#define FW_BLC_SELF_EN_MASK (1U << 31) > > +#define FW_BLC_SELF_FIFO_MASK (1U << 16) /* 945 only */ > > +#define FW_BLC_SELF_EN (1U << 15) /* 945 only */ > > We have the REG_GENMASK* and REG_BIT* macros for exactly this purpose.
TBH, I saw them, but I was not entirely sure why they're needed. And since the entire file is a mixed bag of styles related to this, my idea was not to change it. But thanks for pointing out! I'll use them instead and send v2. -- Cheers, Luca.
