[PATCH v2] drm/amd/amdgpu: Fix assingment in if condition in amdgpu_irq.c

[Alex Deucher]

On Mon, May 22, 2023 at 11:19 AM Alex Deucher <alexdeucher at gmail.com> wrote:
>
> On Mon, May 8, 2023 at 6:48 AM Srinivasan Shanmugam
> <srinivasan.shanmugam at amd.com> wrote:
> >
> > 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 <christian.koenig at amd.com>
> > Cc: Alex Deucher <alexander.deucher at amd.com>
> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
>

Actually, typo in the patch title:
assingment -> assignment
With that fixed, you can add my Ack.



> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>
> > ---
> >
> > v2:
> >
> > - Validate the client_id and src_id values or otherwise
> >   this can crash (Christian)
> >
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 29 +++++++++++++------------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> > index c8413470e057..dfaedb0243ba 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,8 +418,8 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev,
> >  {
> >         u32 ring_index = ih->rptr >> 2;
> >         struct amdgpu_iv_entry entry;
> > -       unsigned client_id, src_id;
> > -       struct amdgpu_irq_src *src;
> > +       unsigned int client_id, src_id;
> > +       struct amdgpu_irq_src *src = NULL;
> >         bool handled = false;
> >         int r;
> >
> > @@ -446,7 +446,8 @@ 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 (client_id < AMDGPU_IRQ_CLIENTID_MAX && src_id < AMDGPU_MAX_IRQ_SRC_ID) {
> > +               src = adev->irq.client[client_id].sources[src_id];
> >                 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);
> >
> > --
> > 2.25.1
> >