* [PATCH] ppc/pnv: Fix number of I2C engines and ports for power9/10
@ 2023-10-23 16:52 Glenn Miles
2023-10-24 5:47 ` Cédric Le Goater
0 siblings, 1 reply; 3+ messages in thread
From: Glenn Miles @ 2023-10-23 16:52 UTC (permalink / raw)
To: qemu-ppc; +Cc: Glenn Miles, qemu-devel, clg, npiggin, fbarrat
Power9 is supposed to have 4 PIB-connected I2C engines with the
following number of ports on each engine:
0: 2
1: 13
2: 2
3: 2
Power10 also has 4 engines but has the following number of ports
on each engine:
0: 14
1: 14
2: 2
3: 16
Current code assumes that they all have the same (maximum) number.
This can be a problem if software expects to see a certain number
of ports present (Power Hypervisor seems to care).
Fixed this by adding separate tables for power9 and power10 that
map the I2C controller number to the number of I2C buses that should
be attached for that engine.
Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
---
Based-on: <20231017221434.810363-1-milesg@linux.vnet.ibm.com>
([PATCH] ppc/pnv: Connect PNV I2C controller to powernv10)
hw/ppc/pnv.c | 12 ++++++++----
include/hw/ppc/pnv_chip.h | 5 +----
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 2655b6e506..6ad4a1a7b1 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1507,6 +1507,8 @@ static void pnv_chip_power9_pec_realize(PnvChip *chip, Error **errp)
}
}
+static int pnv_power9_i2c_ports_per_ctlr[PNV9_CHIP_MAX_I2C] = {2, 13, 2, 2};
+
static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
{
PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
@@ -1626,7 +1628,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
Object *obj = OBJECT(&chip9->i2c[i]);
object_property_set_int(obj, "engine", i + 1, &error_fatal);
- object_property_set_int(obj, "num-busses", pcc->i2c_num_ports,
+ object_property_set_int(obj, "num-busses",
+ pnv_power9_i2c_ports_per_ctlr[i],
&error_fatal);
object_property_set_link(obj, "chip", OBJECT(chip), &error_abort);
if (!qdev_realize(DEVICE(obj), NULL, errp)) {
@@ -1667,7 +1670,6 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
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,
&k->parent_realize);
@@ -1751,6 +1753,8 @@ static void pnv_chip_power10_phb_realize(PnvChip *chip, Error **errp)
}
}
+static int pnv_power10_i2c_ports_per_ctlr[PNV10_CHIP_MAX_I2C] = {14, 14, 2, 16};
+
static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
{
PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
@@ -1877,7 +1881,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
Object *obj = OBJECT(&chip10->i2c[i]);
object_property_set_int(obj, "engine", i + 1, &error_fatal);
- object_property_set_int(obj, "num-busses", pcc->i2c_num_ports,
+ object_property_set_int(obj, "num-busses",
+ pnv_power10_i2c_ports_per_ctlr[i],
&error_fatal);
object_property_set_link(obj, "chip", OBJECT(chip), &error_abort);
if (!qdev_realize(DEVICE(obj), NULL, errp)) {
@@ -1918,7 +1923,6 @@ static void pnv_chip_power10_class_init(ObjectClass *klass, void *data)
dc->desc = "PowerNV Chip POWER10";
k->num_pecs = PNV10_CHIP_MAX_PEC;
k->i2c_num_engines = PNV10_CHIP_MAX_I2C;
- k->i2c_num_ports = PNV10_CHIP_MAX_I2C_PORTS;
device_class_set_parent_realize(dc, pnv_chip_power10_realize,
&k->parent_realize);
diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 5815d96ecf..be1fec5698 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -88,8 +88,7 @@ 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
+#define PNV9_CHIP_MAX_I2C 4
PnvI2C i2c[PNV9_CHIP_MAX_I2C];
};
@@ -122,7 +121,6 @@ struct Pnv10Chip {
PnvPhb4PecState pecs[PNV10_CHIP_MAX_PEC];
#define PNV10_CHIP_MAX_I2C 4
-#define PNV10_CHIP_MAX_I2C_PORTS 2
PnvI2C i2c[PNV10_CHIP_MAX_I2C];
};
@@ -140,7 +138,6 @@ struct PnvChipClass {
uint32_t num_phbs;
uint32_t i2c_num_engines;
- uint32_t i2c_num_ports;
DeviceRealize parent_realize;
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ppc/pnv: Fix number of I2C engines and ports for power9/10
2023-10-23 16:52 [PATCH] ppc/pnv: Fix number of I2C engines and ports for power9/10 Glenn Miles
@ 2023-10-24 5:47 ` Cédric Le Goater
2023-10-24 17:17 ` Miles Glenn
0 siblings, 1 reply; 3+ messages in thread
From: Cédric Le Goater @ 2023-10-24 5:47 UTC (permalink / raw)
To: Glenn Miles, qemu-ppc; +Cc: qemu-devel, npiggin, fbarrat
Hello Glenn,
On 10/23/23 18:52, Glenn Miles wrote:
> Power9 is supposed to have 4 PIB-connected I2C engines with the
> following number of ports on each engine:
>
> 0: 2
> 1: 13
> 2: 2
> 3: 2
>
> Power10 also has 4 engines but has the following number of ports
> on each engine:
>
> 0: 14
> 1: 14
> 2: 2
> 3: 16
>
> Current code assumes that they all have the same (maximum) number.
> This can be a problem if software expects to see a certain number
> of ports present (Power Hypervisor seems to care).
Nice ! Do you have plans to populate the I2C busses ?
> Fixed this by adding separate tables for power9 and power10 that
> map the I2C controller number to the number of I2C buses that should
> be attached for that engine.
>
> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> ---
>
> Based-on: <20231017221434.810363-1-milesg@linux.vnet.ibm.com>
> ([PATCH] ppc/pnv: Connect PNV I2C controller to powernv10)
>
> hw/ppc/pnv.c | 12 ++++++++----
> include/hw/ppc/pnv_chip.h | 5 +----
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 2655b6e506..6ad4a1a7b1 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1507,6 +1507,8 @@ static void pnv_chip_power9_pec_realize(PnvChip *chip, Error **errp)
> }
> }
>
> +static int pnv_power9_i2c_ports_per_ctlr[PNV9_CHIP_MAX_I2C] = {2, 13, 2, 2};
The ports array could be under PnvChipClass. Anyhow,
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> {
> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
> @@ -1626,7 +1628,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> Object *obj = OBJECT(&chip9->i2c[i]);
>
> object_property_set_int(obj, "engine", i + 1, &error_fatal);
> - object_property_set_int(obj, "num-busses", pcc->i2c_num_ports,
> + object_property_set_int(obj, "num-busses",
> + pnv_power9_i2c_ports_per_ctlr[i],
> &error_fatal);
> object_property_set_link(obj, "chip", OBJECT(chip), &error_abort);
> if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> @@ -1667,7 +1670,6 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> 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,
> &k->parent_realize);
> @@ -1751,6 +1753,8 @@ static void pnv_chip_power10_phb_realize(PnvChip *chip, Error **errp)
> }
> }
>
> +static int pnv_power10_i2c_ports_per_ctlr[PNV10_CHIP_MAX_I2C] = {14, 14, 2, 16};
> +
> static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
> {
> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
> @@ -1877,7 +1881,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
> Object *obj = OBJECT(&chip10->i2c[i]);
>
> object_property_set_int(obj, "engine", i + 1, &error_fatal);
> - object_property_set_int(obj, "num-busses", pcc->i2c_num_ports,
> + object_property_set_int(obj, "num-busses",
> + pnv_power10_i2c_ports_per_ctlr[i],
> &error_fatal);
> object_property_set_link(obj, "chip", OBJECT(chip), &error_abort);
> if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> @@ -1918,7 +1923,6 @@ static void pnv_chip_power10_class_init(ObjectClass *klass, void *data)
> dc->desc = "PowerNV Chip POWER10";
> k->num_pecs = PNV10_CHIP_MAX_PEC;
> k->i2c_num_engines = PNV10_CHIP_MAX_I2C;
> - k->i2c_num_ports = PNV10_CHIP_MAX_I2C_PORTS;
>
> device_class_set_parent_realize(dc, pnv_chip_power10_realize,
> &k->parent_realize);
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 5815d96ecf..be1fec5698 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -88,8 +88,7 @@ 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
> +#define PNV9_CHIP_MAX_I2C 4
> PnvI2C i2c[PNV9_CHIP_MAX_I2C];
> };
>
> @@ -122,7 +121,6 @@ struct Pnv10Chip {
> PnvPhb4PecState pecs[PNV10_CHIP_MAX_PEC];
>
> #define PNV10_CHIP_MAX_I2C 4
> -#define PNV10_CHIP_MAX_I2C_PORTS 2
> PnvI2C i2c[PNV10_CHIP_MAX_I2C];
> };
>
> @@ -140,7 +138,6 @@ struct PnvChipClass {
> uint32_t num_phbs;
>
> uint32_t i2c_num_engines;
> - uint32_t i2c_num_ports;
>
> DeviceRealize parent_realize;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ppc/pnv: Fix number of I2C engines and ports for power9/10
2023-10-24 5:47 ` Cédric Le Goater
@ 2023-10-24 17:17 ` Miles Glenn
0 siblings, 0 replies; 3+ messages in thread
From: Miles Glenn @ 2023-10-24 17:17 UTC (permalink / raw)
To: Cédric Le Goater, qemu-ppc; +Cc: qemu-devel, npiggin, fbarrat
On Tue, 2023-10-24 at 07:47 +0200, Cédric Le Goater wrote:
> Hello Glenn,
>
> On 10/23/23 18:52, Glenn Miles wrote:
> > Power9 is supposed to have 4 PIB-connected I2C engines with the
> > following number of ports on each engine:
> >
> > 0: 2
> > 1: 13
> > 2: 2
> > 3: 2
> >
> > Power10 also has 4 engines but has the following number of ports
> > on each engine:
> >
> > 0: 14
> > 1: 14
> > 2: 2
> > 3: 16
> >
> > Current code assumes that they all have the same (maximum) number.
> > This can be a problem if software expects to see a certain number
> > of ports present (Power Hypervisor seems to care).
>
> Nice ! Do you have plans to populate the I2C busses ?
>
Yes, I'll be adding a pca9552 (possibly two) very soon. There's also a
a change coming for fixing an i2c master reset issue and something for
resetting the connected I2C buses at realization so that the slave
devices also are reset. I think I'll add these as a series.
> > Fixed this by adding separate tables for power9 and power10 that
> > map the I2C controller number to the number of I2C buses that
> > should
> > be attached for that engine.
> >
> > Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> > ---
> >
> > Based-on: <20231017221434.810363-1-milesg@linux.vnet.ibm.com>
> > ([PATCH] ppc/pnv: Connect PNV I2C controller to powernv10)
> >
> > hw/ppc/pnv.c | 12 ++++++++----
> > include/hw/ppc/pnv_chip.h | 5 +----
> > 2 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 2655b6e506..6ad4a1a7b1 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -1507,6 +1507,8 @@ static void
> > pnv_chip_power9_pec_realize(PnvChip *chip, Error **errp)
> > }
> > }
> >
> > +static int pnv_power9_i2c_ports_per_ctlr[PNV9_CHIP_MAX_I2C] = {2,
> > 13, 2, 2};
>
> The ports array could be under PnvChipClass.
Ok, I can look into doing that.
> Anyhow,
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks,
>
> C.
>
>
>
>
> > static void pnv_chip_power9_realize(DeviceState *dev, Error
> > **errp)
> > {
> > PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
> > @@ -1626,7 +1628,8 @@ static void
> > pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> > Object *obj = OBJECT(&chip9->i2c[i]);
> >
> > object_property_set_int(obj, "engine", i + 1,
> > &error_fatal);
> > - object_property_set_int(obj, "num-busses", pcc-
> > >i2c_num_ports,
> > + object_property_set_int(obj, "num-busses",
> > + pnv_power9_i2c_ports_per_ctlr[i],
> > &error_fatal);
> > object_property_set_link(obj, "chip", OBJECT(chip),
> > &error_abort);
> > if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> > @@ -1667,7 +1670,6 @@ static void
> > pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> > 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,
> > &k->parent_realize);
> > @@ -1751,6 +1753,8 @@ static void
> > pnv_chip_power10_phb_realize(PnvChip *chip, Error **errp)
> > }
> > }
> >
> > +static int pnv_power10_i2c_ports_per_ctlr[PNV10_CHIP_MAX_I2C] =
> > {14, 14, 2, 16};
> > +
> > static void pnv_chip_power10_realize(DeviceState *dev, Error
> > **errp)
> > {
> > PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
> > @@ -1877,7 +1881,8 @@ static void
> > pnv_chip_power10_realize(DeviceState *dev, Error **errp)
> > Object *obj = OBJECT(&chip10->i2c[i]);
> >
> > object_property_set_int(obj, "engine", i + 1,
> > &error_fatal);
> > - object_property_set_int(obj, "num-busses", pcc-
> > >i2c_num_ports,
> > + object_property_set_int(obj, "num-busses",
> > + pnv_power10_i2c_ports_per_ctlr[i],
> > &error_fatal);
> > object_property_set_link(obj, "chip", OBJECT(chip),
> > &error_abort);
> > if (!qdev_realize(DEVICE(obj), NULL, errp)) {
> > @@ -1918,7 +1923,6 @@ static void
> > pnv_chip_power10_class_init(ObjectClass *klass, void *data)
> > dc->desc = "PowerNV Chip POWER10";
> > k->num_pecs = PNV10_CHIP_MAX_PEC;
> > k->i2c_num_engines = PNV10_CHIP_MAX_I2C;
> > - k->i2c_num_ports = PNV10_CHIP_MAX_I2C_PORTS;
> >
> > device_class_set_parent_realize(dc, pnv_chip_power10_realize,
> > &k->parent_realize);
> > diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> > index 5815d96ecf..be1fec5698 100644
> > --- a/include/hw/ppc/pnv_chip.h
> > +++ b/include/hw/ppc/pnv_chip.h
> > @@ -88,8 +88,7 @@ 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
> > +#define PNV9_CHIP_MAX_I2C 4
> > PnvI2C i2c[PNV9_CHIP_MAX_I2C];
> > };
> >
> > @@ -122,7 +121,6 @@ struct Pnv10Chip {
> > PnvPhb4PecState pecs[PNV10_CHIP_MAX_PEC];
> >
> > #define PNV10_CHIP_MAX_I2C 4
> > -#define PNV10_CHIP_MAX_I2C_PORTS 2
> > PnvI2C i2c[PNV10_CHIP_MAX_I2C];
> > };
> >
> > @@ -140,7 +138,6 @@ struct PnvChipClass {
> > uint32_t num_phbs;
> >
> > uint32_t i2c_num_engines;
> > - uint32_t i2c_num_ports;
> >
> > DeviceRealize parent_realize;
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-24 17:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-23 16:52 [PATCH] ppc/pnv: Fix number of I2C engines and ports for power9/10 Glenn Miles
2023-10-24 5:47 ` Cédric Le Goater
2023-10-24 17:17 ` Miles Glenn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).