From: Greg Kurz <groug@kaod.org>

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 <groug@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 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));
+}
+
 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 = {
-- 
2.21.1

On Mon, Jan 06, 2020 at 03:56:44PM +0100, Cédric Le Goater wrote:
> From: Greg Kurz <groug@kaod.org>
> 
> 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 <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  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.

> +}
> +
>  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 = {

-- 
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

On Tue, 7 Jan 2020 11:23:44 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Jan 06, 2020 at 03:56:44PM +0100, Cédric Le Goater wrote:
> > From: Greg Kurz <groug@kaod.org>
> > 
> > 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 <groug@kaod.org>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >  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 = {
>