Re: [PATCH kvm-unit-tests v2 11/17] pci: provide pci_enable_defaults()

[Peter Xu <peterx@redhat.com>]

On Thu, Nov 10, 2016 at 08:33:42PM +0100, Andrew Jones wrote:
> On Wed, Nov 09, 2016 at 10:10:18AM -0500, Peter Xu wrote:
> > Provide a function to do most of the common PCI init work.
> > 
> > Suggested-by: Andrew Jones <drjones@redhat.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  lib/pci.c | 8 ++++++++
> >  lib/pci.h | 1 +
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/lib/pci.c b/lib/pci.c
> > index fd17ea5..971f02e 100644
> > --- a/lib/pci.c
> > +++ b/lib/pci.c
> > @@ -249,3 +249,11 @@ void pci_scan_bars(struct pci_dev *dev)
> >  		dev->bar[i] = pci_bar_get_addr(dev, i);
> >  	}
> >  }
> > +
> > +int pci_enable_defaults(struct pci_dev *dev)
> > +{
> > +	pci_scan_bars(dev);
> > +	/* Enable device DMA operations */
> > +	pci_cmd_set_clr(dev, PCI_COMMAND_MASTER, 0);
> > +	return 0;
> 
> Shouldn't this be a void function that just asserts on
> any errors it detects? I'm not sure why we're [currently
> unconditionally] returning zero.

This is my intention since I think this function might grow in the
future with lots of stuffs inside, and this function will need a
return code as long as any one of the future small functions will
return an error. However that's really not a strong reason to have
it... will remove it for your r-b. :-)

Thanks,

-- peterx