[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] aspeed: i2c: Fix DMA len write-enable bit handling
From: |
Peter Delevoryas |
Subject: |
Re: [PATCH] aspeed: i2c: Fix DMA len write-enable bit handling |
Date: |
Fri, 24 Jun 2022 20:34:43 +0000 |
> On Jun 24, 2022, at 1:31 PM, Peter Delevoryas <pdel@fb.com> wrote:
>
> I noticed i2c rx transfers were getting shortened to "1" on Zephyr. It
> seems to be because the Zephyr i2c driver sets the RX DMA len with the
> RX field write-enable bit set (bit 31) to avoid a read-modify-write. [1]
>
> /* 0x1C : I2CM Master DMA Transfer Length Register */
>
> I think we should be checking the write-enable bits on the incoming
> value, not checking the register array. I'm not sure we're even writing
> the write-enable bits to the register array, actually.
>
> [1]
> https://github.com/AspeedTech-BMC/zephyr/blob/db3dbcc9c52e67a47180890ac938ed380b33f91c/drivers/i2c/i2c_aspeed.c#L145-L148
Arg, forgot this:
Fixes: ba2cccd64e90f34 ("aspeed: i2c: Add new mode support”)
Should I resend as v2?
Thanks,
Peter
>
> Signed-off-by: Peter Delevoryas <pdel@fb.com>
> ---
> hw/i2c/aspeed_i2c.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 37ae1f2e04..c4fce7474a 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -644,18 +644,18 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus,
> hwaddr offset,
> RX_BUF_LEN) + 1;
> break;
> case A_I2CM_DMA_LEN:
> - w1t = ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
> - ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN_W1T);
> + w1t = FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
> + FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN_W1T);
> /* If none of the w1t bits are set, just write to the reg as normal.
> */
> if (!w1t) {
> bus->regs[R_I2CM_DMA_LEN] = value;
> break;
> }
> - if (ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN_W1T)) {
> + if (FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T)) {
> ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN, RX_BUF_LEN,
> FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN));
> }
> - if (ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN_W1T)) {
> + if (FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN_W1T)) {
> ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN, TX_BUF_LEN,
> FIELD_EX32(value, I2CM_DMA_LEN, TX_BUF_LEN));
> }
> --
> 2.30.2
>