On Wed, Feb 2, 2022 at 8:34 AM Patrick Venture <vent...@google.com> wrote:
>
>
> On Tue, Feb 1, 2022 at 12:54 PM Patrick Venture <vent...@google.com>
> wrote:
>
>>
>>
>> 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?
>>
>
> 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é <f4...@amsat.org>
>>> Tested-by: Philippe Mathieu-Daudé <f4...@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.
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'll have v2 out shortly.
thanks,
Patrick
