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

["Philippe Mathieu-Daudé via" <qemu-devel@nongnu.org>]

On 1/2/22 21:54, Patrick Venture wrote:
> 
> 
> On Tue, Feb 1, 2022 at 11:02 AM Philippe Mathieu-Daudé <f4bug@amsat.org 
> <mailto:f4bug@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 <wuhaotsh@google.com
>     <mailto:wuhaotsh@google.com>>
>      > Signed-off-by: Patrick Venture <venture@google.com
>     <mailto:venture@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?

On the monitor:

(qemu) info qtree
bus: main-system-bus
   type System
   ...
   dev: npcm7xx-smbus, id ""
     gpio-out "sysbus-irq" 1
     mmio 00000000f008d000/0000000000001000
     bus: i2c-bus
       type i2c-bus
       dev: pca9548, id ""
         address = 119 (0x77)
         bus: channel[*]
           type i2c-bus
         bus: channel[*]
           type i2c-bus
         bus: channel[*]
           type i2c-bus
           dev: tmp105, id ""
             gpio-out "" 1
             address = 73 (0x49)
         bus: channel[*]
           type i2c-bus
           dev: tmp105, id ""
             gpio-out "" 1
             address = 72 (0x48)
         bus: channel[*]
           type i2c-bus
           dev: tmp105, id ""
             gpio-out "" 1
             address = 73 (0x49)
         bus: channel[*]
           type i2c-bus
           dev: tmp105, id ""
             gpio-out "" 1
             address = 72 (0x48)
         bus: channel[*]
           type i2c-bus
         bus: channel[*]
           type i2c-bus

>     -- >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 this snippet:

(qemu) info qtree
bus: main-system-bus
   type System
   ...
   dev: npcm7xx-smbus, id ""
     gpio-out "sysbus-irq" 1
     mmio 00000000f008d000/0000000000001000
     bus: i2c-bus
       type i2c-bus
       dev: pca9548, id ""
         address = 119 (0x77)
         bus: i2c.7
           type i2c-bus
         bus: i2c.6
           type i2c-bus
         bus: i2c.5
           type i2c-bus
           dev: tmp105, id ""
             gpio-out "" 1
             address = 73 (0x49)
         bus: i2c.4
           type i2c-bus
           dev: tmp105, id ""
             gpio-out "" 1
             address = 72 (0x48)
         bus: i2c.3
           type i2c-bus
           dev: tmp105, id ""
             gpio-out "" 1
             address = 73 (0x49)
         bus: i2c.2
           type i2c-bus
           dev: tmp105, id ""
             gpio-out "" 1
             address = 72 (0x48)
         bus: i2c.1
           type i2c-bus
         bus: i2c.0
           type i2c-bus

Regards,

Phil.