Re: [PATCH] hw/i2c: flatten pca954x mux device

[Philippe Mathieu-Daudé]

On 2/2/22 17:40, Patrick Venture wrote:

>     Philippe,
>
>     I0202 08:29:45.380384  6641 stream.go:31] qemu: child buses at
>     "pca9546": "channel[*]", "channel[*]", "channel[*]", "channel[*]"
>
>     Ok, so that's interesting.  In one system (using qom-list) it's
>     correct, but then when using it to do path assignment
>     (qdev-monitor), it fails...
>
>     I'm not as fond of the name i2c-bus.%d, since they're referred to as
>     channels in the datasheet.  If I do the manual name creation, can I
>     keep the name channel or should I pivot over?
>
>     Thanks
>
>
>             -- >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é <f4bug@amsat.org
>             <mailto:f4bug@amsat.org>>
>             Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org
>             <mailto:f4bug@amsat.org>>
>
>
> Just saw your reply, and found a bunch of other non-spam in my spam 
> folder.  I sent the message to the anti-spam team, hopefully that'll 
> resolve this for myself and presumably others.

Thanks. I suppose the problem is the amsat.org domain.

> I definitely see the same result with the qdev-monitor, but was really 
> surprised that the qom-list worked.  I'll explicitly set the name, and 
> i2c.%d is fine.  The detail that they're channels is not really 
> important to the end user presumably.

I agree it is better to follow datasheets, thus I am fine if you
change and use channel. How would it look like? "channel.0"?
FYI qdev busses are described in docs/qdev-device-use.txt.

We should be able to plug a device using some command line
such "-device i2c_test_dev,bus=channel.0,addr=0x55".
I wonder how to select the base PCA9548 ...

Maybe we need to pass the PCA ID to pca954x_init(), so we can
name "channel.2.0" for the 1st channel on the 2nd PCA?

Regards,

Phil.