Re: [Intel-xe] [PATCH v5 1/6] drm/xe: Add XE_MISSING_CASE macro

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 22 Sep 2023, "Gupta, Anshuman" <anshuman.gupta@intel.com> wrote:
>> -----Original Message-----
>> From: Andi Shyti <andi.shyti@linux.intel.com>
>> Sent: Friday, September 22, 2023 8:46 PM
>> To: Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Cc: Nilawar, Badal <badal.nilawar@intel.com>; intel-xe@lists.freedesktop.org;
>> linux-hwmon@vger.kernel.org; Gupta, Anshuman
>> <anshuman.gupta@intel.com>; Dixit, Ashutosh <ashutosh.dixit@intel.com>;
>> linux@roeck-us.net; andi.shyti@linux.intel.com; Tauro, Riana
>> <riana.tauro@intel.com>; Brost, Matthew <matthew.brost@intel.com>
>> Subject: Re: [PATCH v5 1/6] drm/xe: Add XE_MISSING_CASE macro
>> 
>> Hi Rodrigo,
>> 
>> On Thu, Sep 21, 2023 at 12:03:26PM -0400, Rodrigo Vivi wrote:
>> > On Thu, Sep 21, 2023 at 03:55:14PM +0530, Badal Nilawar wrote:
>> > > Add XE_MISSING_CASE macro to handle missing switch case
>> > >
>> > > v2: Add comment about macro usage (Himal)
>> > >
>> > > Cc: Andi Shyti <andi.shyti@linux.intel.com>
>> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> > > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> > > ---
>> > >  drivers/gpu/drm/xe/xe_macros.h | 4 ++++
>> > >  1 file changed, 4 insertions(+)
>> > >
>> > > diff --git a/drivers/gpu/drm/xe/xe_macros.h
>> > > b/drivers/gpu/drm/xe/xe_macros.h index daf56c846d03..6c74c69920ed
>> > > 100644
>> > > --- a/drivers/gpu/drm/xe/xe_macros.h
>> > > +++ b/drivers/gpu/drm/xe/xe_macros.h
>> > > @@ -15,4 +15,8 @@
>> > >  			    "Ioctl argument check failed at %s:%d: %s", \
>> > >  			    __FILE__, __LINE__, #cond), 1))
>> > >
>> > > +/* Parameter to macro should be a variable name */ #define
>> > > +XE_MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \
>> > > +				__stringify(x), (long)(x))
>> > > +
>> >
>> > No, please! Let's not add unnecessary macros.
>> 
>> it's not a bad idea, actually... in i915 we have the MISSING_CASE for switch
>> cases with a defined number of cases and print warnings in case none if them
>> is the one inside the switch.
> IMHO Our CI aborts the on MISSING_CASE, which is not recoverable, drm_warn would
> Be better alternative here.

The whole point is that it aborts, so it won't get ignored. It's only
for cases like this.

BR,
Jani.


> Thanks,
> Anshuman Gupta.
>> 
>> It's so widely used and actually nice to have that I thought many times to
>> move it to some core kernel libraries.
>> 
>> Andi

-- 
Jani Nikula, Intel