[Intel-gfx] [PATCH 09/13] drm/i915/skl: Use MISSING_CASE() in skl_wrpll_params_populate()

[Paulo Zanoni]

2015-05-28 4:51 GMT-03:00 Daniel Vetter <daniel at ffwll.ch>:
> On Wed, May 27, 2015 at 03:40:32PM -0300, Paulo Zanoni wrote:
>> 2015-05-07 14:38 GMT-03:00 Damien Lespiau <damien.lespiau at intel.com>:
>> > We now have a special macro for those cases.
>>
>> I'm not sure if this patch is an improvement. Before it, we always
>> knew which "switch" statement was bad since we used to print either
>> "PDiv" or "KDiv". After the patch, it will not be possible to know
>> from which switch statement the error came from. Of course, there's
>> the advantage of at least knowing the value. I'd vote to either skip
>> this patch, or improve the MISSING_CASE macro to be able to account
>> for multiple uses on the same function. But I'm open to arugmentation
>> :)
>
> MISSING_CASE is a WARN, which also prints the line number. Not enough?

Line numbers are not very useful unless you're absolutely sure which
tree/commit someone is running. And it still takes a lot of work to
checkout the correct tree/commit and discover which of the WARNs is on
that specific line.

> -Daniel
>
>>
>> >
>> > Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_ddi.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> > index b9d5d65..ab327a1 100644
>> > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> > @@ -1145,7 +1145,7 @@ static void skl_wrpll_params_populate(struct skl_wrpll_params *params,
>> >                 params->pdiv = 4;
>> >                 break;
>> >         default:
>> > -               WARN(1, "Incorrect PDiv\n");
>> > +               MISSING_CASE(p0);
>> >         }
>> >
>> >         switch (p2) {
>> > @@ -1162,7 +1162,7 @@ static void skl_wrpll_params_populate(struct skl_wrpll_params *params,
>> >                 params->kdiv = 3;
>> >                 break;
>> >         default:
>> > -               WARN(1, "Incorrect KDiv\n");
>> > +               MISSING_CASE(p2);
>> >         }
>> >
>> >         params->qdiv_ratio = p1;
>> > --
>> > 2.1.0
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Paulo Zanoni