diff mbox series

[8/9] npu2: Remove redundant assignment to p->phb_nvlink.scan_map

Message ID 1547049531-759-8-git-send-email-arbab@linux.ibm.com
State Accepted
Headers show
Series [1/9] Remove duplicate npu2-common.o from $(HW_OBJS) | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success master/apply_patch Successfully applied
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master

Commit Message

Reza Arbab Jan. 9, 2019, 3:58 p.m. UTC
In npu2_populate_devices(), we do

    p->phb_nvlink.scan_map |= 0x1 << ((dev->bdfn & 0xf8) >> 3);
    :

    dev->nvlink.pvd = pci_virt_add_device(&p->phb_nvlink, dev->bdfn, 0x100, dev);
    /* At this point, dev->nvlink.pvd->bdfn = dev->bdfn */

    if (dev->nvlink.pvd) {
        p->phb_nvlink.scan_map |= 0x1 << ((dev->nvlink.pvd->bdfn & 0xf8) >> 3);
        :
    }

Because dev->nvlink.pvd->bdfn equals dev->bdfn, the second assignment to
scan_map is redundant. Remove it.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 hw/npu2.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Andrew Donnellan Jan. 10, 2019, 12:06 a.m. UTC | #1
On 10/1/19 2:58 am, Reza Arbab wrote:
> In npu2_populate_devices(), we do
> 
>      p->phb_nvlink.scan_map |= 0x1 << ((dev->bdfn & 0xf8) >> 3);
>      :
> 
>      dev->nvlink.pvd = pci_virt_add_device(&p->phb_nvlink, dev->bdfn, 0x100, dev);
>      /* At this point, dev->nvlink.pvd->bdfn = dev->bdfn */
> 
>      if (dev->nvlink.pvd) {
>          p->phb_nvlink.scan_map |= 0x1 << ((dev->nvlink.pvd->bdfn & 0xf8) >> 3);
>          :
>      }
> 
> Because dev->nvlink.pvd->bdfn equals dev->bdfn, the second assignment to
> scan_map is redundant. Remove it.
> 
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

(though perhaps it makes more sense to get rid of the first scan map 
assignment and leave this one? idk)

> ---
>   hw/npu2.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/hw/npu2.c b/hw/npu2.c
> index 8418e04..68a5382 100644
> --- a/hw/npu2.c
> +++ b/hw/npu2.c
> @@ -1838,11 +1838,8 @@ static void npu2_populate_devices(struct npu2 *p,
>   
>   		/* Initialize PCI virtual device */
>   		dev->nvlink.pvd = pci_virt_add_device(&p->phb_nvlink, dev->bdfn, 0x100, dev);
> -		if (dev->nvlink.pvd) {
> -			p->phb_nvlink.scan_map |=
> -				0x1 << ((dev->nvlink.pvd->bdfn & 0xf8) >> 3);
> +		if (dev->nvlink.pvd)
>   			npu2_populate_cfg(dev);
> -		}
>   
>   		index++;
>   	}
>
Reza Arbab Jan. 10, 2019, 6:01 p.m. UTC | #2
On Thu, Jan 10, 2019 at 11:06:37AM +1100, Andrew Donnellan wrote:
>(though perhaps it makes more sense to get rid of the first scan map 
>assignment and leave this one? idk)

Sure, that is a little better because then the assignment wouldn't occur 
at all if pci_virt_add_device() fails. I'll do a v2.

Why did I send all these as a set instead of individual patches? :)
diff mbox series

Patch

diff --git a/hw/npu2.c b/hw/npu2.c
index 8418e04..68a5382 100644
--- a/hw/npu2.c
+++ b/hw/npu2.c
@@ -1838,11 +1838,8 @@  static void npu2_populate_devices(struct npu2 *p,
 
 		/* Initialize PCI virtual device */
 		dev->nvlink.pvd = pci_virt_add_device(&p->phb_nvlink, dev->bdfn, 0x100, dev);
-		if (dev->nvlink.pvd) {
-			p->phb_nvlink.scan_map |=
-				0x1 << ((dev->nvlink.pvd->bdfn & 0xf8) >> 3);
+		if (dev->nvlink.pvd)
 			npu2_populate_cfg(dev);
-		}
 
 		index++;
 	}