On Thu, Oct 6, 2022 at 5:29 AM Juergen Gross <jgross@xxxxxxxx> wrote: > > pcifront_try_connect() and pcifront_attach_devices() share a large > chunk of duplicated code for reading the config information from > Xenstore, which only differs regarding a function call. > > Put that code into a new sub-function. While at it fix the error > reporting in case the root-xx node had the wrong format. > > As the return value of pcifront_try_connect() and > pcifront_attach_devices() are not used anywhere make those functions > return void. As an additional bonus this removes the dubious return > of -EFAULT in case of an unexpected driver state. > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx> > --- > drivers/pci/xen-pcifront.c | 133 +++++++++++-------------------------- > 1 file changed, 40 insertions(+), 93 deletions(-) > > diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c > index 689271c4245c..a68e47dcdd7e 100644 > --- a/drivers/pci/xen-pcifront.c > +++ b/drivers/pci/xen-pcifront.c > @@ -819,76 +819,79 @@ static int pcifront_publish_info(struct pcifront_device *pdev) > err = xenbus_scanf(XBT_NIL, pdev->xdev->otherend, > "root_num", "%d", &num_roots); > if (err == -ENOENT) { > xenbus_dev_error(pdev->xdev, err, > "No PCI Roots found, trying 0000:00"); > - err = pcifront_scan_root(pdev, 0, 0); > + if (rescan) > + err = pcifront_rescan_root(pdev, 0, 0); > + else > + err = pcifront_scan_root(pdev, 0, 0); Early in pcifront_rescan_root(), we have: b = pci_find_bus(domain, bus); if (!b) /* If the bus is unknown, create it. */ return pcifront_scan_root(pdev, domain, bus); pcifront_scan_root() does some allocation, but the later scanning matches that of pcifront_rescan_root(). So I think we can just always call pcifront_rescan_root() and it should do the right thing. That drops the need for the rescan boolean. Regardless of the above idea: Reviewed-by: Jason Andryuk <jandryuk@xxxxxxxxx> Regards, Jason