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

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

On Thu, 21 Sep 2023, "Nilawar, Badal" <badal.nilawar@intel.com> wrote:
> On 21-09-2023 21:33, 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.
> This was suggested by Andy, in fact he suggested to reuse existing 
> MISSING_CASE macro from i915. As I couldn't find common place to move it 
> I went with creating new one.
>
> I will drop this patch and simply use drm_warn.

Please use drm_WARN() or drm_WARN_ON(). With panic_on_warn=1 in CI,
it'll oops the machine, and we'll actually catch these as opposed to
just leaving them as lines in dmesg.

I guess the main purpose of MISSING_CASE() in i915 was to unify the
behaviour, though I think that was also misused.

BR,
Jani.

>
> Regards,
> Badal
>> 
>>>   #endif
>>> -- 
>>> 2.25.1
>>>

-- 
Jani Nikula, Intel