[PATCH v4 07/11] ppc/pnv: Fix PNV I2C invalid status after reset

Glenn Miles posted 11 patches 1 year ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Glenn Miles <milesg@linux.vnet.ibm.com>, "Cédric Le Goater" <clg@kaod.org>, Nicholas Piggin <npiggin@gmail.com>, "Frédéric Barrat" <fbarrat@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH v4 07/11] ppc/pnv: Fix PNV I2C invalid status after reset
Posted by Glenn Miles 1 year ago
The PNV I2C Controller was clearing the status register
after a reset without repopulating the "upper threshold
for I2C ports", "Command Complete" and the SCL/SDA input
level fields.

Fixed this for resets caused by a system reset as well
as from writing to the "Immediate Reset" register.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Fixes: 263b81ee15af ("ppc/pnv: Add an I2C controller model")
Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
---

No changes from previous version

 hw/ppc/pnv_i2c.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
index b2c738da50..f80589157b 100644
--- a/hw/ppc/pnv_i2c.c
+++ b/hw/ppc/pnv_i2c.c
@@ -462,6 +462,23 @@ static uint64_t pnv_i2c_xscom_read(void *opaque, hwaddr addr,
     return val;
 }
 
+static void pnv_i2c_reset(void *dev)
+{
+    PnvI2C *i2c = PNV_I2C(dev);
+
+    memset(i2c->regs, 0, sizeof(i2c->regs));
+
+    i2c->regs[I2C_STAT_REG] =
+        SETFIELD(I2C_STAT_UPPER_THRS, 0ull, i2c->num_busses - 1) |
+        I2C_STAT_CMD_COMP | I2C_STAT_SCL_INPUT_LEVEL |
+        I2C_STAT_SDA_INPUT_LEVEL;
+    i2c->regs[I2C_EXTD_STAT_REG] =
+        SETFIELD(I2C_EXTD_STAT_FIFO_SIZE, 0ull, PNV_I2C_FIFO_SIZE) |
+        SETFIELD(I2C_EXTD_STAT_I2C_VERSION, 0ull, 23); /* last version */
+
+    fifo8_reset(&i2c->fifo);
+}
+
 static void pnv_i2c_xscom_write(void *opaque, hwaddr addr,
                                 uint64_t val, unsigned size)
 {
@@ -499,16 +516,7 @@ static void pnv_i2c_xscom_write(void *opaque, hwaddr addr,
         break;
 
     case I2C_RESET_I2C_REG:
-        i2c->regs[I2C_MODE_REG] = 0;
-        i2c->regs[I2C_CMD_REG] = 0;
-        i2c->regs[I2C_WATERMARK_REG] = 0;
-        i2c->regs[I2C_INTR_MASK_REG] = 0;
-        i2c->regs[I2C_INTR_COND_REG] = 0;
-        i2c->regs[I2C_INTR_RAW_COND_REG] = 0;
-        i2c->regs[I2C_STAT_REG] = 0;
-        i2c->regs[I2C_RESIDUAL_LEN_REG] = 0;
-        i2c->regs[I2C_EXTD_STAT_REG] &=
-            (I2C_EXTD_STAT_FIFO_SIZE | I2C_EXTD_STAT_I2C_VERSION);
+        pnv_i2c_reset(i2c);
         break;
 
     case I2C_RESET_ERRORS:
@@ -620,20 +628,6 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void *fdt,
     return 0;
 }
 
-static void pnv_i2c_reset(void *dev)
-{
-    PnvI2C *i2c = PNV_I2C(dev);
-
-    memset(i2c->regs, 0, sizeof(i2c->regs));
-
-    i2c->regs[I2C_STAT_REG] = I2C_STAT_CMD_COMP;
-    i2c->regs[I2C_EXTD_STAT_REG] =
-        SETFIELD(I2C_EXTD_STAT_FIFO_SIZE, 0ull, PNV_I2C_FIFO_SIZE) |
-        SETFIELD(I2C_EXTD_STAT_I2C_VERSION, 0ull, 23); /* last version */
-
-    fifo8_reset(&i2c->fifo);
-}
-
 static void pnv_i2c_realize(DeviceState *dev, Error **errp)
 {
     PnvI2C *i2c = PNV_I2C(dev);
-- 
2.31.1


Re: [PATCH v4 07/11] ppc/pnv: Fix PNV I2C invalid status after reset
Posted by Cédric Le Goater 1 year ago
On 11/21/23 00:51, Glenn Miles wrote:
> The PNV I2C Controller was clearing the status register
> after a reset without repopulating the "upper threshold
> for I2C ports", "Command Complete" and the SCL/SDA input
> level fields.
> 
> Fixed this for resets caused by a system reset as well
> as from writing to the "Immediate Reset" register.
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Fixes: 263b81ee15af ("ppc/pnv: Add an I2C controller model")
> Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> ---
> 
> No changes from previous version

This patch was merged upstream now.

C.




> 
>   hw/ppc/pnv_i2c.c | 42 ++++++++++++++++++------------------------
>   1 file changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
> index b2c738da50..f80589157b 100644
> --- a/hw/ppc/pnv_i2c.c
> +++ b/hw/ppc/pnv_i2c.c
> @@ -462,6 +462,23 @@ static uint64_t pnv_i2c_xscom_read(void *opaque, hwaddr addr,
>       return val;
>   }
>   
> +static void pnv_i2c_reset(void *dev)
> +{
> +    PnvI2C *i2c = PNV_I2C(dev);
> +
> +    memset(i2c->regs, 0, sizeof(i2c->regs));
> +
> +    i2c->regs[I2C_STAT_REG] =
> +        SETFIELD(I2C_STAT_UPPER_THRS, 0ull, i2c->num_busses - 1) |
> +        I2C_STAT_CMD_COMP | I2C_STAT_SCL_INPUT_LEVEL |
> +        I2C_STAT_SDA_INPUT_LEVEL;
> +    i2c->regs[I2C_EXTD_STAT_REG] =
> +        SETFIELD(I2C_EXTD_STAT_FIFO_SIZE, 0ull, PNV_I2C_FIFO_SIZE) |
> +        SETFIELD(I2C_EXTD_STAT_I2C_VERSION, 0ull, 23); /* last version */
> +
> +    fifo8_reset(&i2c->fifo);
> +}
> +
>   static void pnv_i2c_xscom_write(void *opaque, hwaddr addr,
>                                   uint64_t val, unsigned size)
>   {
> @@ -499,16 +516,7 @@ static void pnv_i2c_xscom_write(void *opaque, hwaddr addr,
>           break;
>   
>       case I2C_RESET_I2C_REG:
> -        i2c->regs[I2C_MODE_REG] = 0;
> -        i2c->regs[I2C_CMD_REG] = 0;
> -        i2c->regs[I2C_WATERMARK_REG] = 0;
> -        i2c->regs[I2C_INTR_MASK_REG] = 0;
> -        i2c->regs[I2C_INTR_COND_REG] = 0;
> -        i2c->regs[I2C_INTR_RAW_COND_REG] = 0;
> -        i2c->regs[I2C_STAT_REG] = 0;
> -        i2c->regs[I2C_RESIDUAL_LEN_REG] = 0;
> -        i2c->regs[I2C_EXTD_STAT_REG] &=
> -            (I2C_EXTD_STAT_FIFO_SIZE | I2C_EXTD_STAT_I2C_VERSION);
> +        pnv_i2c_reset(i2c);
>           break;
>   
>       case I2C_RESET_ERRORS:
> @@ -620,20 +628,6 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void *fdt,
>       return 0;
>   }
>   
> -static void pnv_i2c_reset(void *dev)
> -{
> -    PnvI2C *i2c = PNV_I2C(dev);
> -
> -    memset(i2c->regs, 0, sizeof(i2c->regs));
> -
> -    i2c->regs[I2C_STAT_REG] = I2C_STAT_CMD_COMP;
> -    i2c->regs[I2C_EXTD_STAT_REG] =
> -        SETFIELD(I2C_EXTD_STAT_FIFO_SIZE, 0ull, PNV_I2C_FIFO_SIZE) |
> -        SETFIELD(I2C_EXTD_STAT_I2C_VERSION, 0ull, 23); /* last version */
> -
> -    fifo8_reset(&i2c->fifo);
> -}
> -
>   static void pnv_i2c_realize(DeviceState *dev, Error **errp)
>   {
>       PnvI2C *i2c = PNV_I2C(dev);