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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux