Re: [PATCH v3 11/12] pnv/psi: Add device reset hook

[Greg Kurz]

On Tue, 7 Jan 2020 11:23:44 +1100
David Gibson <address@hidden> wrote:

> On Mon, Jan 06, 2020 at 03:56:44PM +0100, Cédric Le Goater wrote:
> > From: Greg Kurz <address@hidden>
> > 
> > And call it from a QEMU reset handler. This allows each PNV child class to
> > override the reset hook if needed, eg. POWER8 doesn't but POWER9 does.
> > The proper way to do that would be to use device_class_set_parent_reset(),
> > but defining a Pnv8PsiClass and a Pnv9PsiClass types with a parent_reset
> > pointer adds a fair amount of code. Calling pnv_psi_reset() explicitely is
> > fine for now.
> > 
> > A subsequent patch will consolidate the call to qemu_register_reset() in
> > a single place.
> > 
> > Signed-off-by: Greg Kurz <address@hidden>
> > Signed-off-by: Cédric Le Goater <address@hidden>
> > ---
> >  hw/ppc/pnv_psi.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> > index 4da3d2568624..cf21e42d93b7 100644
> > --- a/hw/ppc/pnv_psi.c
> > +++ b/hw/ppc/pnv_psi.c
> > @@ -455,7 +455,7 @@ static const MemoryRegionOps pnv_psi_xscom_ops = {
> >      }
> >  };
> >  
> > -static void pnv_psi_reset(void *dev)
> > +static void pnv_psi_reset(DeviceState *dev)
> >  {
> >      PnvPsi *psi = PNV_PSI(dev);
> >  
> > @@ -464,6 +464,13 @@ static void pnv_psi_reset(void *dev)
> >      psi->regs[PSIHB_XSCOM_BAR] = psi->bar | PSIHB_BAR_EN;
> >  }
> >  
> > +static void pnv_psi_reset_handler(void *dev)
> > +{
> > +    DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > +
> > +    dc->reset(DEVICE(dev));
> 
> I think it would be better to use device_reset() here rather than
> explicitly accessing the dc->reset pointer yourself.
> 

Of course ! Not sure why I didn't do that...

> > +}
> > +
> >  static void pnv_psi_power8_instance_init(Object *obj)
> >  {
> >      Pnv8Psi *psi8 = PNV8_PSI(obj);
> > @@ -526,7 +533,7 @@ static void pnv_psi_power8_realize(DeviceState *dev, 
> > Error **errp)
> >              ((uint64_t) i << PSIHB_XIVR_SRC_SH);
> >      }
> >  
> > -    qemu_register_reset(pnv_psi_reset, dev);
> > +    qemu_register_reset(pnv_psi_reset_handler, dev);
> >  }
> >  
> >  static int pnv_psi_dt_xscom(PnvXScomInterface *dev, void *fdt, int 
> > xscom_offset)
> > @@ -809,7 +816,7 @@ static void pnv_psi_power9_irq_set(PnvPsi *psi, int 
> > irq, bool state)
> >      qemu_set_irq(psi->qirqs[irq], state);
> >  }
> >  
> > -static void pnv_psi_power9_reset(void *dev)
> > +static void pnv_psi_power9_reset(DeviceState *dev)
> >  {
> >      Pnv9Psi *psi = PNV9_PSI(dev);
> >  
> > @@ -863,7 +870,7 @@ static void pnv_psi_power9_realize(DeviceState *dev, 
> > Error **errp)
> >  
> >      pnv_psi_set_bar(psi, psi->bar | PSIHB_BAR_EN);
> >  
> > -    qemu_register_reset(pnv_psi_power9_reset, dev);
> > +    qemu_register_reset(pnv_psi_reset_handler, dev);
> >  }
> >  
> >  static void pnv_psi_power9_class_init(ObjectClass *klass, void *data)
> > @@ -875,6 +882,7 @@ static void pnv_psi_power9_class_init(ObjectClass 
> > *klass, void *data)
> >  
> >      dc->desc    = "PowerNV PSI Controller POWER9";
> >      dc->realize = pnv_psi_power9_realize;
> > +    dc->reset   = pnv_psi_power9_reset;
> >  
> >      ppc->xscom_pcba = PNV9_XSCOM_PSIHB_BASE;
> >      ppc->xscom_size = PNV9_XSCOM_PSIHB_SIZE;
> > @@ -927,6 +935,7 @@ static void pnv_psi_class_init(ObjectClass *klass, void 
> > *data)
> >  
> >      dc->desc = "PowerNV PSI Controller";
> >      dc->props = pnv_psi_properties;
> > +    dc->reset = pnv_psi_reset;
> >  }
> >  
> >  static const TypeInfo pnv_psi_info = {
>

pgpvTMSmf5KKB.pgp
Description: OpenPGP digital signature