On Tue, 04 Oct 2022, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> On Fri, 30 Sep 2022, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_reg_defs.h b/drivers/gpu/drm/i915/i915_reg_defs.h
>> index 8f486f77609f..e823869b9afd 100644
>> --- a/drivers/gpu/drm/i915/i915_reg_defs.h
>> +++ b/drivers/gpu/drm/i915/i915_reg_defs.h
>> @@ -104,22 +104,16 @@ typedef struct {
>>
>> #define _MMIO(r) ((const i915_reg_t){ .reg = (r) })
>>
>> -#define INVALID_MMIO_REG _MMIO(0)
>> -
>> -static __always_inline u32 i915_mmio_reg_offset(i915_reg_t reg)
>> -{
>> - return reg.reg;
>> -}
>> +typedef struct {
>> + u32 reg;
>> +} i915_mcr_reg_t;
>>
>> -static inline bool i915_mmio_reg_equal(i915_reg_t a, i915_reg_t b)
>> -{
>> - return i915_mmio_reg_offset(a) == i915_mmio_reg_offset(b);
>> -}
>> +#define INVALID_MMIO_REG _MMIO(0)
>>
>> -static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>> -{
>> - return !i915_mmio_reg_equal(reg, INVALID_MMIO_REG);
>> -}
>> +/* These macros can be used on either i915_reg_t or i915_mcr_reg_t */
>> +#define i915_mmio_reg_offset(r) (r.reg)
>> +#define i915_mmio_reg_equal(a, b) (i915_mmio_reg_offset(a) == i915_mmio_reg_offset(b))
>> +#define i915_mmio_reg_valid(r) (!i915_mmio_reg_equal(r, INVALID_MMIO_REG))
>>
>
> I don't really like losing the type safety here. The whole and only
> purpose of typedeffing i915_reg_t as a struct instead of just u32 was
> the strict type safety.
PS. Changes like this should really be highlighted better, in the commit
subject and title. Now it feels like it's hidden within a big commit
within a big series, without even mentioning it in the commit message.
BR,
Jani.
>
> BR,
> Jani.
--
Jani Nikula, Intel Open Source Graphics Center
