Re: ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/2] drm/i915/icl: Add allowed DP rates for Icelake

From: Jani Nikula <jani.nikula@intel.com>
To: Paulo Zanoni <paulo.r.zanoni@intel.com>, intel-gfx@lists.freedesktop.org
Cc: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Subject: Re: ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/2] drm/i915/icl: Add allowed DP rates for Icelake
Date: Tue, 12 Jun 2018 11:46:08 +0300	[thread overview]
Message-ID: <87k1r42y0f.fsf@intel.com> (raw)
In-Reply-To: <1528762852.2604.15.camel@intel.com>

On Mon, 11 Jun 2018, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Em Seg, 2018-06-11 às 22:35 +0000, Patchwork escreveu:
>> == Series Details ==
>> 
>> Series: series starting with [CI,1/2] drm/i915/icl: Add allowed DP
>> rates for Icelake
>> URL   : https://patchwork.freedesktop.org/series/44595/
>> State : warning
>> 
>> == Summary ==
>> 
>> $ dim checkpatch origin/drm-tip
>> e6e6b2f7af58 drm/i915/icl: Add allowed DP rates for Icelake
>> 3fe43cb729fe drm/i915/dp: Add support for HBR3 and TPS4 during link
>> training
>> -:26: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
>> #26: FILE: drivers/gpu/drm/i915/i915_reg.h:8694:
>> +#define  DP_TP_CTL_LINK_TRAIN_PAT4		(5<<8)
>
> Dear maintainers,
>
> I get this type of error way too often. What's the most desirable thing
> here?
>
> 1 - Make it "(5 << 8)" so checkpatch doesn't complain, which will leave
> the coding style inconsistent with the surrounding lines.

I don't like the inconsistency.

> 2 - Drive-by fix all the bits around it so everybody in the same
> definition has nice spaces, 2.a: in the same patch, 2.b: in a separate
> patch.

Fine by me. Both a and b. I was kind of hoping this would have happened
more.

> 3 - Just ignore the checkpatch message, push code as-is.

Also fine by me.

> 4 - Blacklist this check from checkpatch.

Unfortunately the SPACING class in checkpatch would silence much, much
more than just this specific thing, so it would be a net negative.

> 5 - Submit a separate patch fixing all the spacing errors on i915_reg.h
> once and for all. Live happily ever after.

It would be annoying for a while with conflicts, but I'd be fine. Not
sure if it would be better to do it in some arbitrary chunks rather than
mass change.

> 6 - Submit a separate patch converting everything to BIT() on
> i915_reg.h.

Same as above.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx