Re: [PATCH v2 2/2] ppc/pnv: Connect I2C controller model to powernv9 chip

2023-10-16 Thread Miles Glenn
On Fri, 2023-10-13 at 09:06 +0200, Cédric Le Goater wrote:
> On 10/12/23 22:08, Glenn Miles wrote:
> > From: Cédric Le Goater 
> > 
> > Wires up three I2C controller instances to the powernv9 chip
> > XSCOM address space.
> > 
> > Each controller instance is wired up to a single I2C bus of
> > its own.  No other I2C devices are connected to the buses
> > at this time.
> > 
> > Signed-off-by: Cédric Le Goater 
> > [milesg: Split wiring from addition of model itself]
> > [milesg: Added new commit message]
> > [milesg: Moved hardcoded attributes into PnvChipClass]
> > Signed-off-by: Glenn Miles 
> > ---
> > 
> > Changes in v2:
> >  - Moved some hardcoded attributes into PnvChipClass
> > 
> >   hw/ppc/pnv.c  | 29 +
> >   include/hw/ppc/pnv_chip.h |  8 
> >   2 files changed, 37 insertions(+)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index eb54f93986..7db6f3abe5 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -1438,6 +1438,10 @@ static void
> > pnv_chip_power9_instance_init(Object *obj)
> >   object_initialize_child(obj, "pec[*]", >pecs[i],
> >   TYPE_PNV_PHB4_PEC);
> >   }
> > +
> > +for (i = 0; i < pcc->i2c_num_engines; i++) {
> > +object_initialize_child(obj, "i2c[*]", >i2c[i],
> > TYPE_PNV_I2C);
> > +}
> >   }
> >   
> >   static void pnv_chip_quad_realize_one(PnvChip *chip, PnvQuad *eq,
> > @@ -1510,6 +1514,7 @@ static void
> > pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> >   PnvChip *chip = PNV_CHIP(dev);
> >   Pnv9Psi *psi9 = >psi;
> >   Error *local_err = NULL;
> > +int i;
> >   
> >   /* XSCOM bridge is first */
> >   pnv_xscom_realize(chip, PNV9_XSCOM_SIZE, _err);
> > @@ -1613,6 +1618,28 @@ static void
> > pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> >   error_propagate(errp, local_err);
> >   return;
> >   }
> > +
> > +/*
> > + * I2C
> > + * TODO: The number of busses is specific to each platform
> 
> I would remove the TODO now,
> 
Ok, done.

> Reviewed-by: Cédric Le Goater 
> 
> Thanks,
> 
> C.
> 
> 

Thank for the review!  I'll be sending out a v3 shortly.

Glenn

> > + */
> > +for (i = 0; i < pcc->i2c_num_engines; i++) {
> > +Object *obj =  OBJECT(>i2c[i]);
> > +
> > +object_property_set_int(obj, "engine", i + 1,
> > _fatal);
> > +object_property_set_int(obj, "num-busses", pcc-
> > >i2c_num_ports,
> > +_fatal);
> > +object_property_set_link(obj, "chip", OBJECT(chip),
> > _abort);
> > +if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> > +return;
> > +}
> > +pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
> > +   chip9->i2c[i].engine *
> > PNV9_XSCOM_I2CM_SIZE,
> > +>i2c[i].xscom_regs);
> > +qdev_connect_gpio_out(DEVICE(>i2c[i]), 0,
> > +  qdev_get_gpio_in(DEVICE(
> > >psi),
> > +   PSIHB9_IRQ_SBE_I2C)
> > );
> > +}
> >   }
> >   
> >   static uint32_t pnv_chip_power9_xscom_pcba(PnvChip *chip,
> > uint64_t addr)
> > @@ -1640,6 +1667,8 @@ static void
> > pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> >   k->xscom_pcba = pnv_chip_power9_xscom_pcba;
> >   dc->desc = "PowerNV Chip POWER9";
> >   k->num_pecs = PNV9_CHIP_MAX_PEC;
> > +k->i2c_num_engines = PNV9_CHIP_MAX_I2C;
> > +k->i2c_num_ports = PNV9_CHIP_MAX_I2C_PORTS;
> >   
> >   device_class_set_parent_realize(dc, pnv_chip_power9_realize,
> >   >parent_realize);
> > diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> > index 53e1d921d7..90cfbad1a5 100644
> > --- a/include/hw/ppc/pnv_chip.h
> > +++ b/include/hw/ppc/pnv_chip.h
> > @@ -9,6 +9,7 @@
> >   #include "hw/ppc/pnv_psi.h"
> >   #include "hw/ppc/pnv_sbe.h"
> >   #include "hw/ppc/pnv_xive.h"
> > +#include "hw/ppc/pnv_i2c.h"
> >   #include "hw/sysbus.h"
> >   
> >   OBJECT_DECLARE_TYPE(PnvChip, PnvChipClass,
> > @@ -86,6 +87,10 @@ struct Pnv9Chip {
> >   
> >   #define PNV9_CHIP_MAX_PEC 3
> >   PnvPhb4PecState pecs[PNV9_CHIP_MAX_PEC];
> > +
> > +#define PNV9_CHIP_MAX_I2C 3
> > +#define PNV9_CHIP_MAX_I2C_PORTS 1
> > +PnvI2C  i2c[PNV9_CHIP_MAX_I2C];
> >   };
> >   
> >   /*
> > @@ -130,6 +135,9 @@ struct PnvChipClass {
> >   uint32_t num_pecs;
> >   uint32_t num_phbs;
> >   
> > +uint32_t i2c_num_engines;
> > +uint32_t i2c_num_ports;
> > +
> >   DeviceRealize parent_realize;
> >   
> >   uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);




Re: [PATCH v2 2/2] ppc/pnv: Connect I2C controller model to powernv9 chip

2023-10-13 Thread Cédric Le Goater

On 10/12/23 22:08, Glenn Miles wrote:

From: Cédric Le Goater 

Wires up three I2C controller instances to the powernv9 chip
XSCOM address space.

Each controller instance is wired up to a single I2C bus of
its own.  No other I2C devices are connected to the buses
at this time.

Signed-off-by: Cédric Le Goater 
[milesg: Split wiring from addition of model itself]
[milesg: Added new commit message]
[milesg: Moved hardcoded attributes into PnvChipClass]
Signed-off-by: Glenn Miles 
---

Changes in v2:
 - Moved some hardcoded attributes into PnvChipClass

  hw/ppc/pnv.c  | 29 +
  include/hw/ppc/pnv_chip.h |  8 
  2 files changed, 37 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index eb54f93986..7db6f3abe5 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1438,6 +1438,10 @@ static void pnv_chip_power9_instance_init(Object *obj)
  object_initialize_child(obj, "pec[*]", >pecs[i],
  TYPE_PNV_PHB4_PEC);
  }
+
+for (i = 0; i < pcc->i2c_num_engines; i++) {
+object_initialize_child(obj, "i2c[*]", >i2c[i], TYPE_PNV_I2C);
+}
  }
  
  static void pnv_chip_quad_realize_one(PnvChip *chip, PnvQuad *eq,

@@ -1510,6 +1514,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
Error **errp)
  PnvChip *chip = PNV_CHIP(dev);
  Pnv9Psi *psi9 = >psi;
  Error *local_err = NULL;
+int i;
  
  /* XSCOM bridge is first */

  pnv_xscom_realize(chip, PNV9_XSCOM_SIZE, _err);
@@ -1613,6 +1618,28 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
Error **errp)
  error_propagate(errp, local_err);
  return;
  }
