Re: [PATCH] hw/i2c/imx_i2c: Always set interrupt status bit if interrupt condition occurs

[Corey Minyard]

On Wed, May 07, 2025 at 02:40:40PM +0200, Bernhard Beschow wrote:
> According to the i.MX 8M Plus reference manual, the status flag I2C_I2SR[IIF]
> continues to be set when an interrupt condition occurs even when I2C 
> interrupts
> are disabled (I2C_I2CR[IIEN] is clear). However, the device model only sets 
> the
> flag when I2C interrupts are enabled which causes U-Boot to loop forever. Fix
> the device model by always setting the flag and let I2C_I2CR[IIEN] guard I2C
> interrupts only.
> 
> Also remove the comment in the code since it merely stated the obvious and 
> would
> be outdated now.

This looks good to me.  I can give you an:

Acked-by: Corey Minyard <cminyard@mvista.com>

or I can take it into my tree.

-corey

> 
> Fixes: 20d0f9cf6a41 ("i.MX: Add I2C controller emulator")
> cc: qemu-stable
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/i2c/imx_i2c.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
> index 91f84c2ad7..d26177c85d 100644
> --- a/hw/i2c/imx_i2c.c
> +++ b/hw/i2c/imx_i2c.c
> @@ -79,13 +79,12 @@ static void imx_i2c_reset(DeviceState *dev)
>  
>  static inline void imx_i2c_raise_interrupt(IMXI2CState *s)
>  {
> -    /*
> -     * raise an interrupt if the device is enabled and it is configured
> -     * to generate some interrupts.
> -     */
> -    if (imx_i2c_is_enabled(s) && imx_i2c_interrupt_is_enabled(s)) {
> +    if (imx_i2c_is_enabled(s)) {
>          s->i2sr |= I2SR_IIF;
> -        qemu_irq_raise(s->irq);
> +
> +        if (imx_i2c_interrupt_is_enabled(s)) {
> +            qemu_irq_raise(s->irq);
> +        }
>      }
>  }
>  
> -- 
> 2.49.0
> 
>