| Message ID | 20230811054220.99562-1-francis_yuu@stu.pku.edu.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/4] hw/i2c/aspeed: Fix I2CD_POOL_CTRL register bit field defination | expand |
Hello Hang, It is good practice to send a cover letter giving a quick summary of the changes. On 8/11/23 07:42, Hang Yu wrote: > Fixed inconsistency between the regisiter bit field defination in headfile Fixed inconsistency between the register bit field definition header file > and the ast2600 datasheet. The reg name is I2CD1C:Pool Buffer Control > Register in old register mode and I2CC0C: Master/Slave Pool Buffer Control > Register in new register mode. They share bit field > [12:8]:Transmit Data Byte Count and bit field > [29:24]:Actual Received Pool Buffer Size according to the datasheet. > > Signed-off-by: Hang Yu <francis_yuu@stu.pku.edu.cn> The initial macros included a '+ 1' #define I2CD_POOL_RX_SIZE(x) ((((x) >> 16) & 0xff) + 1) #define I2CD_POOL_TX_COUNT(x) ((((x) >> 8) & 0xff) + 1) which was not reported in commit 3be3d6ccf2ad ("aspeed: i2c: Migrate to registerfields API"). I think patch 1 and 2 should be folded, could you please respin a v3 and add a Fixes tag ? It is too late for the 8.1 cycle but these changes would be good candidate for QEMU stable. I will queue them for 8.2. Also, they fix booting v08.06 SDK which is great. Here is an extra patch to change the QEMU tests to the latest version : https://github.com/legoater/qemu/commit/08243703f48f5e9c263773a846f7dc655cd64bfa You can include it in your series if you want. Thanks, C. > --- > include/hw/i2c/aspeed_i2c.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h > index 51c944efea..2e1e15aaf0 100644 > --- a/include/hw/i2c/aspeed_i2c.h > +++ b/include/hw/i2c/aspeed_i2c.h > @@ -139,9 +139,9 @@ REG32(I2CD_CMD, 0x14) /* I2CD Command/Status */ > REG32(I2CD_DEV_ADDR, 0x18) /* Slave Device Address */ > SHARED_FIELD(SLAVE_DEV_ADDR1, 0, 7) > REG32(I2CD_POOL_CTRL, 0x1C) /* Pool Buffer Control */ > - SHARED_FIELD(RX_COUNT, 24, 5) > + SHARED_FIELD(RX_COUNT, 24, 6) > SHARED_FIELD(RX_SIZE, 16, 5) > - SHARED_FIELD(TX_COUNT, 9, 5) > + SHARED_FIELD(TX_COUNT, 8, 5) > FIELD(I2CD_POOL_CTRL, OFFSET, 2, 6) /* AST2400 */ > REG32(I2CD_BYTE_BUF, 0x20) /* Transmit/Receive Byte Buffer */ > SHARED_FIELD(RX_BUF, 8, 8)
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index 51c944efea..2e1e15aaf0 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -139,9 +139,9 @@ REG32(I2CD_CMD, 0x14) /* I2CD Command/Status */ REG32(I2CD_DEV_ADDR, 0x18) /* Slave Device Address */ SHARED_FIELD(SLAVE_DEV_ADDR1, 0, 7) REG32(I2CD_POOL_CTRL, 0x1C) /* Pool Buffer Control */ - SHARED_FIELD(RX_COUNT, 24, 5) + SHARED_FIELD(RX_COUNT, 24, 6) SHARED_FIELD(RX_SIZE, 16, 5) - SHARED_FIELD(TX_COUNT, 9, 5) + SHARED_FIELD(TX_COUNT, 8, 5) FIELD(I2CD_POOL_CTRL, OFFSET, 2, 6) /* AST2400 */ REG32(I2CD_BYTE_BUF, 0x20) /* Transmit/Receive Byte Buffer */ SHARED_FIELD(RX_BUF, 8, 8)
Fixed inconsistency between the regisiter bit field defination in headfile and the ast2600 datasheet. The reg name is I2CD1C:Pool Buffer Control Register in old register mode and I2CC0C: Master/Slave Pool Buffer Control Register in new register mode. They share bit field [12:8]:Transmit Data Byte Count and bit field [29:24]:Actual Received Pool Buffer Size according to the datasheet. Signed-off-by: Hang Yu <francis_yuu@stu.pku.edu.cn> --- include/hw/i2c/aspeed_i2c.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)