diff mbox series

[v2,3/8] ppc/pnv: PNV I2C engines assigned incorrect XSCOM addresses

Message ID 20231110194925.475909-4-milesg@linux.vnet.ibm.com
State New
Headers show
Series Add powernv10 I2C devices and tests | expand

Commit Message

Glenn Miles Nov. 10, 2023, 7:49 p.m. UTC
The PNV I2C engines for power9 and power10 were being assigned a base
XSCOM address that was off by one I2C engine's address range such
that engine 0 had engine 1's address and so on.  The xscom address
assignment was being based on the device tree engine numbering, which
starts at 1.  Rather than changing the device tree numbering to start
with 0, the addressing was changed to be based on the existing device
tree numbers minus one.

Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
---
 hw/ppc/pnv.c     | 6 ++++--
 hw/ppc/pnv_i2c.c | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Cédric Le Goater Nov. 13, 2023, 9:07 a.m. UTC | #1
On 11/10/23 20:49, Glenn Miles wrote:
> The PNV I2C engines for power9 and power10 were being assigned a base
> XSCOM address that was off by one I2C engine's address range such
> that engine 0 had engine 1's address and so on.  The xscom address
> assignment was being based on the device tree engine numbering, which
> starts at 1.  Rather than changing the device tree numbering to start
> with 0, the addressing was changed to be based on the existing device
> tree numbers minus one.
> 
> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>

Looks like a fix to me. Please add a Fixes: tag and the patch will
be queued for 8.2

Thanks,

C.


