Message ID | 20230220081252.25348-2-qianfanguijin@163.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2,1/3] hw: allwinner-i2c: Make the trace message more readable | expand |
On Mon, Feb 20, 2023 at 9:13 AM <qianfanguijin@163.com> wrote: > > From: qianfan Zhao <qianfanguijin@163.com> > > TWI_CNTR_INT_FLAG is W1C(write 1 to clear and write 0 has non-effect) > register on SUN6i based SoCs, we should lower interrupt when the guest > set this bit. > > The linux kernel will hang in irq handler(mv64xxx_i2c_intr) if no > device connected on the i2c bus, next is the trace log: > > allwinner_i2c_write write CNTR(0x0c): 0xc4 A_ACK BUS_EN INT_EN > allwinner_i2c_write write CNTR(0x0c): 0xcc A_ACK INT_FLAG BUS_EN INT_EN > allwinner_i2c_read read CNTR(0x0c): 0xcc A_ACK INT_FLAG BUS_EN INT_EN > allwinner_i2c_read read STAT(0x10): 0x20 STAT_M_ADDR_WR_NACK > allwinner_i2c_write write CNTR(0x0c): 0x54 A_ACK M_STP BUS_EN > allwinner_i2c_write write CNTR(0x0c): 0x4c A_ACK INT_FLAG BUS_EN > allwinner_i2c_read read CNTR(0x0c): 0x4c A_ACK INT_FLAG BUS_EN > allwinner_i2c_read read STAT(0x10): 0xf8 STAT_IDLE > allwinner_i2c_write write CNTR(0x0c): 0x54 A_ACK M_STP BUS_EN > allwinner_i2c_write write CNTR(0x0c): 0x4c A_ACK INT_FLAG BUS_EN > allwinner_i2c_read read CNTR(0x0c): 0x4c A_ACK INT_FLAG BUS_EN > allwinner_i2c_read read STAT(0x10): 0xf8 STAT_IDLE > ... > > Fix it. > > Signed-off-by: qianfan Zhao <qianfanguijin@163.com> > --- > hw/i2c/allwinner-i2c.c | 26 ++++++++++++++++++++++++-- > include/hw/i2c/allwinner-i2c.h | 6 ++++++ > 2 files changed, 30 insertions(+), 2 deletions(-) > > diff --git a/hw/i2c/allwinner-i2c.c b/hw/i2c/allwinner-i2c.c > index fa650e7e02..819638d740 100644 > --- a/hw/i2c/allwinner-i2c.c > +++ b/hw/i2c/allwinner-i2c.c > @@ -463,10 +463,16 @@ static void allwinner_i2c_write(void *opaque, hwaddr offset, > s->stat = STAT_FROM_STA(STAT_IDLE); > s->cntr &= ~TWI_CNTR_M_STP; > } > - if ((s->cntr & TWI_CNTR_INT_FLAG) == 0) { > - /* Interrupt flag cleared */ > + > + if (!s->irq_clear_inverted && !(s->cntr & TWI_CNTR_INT_FLAG)) { > + /* Write 0 to clear this flag */ > + qemu_irq_lower(s->irq); > + } else if (s->irq_clear_inverted && (s->cntr & TWI_CNTR_INT_FLAG)) { > + /* Write 1 to clear this flag */ > + s->cntr &= ~TWI_CNTR_INT_FLAG; > qemu_irq_lower(s->irq); > } > + > if ((s->cntr & TWI_CNTR_A_ACK) == 0) { > if (STAT_TO_STA(s->stat) == STAT_M_DATA_RX_ACK) { > s->stat = STAT_FROM_STA(STAT_M_DATA_RX_NACK); > @@ -557,9 +563,25 @@ static const TypeInfo allwinner_i2c_type_info = { > .class_init = allwinner_i2c_class_init, > }; > > +static void allwinner_i2c_sun6i_init(Object *obj) > +{ > + AWI2CState *s = AW_I2C(obj); > + > + s->irq_clear_inverted = true; > +} > + > +static const TypeInfo allwinner_i2c_sun6i_type_info = { > + .name = TYPE_AW_I2C_SUN6I, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(AWI2CState), > + .instance_init = allwinner_i2c_sun6i_init, > + .class_init = allwinner_i2c_class_init, > +}; > + > static void allwinner_i2c_register_types(void) > { > type_register_static(&allwinner_i2c_type_info); > + type_register_static(&allwinner_i2c_sun6i_type_info); > } > > type_init(allwinner_i2c_register_types) > diff --git a/include/hw/i2c/allwinner-i2c.h b/include/hw/i2c/allwinner-i2c.h > index 4f378b86ba..0e325d265e 100644 > --- a/include/hw/i2c/allwinner-i2c.h > +++ b/include/hw/i2c/allwinner-i2c.h > @@ -28,6 +28,10 @@ > #include "qom/object.h" > > #define TYPE_AW_I2C "allwinner.i2c" > + > +/** Allwinner I2C sun6i family and newer (A31, H2+, H3, etc) */ > +#define TYPE_AW_I2C_SUN6I TYPE_AW_I2C "-sun6i" > + > OBJECT_DECLARE_SIMPLE_TYPE(AWI2CState, AW_I2C) > > #define AW_I2C_MEM_SIZE 0x24 > @@ -50,6 +54,8 @@ struct AWI2CState { > uint8_t srst; > uint8_t efr; > uint8_t lcr; > + > + bool irq_clear_inverted; > }; > > #endif /* ALLWINNER_I2C_H */ > -- > 2.25.1 > Tried this patch with avocado for cubieboard and orangepi-pc, all tests are passing. So for me: Reviewed-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com> Tested-by: Strahinja Jankovic <strahinja.p.jankovic@gmail.com>
On Mon, 20 Feb 2023 at 08:13, <qianfanguijin@163.com> wrote: > > From: qianfan Zhao <qianfanguijin@163.com> > > TWI_CNTR_INT_FLAG is W1C(write 1 to clear and write 0 has non-effect) > register on SUN6i based SoCs, we should lower interrupt when the guest > set this bit. > > The linux kernel will hang in irq handler(mv64xxx_i2c_intr) if no > device connected on the i2c bus, next is the trace log: > > allwinner_i2c_write write CNTR(0x0c): 0xc4 A_ACK BUS_EN INT_EN > allwinner_i2c_write write CNTR(0x0c): 0xcc A_ACK INT_FLAG BUS_EN INT_EN > allwinner_i2c_read read CNTR(0x0c): 0xcc A_ACK INT_FLAG BUS_EN INT_EN > allwinner_i2c_read read STAT(0x10): 0x20 STAT_M_ADDR_WR_NACK > allwinner_i2c_write write CNTR(0x0c): 0x54 A_ACK M_STP BUS_EN > allwinner_i2c_write write CNTR(0x0c): 0x4c A_ACK INT_FLAG BUS_EN > allwinner_i2c_read read CNTR(0x0c): 0x4c A_ACK INT_FLAG BUS_EN > allwinner_i2c_read read STAT(0x10): 0xf8 STAT_IDLE > allwinner_i2c_write write CNTR(0x0c): 0x54 A_ACK M_STP BUS_EN > allwinner_i2c_write write CNTR(0x0c): 0x4c A_ACK INT_FLAG BUS_EN > allwinner_i2c_read read CNTR(0x0c): 0x4c A_ACK INT_FLAG BUS_EN > allwinner_i2c_read read STAT(0x10): 0xf8 STAT_IDLE > ... > > Fix it. > > Signed-off-by: qianfan Zhao <qianfanguijin@163.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/hw/i2c/allwinner-i2c.c b/hw/i2c/allwinner-i2c.c index fa650e7e02..819638d740 100644 --- a/hw/i2c/allwinner-i2c.c +++ b/hw/i2c/allwinner-i2c.c @@ -463,10 +463,16 @@ static void allwinner_i2c_write(void *opaque, hwaddr offset, s->stat = STAT_FROM_STA(STAT_IDLE); s->cntr &= ~TWI_CNTR_M_STP; } - if ((s->cntr & TWI_CNTR_INT_FLAG) == 0) { - /* Interrupt flag cleared */ + + if (!s->irq_clear_inverted && !(s->cntr & TWI_CNTR_INT_FLAG)) { + /* Write 0 to clear this flag */ + qemu_irq_lower(s->irq); + } else if (s->irq_clear_inverted && (s->cntr & TWI_CNTR_INT_FLAG)) { + /* Write 1 to clear this flag */ + s->cntr &= ~TWI_CNTR_INT_FLAG; qemu_irq_lower(s->irq); } + if ((s->cntr & TWI_CNTR_A_ACK) == 0) { if (STAT_TO_STA(s->stat) == STAT_M_DATA_RX_ACK) { s->stat = STAT_FROM_STA(STAT_M_DATA_RX_NACK); @@ -557,9 +563,25 @@ static const TypeInfo allwinner_i2c_type_info = { .class_init = allwinner_i2c_class_init, }; +static void allwinner_i2c_sun6i_init(Object *obj) +{ + AWI2CState *s = AW_I2C(obj); + + s->irq_clear_inverted = true; +} + +static const TypeInfo allwinner_i2c_sun6i_type_info = { + .name = TYPE_AW_I2C_SUN6I, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(AWI2CState), + .instance_init = allwinner_i2c_sun6i_init, + .class_init = allwinner_i2c_class_init, +}; + static void allwinner_i2c_register_types(void) { type_register_static(&allwinner_i2c_type_info); + type_register_static(&allwinner_i2c_sun6i_type_info); } type_init(allwinner_i2c_register_types) diff --git a/include/hw/i2c/allwinner-i2c.h b/include/hw/i2c/allwinner-i2c.h index 4f378b86ba..0e325d265e 100644 --- a/include/hw/i2c/allwinner-i2c.h +++ b/include/hw/i2c/allwinner-i2c.h @@ -28,6 +28,10 @@ #include "qom/object.h" #define TYPE_AW_I2C "allwinner.i2c" + +/** Allwinner I2C sun6i family and newer (A31, H2+, H3, etc) */ +#define TYPE_AW_I2C_SUN6I TYPE_AW_I2C "-sun6i" + OBJECT_DECLARE_SIMPLE_TYPE(AWI2CState, AW_I2C) #define AW_I2C_MEM_SIZE 0x24 @@ -50,6 +54,8 @@ struct AWI2CState { uint8_t srst; uint8_t efr; uint8_t lcr; + + bool irq_clear_inverted; }; #endif /* ALLWINNER_I2C_H */