On Fri, 02 Mar 2018, Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> wrote: > 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@xxxxxxxxx> 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 Doing a bit of git grep -cE with those seems to indicate that code with uint(32|16|8)_t promotes *more* use of those. It's natural and it goes by the rule of following the style surrounding your changes. > I don't consider that undoable. It's doable. Only a question of whether we want to do that or not. > 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. FWIW I think BIT(n) is simply better than open-coding (1 << n), while the kernel vs. C types is more of an aestheical thing. > 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. Agreed on the machine overlords. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