> ---
>   hw/ppc/pnv.c     | 6 ++++--
>   hw/ppc/pnv_i2c.c | 2 +-
>   2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 0b24d7d8ed..516434bc9c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1623,7 +1623,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>               return;
>           }
>           pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
> -                               chip9->i2c[i].engine * PNV9_XSCOM_I2CM_SIZE,
> +                                (chip9->i2c[i].engine - 1) *
> +                                        PNV9_XSCOM_I2CM_SIZE,
>                                   &chip9->i2c[i].xscom_regs);
>           qdev_connect_gpio_out(DEVICE(&chip9->i2c[i]), 0,
>                                 qdev_get_gpio_in(DEVICE(&chip9->psi),
> @@ -1871,7 +1872,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>               return;
>           }
>           pnv_xscom_add_subregion(chip, PNV10_XSCOM_I2CM_BASE +
> -                                chip10->i2c[i].engine * PNV10_XSCOM_I2CM_SIZE,
> +                                (chip10->i2c[i].engine - 1) *
> +                                        PNV10_XSCOM_I2CM_SIZE,
>                                   &chip10->i2c[i].xscom_regs);
>           qdev_connect_gpio_out(DEVICE(&chip10->i2c[i]), 0,
>                                 qdev_get_gpio_in(DEVICE(&chip10->psi),
> diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
> index f75e59e709..b2c738da50 100644
> --- a/hw/ppc/pnv_i2c.c
> +++ b/hw/ppc/pnv_i2c.c
> @@ -593,7 +593,7 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void *fdt,
>       int i2c_offset;
>       const char i2c_compat[] = "ibm,power8-i2cm\0ibm,power9-i2cm";
>       uint32_t i2c_pcba = PNV9_XSCOM_I2CM_BASE +
> -        i2c->engine * PNV9_XSCOM_I2CM_SIZE;
> +        (i2c->engine - 1) * PNV9_XSCOM_I2CM_SIZE;
>       uint32_t reg[2] = {
>           cpu_to_be32(i2c_pcba),
>           cpu_to_be32(PNV9_XSCOM_I2CM_SIZE)
Glenn Miles Nov. 14, 2023, 3:27 p.m. UTC | #2
On Mon, 2023-11-13 at 10:07 +0100, Cédric Le Goater wrote:
> On 11/10/23 20:49, Glenn Miles wrote:
> > The PNV I2C engines for power9 and power10 were being assigned a
> > base
> > XSCOM address that was off by one I2C engine's address range such
> > that engine 0 had engine 1's address and so on.  The xscom address
> > assignment was being based on the device tree engine numbering,
> > which
> > starts at 1.  Rather than changing the device tree numbering to
> > start
> > with 0, the addressing was changed to be based on the existing
> > device
> > tree numbers minus one.
> > 
> > Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> 
> Looks like a fix to me. Please add a Fixes: tag and the patch will
> be queued for 8.2
> 
> Thanks,
> 
> C.
> 

Yes, I agree.  Sorry, still learning about all of these tags!

Thanks,

Glenn

> 
> > ---
> >   hw/ppc/pnv.c     | 6 ++++--
> >   hw/ppc/pnv_i2c.c | 2 +-
> >   2 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 0b24d7d8ed..516434bc9c 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -1623,7 +1623,8 @@ static void
> > pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> >               return;
> >           }
> >           pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
> > -                               chip9->i2c[i].engine *
> > PNV9_XSCOM_I2CM_SIZE,
> > +                                (chip9->i2c[i].engine - 1) *
> > +                                        PNV9_XSCOM_I2CM_SIZE,
> >                                   &chip9->i2c[i].xscom_regs);
> >           qdev_connect_gpio_out(DEVICE(&chip9->i2c[i]), 0,
> >                                 qdev_get_gpio_in(DEVICE(&chip9-
> > >psi),
> > @@ -1871,7 +1872,8 @@ static void
> > pnv_chip_power10_realize(DeviceState *dev, Error **errp)
> >               return;
> >           }
> >           pnv_xscom_add_subregion(chip, PNV10_XSCOM_I2CM_BASE +
> > -                                chip10->i2c[i].engine *
> > PNV10_XSCOM_I2CM_SIZE,
> > +                                (chip10->i2c[i].engine - 1) *
> > +                                        PNV10_XSCOM_I2CM_SIZE,
> >                                   &chip10->i2c[i].xscom_regs);
> >           qdev_connect_gpio_out(DEVICE(&chip10->i2c[i]), 0,
> >                                 qdev_get_gpio_in(DEVICE(&chip10-
> > >psi),
> > diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
> > index f75e59e709..b2c738da50 100644
> > --- a/hw/ppc/pnv_i2c.c
> > +++ b/hw/ppc/pnv_i2c.c
> > @@ -593,7 +593,7 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface
> > *dev, void *fdt,
> >       int i2c_offset;
> >       const char i2c_compat[] = "ibm,power8-i2cm\0ibm,power9-i2cm";
> >       uint32_t i2c_pcba = PNV9_XSCOM_I2CM_BASE +
> > -        i2c->engine * PNV9_XSCOM_I2CM_SIZE;
> > +        (i2c->engine - 1) * PNV9_XSCOM_I2CM_SIZE;
> >       uint32_t reg[2] = {
> >           cpu_to_be32(i2c_pcba),
> >           cpu_to_be32(PNV9_XSCOM_I2CM_SIZE)
diff mbox series

Patch

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0b24d7d8ed..516434bc9c 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1623,7 +1623,8 @@  static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
             return;
         }
         pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
-                               chip9->i2c[i].engine * PNV9_XSCOM_I2CM_SIZE,
+                                (chip9->i2c[i].engine - 1) *
+                                        PNV9_XSCOM_I2CM_SIZE,
                                 &chip9->i2c[i].xscom_regs);
         qdev_connect_gpio_out(DEVICE(&chip9->i2c[i]), 0,
                               qdev_get_gpio_in(DEVICE(&chip9->psi),
@@ -1871,7 +1872,8 @@  static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
             return;
         }
         pnv_xscom_add_subregion(chip, PNV10_XSCOM_I2CM_BASE +
-                                chip10->i2c[i].engine * PNV10_XSCOM_I2CM_SIZE,
+                                (chip10->i2c[i].engine - 1) *
+                                        PNV10_XSCOM_I2CM_SIZE,
                                 &chip10->i2c[i].xscom_regs);
         qdev_connect_gpio_out(DEVICE(&chip10->i2c[i]), 0,
                               qdev_get_gpio_in(DEVICE(&chip10->psi),
diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
index f75e59e709..b2c738da50 100644
--- a/hw/ppc/pnv_i2c.c
+++ b/hw/ppc/pnv_i2c.c
@@ -593,7 +593,7 @@  static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void *fdt,
     int i2c_offset;
     const char i2c_compat[] = "ibm,power8-i2cm\0ibm,power9-i2cm";
     uint32_t i2c_pcba = PNV9_XSCOM_I2CM_BASE +
-        i2c->engine * PNV9_XSCOM_I2CM_SIZE;
+        (i2c->engine - 1) * PNV9_XSCOM_I2CM_SIZE;
     uint32_t reg[2] = {
         cpu_to_be32(i2c_pcba),
         cpu_to_be32(PNV9_XSCOM_I2CM_SIZE)