On Wed, Sep 17, 2025 at 01:44:48PM +0200, Luc Michel wrote: > Now that we have a simple decoding logic for all the banked registers, > remove the register API usage for them. This restricts the register API > usage to only the base registers (from 0x0 to 0xec). > > This also removes all the custom code that was creating register > descriptors for the register API and was leading to memory leaks when > the device was finalized. > > Signed-off-by: Luc Michel <[email protected]>
Reviewed-by: Francisco Iglesias <[email protected]> > --- > include/hw/net/xlnx-versal-canfd.h | 8 - > hw/net/can/xlnx-versal-canfd.c | 295 +---------------------------- > 2 files changed, 5 insertions(+), 298 deletions(-) > > diff --git a/include/hw/net/xlnx-versal-canfd.h > b/include/hw/net/xlnx-versal-canfd.h > index ad3104dd13f..396f90d6dc1 100644 > --- a/include/hw/net/xlnx-versal-canfd.h > +++ b/include/hw/net/xlnx-versal-canfd.h > @@ -52,18 +52,10 @@ typedef struct XlnxVersalCANFDState { > > qemu_irq irq_canfd_int; > qemu_irq irq_addr_err; > > RegisterInfo reg_info[XLNX_VERSAL_CANFD_R_MAX]; > - RegisterAccessInfo *tx_regs; > - RegisterAccessInfo *rx0_regs; > - RegisterAccessInfo *rx1_regs; > - RegisterAccessInfo *af_regs; > - RegisterAccessInfo *txe_regs; > - RegisterAccessInfo *rx_mailbox_regs; > - RegisterAccessInfo *af_mask_regs_mailbox; > - > uint32_t regs[XLNX_VERSAL_CANFD_R_MAX]; > > ptimer_state *canfd_timer; > > CanBusClientState bus_client; > diff --git a/hw/net/can/xlnx-versal-canfd.c b/hw/net/can/xlnx-versal-canfd.c > index 81615bc52a6..49f1b174b70 100644 > --- a/hw/net/can/xlnx-versal-canfd.c > +++ b/hw/net/can/xlnx-versal-canfd.c > @@ -1425,50 +1425,10 @@ static void filter_reg_write(XlnxVersalCANFDState *s, > hwaddr addr, > HWADDR_PRIx " changed while filter %zu enabled\n", > path, addr, bank_idx + 1); > } > } > > -static uint64_t filter_mask(RegisterInfo *reg, uint64_t val64) > -{ > - XlnxVersalCANFDState *s = XILINX_CANFD(reg->opaque); > - uint32_t reg_idx = (reg->access->addr) / 4; > - uint32_t val = val64; > - uint32_t filter_offset = (reg_idx - R_AFMR_REGISTER) / 2; > - > - if (!(s->regs[R_ACCEPTANCE_FILTER_CONTROL_REGISTER] & > - (1 << filter_offset))) { > - s->regs[reg_idx] = val; > - } else { > - g_autofree char *path = object_get_canonical_path(OBJECT(s)); > - > - qemu_log_mask(LOG_GUEST_ERROR, "%s: Acceptance filter %d not > enabled\n", > - path, filter_offset + 1); > - } > - > - return s->regs[reg_idx]; > -} > - > -static uint64_t filter_id(RegisterInfo *reg, uint64_t val64) > -{ > - XlnxVersalCANFDState *s = XILINX_CANFD(reg->opaque); > - hwaddr reg_idx = (reg->access->addr) / 4; > - uint32_t val = val64; > - uint32_t filter_offset = (reg_idx - R_AFIR_REGISTER) / 2; > - > - if (!(s->regs[R_ACCEPTANCE_FILTER_CONTROL_REGISTER] & > - (1 << filter_offset))) { > - s->regs[reg_idx] = val; > - } else { > - g_autofree char *path = object_get_canonical_path(OBJECT(s)); > - > - qemu_log_mask(LOG_GUEST_ERROR, "%s: Acceptance filter %d not > enabled\n", > - path, filter_offset + 1); > - } > - > - return s->regs[reg_idx]; > -} > - > static uint64_t canfd_tx_fifo_status_prew(RegisterInfo *reg, uint64_t val64) > { > XlnxVersalCANFDState *s = XILINX_CANFD(reg->opaque); > uint32_t val = val64; > uint8_t read_ind = 0; > @@ -1590,129 +1550,10 @@ static uint64_t canfd_write_check_prew(RegisterInfo > *reg, uint64_t val64) > return val; > } > return 0; > } > > -static const RegisterAccessInfo canfd_tx_regs[] = { > - { .name = "TB_ID_REGISTER", .addr = A_TB_ID_REGISTER, > - },{ .name = "TB0_DLC_REGISTER", .addr = A_TB0_DLC_REGISTER, > - },{ .name = "TB_DW0_REGISTER", .addr = A_TB_DW0_REGISTER, > - },{ .name = "TB_DW1_REGISTER", .addr = A_TB_DW1_REGISTER, > - },{ .name = "TB_DW2_REGISTER", .addr = A_TB_DW2_REGISTER, > - },{ .name = "TB_DW3_REGISTER", .addr = A_TB_DW3_REGISTER, > - },{ .name = "TB_DW4_REGISTER", .addr = A_TB_DW4_REGISTER, > - },{ .name = "TB_DW5_REGISTER", .addr = A_TB_DW5_REGISTER, > - },{ .name = "TB_DW6_REGISTER", .addr = A_TB_DW6_REGISTER, > - },{ .name = "TB_DW7_REGISTER", .addr = A_TB_DW7_REGISTER, > - },{ .name = "TB_DW8_REGISTER", .addr = A_TB_DW8_REGISTER, > - },{ .name = "TB_DW9_REGISTER", .addr = A_TB_DW9_REGISTER, > - },{ .name = "TB_DW10_REGISTER", .addr = A_TB_DW10_REGISTER, > - },{ .name = "TB_DW11_REGISTER", .addr = A_TB_DW11_REGISTER, > - },{ .name = "TB_DW12_REGISTER", .addr = A_TB_DW12_REGISTER, > - },{ .name = "TB_DW13_REGISTER", .addr = A_TB_DW13_REGISTER, > - },{ .name = "TB_DW14_REGISTER", .addr = A_TB_DW14_REGISTER, > - },{ .name = "TB_DW15_REGISTER", .addr = A_TB_DW15_REGISTER, > - } > -}; > - > -static const RegisterAccessInfo canfd_rx0_regs[] = { > - { .name = "RB_ID_REGISTER", .addr = A_RB_ID_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DLC_REGISTER", .addr = A_RB_DLC_REGISTER, > - .ro = 0xfe1fffff, > - },{ .name = "RB_DW0_REGISTER", .addr = A_RB_DW0_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW1_REGISTER", .addr = A_RB_DW1_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW2_REGISTER", .addr = A_RB_DW2_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW3_REGISTER", .addr = A_RB_DW3_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW4_REGISTER", .addr = A_RB_DW4_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW5_REGISTER", .addr = A_RB_DW5_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW6_REGISTER", .addr = A_RB_DW6_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW7_REGISTER", .addr = A_RB_DW7_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW8_REGISTER", .addr = A_RB_DW8_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW9_REGISTER", .addr = A_RB_DW9_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW10_REGISTER", .addr = A_RB_DW10_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW11_REGISTER", .addr = A_RB_DW11_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW12_REGISTER", .addr = A_RB_DW12_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW13_REGISTER", .addr = A_RB_DW13_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW14_REGISTER", .addr = A_RB_DW14_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "RB_DW15_REGISTER", .addr = A_RB_DW15_REGISTER, > - .ro = 0xffffffff, > - } > -}; > - > -static const RegisterAccessInfo canfd_rx1_regs[] = { > - { .name = "RB_ID_REGISTER_1", .addr = A_RB_ID_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DLC_REGISTER_1", .addr = A_RB_DLC_REGISTER_1, > - .ro = 0xfe1fffff, > - },{ .name = "RB0_DW0_REGISTER_1", .addr = A_RB0_DW0_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW1_REGISTER_1", .addr = A_RB_DW1_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW2_REGISTER_1", .addr = A_RB_DW2_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW3_REGISTER_1", .addr = A_RB_DW3_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW4_REGISTER_1", .addr = A_RB_DW4_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW5_REGISTER_1", .addr = A_RB_DW5_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW6_REGISTER_1", .addr = A_RB_DW6_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW7_REGISTER_1", .addr = A_RB_DW7_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW8_REGISTER_1", .addr = A_RB_DW8_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW9_REGISTER_1", .addr = A_RB_DW9_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW10_REGISTER_1", .addr = A_RB_DW10_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW11_REGISTER_1", .addr = A_RB_DW11_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW12_REGISTER_1", .addr = A_RB_DW12_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW13_REGISTER_1", .addr = A_RB_DW13_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW14_REGISTER_1", .addr = A_RB_DW14_REGISTER_1, > - .ro = 0xffffffff, > - },{ .name = "RB_DW15_REGISTER_1", .addr = A_RB_DW15_REGISTER_1, > - .ro = 0xffffffff, > - } > -}; > - > -/* Acceptance filter registers. */ > -static const RegisterAccessInfo canfd_af_regs[] = { > - { .name = "AFMR_REGISTER", .addr = A_AFMR_REGISTER, > - .pre_write = filter_mask, > - },{ .name = "AFIR_REGISTER", .addr = A_AFIR_REGISTER, > - .pre_write = filter_id, > - } > -}; > - > -static const RegisterAccessInfo canfd_txe_regs[] = { > - { .name = "TXE_FIFO_TB_ID_REGISTER", .addr = > A_TXE_FIFO_TB_ID_REGISTER, > - .ro = 0xffffffff, > - },{ .name = "TXE_FIFO_TB_DLC_REGISTER", .addr = > A_TXE_FIFO_TB_DLC_REGISTER, > - .ro = 0xffffffff, > - } > -}; > - > static const RegisterAccessInfo canfd_regs_info[] = { > { .name = "SOFTWARE_RESET_REGISTER", .addr = > A_SOFTWARE_RESET_REGISTER, > .pre_write = canfd_srr_pre_write, > },{ .name = "MODE_SELECT_REGISTER", .addr = A_MODE_SELECT_REGISTER, > .pre_write = canfd_msr_pre_write, > @@ -2001,146 +1842,20 @@ static int > xlnx_canfd_connect_to_bus(XlnxVersalCANFDState *s, > s->bus_client.info = &canfd_xilinx_bus_client_info; > > return can_bus_insert_client(bus, &s->bus_client); > } > > -#define NUM_REG_PER_AF ARRAY_SIZE(canfd_af_regs) > -#define NUM_AF 32 > -#define NUM_REG_PER_TXE ARRAY_SIZE(canfd_txe_regs) > -#define NUM_TXE 32 > - > -static int canfd_populate_regarray(XlnxVersalCANFDState *s, > - RegisterInfoArray *r_array, int pos, > - const RegisterAccessInfo *rae, > - int num_rae) > -{ > - int i; > - > - for (i = 0; i < num_rae; i++) { > - int index = rae[i].addr / 4; > - RegisterInfo *r = &s->reg_info[index]; > - > - object_initialize(r, sizeof(*r), TYPE_REGISTER); > - > - *r = (RegisterInfo) { > - .data = &s->regs[index], > - .data_size = sizeof(uint32_t), > - .access = &rae[i], > - .opaque = OBJECT(s), > - }; > - > - r_array->r[i + pos] = r; > - } > - return i + pos; > -} > - > -static void canfd_create_rai(RegisterAccessInfo *rai_array, > - const RegisterAccessInfo *canfd_regs, > - int template_rai_array_sz, > - int num_template_to_copy) > -{ > - int i; > - int reg_num; > - > - for (reg_num = 0; reg_num < num_template_to_copy; reg_num++) { > - int pos = reg_num * template_rai_array_sz; > - > - memcpy(rai_array + pos, canfd_regs, > - template_rai_array_sz * sizeof(RegisterAccessInfo)); > - > - for (i = 0; i < template_rai_array_sz; i++) { > - const char *name = canfd_regs[i].name; > - uint64_t addr = canfd_regs[i].addr; > - rai_array[i + pos].name = g_strdup_printf("%s%d", name, reg_num); > - rai_array[i + pos].addr = addr + pos * 4; > - } > - } > -} > - > -static RegisterInfoArray *canfd_create_regarray(XlnxVersalCANFDState *s) > -{ > - const char *device_prefix = object_get_typename(OBJECT(s)); > - uint64_t memory_size = XLNX_VERSAL_CANFD_R_MAX * 4; > - int num_regs; > - int pos = 0; > - RegisterInfoArray *r_array; > - > - num_regs = ARRAY_SIZE(canfd_regs_info) + > - s->cfg.tx_fifo * NUM_REGS_PER_MSG_SPACE + > - s->cfg.rx0_fifo * NUM_REGS_PER_MSG_SPACE + > - NUM_AF * NUM_REG_PER_AF + > - NUM_TXE * NUM_REG_PER_TXE; > - > - s->tx_regs = g_new0(RegisterAccessInfo, > - s->cfg.tx_fifo * ARRAY_SIZE(canfd_tx_regs)); > - > - canfd_create_rai(s->tx_regs, canfd_tx_regs, > - ARRAY_SIZE(canfd_tx_regs), s->cfg.tx_fifo); > - > - s->rx0_regs = g_new0(RegisterAccessInfo, > - s->cfg.rx0_fifo * ARRAY_SIZE(canfd_rx0_regs)); > - > - canfd_create_rai(s->rx0_regs, canfd_rx0_regs, > - ARRAY_SIZE(canfd_rx0_regs), s->cfg.rx0_fifo); > - > - s->af_regs = g_new0(RegisterAccessInfo, > - NUM_AF * ARRAY_SIZE(canfd_af_regs)); > - > - canfd_create_rai(s->af_regs, canfd_af_regs, > - ARRAY_SIZE(canfd_af_regs), NUM_AF); > - > - s->txe_regs = g_new0(RegisterAccessInfo, > - NUM_TXE * ARRAY_SIZE(canfd_txe_regs)); > - > - canfd_create_rai(s->txe_regs, canfd_txe_regs, > - ARRAY_SIZE(canfd_txe_regs), NUM_TXE); > - > - if (s->cfg.enable_rx_fifo1) { > - num_regs += s->cfg.rx1_fifo * NUM_REGS_PER_MSG_SPACE; > - > - s->rx1_regs = g_new0(RegisterAccessInfo, > - s->cfg.rx1_fifo * ARRAY_SIZE(canfd_rx1_regs)); > - > - canfd_create_rai(s->rx1_regs, canfd_rx1_regs, > - ARRAY_SIZE(canfd_rx1_regs), s->cfg.rx1_fifo); > - } > - > - r_array = g_new0(RegisterInfoArray, 1); > - r_array->r = g_new0(RegisterInfo * , num_regs); > - r_array->num_elements = num_regs; > - r_array->prefix = device_prefix; > - > - pos = canfd_populate_regarray(s, r_array, pos, > - canfd_regs_info, > - ARRAY_SIZE(canfd_regs_info)); > - pos = canfd_populate_regarray(s, r_array, pos, > - s->tx_regs, s->cfg.tx_fifo * > - NUM_REGS_PER_MSG_SPACE); > - pos = canfd_populate_regarray(s, r_array, pos, > - s->rx0_regs, s->cfg.rx0_fifo * > - NUM_REGS_PER_MSG_SPACE); > - if (s->cfg.enable_rx_fifo1) { > - pos = canfd_populate_regarray(s, r_array, pos, > - s->rx1_regs, s->cfg.rx1_fifo * > - NUM_REGS_PER_MSG_SPACE); > - } > - pos = canfd_populate_regarray(s, r_array, pos, > - s->af_regs, NUM_AF * NUM_REG_PER_AF); > - pos = canfd_populate_regarray(s, r_array, pos, > - s->txe_regs, NUM_TXE * NUM_REG_PER_TXE); > - > - memory_region_init_io(&r_array->mem, OBJECT(s), &canfd_ops, r_array, > - device_prefix, memory_size); > - return r_array; > -} > - > static void canfd_realize(DeviceState *dev, Error **errp) > { > XlnxVersalCANFDState *s = XILINX_CANFD(dev); > RegisterInfoArray *reg_array; > > - reg_array = canfd_create_regarray(s); > + reg_array = register_init_block32(dev, canfd_regs_info, > + ARRAY_SIZE(canfd_regs_info), > s->reg_info, > + s->regs, &canfd_regs_ops, false, > + A_RX_FIFO_WATERMARK_REGISTER > + + sizeof(uint32_t)); > memory_region_add_subregion(&s->iomem, 0x00, ®_array->mem); > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); > sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq_canfd_int); > > if (s->canfdbus) { > -- > 2.50.1 >
