[Intel-gfx] i915 vs checkpatch

Daniel Vetter daniel at ffwll.ch
Mon Mar 5 08:14:27 UTC 2018


On Thu, Mar 01, 2018 at 11:47:06AM +0200, Arkadiusz Hiler wrote:
> Hey all,
> 
> Since not so long ago our CI is running and reporting sparse and
> checkpatch. Sparse is doing just fine but I had to disable checkpatch
> for the time being - too much "false" positives causing people to
> complain. It's simply confusing to see one thing in the code, and
> fitting your change in only to get a report that it's wrong.
> 
> We are explicitly going against couple of the recommendations it tries
> to enforce, e.g. not using BIT macro, splitting quoted strings:
> https://lists.freedesktop.org/archives/intel-gfx/2018-February/156613.html
> 
> IMHO we should make a couple of decisions here:
>  1. Do we really want to use the checkpatch / have CI reports?
>  2. Which of the checkpatch checks we want to disabled for i915?
>  3. How strongly do we want to enforce the rest?
>  4. Do we want to change what's already in the tree, for compliance?
> 
> Recent-ish drm-tip, vanilla checkpatch on i915 reports:
> total: 399 errors, 3573 warnings, 209374 lines checked

I'd recommend not making checkpatch ever fail CI, but at most warning.
Plus silence the ones we obviously think are silly (or currently
inconsistent in our code).

The reason is that checkpatch isn't really maintained by a consensus
approach, but it's mostly just Joe Perches doing what he feels like. It's
better again, but in past kernel releases it has become opionated about
pointless bikesheds to the point I simply had to ignore most of it. It'd
be great if we have an authoritative answer to all code layout questions,
but in reality we don't.

I think the ingore list is probably best kept within maintainer-tools
itself, that way we at least have visibility into it from committers.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Intel-gfx mailing list