Re: [PATCH] i2c: aspeed: Nullify bus messages after timeout

[Andrew Jeffery <andrew@codeconstruct.com.au>]

On Fri, 2025-01-31 at 16:29 -0600, Eddie James wrote:
> For multimaster case, it's conceivable that an interrupt comes
> in after the transfer times out and attempts to use bus messages
> that have already been freed by the i2c core.

This description seems a little vague. Did you hit this case in
practice?

> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/i2c-aspeed.c
> b/drivers/i2c/busses/i2c-aspeed.c
> index 1550d3d552aed..e344dcc2233fe 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -731,6 +731,7 @@ static int aspeed_i2c_master_xfer(struct
> i2c_adapter *adap,
>                  * master command.
>                  */
>                 spin_lock_irqsave(&bus->lock, flags);
> +               bus->msgs = NULL;

It feels like there's buggy code elsewhere in the driver that this is
protecting (broken state machine)? I've had a look at the
aspeed_i2c_master_irq() implementation and can't see what though, as we
take an early-exit (before indexing into bus->msgs) if bus-
>master_state is INACTIVE or PENDING.

Can you be a bit more specific about where the issue lies?

Andrew


>                 if (bus->master_state == ASPEED_I2C_MASTER_PENDING)
>                         bus->master_state =
> ASPEED_I2C_MASTER_INACTIVE;
>                 spin_unlock_irqrestore(&bus->lock, flags);