[PATCH v3 2/2] i2c: aspeed: Acknowledge Tx done with and without ACK irq late
Andi Shyti
andi.shyti at kernel.org
Sun Dec 10 07:44:55 AEDT 2023
Hi Quan,
[...]
> - /* Ack all interrupts except for Rx done */
> - writel(irq_received & ~ASPEED_I2CD_INTR_RX_DONE,
> - bus->base + ASPEED_I2C_INTR_STS_REG);
> +
> + /*
> + * Early acking of INTR_RX_DONE and INTR_TX_[ACK|NAK] would indicate HW to
> + * start receiving or sending new data, and this may cause a race condition
> + * as the irq handler has not yet handled these irqs but is being acked.
> + * Let's ack them late at the end of the irq handler when those are truly processed.
> + */
> + irq_ack_last = ASPEED_I2CD_INTR_RX_DONE | ASPEED_I2CD_INTR_TX_ACK | ASPEED_I2CD_INTR_TX_NAK;
> + writel(irq_received & ~irq_ack_last, bus->base + ASPEED_I2C_INTR_STS_REG);
I like Andrews suggestion of having irq_ack_last as a define that
is already negated, instead of negating it in the writel, which
makes it a bit difficult to read.
Besides, ack_last, as a name is not very meaningful, I'd rather
call it irq_ack_rx_tx (or something similar).
But I'm not going to block it for this, up to you if you want to
send a new version.
Reviewed-by: Andi Shyti <andi.shyti at kernel.org>
Thanks,
Andi
More information about the openbmc
mailing list