Am 05.05.23 um 16:59 schrieb Srinivasan Shanmugam:
Assignments in if condition are less readable and error-prone.Fixes below error & warnings reported by checkpatch" ERROR: do not use assignment in if condition + } else if ((src = adev->irq.client[client_id].sources[src_id])) { WARNING: braces {} are not necessary for any arm of this statement WARNING: Prefer 'unsigned int' to bare use of 'unsigned' Cc: Christian König <[email protected]> Cc: Alex Deucher <[email protected]> Signed-off-by: Srinivasan Shanmugam <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 27 +++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index c8413470e057..f312e8ca0015 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -110,7 +110,7 @@ const char *soc15_ih_clientid_name[] = { void amdgpu_irq_disable_all(struct amdgpu_device *adev) { unsigned long irqflags; - unsigned i, j, k; + unsigned int i, j, k; int r;spin_lock_irqsave(&adev->irq.lock, irqflags);@@ -269,11 +269,11 @@ int amdgpu_irq_init(struct amdgpu_device *adev) int nvec = pci_msix_vec_count(adev->pdev); unsigned int flags;- if (nvec <= 0) {+ if (nvec <= 0) flags = PCI_IRQ_MSI; - } else { + else flags = PCI_IRQ_MSI | PCI_IRQ_MSIX; - } + /* we only need one vector */ nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags); if (nvec > 0) { @@ -332,7 +332,7 @@ void amdgpu_irq_fini_hw(struct amdgpu_device *adev) */ void amdgpu_irq_fini_sw(struct amdgpu_device *adev) { - unsigned i, j; + unsigned int i, j;for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) {if (!adev->irq.client[i].sources) @@ -366,7 +366,7 @@ void amdgpu_irq_fini_sw(struct amdgpu_device *adev) * 0 on success or error code otherwise */ int amdgpu_irq_add_id(struct amdgpu_device *adev, - unsigned client_id, unsigned src_id, + unsigned int client_id, unsigned int src_id, struct amdgpu_irq_src *source) { if (client_id >= AMDGPU_IRQ_CLIENTID_MAX) @@ -418,7 +418,7 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev, { u32 ring_index = ih->rptr >> 2; struct amdgpu_iv_entry entry; - unsigned client_id, src_id; + unsigned int client_id, src_id; struct amdgpu_irq_src *src; bool handled = false; int r; @@ -431,6 +431,7 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev,client_id = entry.client_id;src_id = entry.src_id; + src = adev->irq.client[client_id].sources[src_id];
That won't work like this. We first need to validate the client_id and src_id values or otherwise this can crash.
That's why we have the assignment inside the if in the first place. Christian.
if (client_id >= AMDGPU_IRQ_CLIENTID_MAX) {DRM_DEBUG("Invalid client_id in IV: %d\n", client_id); @@ -446,7 +447,7 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev, DRM_DEBUG("Unregistered interrupt client_id: %d src_id: %d\n", client_id, src_id);- } else if ((src = adev->irq.client[client_id].sources[src_id])) {+ } else if (src) { r = src->funcs->process(adev, src, &entry); if (r < 0) DRM_ERROR("error processing interrupt (%d)\n", r); @@ -493,7 +494,7 @@ void amdgpu_irq_delegate(struct amdgpu_device *adev, * Updates interrupt state for the specific source (all ASICs). */ int amdgpu_irq_update(struct amdgpu_device *adev, - struct amdgpu_irq_src *src, unsigned type) + struct amdgpu_irq_src *src, unsigned int type) { unsigned long irqflags; enum amdgpu_interrupt_state state; @@ -556,7 +557,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct amdgpu_device *adev) * 0 on success or error code otherwise */ int amdgpu_irq_get(struct amdgpu_device *adev, struct amdgpu_irq_src *src, - unsigned type) + unsigned int type) { if (!adev->irq.installed) return -ENOENT; @@ -586,7 +587,7 @@ int amdgpu_irq_get(struct amdgpu_device *adev, struct amdgpu_irq_src *src, * 0 on success or error code otherwise */ int amdgpu_irq_put(struct amdgpu_device *adev, struct amdgpu_irq_src *src, - unsigned type) + unsigned int type) { if (!adev->irq.installed) return -ENOENT; @@ -620,7 +621,7 @@ int amdgpu_irq_put(struct amdgpu_device *adev, struct amdgpu_irq_src *src, * invalid parameters */ bool amdgpu_irq_enabled(struct amdgpu_device *adev, struct amdgpu_irq_src *src, - unsigned type) + unsigned int type) { if (!adev->irq.installed) return false; @@ -733,7 +734,7 @@ void amdgpu_irq_remove_domain(struct amdgpu_device *adev) * Returns: * Linux IRQ */ -unsigned amdgpu_irq_create_mapping(struct amdgpu_device *adev, unsigned src_id) +unsigned int amdgpu_irq_create_mapping(struct amdgpu_device *adev, unsigned int src_id) { adev->irq.virq[src_id] = irq_create_mapping(adev->irq.domain, src_id);
