Re: [PATCH v4 15/19] spapr: Remove SpaprIrq::nr_msis

From: David Gibson <david@gibson.dropbear.id.au>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: "Jason Wang" <jasowang@redhat.com>,
	"Riku Voipio" <riku.voipio@iki.fi>,
	"Laurent Vivier" <laurent@vivier.eu>,
	qemu-devel@nongnu.org, groug@kaod.org, qemu-ppc@nongnu.org,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	philmd@redhat.com
Subject: Re: [PATCH v4 15/19] spapr: Remove SpaprIrq::nr_msis
Date: Thu, 10 Oct 2019 12:56:43 +1100	[thread overview]
Message-ID: <20191010015643.GA28552@umbus.fritz.box> (raw)
In-Reply-To: <cbf330ce-2f95-5af1-aff5-8bed120c7317@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 3024 bytes --]

On Wed, Oct 09, 2019 at 05:59:13PM +0200, Cédric Le Goater wrote:
> On 09/10/2019 08:08, David Gibson wrote:
> > The nr_msis value we use here has to line up with whether we're using
> > legacy or modern irq allocation.  Therefore it's safer to derive it based
> > on legacy_irq_allocation rather than having SpaprIrq contain a canned
> > value.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> one minor typo below,
> 
> > ---
> >  hw/ppc/spapr.c              |  5 ++---
> >  hw/ppc/spapr_irq.c          | 26 +++++++++++++++++---------
> >  hw/ppc/spapr_pci.c          |  7 ++++---
> >  include/hw/pci-host/spapr.h |  4 ++--
> >  include/hw/ppc/spapr_irq.h  |  4 +---
> >  5 files changed, 26 insertions(+), 20 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index e880db5d38..153cc54354 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1275,7 +1275,7 @@ static void *spapr_build_fdt(SpaprMachineState *spapr)
> >      }
> >  
> >      QLIST_FOREACH(phb, &spapr->phbs, list) {
> > -        ret = spapr_dt_phb(phb, PHANDLE_INTC, fdt, spapr->irq->nr_msis, NULL);
> > +        ret = spapr_dt_phb(spapr, phb, PHANDLE_INTC, fdt, NULL);
> >          if (ret < 0) {
> >              error_report("couldn't setup PCI devices in fdt");
> >              exit(1);
> > @@ -3910,8 +3910,7 @@ int spapr_phb_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
> >          return -1;
> >      }
> >  
> > -    if (spapr_dt_phb(sphb, intc_phandle, fdt, spapr->irq->nr_msis,
> > -                     fdt_start_offset)) {
> > +    if (spapr_dt_phb(spapr, sphb, intc_phandle, fdt, fdt_start_offset)) {
> >          error_setg(errp, "unable to create FDT node for PHB %d", sphb->index);
> >          return -1;
> >      }
> > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> > index f3d18b1dad..076da31501 100644
> > --- a/hw/ppc/spapr_irq.c
> > +++ b/hw/ppc/spapr_irq.c
> > @@ -29,9 +29,14 @@ static const TypeInfo spapr_intc_info = {
> >      .class_size = sizeof(SpaprInterruptControllerClass),
> >  };
> >  
> > -void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis)
> > +static void spapr_irq_msi_init(SpaprMachineState *spapr)
> >  {
> > -    spapr->irq_map_nr = nr_msis;
> > +    if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
> > +        /* Legacy mode doesn't use this allocater */
> 
> allocater -> allocator

Huh.  I had surprising difficulty trying to confirm what the correct
spelling really is here.  Actual dictionaries seem to thing that
neither "allocater" nor "allocator" are words.  Still "allocator"
seems to be by far the more common variant, so I've changed it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]