Hi Joe, > Subject: [PATCH 08/19] hw/i3c/dw-i3c: Add register RO field masks > > Adds read-only register masks for the DwC I3C controller. > > Signed-off-by: Joe Komlodi <[email protected]> > > Reviewed-by: Patrick Venture <[email protected]> > --- > hw/i3c/dw-i3c.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/hw/i3c/dw-i3c.c b/hw/i3c/dw-i3c.c index 845fbd5efd..4f40f999f4 > 100644 > --- a/hw/i3c/dw-i3c.c > +++ b/hw/i3c/dw-i3c.c > @@ -313,6 +313,45 @@ static const uint32_t > dw_i3c_resets[DW_I3C_NR_REGS] = { > [R_SLAVE_CONFIG] = 0x00000023, > }; > > +static const uint32_t dw_i3c_ro[DW_I3C_NR_REGS] = { > + [R_DEVICE_CTRL] = 0x04fffe00, > + [R_DEVICE_ADDR] = 0x7f807f80, > + [R_HW_CAPABILITY] = 0xffffffff, > + [R_IBI_QUEUE_STATUS] = 0xffffffff, > + [R_DATA_BUFFER_THLD_CTRL] = 0xf8f8f8f8, > + [R_IBI_QUEUE_CTRL] = 0xfffffff0, > + [R_RESET_CTRL] = 0xffffffc0, > + [R_SLV_EVENT_CTRL] = 0xffffff3f, > + [R_INTR_STATUS] = 0xffff809f, > + [R_INTR_STATUS_EN] = 0xffff8080, > + [R_INTR_SIGNAL_EN] = 0xffff8080, > + [R_INTR_FORCE] = 0xffff8000, > + [R_QUEUE_STATUS_LEVEL] = 0xffffffff, > + [R_DATA_BUFFER_STATUS_LEVEL] = 0xffffffff, > + [R_PRESENT_STATE] = 0xffffffff, > + [R_CCC_DEVICE_STATUS] = 0xffffffff, > + [R_I3C_VER_ID] = 0xffffffff, > + [R_I3C_VER_TYPE] = 0xffffffff, > + [R_DEVICE_ADDR_TABLE_POINTER] = 0xffffffff, > + [R_DEV_CHAR_TABLE_POINTER] = 0xffcbffff, > + [R_SLV_PID_VALUE] = 0xffff0fff, > + [R_SLV_CHAR_CTRL] = 0xffffffff, > + [A_VENDOR_SPECIFIC_REG_POINTER] = 0xffffffff, > + [R_SLV_MAX_LEN] = 0xffffffff, > + [R_MAX_READ_TURNAROUND] = 0xffffffff, > + [R_MAX_DATA_SPEED] = 0xffffffff, > + [R_SLV_INTR_REQ] = 0xfffffff0, > + [R_SLV_TSX_SYMBL_TIMING] = 0xffffffc0, > + [R_DEVICE_CTRL_EXTENDED] = 0xfffffff8, > + [R_SCL_I3C_OD_TIMING] = 0xff00ff00, > + [R_SCL_I3C_PP_TIMING] = 0xff00ff00, > + [R_SCL_I2C_FMP_TIMING] = 0xff000000, > + [R_SCL_EXT_TERMN_LCNT_TIMING] = 0x0000fff0, > + [R_BUS_IDLE_TIMING] = 0xfff00000, > + [R_EXTENDED_CAPABILITY] = 0xffffffff, > + [R_SLAVE_CONFIG] = 0xffffffff, > +}; > + > static uint64_t dw_i3c_read(void *opaque, hwaddr offset, unsigned size) { > DWI3C *s = DW_I3C(opaque); > @@ -341,6 +380,7 @@ static void dw_i3c_write(void *opaque, hwaddr offset, > uint64_t value, > > trace_dw_i3c_write(s->id, offset, value); > > + value &= ~dw_i3c_ro[addr]; > switch (addr) { The following registers are read-only, and the code already checks their access based solely on the register address. Why do we still need to define them in dw_i3c_ro? case R_HW_CAPABILITY: case R_RESPONSE_QUEUE_PORT: case R_IBI_QUEUE_DATA: case R_QUEUE_STATUS_LEVEL: case R_PRESENT_STATE: case R_CCC_DEVICE_STATUS: case R_DEVICE_ADDR_TABLE_POINTER: case R_VENDOR_SPECIFIC_REG_POINTER: case R_SLV_CHAR_CTRL: case R_SLV_MAX_LEN: case R_MAX_READ_TURNAROUND: case R_I3C_VER_ID: case R_I3C_VER_TYPE: case R_EXTENDED_CAPABILITY:
Thanks-Jamin > case R_HW_CAPABILITY: > case R_RESPONSE_QUEUE_PORT: > -- > 2.50.0.rc1.591.g9c95f17f64-goog
