[AMD Official Use Only - AMD Internal Distribution Only] HI Felix
-----Original Message----- From: Kuehling, Felix <felix.kuehl...@amd.com> Sent: Wednesday, June 5, 2024 2:45 AM To: Zhang, Jesse(Jie) <jesse.zh...@amd.com>; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Koenig, Christian <christian.koe...@amd.com>; Huang, Tim <tim.hu...@amd.com> Subject: Re: [PATCH 10/12] drm/amdkfd: remove dead code in kq_initialize On 2024-06-03 04:49, Jesse Zhang wrote: > The queue type can only be KFD_QUEUE_TYPE_DIQ or KFD_QUEUE_TYPE_HIQ, > and the default cannot be reached. I wonder, if you remove the default case, I guess you are relying on the compiler or a static checker to ensure that we can only pass valid enum values to this function. I don't think C compilers are that strict. You could pass a random integer to the function. That said, this function only has two callers, and both of them use a proper enum value. [Zhang, Jesse(Jie)] At the beginning of the function kq_initialize, we check the queue type that can ensure the warning about C compilers. static bool kq_initialize(struct kernel_queue *kq, struct kfd_node *dev, enum kfd_queue_type type, unsigned int queue_size) { struct queue_properties prop; int retval; union PM4_MES_TYPE_3_HEADER nop; if (WARN_ON(type != KFD_QUEUE_TYPE_DIQ && type != KFD_QUEUE_TYPE_HIQ)) return false; ... } Thanks Jesse > > Signed-off-by: Jesse Zhang <jesse.zh...@amd.com> Acked-by: Felix Kuehling <felix.kuehl...@amd.com> > --- > drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c > b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c > index 32c926986dbb..3142b2593e2b 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c > @@ -67,9 +67,6 @@ static bool kq_initialize(struct kernel_queue *kq, struct > kfd_node *dev, > case KFD_QUEUE_TYPE_HIQ: > kq->mqd_mgr = dev->dqm->mqd_mgrs[KFD_MQD_TYPE_HIQ]; > break; > - default: > - pr_err("Invalid queue type %d\n", type); > - return false; > } > > if (!kq->mqd_mgr)