RE: [PATCH v2 08/14] vfio/pci: Re-order vfio_pci_probe()

["Tian, Kevin" <kevin.tian@intel.com>]

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Wednesday, March 17, 2021 6:27 AM
> 
> On Tue, 16 Mar 2021 10:20:58 -0300
> Jason Gunthorpe <jgg@nvidia.com> wrote:
> 
> > On Tue, Mar 16, 2021 at 08:04:55AM +0000, Tian, Kevin wrote:
> > > > @@ -2060,15 +2056,20 @@ static int vfio_pci_probe(struct pci_dev
> *pdev,
> > > > const struct pci_device_id *id)
> > > >  		vfio_pci_set_power_state(vdev, PCI_D3hot);
> > > >  	}
> > > >
> > > > -	return ret;
> > > > +	ret = vfio_add_group_dev(&pdev->dev, &vfio_pci_ops, vdev);
> > > > +	if (ret)
> > > > +		goto out_power;
> > > > +	return 0;
> > > >
> > > > +out_power:
> > > > +	if (!disable_idle_d3)
> > > > +		vfio_pci_set_power_state(vdev, PCI_D0);
> > >
> > > Just curious whether the power state must be recovered upon failure
> here.
> > > From the comment several lines above, the power state is set to an
> unknown
> > > state before doing D3 transaction. From this point it looks fine if leaving
> the
> > > device in D3 since there is no expected state to be recovered?
> >
> > I don't know, this is what the remove function does, so I can't see a
> > reason why remove should do it but not here.
> 
> I'm not sure it matters in either case, we're just trying to be most
> similar to expected driver behavior.  pci_enable_device() puts the
> device in D0 but pci_disable_device() doesn't touch the power state, so
> the device would typically be released from a PCI driver in D0 afaict.
> Thanks,
> 

OK. Then,

Reviewed-by: Kevin Tian <kevin.tian@intel.com>