Hi Joe,
> Subject: [PATCH 07/19] hw/i3c/aspeed_i3c: Add register RO field masks
>
> Adds read-only register masks for the Aspeed I3C controller registers.
>
> Signed-off-by: Joe Komlodi <[email protected]>
>
> Reviewed-by: Patrick Venture <[email protected]>
> ---
> hw/i3c/aspeed_i3c.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/hw/i3c/aspeed_i3c.c b/hw/i3c/aspeed_i3c.c index
> 7a16dfec53..1ad25e5a00 100644
> --- a/hw/i3c/aspeed_i3c.c
> +++ b/hw/i3c/aspeed_i3c.c
> @@ -74,6 +74,21 @@ REG32(I3C6_REG1, 0x64)
> FIELD(I3C6_REG1, SA_EN, 15, 1)
> FIELD(I3C6_REG1, INST_ID, 16, 4)
>
> +static const uint32_t ast2600_i3c_controller_ro[ASPEED_I3C_NR_REGS] = {
> + [R_I3C1_REG0] = 0xfc000000,
According to the datasheet, bits [29:28] are defined as RW (SDA_PULLUP_EN).
Why are these bits being masked as read-only?
> + [R_I3C1_REG1] = 0xfff00000,
According to the datasheet, bits [31:0] are all RW.
Why are bits [31:20] being treated as read-only?
The same questions apply to I3C2 through I3C6.
Thanks-Jamin
> + [R_I3C2_REG0] = 0xfc000000,
> + [R_I3C2_REG1] = 0xfff00000,
> + [R_I3C3_REG0] = 0xfc000000,
> + [R_I3C3_REG1] = 0xfff00000,
> + [R_I3C4_REG0] = 0xfc000000,
> + [R_I3C4_REG1] = 0xfff00000,
> + [R_I3C5_REG0] = 0xfc000000,
> + [R_I3C5_REG1] = 0xfff00000,
> + [R_I3C6_REG0] = 0xfc000000,
> + [R_I3C6_REG1] = 0xfff00000,
> +};
> +
> static uint64_t aspeed_i3c_read(void *opaque, hwaddr addr, unsigned int size)
> {
> AspeedI3CState *s = ASPEED_I3C(opaque); @@ -97,6 +112,7 @@ static
> void aspeed_i3c_write(void *opaque,
>
> addr >>= 2;
>
> + data &= ~ast2600_i3c_controller_ro[addr];
> /* I3C controller register */
> switch (addr) {
> case R_I3C1_REG1:
> --
> 2.50.0.rc1.591.g9c95f17f64-goog
