On Fri, 22 Sep 2023, "Gupta, Anshuman" <anshuman.gupta@xxxxxxxxx> wrote: >> -----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. 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
