On Tue, Feb 1, 2022 at 11:02 AM Philippe Mathieu-Daudé <f4...@amsat.org>
wrote:
> On 1/2/22 17:30, Patrick Venture wrote:
> > Previously this device created N subdevices which each owned an i2c bus.
> > Now this device simply owns the N i2c busses directly.
> >
> > Tested: Verified devices behind mux are still accessible via qmp and i2c
> > from within an arm32 SoC.
> >
> > Reviewed-by: Hao Wu <wuhao...@google.com>
> > Signed-off-by: Patrick Venture <vent...@google.com>
> > ---
> > hw/i2c/i2c_mux_pca954x.c | 75 ++++++----------------------------------
> > 1 file changed, 11 insertions(+), 64 deletions(-)
>
> > static void pca954x_init(Object *obj)
> > {
> > Pca954xState *s = PCA954X(obj);
> > Pca954xClass *c = PCA954X_GET_CLASS(obj);
> > int i;
> >
> > - /* Only initialize the children we expect. */
> > + /* SMBus modules. Cannot fail. */
> > for (i = 0; i < c->nchans; i++) {
> > - object_initialize_child(obj, "channel[*]", &s->channel[i],
> > - TYPE_PCA954X_CHANNEL);
> > + /* start all channels as disabled. */
> > + s->enabled[i] = false;
> > + s->bus[i] = i2c_init_bus(DEVICE(s), "channel[*]");
>
> This is not a QOM property, so you need to initialize manually:
>
that was my suspicion but this is the output I'm seeing:
{'execute': 'qom-list', 'arguments': { 'path':
'/machine/soc/smbus[0]/i2c-bus/child[0]' }}
{"return": [
{"name": "type", "type": "string"},
{"name": "parent_bus", "type": "link<bus>"},
{"name": "realized", "type": "bool"},
{"name": "hotplugged", "type": "bool"},
{"name": "hotpluggable", "type": "bool"},
{"name": "address", "type": "uint8"},
{"name": "channel[3]", "type": "child<i2c-bus>"},
{"name": "channel[0]", "type": "child<i2c-bus>"},
{"name": "channel[1]", "type": "child<i2c-bus>"},
{"name": "channel[2]", "type": "child<i2c-bus>"}
]}
It seems to be naming them via the order they're created.
Is this not behaving how you expect?
>
> -- >8 --
> diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> index f9ce633b3a..a9517b612a 100644
> --- a/hw/i2c/i2c_mux_pca954x.c
> +++ b/hw/i2c/i2c_mux_pca954x.c
> @@ -189,9 +189,11 @@ static void pca954x_init(Object *obj)
>
> /* SMBus modules. Cannot fail. */
> for (i = 0; i < c->nchans; i++) {
> + g_autofree gchar *bus_name = g_strdup_printf("i2c.%d", i);
> +
> /* start all channels as disabled. */
> s->enabled[i] = false;
> - s->bus[i] = i2c_init_bus(DEVICE(s), "channel[*]");
> + s->bus[i] = i2c_init_bus(DEVICE(s), bus_name);
> }
> }
>
> ---
>
> (look at HMP 'info qtree' output).
>
> > }
> > }
>
> With the change:
> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> Tested-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>
