| 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 |
| 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 |
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++; > } >
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 --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++; }
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(-)