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

[Gupta, Anshuman]

> -----Original Message-----
> From: Andi Shyti <andi.shyti at linux.intel.com>
> Sent: Friday, September 22, 2023 8:46 PM
> To: Vivi, Rodrigo <rodrigo.vivi at intel.com>
> Cc: Nilawar, Badal <badal.nilawar at intel.com>; intel-xe at lists.freedesktop.org;
> linux-hwmon at vger.kernel.org; Gupta, Anshuman
> <anshuman.gupta at intel.com>; Dixit, Ashutosh <ashutosh.dixit at intel.com>;
> linux at roeck-us.net; andi.shyti at linux.intel.com; Tauro, Riana
> <riana.tauro at intel.com>; Brost, Matthew <matthew.brost at 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 at linux.intel.com>
> > > Signed-off-by: Badal Nilawar <badal.nilawar at intel.com>
> > > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at 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.
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