Re: [PATCH 1/5] drm/i915/uc: Move intel_uc_fw_status_repr() to intel_uc.c

["Srivatsa, Anusha" <anusha.srivatsa@intel.com>]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Michal Wajdeczko
>Sent: Tuesday, March 28, 2017 5:51 AM
>To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>Cc: intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 1/5] drm/i915/uc: Move intel_uc_fw_status_repr()
>to intel_uc.c
>
>On Tue, Mar 28, 2017 at 10:27:28AM +0200, Michal Wajdeczko wrote:
>> On Tue, Mar 28, 2017 at 10:05:31AM +0300, Joonas Lahtinen wrote:
>> > On ma, 2017-03-27 at 17:19 +0000, Michal Wajdeczko wrote:
>> > > The file fits better. Also use "<invalid>" for invalid case.
>> > >
>> > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> >
>> > <SNIP>
>> >
>> > > @@ -26,6 +26,24 @@
>> > >  #include "intel_uc.h"
>> > >  #include <linux/firmware.h>
>> > >
>> > > +
>> > > +/* User-friendly representation of an enum */ const char
>> > > +*intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>> >
>> > This could be static inline in the .h too.
>> >
>> > > +{
>> > > +	switch (status) {
>> > > +	case INTEL_UC_FIRMWARE_FAIL:
>> > > +		return "FAIL";
>> > > +	case INTEL_UC_FIRMWARE_NONE:
>> > > +		return "NONE";
>> > > +	case INTEL_UC_FIRMWARE_PENDING:
>> > > +		return "PENDING";
>> > > +	case INTEL_UC_FIRMWARE_SUCCESS:
>> > > +		return "SUCCESS";
>> > > +	default:
>> >
>> > Add MISSING_CASE(status); here.
>>
>> This will require another patch that will move definition of
>> MISSING_CASE macro from i915_drv.h to i915_utils.h as intel_uc.h is
>> included now ahead of MISSING_CASE definition... stay tuned ;)
>>
>
>There is also other option: we can drop "default" case and then rely on the
>compiler to complain at build time when we miss any enum.

But wont having a default option make it more readable and friendly? Just a thought.... I like it the way it is now.....

Anusha 
>-Michal
>
>_______________________________________________
>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