Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/2] drm/i915/icl: Add allowed DP rates for Icelake
Em Qua, 2018-06-13 às 11:07 +0300, Jani Nikula escreveu: > On Tue, 12 Jun 2018, Rodrigo Vivi wrote: > > Do we really want BIT everywhere?! > > I think I'd go for everywhere except part of a register field value: > While I completely agree with your reasoning, this means we'll kinda always want to blacklist the BIT_MACRO checkpath type because checkpatch won't know about these exceptions, which means we won't actually need to convert everything to BIT() since no false negative emails anyway. Anyway, I submitted a patch to fix the spacing issues, I'd love to have some comments from the maintainers on it. Thanks, Paulo > #define SINGLE_BIT_OKAY BIT(25) > #define FIELD_SHIFT 20 > #define FIELD_MASK(0xf << 20) > #define FIELD_FOO_PLEASE_NO BIT(20) /* Don't do > this */ > #define FIELD_FOO (1 << 20) /* This is > consistent */ > #define FIELD_BAR (2 << 20) > #define FIELD_BAZ (3 << 20) > > > BR, > Jani. > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/2] drm/i915/icl: Add allowed DP rates for Icelake
On Tue, 12 Jun 2018, Rodrigo Vivi wrote: > Do we really want BIT everywhere?! I think I'd go for everywhere except part of a register field value: #define SINGLE_BIT_OKAY BIT(25) #define FIELD_SHIFT 20 #define FIELD_MASK (0xf << 20) #define FIELD_FOO_PLEASE_NO BIT(20) /* Don't do this */ #define FIELD_FOO (1 << 20) /* This is consistent */ #define FIELD_BAR (2 << 20) #define FIELD_BAZ (3 << 20) 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
Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/2] drm/i915/icl: Add allowed DP rates for Icelake
On Tue, Jun 12, 2018 at 11:46:08AM +0300, Jani Nikula wrote: > On Mon, 11 Jun 2018, Paulo Zanoni wrote: > > Em Seg, 2018-06-11 às 22:35 +, 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. me neither... > > > 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. what I'm currently doing... > > > 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. Let's keep the style we want there even if this cause warnings while we haven't finished the standardization. > > > 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. I believe I prefer one mass commit. So we convert once for all and cause rebase conflict on internal branch only once. So we solve all at one and be happy... > > > 6 - Submit a separate patch converting everything to BIT() on > > i915_reg.h. > > Same as above. Do we really want BIT everywhere?! Thanks, Rodrigo. > > 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/2] drm/i915/icl: Add allowed DP rates for Icelake
On Mon, 11 Jun 2018, Paulo Zanoni wrote: > Em Seg, 2018-06-11 às 22:35 +, 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
Re: [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/2] drm/i915/icl: Add allowed DP rates for Icelake
Em Seg, 2018-06-11 às 22:35 +, 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. 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. 3 - Just ignore the checkpatch message, push code as-is. 4 - Blacklist this check from checkpatch. 5 - Submit a separate patch fixing all the spacing errors on i915_reg.h once and for all. Live happily ever after. 6 - Submit a separate patch converting everything to BIT() on i915_reg.h. Thanks, Paulo > ^ > > total: 0 errors, 0 warnings, 1 checks, 127 lines checked > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,1/2] drm/i915/icl: Add allowed DP rates for Icelake
== 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) ^ total: 0 errors, 0 warnings, 1 checks, 127 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
