On 30/9/26 13:50, Cédric Le Goater wrote:
Note to Peter: I really think keeping duplicating the
UnimplementedDeviceConfig structure is a basic software
development antipattern. Could you reconsider your objection?
What's the background ?
I think that was discussed on IRC, the only ref on the list is:
https://lore.kernel.org/qemu-devel/cafeaca_ob6mhqfaam9iamae5vfxayhmcqkk-gjh+s1mhuvf...@mail.gmail.com/
create_unimplemented_device() calls qdev_create() and allocate on the
heap, my motivation was to have another helper for in-place init (when
the UnimplementedDeviceState state is embedded in the parent). Now we
ended with such helpers like aspeed_mmio_map_unimplemented() which
realize in place, make_unimp_dev() and create_unimp() init and realize
in place and Jamin adds yet another pattern. While all are valids,
I'd rather unify. Maybe this is now back to the "qdev alloc vs qdev
struct embedded" discussion we never figured out.
hw/arm/aspeed_ast1040.c:127: object_initialize_child(obj, "pwm",
&s->pwm, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast1040.c:128: object_initialize_child(obj, "espi",
&s->espi, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast1040.c:129: object_initialize_child(obj, "udc",
&s->udc, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast10x0.c:165: object_initialize_child(obj, "iomem",
&s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast10x0.c:166: object_initialize_child(obj, "pwm",
&s->pwm, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast10x0.c:167: object_initialize_child(obj, "espi",
&s->espi, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast10x0.c:168: object_initialize_child(obj, "udc",
&s->udc, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2400.c:243: object_initialize_child(obj, "iomem",
&s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2400.c:244: object_initialize_child(obj, "video",
&s->video, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2600.c:285: object_initialize_child(obj, "iomem",
&s->iomem, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2600.c:286: object_initialize_child(obj, "video",
&s->video, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/aspeed_ast2600.c:287: object_initialize_child(obj, "dpmcu",
&s->dpmcu, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/bcm2835_peripherals.c:40: object_initialize_child(OBJECT(ps),
name, uds, TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/mps2-tz.c:440: object_initialize_child(OBJECT(mms), name, uds,
TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/musca.c:146: object_initialize_child(OBJECT(mms), name, uds,
TYPE_UNIMPLEMENTED_DEVICE);
hw/arm/bcm2835_peripherals.c:36:void
create_unimp(BCMSocPeripheralBaseState *ps,
hw/arm/bcm2835_peripherals.c:521: create_unimp(s, &s->txp,
"bcm2835-txp", TXP_OFFSET, 0x1000);
hw/arm/bcm2835_peripherals.c:522: create_unimp(s, &s->armtmr,
"bcm2835-sp804", ARMCTRL_TIMER0_1_OFFSET, 0x40);
hw/arm/bcm2835_peripherals.c:523: create_unimp(s, &s->i2s,
"bcm2835-i2s", I2S_OFFSET, 0x100);
hw/arm/bcm2835_peripherals.c:524: create_unimp(s, &s->smi,
"bcm2835-smi", SMI_OFFSET, 0x100);
hw/arm/bcm2835_peripherals.c:525: create_unimp(s, &s->bscsl,
"bcm2835-spis", BSC_SL_OFFSET, 0x100);
hw/arm/bcm2835_peripherals.c:526: create_unimp(s, &s->dbus,
"bcm2835-dbus", DBUS_OFFSET, 0x8000);
hw/arm/bcm2835_peripherals.c:527: create_unimp(s, &s->ave0,
"bcm2835-ave0", AVE0_OFFSET, 0x8000);
hw/arm/bcm2835_peripherals.c:528: create_unimp(s, &s->v3d,
"bcm2835-v3d", V3D_OFFSET, 0x1000);
hw/arm/bcm2835_peripherals.c:529: create_unimp(s, &s->sdramc,
"bcm2835-sdramc", SDRAMC_OFFSET, 0x100);
hw/arm/bcm2838_peripherals.c:182: create_unimp(s_base, &s->clkisp,
"bcm2835-clkisp", CLOCK_ISP_OFFSET,
hw/arm/bcm2838_peripherals.c:196: create_unimp(s_base, &s->asb,
"bcm2838-asb", BRDG_OFFSET, 0x24);
hw/arm/mps2-tz.c:429:static MemoryRegion
*make_unimp_dev(MPS2TZMachineState *mms,
hw/arm/mps2-tz.c:981: { "i2s-audio", make_unimp_dev,
&mms->i2s_audio,
hw/arm/mps2-tz.c:988: { "gfx", make_unimp_dev, &mms->gfx,
0x41000000, 0x140000 },
hw/arm/mps2-tz.c:989: { "gpio0", make_unimp_dev,
&mms->gpio[0], 0x40100000, 0x1000 },
hw/arm/mps2-tz.c:990: { "gpio1", make_unimp_dev,
&mms->gpio[1], 0x40101000, 0x1000 },
hw/arm/mps2-tz.c:991: { "gpio2", make_unimp_dev,
&mms->gpio[2], 0x40102000, 0x1000 },
hw/arm/mps2-tz.c:992: { "gpio3", make_unimp_dev,
&mms->gpio[3], 0x40103000, 0x1000 },
hw/arm/mps2-tz.c:1035: { "i2s-audio", make_unimp_dev,
&mms->i2s_audio,
hw/arm/mps2-tz.c:1045: { "clcd", make_unimp_dev,
&mms->cldc, 0x4130a000, 0x1000 },
hw/arm/mps2-tz.c:1051: { "gpio0", make_unimp_dev,
&mms->gpio[0], 0x41100000, 0x1000 },
hw/arm/mps2-tz.c:1052: { "gpio1", make_unimp_dev,
&mms->gpio[1], 0x41101000, 0x1000 },
hw/arm/mps2-tz.c:1053: { "gpio2", make_unimp_dev,
&mms->gpio[2], 0x41102000, 0x1000 },
hw/arm/mps2-tz.c:1054: { "gpio3", make_unimp_dev,
&mms->gpio[3], 0x41103000, 0x1000 },
hw/arm/mps2-tz.c:1102: { "i2s-audio", make_unimp_dev,
&mms->i2s_audio, 0x49301000, 0x1000 },
hw/arm/mps2-tz.c:1111: { "clcd", make_unimp_dev,
&mms->cldc, 0x4930a000, 0x1000 },
hw/arm/mps2-tz.c:1117: { "gpio0", make_unimp_dev,
&mms->gpio[0], 0x41100000, 0x1000 },
hw/arm/mps2-tz.c:1118: { "gpio1", make_unimp_dev,
&mms->gpio[1], 0x41101000, 0x1000 },
hw/arm/mps2-tz.c:1119: { "gpio2", make_unimp_dev,
&mms->gpio[2], 0x41102000, 0x1000 },
hw/arm/mps2-tz.c:1120: { "gpio3", make_unimp_dev,
&mms->gpio[3], 0x41103000, 0x1000 },
hw/arm/mps2-tz.c:1141: { "sram-timing-adapter",
make_unimp_dev, &mms->ta[0], 0x41700000, 0x1000 },
hw/arm/mps2-tz.c:1142: { "qspi-timing-adapter",
make_unimp_dev, &mms->ta[1], 0x41701000, 0x1000 },
hw/arm/mps2-tz.c:1143: { "ddr4-timing-adapter",
make_unimp_dev, &mms->ta[2], 0x41702000, 0x1000 },
hw/arm/mps2-tz.c:1167: { "i2s-audio", make_unimp_dev,
&mms->i2s_audio, 0x49301000, 0x1000 },
hw/arm/mps2-tz.c:1176: { "clcd", make_unimp_dev,
&mms->cldc, 0x4930a000, 0x1000 },
hw/arm/mps2-tz.c:1182: { "gpio0", make_unimp_dev,
&mms->gpio[0], 0x41100000, 0x1000 },
hw/arm/mps2-tz.c:1183: { "gpio1", make_unimp_dev,
&mms->gpio[1], 0x41101000, 0x1000 },
hw/arm/mps2-tz.c:1184: { "gpio2", make_unimp_dev,
&mms->gpio[2], 0x41102000, 0x1000 },
hw/arm/mps2-tz.c:1185: { "gpio3", make_unimp_dev,
&mms->gpio[3], 0x41103000, 0x1000 },
hw/arm/mps2-tz.c:1195: { "qspi-controller0",
make_unimp_dev, &mms->qspi_controller[0],
hw/arm/mps2-tz.c:1197: { "qspi-controller1",
make_unimp_dev, &mms->qspi_controller[1],
hw/arm/mps2-tz.c:1199: { "qspi-controller2",
make_unimp_dev, &mms->qspi_controller[2],
hw/arm/musca.c:137:static MemoryRegion *make_unimp_dev(MuscaMachineState
*mms,
hw/arm/musca.c:325: { "spi", make_unimp_dev, &mms->spi, 0x3000,
0x1000 },
hw/arm/musca.c:326: { "i2c0", make_unimp_dev, &mms->i2c[0],
0x4000, 0x1000 },
hw/arm/musca.c:327: { "i2c1", make_unimp_dev, &mms->i2c[1],
0x5000, 0x1000 },
hw/arm/musca.c:328: { "i2s", make_unimp_dev, &mms->i2s, 0x6000,
0x1000 },
hw/arm/musca.c:329: { "pwm0", make_unimp_dev, &mms->pwm[0],
0x7000, 0x1000 },
hw/arm/musca.c:331: { "qspi", make_unimp_dev, &mms->qspi, 0xa000,
0x1000 },
hw/arm/musca.c:332: { "timer", make_unimp_dev, &mms->timer,
0xb000, 0x1000 },
hw/arm/musca.c:333: { "scc", make_unimp_dev, &mms->scc, 0xc000,
0x1000 },
hw/arm/musca.c:334: { "pwm1", make_unimp_dev, &mms->pwm[1],
0xe000, 0x1000 },
hw/arm/musca.c:335: { "pwm2", make_unimp_dev, &mms->pwm[2],
0xf000, 0x1000 },
hw/arm/musca.c:336: { "gpio", make_unimp_dev, &mms->gpio,
0x10000, 0x1000 },
hw/arm/musca.c:463: { "eflash0", make_unimp_dev,
&mms->eflash[0],
hw/arm/musca.c:465: { "eflash1", make_unimp_dev,
&mms->eflash[1],
hw/arm/musca.c:467: { "qspi", make_unimp_dev, &mms->qspi,
0x42800000, 0x100000 },
hw/arm/musca.c:472: { "mhu0", make_unimp_dev,
&mms->mhu[0], 0x42600000, 0x100000 },
hw/arm/musca.c:473: { "mhu1", make_unimp_dev,
&mms->mhu[1], 0x42700000, 0x100000 },
hw/arm/musca.c:484: { "pwm0", make_unimp_dev,
&mms->pwm[0], 0x40101000, 0x1000 },
hw/arm/musca.c:485: { "pwm1", make_unimp_dev,
&mms->pwm[1], 0x40102000, 0x1000 },
hw/arm/musca.c:486: { "pwm2", make_unimp_dev,
&mms->pwm[2], 0x40103000, 0x1000 },
hw/arm/musca.c:487: { "i2s", make_unimp_dev, &mms->i2s,
0x40104000, 0x1000 },
hw/arm/musca.c:490: { "i2c0", make_unimp_dev,
&mms->i2c[0], 0x40108000, 0x1000 },
hw/arm/musca.c:491: { "i2c1", make_unimp_dev,
&mms->i2c[1], 0x40109000, 0x1000 },
hw/arm/musca.c:492: { "spi", make_unimp_dev, &mms->spi,
0x4010a000, 0x1000 },
hw/arm/musca.c:493: { "scc", make_unimp_dev, &mms->scc,
0x5010b000, 0x1000 },
hw/arm/musca.c:494: { "timer", make_unimp_dev,
&mms->timer, 0x4010c000, 0x1000 },
hw/arm/musca.c:496: { "pvt", make_unimp_dev, &mms->pvt,
0x4010e000, 0x1000 },
hw/arm/musca.c:497: { "sdio", make_unimp_dev, &mms->sdio,
0x4010f000, 0x1000 },
hw/arm/musca.c:503: { "gpio", make_unimp_dev, &mms->gpio,
0x41000000, 0x1000 },
