[Intel-gfx] i915 vs checkpatch

Joonas Lahtinen joonas.lahtinen at linux.intel.com
Fri Mar 2 09:37:29 UTC 2018


Quoting Rodrigo Vivi (2018-03-01 20:00:07)
> On Thu, Mar 01, 2018 at 06:13:31PM +0200, Jani Nikula wrote:
> > 
> > I went through the recent checkpatch reports, and here's my take.
> > 
> > On Thu, 01 Mar 2018, Arkadiusz Hiler <arkadiusz.hiler at intel.com> wrote:
> > >  2. Which of the checkpatch checks we want to disabled for i915?
> > 
> > I'd like to have these silenced:
> > 
> > CHECK: No space is necessary after a cast
> > WARNING: line over 80 characters
> > WARNING: quoted string split across lines
> > 
> > I'd prefer we conform to the last two too, but there's just too much
> > noise and too many cases where we explicitly should ignore them.
> > 
> > For the time being, I think we may have to silence these ones too, but
> > I'd like us to discuss enforcing them:
> > 
> > CHECK: Prefer kernel type 'u16' over 'uint16_t'
> > CHECK: Prefer kernel type 'u32' over 'uint32_t'
> > CHECK: Prefer kernel type 'u64' over 'uint64_t'
> > CHECK: Prefer kernel type 'u8' over 'uint8_t'
> > CHECK: Prefer using the BIT macro
> > 
> > The BIT macros is one that I'd consider accepting a one-time conversion
> > of i915_reg.h and after that use it exclusively. But up to debate.
> 
> For this one I just wonder if we would need to do a massive
> change before. Because it would get ugly to have mixed cases.

Yep, the mixed cases are bit tough to automatically enforce. So the
transitional phase will always be troublesome, and trying to make that
shorter makes some sense to me.

Traditionally we've avoided mass changes just for the changes, but we
have to assess the value of doing it against what we get. That is
getting automatic enforcement, once we've converted over.

We're not that far off the mark with u(32|16|8) vs uint(32|16|8)_t:

i915$ git grep -E "uint(32|16|8)_t" | wc -l
852
i915$ git grep -E "u(32|16|8)" | wc -l
3857

I don't consider that undoable.

BIT() is in the minority at the moment, so it might benefit even more as
people often cargo-cult the programming style from other places in the code.

I think it might be worthy doing these two changes to get the automatic
enforemend and avoid the codebase staying in limbo. Machine overlords are
way better at enforcing any code checks than us humans.

Regards, Joonas


More information about the Intel-gfx mailing list