| Message ID | 20180323195853.4599-1-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 23, 2018 at 12:58:53PM -0700, Paulo Zanoni wrote: > Protect the macro parameters with parens in order to avoid priority > issues on macro evaluation when the macro argument is not a single > operand. > > This is not a problem today, but it could be in the future. I found > this while reviewing a patch that introduces new callers for the > macros. > > Reference: commit 04416108ccea ("drm/i915/cnl: Add registers related to voltage swing sequences.") > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index da2f6c623ab2..49c90e1aa796 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1729,9 +1729,9 @@ enum i915_power_well_id { > > #define CNL_PORT_TX_DW2_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP((port), 2)) > #define CNL_PORT_TX_DW2_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0((port), 2)) > -#define SWING_SEL_UPPER(x) ((x >> 3) << 15) > +#define SWING_SEL_UPPER(x) (((x) >> 3) << 15) > #define SWING_SEL_UPPER_MASK (1 << 15) > -#define SWING_SEL_LOWER(x) ((x & 0x7) << 11) > +#define SWING_SEL_LOWER(x) (((x) & 0x7) << 11) > #define SWING_SEL_LOWER_MASK (0x7 << 11) > #define RCOMP_SCALAR(x) ((x) << 0) > #define RCOMP_SCALAR_MASK (0xFF << 0) > -- > 2.14.3 >
On Fri, 23 Mar 2018, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote: > Protect the macro parameters with parens in order to avoid priority > issues on macro evaluation when the macro argument is not a single > operand. > > This is not a problem today, but it could be in the future. I found > this while reviewing a patch that introduces new callers for the > macros. > > Reference: commit 04416108ccea ("drm/i915/cnl: Add registers related to voltage swing sequences.") > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index da2f6c623ab2..49c90e1aa796 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1729,9 +1729,9 @@ enum i915_power_well_id { > > #define CNL_PORT_TX_DW2_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP((port), 2)) > #define CNL_PORT_TX_DW2_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0((port), 2)) > -#define SWING_SEL_UPPER(x) ((x >> 3) << 15) > +#define SWING_SEL_UPPER(x) (((x) >> 3) << 15) > #define SWING_SEL_UPPER_MASK (1 << 15) > -#define SWING_SEL_LOWER(x) ((x & 0x7) << 11) > +#define SWING_SEL_LOWER(x) (((x) & 0x7) << 11) Unrelated to the patch at hand, but why do we have >> 3 and & 7 here like this? We could have a single SWING_SEL_MASK() and SWING_SEL() that would do the split to two parts. For future follow-up... BR, Jani. > #define SWING_SEL_LOWER_MASK (0x7 << 11) > #define RCOMP_SCALAR(x) ((x) << 0) > #define RCOMP_SCALAR_MASK (0xFF << 0)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index da2f6c623ab2..49c90e1aa796 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1729,9 +1729,9 @@ enum i915_power_well_id { #define CNL_PORT_TX_DW2_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP((port), 2)) #define CNL_PORT_TX_DW2_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0((port), 2)) -#define SWING_SEL_UPPER(x) ((x >> 3) << 15) +#define SWING_SEL_UPPER(x) (((x) >> 3) << 15) #define SWING_SEL_UPPER_MASK (1 << 15) -#define SWING_SEL_LOWER(x) ((x & 0x7) << 11) +#define SWING_SEL_LOWER(x) (((x) & 0x7) << 11) #define SWING_SEL_LOWER_MASK (0x7 << 11) #define RCOMP_SCALAR(x) ((x) << 0) #define RCOMP_SCALAR_MASK (0xFF << 0)
Protect the macro parameters with parens in order to avoid priority issues on macro evaluation when the macro argument is not a single operand. This is not a problem today, but it could be in the future. I found this while reviewing a patch that introduces new callers for the macros. Reference: commit 04416108ccea ("drm/i915/cnl: Add registers related to voltage swing sequences.") Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)