+
+/*
+ * I2C
+ * TODO: The number of busses is specific to each platform



I would remove the TODO now,

Reviewed-by: Cédric Le Goater 

Thanks,

C.



+ */
+for (i = 0; i < pcc->i2c_num_engines; i++) {
+Object *obj =  OBJECT(>i2c[i]);
+
+object_property_set_int(obj, "engine", i + 1, _fatal);
+object_property_set_int(obj, "num-busses", pcc->i2c_num_ports,
+_fatal);
+object_property_set_link(obj, "chip", OBJECT(chip), _abort);
+if (!qdev_realize(DEVICE(obj), NULL, errp)) {
+return;
+}
+pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
+   chip9->i2c[i].engine * PNV9_XSCOM_I2CM_SIZE,
+>i2c[i].xscom_regs);
+qdev_connect_gpio_out(DEVICE(>i2c[i]), 0,
+  qdev_get_gpio_in(DEVICE(>psi),
+   PSIHB9_IRQ_SBE_I2C));
+}
  }
  
  static uint32_t pnv_chip_power9_xscom_pcba(PnvChip *chip, uint64_t addr)

@@ -1640,6 +1667,8 @@ static void pnv_chip_power9_class_init(ObjectClass 
*klass, void *data)
  k->xscom_pcba = pnv_chip_power9_xscom_pcba;
  dc->desc = "PowerNV Chip POWER9";
  k->num_pecs = PNV9_CHIP_MAX_PEC;
+k->i2c_num_engines = PNV9_CHIP_MAX_I2C;
+k->i2c_num_ports = PNV9_CHIP_MAX_I2C_PORTS;
  
  device_class_set_parent_realize(dc, pnv_chip_power9_realize,

  >parent_realize);
diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 53e1d921d7..90cfbad1a5 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -9,6 +9,7 @@
  #include "hw/ppc/pnv_psi.h"
  #include "hw/ppc/pnv_sbe.h"
  #include "hw/ppc/pnv_xive.h"
