On Sat, 10 Sept 2022 at 07:13, Vikram Garhwal <vikram.garh...@amd.com> wrote: > > Connect CANFD0 and CANFD1 on the Versal-virt machine. > > Signed-off-by: Vikram Garhwal <vikram.garh...@amd.com> > --- > hw/arm/xlnx-versal-virt.c | 45 ++++++++++++++++++++++++++++++++++++ > hw/arm/xlnx-versal.c | 37 +++++++++++++++++++++++++++++ > include/hw/arm/xlnx-versal.h | 12 ++++++++++ > 3 files changed, 94 insertions(+) > > diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c > index 37fc9b919c..062e6f2a95 100644 > --- a/hw/arm/xlnx-versal-virt.c > +++ b/hw/arm/xlnx-versal-virt.c > @@ -40,9 +40,11 @@ struct VersalVirt { > uint32_t clk_25Mhz; > uint32_t usb; > uint32_t dwc; > + uint32_t canfd[2]; > } phandle; > struct arm_boot_info binfo; > > + CanBusState *canbus[XLNX_VERSAL_NR_CANFD]; > struct { > bool secure; > } cfg; > @@ -235,6 +237,34 @@ static void fdt_add_uart_nodes(VersalVirt *s) > } > } > > +static void fdt_add_canfd_nodes(VersalVirt *s) > +{ > + uint64_t addrs[] = { MM_CANFD0, MM_CANFD1 }; > + uint32_t size[] = {MM_CANFD0_SIZE, MM_CANFD1_SIZE};
Can you be consistent about how you format the spacing on this kind of thing, please? > + unsigned int irqs[] = { VERSAL_CANFD0_IRQ_0, VERSAL_CANFD1_IRQ_0 }; > + const char compat[] = "xlnx,versal-canfd"; > + int i; > + > + /* Create and connect CANFD0 and CANFD1 nodes to canbus0. */ > + for (i = 0; i < ARRAY_SIZE(addrs); i++) { > + char *name = g_strdup_printf("/canfd@%" PRIx64, addrs[i]); > + qemu_fdt_add_subnode(s->fdt, name); > + qemu_fdt_setprop_cell(s->fdt, name, "rx-fifo0", 0x40); > + qemu_fdt_setprop_cell(s->fdt, name, "enable-rx-fifo1", 0x1); > + qemu_fdt_setprop_cell(s->fdt, name, "rx-fifo1", 0x40); > + > + qemu_fdt_setprop_cells(s->fdt, name, "interrupts", > + GIC_FDT_IRQ_TYPE_SPI, irqs[i], > + GIC_FDT_IRQ_FLAGS_LEVEL_HI); > + qemu_fdt_setprop_sized_cells(s->fdt, name, "reg", > + 2, addrs[i], 2, size[i]); > + qemu_fdt_setprop(s->fdt, name, "compatible", > + compat, sizeof(compat)); Where there is no embedded NUL byte in the compat string, you can use qemu_fdt_setprop_string(): qemu_fdt_setprop_string(s->fdt, name, "compatible", "xlnx,versal-canfd"); > + > + g_free(name); > + } > +} > + > static void fdt_add_fixed_link_nodes(VersalVirt *s, char *gemname, > uint32_t phandle) > { > @@ -639,12 +669,17 @@ static void versal_virt_init(MachineState *machine) > TYPE_XLNX_VERSAL); > object_property_set_link(OBJECT(&s->soc), "ddr", OBJECT(machine->ram), > &error_abort); > + object_property_set_link(OBJECT(&s->soc), "canbus0", > OBJECT(s->canbus[0]), > + &error_abort); > + object_property_set_link(OBJECT(&s->soc), "canbus1", > OBJECT(s->canbus[1]), > + &error_abort); > sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal); > > fdt_create(s); > create_virtio_regions(s); > fdt_add_gem_nodes(s); > fdt_add_uart_nodes(s); > + fdt_add_canfd_nodes(s); > fdt_add_gic_nodes(s); > fdt_add_timer_nodes(s); > fdt_add_zdma_nodes(s); > @@ -712,6 +747,16 @@ static void versal_virt_init(MachineState *machine) > > static void versal_virt_machine_instance_init(Object *obj) > { > + VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj); > + > + object_property_add_link(obj, "canbus0", TYPE_CAN_BUS, > + (Object **)&s->canbus[0], > + object_property_allow_set_link, > + 0); > + object_property_add_link(obj, "canbus1", TYPE_CAN_BUS, > + (Object **)&s->canbus[1], > + object_property_allow_set_link, > + 0); What are these for ? > } > > static void versal_virt_machine_class_init(ObjectClass *oc, void *data) > diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c > index 57276e1506..452f433eb4 100644 > --- a/hw/arm/xlnx-versal.c > +++ b/hw/arm/xlnx-versal.c > @@ -185,6 +185,38 @@ static void versal_create_uarts(Versal *s, qemu_irq *pic) > } > } > > +static void versal_create_canfds(Versal *s, qemu_irq *pic) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(s->lpd.iou.canfd); i++) { > + static const int irqs[] = { VERSAL_CANFD0_IRQ_0, > VERSAL_CANFD1_IRQ_0}; > + static const uint64_t addrs[] = { MM_CANFD0, MM_CANFD1 }; > + char *name = g_strdup_printf("canfd%d", i); > + DeviceState *dev; > + MemoryRegion *mr; > + > + object_initialize_child(OBJECT(s), name, &s->lpd.iou.canfd[i], > + TYPE_XILINX_CANFD); > + dev = DEVICE(&s->lpd.iou.canfd[i]); You never use dev directly as a DeviceState, so you might as well make it a SysBusDevice and save some of the cast macros below. > + > + object_property_set_int(OBJECT(&s->lpd.iou.canfd[i]), "ext_clk_freq", > + XLNX_VERSAL_CANFD_REF_CLK , &error_abort); > + > + object_property_set_link(OBJECT(dev), "canfdbus", > + OBJECT(s->lpd.iou.canbus[i]), > + &error_abort); > + > + sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal); > + > + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0); > + memory_region_add_subregion(&s->mr_ps, addrs[i], mr); > + > + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irqs[i]]); > + g_free(name); > + } > +} thanks -- PMM