Emmanuel Blot <[email protected]> writes:
> Slaves created with i2c_slave_create_simple() were left unparented and
> showed up under /machine/unattached with no stable QOM path. Add each
> slave as a QOM child of its bus, named after its I2C address, so it has
> a deterministic and addressable QOM path.
Could use an example.
Doesn't tell which machines are affected.
To find affected machines, I captured output of "info qom-tree" by running
$ echo -e 'info qom-tree\nq' | qemu-system-TARGET -M MACHINE -S -display
none -monitor stdio
for all targets and all their machine types.
This fails on my box for a number of machines: misses some image file,
needs Xen, ... These are
an5206
boston
boston-aia
canon-a1100
leon3_generic
loongson3-virt
mcf5208evb
microchip-icicle-kit
niagara
nitro
q800
sx1
sx1-v1
xenfv-4.2
xenpv
xenpvh
Testing them manually would be possible. Not by me.
Of the ones that run, output differs for
anacapa-bmc
ast1030-evb
ast1040-evb
ast1060-evb
ast2500-evb
ast2600-evb
ast2700a1-evb
ast2700a2-evb
ast2700fc
bletchley-bmc
bpim2u
catalina-bmc
cubieboard
fby35-bmc
fuji-bmc
g220a-bmc
gb200nvl-bmc
kudo-bmc
lm3s811evb
mpc8544ds
npcm750-evb
npcm845-evb
palmetto-bmc
powernv10-rainier
ppce500
quanta-gsj
quanta-q71l-bmc
rainier-bmc
realview-eb
realview-eb-mpcore
realview-pb-a8
realview-pbx-a9
romulus-bmc
sam460ex
supermicro-x11spi-bmc
supermicrox11-bmc
tiogapass-bmc
tt-atlantis
versatileab
versatilepb
vexpress-a15
vexpress-a9
witherspoon-bmc
yosemitev2-bmc
Looking for one with a short diff, I found realview-eb:
/device[26] (versatile_i2c)
/arm_sbcon_i2c[0] (memory-region)
/i2c (i2c-bus)
- /device[27] (ds1338)
+ /0x68 (ds1338)
/device[2] (realview_gic)
/gic (arm_gic)
/gic_cpu[0] (memory-region)
The QOM path of this ds1338 device changes from
/machine/unattached/device[27]
to
/machine/unattached/device[26]/0x68
The new path still isn't stable, because the parent's path isn't. This
caveat should be noted in the commit message.
An example of successful stabilization is tt-atlantis, where the QOM
paths of ds1338 and tmp105 change from
/machine/unattached/device[21]
/machine/unattached/device[22]
to
/machine/i2c[0]/i2c-bus/0x6f
/machine/i2c[4]/i2c-bus/0x48
> Signed-off-by: Emmanuel Blot <[email protected]>
> ---
> hw/i2c/core.c | 3 +++
> include/hw/i2c/i2c.h | 7 +++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i2c/core.c b/hw/i2c/core.c
> index 54f6bdca88..0bbce45d48 100644
> --- a/hw/i2c/core.c
> +++ b/hw/i2c/core.c
> @@ -382,6 +382,9 @@ I2CSlave *i2c_slave_create_simple(I2CBus *bus, const char
> *name, uint8_t addr)
> {
> I2CSlave *dev = i2c_slave_new(name, addr);
>
> + g_autofree char *childname = g_strdup_printf("0x%02x", addr);
Is the hex address a satisfactory child name?
> + object_property_add_child(OBJECT(bus), childname, OBJECT(dev));
> +
> i2c_slave_realize_and_unref(dev, bus, &error_abort);
>
> return dev;
> diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
> index dd5930f4b5..dc557bbf3f 100644
> --- a/include/hw/i2c/i2c.h
> +++ b/include/hw/i2c/i2c.h
> @@ -166,13 +166,16 @@ bool i2c_scan_bus(I2CBus *bus, uint8_t address, bool
> broadcast,
> I2CSlave *i2c_slave_new(const char *name, uint8_t addr);
>
> /**
> - * Create and realize an I2C slave device on the heap.
> + * Create and realize an I2C slave device on the heap, add the device as a
> + * child of its parent bus.
> + *
> * @bus: I2C bus to put it on
> * @name: I2C slave device type name
> * @addr: I2C address of the slave when put on a bus
> *
> * Create the device state structure, initialize it, put it on the
> - * specified @bus, and drop the reference to it (the device is realized).
> + * specified @bus, parent it, and drop the reference to it (the device is
> + * realized).
> */
> I2CSlave *i2c_slave_create_simple(I2CBus *bus, const char *name, uint8_t
> addr);