+#include "hw/ppc/pnv_i2c.h"
  #include "hw/sysbus.h"
  
  OBJECT_DECLARE_TYPE(PnvChip, PnvChipClass,

@@ -86,6 +87,10 @@ struct Pnv9Chip {
  
  #define PNV9_CHIP_MAX_PEC 3

  PnvPhb4PecState pecs[PNV9_CHIP_MAX_PEC];
+
+#define PNV9_CHIP_MAX_I2C 3
+#define PNV9_CHIP_MAX_I2C_PORTS 1
+PnvI2C  i2c[PNV9_CHIP_MAX_I2C];
  };
  
  /*

@@ -130,6 +135,9 @@ struct PnvChipClass {
  uint32_t num_pecs;
  uint32_t num_phbs;
  
+uint32_t i2c_num_engines;

+uint32_t i2c_num_ports;
+
  DeviceRealize parent_realize;
  
  uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);





[PATCH v2 2/2] ppc/pnv: Connect I2C controller model to powernv9 chip

2023-10-12 Thread Glenn Miles
From: Cédric Le Goater 

Wires up three I2C controller instances to the powernv9 chip
XSCOM address space.

Each controller instance is wired up to a single I2C bus of
its own.  No other I2C devices are connected to the buses
at this time.

Signed-off-by: Cédric Le Goater 
[milesg: Split wiring from addition of model itself]
[milesg: Added new commit message]
[milesg: Moved hardcoded attributes into PnvChipClass]
Signed-off-by: Glenn Miles 
---

Changes in v2:
- Moved some hardcoded attributes into PnvChipClass

 hw/ppc/pnv.c  | 29 +
 include/hw/ppc/pnv_chip.h |  8 
 2 files changed, 37 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index eb54f93986..7db6f3abe5 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1438,6 +1438,10 @@ static void pnv_chip_power9_instance_init(Object *obj)
 object_initialize_child(obj, "pec[*]", >pecs[i],
 TYPE_PNV_PHB4_PEC);
 }
+
+for (i = 0; i < pcc->i2c_num_engines; i++) {
+object_initialize_child(obj, "i2c[*]", >i2c[i], TYPE_PNV_I2C);
+}
 }
 
 static void pnv_chip_quad_realize_one(PnvChip *chip, PnvQuad *eq,
@@ -1510,6 +1514,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
Error **errp)
 PnvChip *chip = PNV_CHIP(dev);
 Pnv9Psi *psi9 = >psi;
 Error *local_err = NULL;
+int i;
 
 /* XSCOM bridge is first */
 pnv_xscom_realize(chip, PNV9_XSCOM_SIZE, _err);
@@ -1613,6 +1618,28 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
Error **errp)
 error_propagate(errp, local_err);
 return;
 }
+
+/*
+ * I2C
+ * TODO: The number of busses is specific to each platform
+ */
+for (i = 0; i < pcc->i2c_num_engines; i++) {
+Object *obj =  OBJECT(>i2c[i]);
+
+object_property_set_int(obj, "engine", i + 1, _fatal);
+object_property_set_int(obj, "num-busses", pcc->i2c_num_ports,
+_fatal);
+object_property_set_link(obj, "chip", OBJECT(chip), _abort);
+if (!qdev_realize(DEVICE(obj), NULL, errp)) {
+return;
+}
+pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
+   chip9->i2c[i].engine * PNV9_XSCOM_I2CM_SIZE,
+>i2c[i].xscom_regs);
+qdev_connect_gpio_out(DEVICE(>i2c[i]), 0,
+  qdev_get_gpio_in(DEVICE(>psi),
+   PSIHB9_IRQ_SBE_I2C));
+}
 }
 
 static uint32_t pnv_chip_power9_xscom_pcba(PnvChip *chip, uint64_t addr)
@@ -1640,6 +1667,8 @@ static void pnv_chip_power9_class_init(ObjectClass 
*klass, void *data)
 k->xscom_pcba = pnv_chip_power9_xscom_pcba;
 dc->desc = "PowerNV Chip POWER9";
 k->num_pecs = PNV9_CHIP_MAX_PEC;
+k->i2c_num_engines = PNV9_CHIP_MAX_I2C;
+k->i2c_num_ports = PNV9_CHIP_MAX_I2C_PORTS;
 
 device_class_set_parent_realize(dc, pnv_chip_power9_realize,
 >parent_realize);
diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 53e1d921d7..90cfbad1a5 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -9,6 +9,7 @@
 #include "hw/ppc/pnv_psi.h"
 #include "hw/ppc/pnv_sbe.h"
 #include "hw/ppc/pnv_xive.h"
+#include "hw/ppc/pnv_i2c.h"
 #include "hw/sysbus.h"
 
 OBJECT_DECLARE_TYPE(PnvChip, PnvChipClass,
@@ -86,6 +87,10 @@ struct Pnv9Chip {
 
 #define PNV9_CHIP_MAX_PEC 3
 PnvPhb4PecState pecs[PNV9_CHIP_MAX_PEC];
+
+#define PNV9_CHIP_MAX_I2C 3
+#define PNV9_CHIP_MAX_I2C_PORTS 1
+PnvI2C  i2c[PNV9_CHIP_MAX_I2C];
 };
 
 /*
@@ -130,6 +135,9 @@ struct PnvChipClass {
 uint32_t num_pecs;
 uint32_t num_phbs;
 
+uint32_t i2c_num_engines;
+uint32_t i2c_num_ports;
+
 DeviceRealize parent_realize;
 
 uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
-- 
2.31.1