standardizing i2c device ids

standardizing i2c device ids

[Patrick Leis]

[-- Attachment #1: Type: text/plain, Size: 527 bytes --]

Corey and Peter,

My team builds lots of configurations for Qemu boards, and one pain point
has been that the qom path for a device depends on the device insertion
order, child[0], child[1] and the like.  I noticed that the qdev paths for
devices also exist by their device id property.  By default, this ends up
being the device type name.  I was wondering if it made sense to override
this with the device type plus the smbus address?  I did something similar
with the i2c mux device, to resolve part of this issue.

Patrick

[-- Attachment #2: Type: text/html, Size: 602 bytes --]

Re: standardizing i2c device ids

[Philippe Mathieu-Daudé]

Hi Patrick,

On 21/6/24 00:03, Patrick Leis wrote:
> Corey and Peter,
> 
> My team builds lots of configurations for Qemu boards, and one pain 
> point has been that the qom path for a device depends on the device 
> insertion order, child[0], child[1] and the like.  I noticed that the 
> qdev paths for devices also exist by their device id property.  By 
> default, this ends up being the device type name.  I was wondering if it 
> made sense to override this with the device type plus the smbus 
> address?  I did something similar with the i2c mux device, to resolve 
> part of this issue.

Including Markus since we discussed this with him last year, but
I don't remember correctly what was agreed / decided :S

Regards,

Phil.

Re: standardizing i2c device ids

[Patrick Leis]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

On Thu, Jun 20, 2024 at 11:26 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Hi Patrick,
>
> On 21/6/24 00:03, Patrick Leis wrote:
> > Corey and Peter,
> >
> > My team builds lots of configurations for Qemu boards, and one pain
> > point has been that the qom path for a device depends on the device
> > insertion order, child[0], child[1] and the like.  I noticed that the
> > qdev paths for devices also exist by their device id property.  By
> > default, this ends up being the device type name.  I was wondering if it
> > made sense to override this with the device type plus the smbus
> > address?  I did something similar with the i2c mux device, to resolve
> > part of this issue.
>
> Including Markus since we discussed this with him last year, but
> I don't remember correctly what was agreed / decided :S
>

Thanks :)

I'd really like to be able to access devices with paths that specify the
device I want specifically :)


>
> Regards,
>
> Phil.
>

[-- Attachment #2: Type: text/html, Size: 1579 bytes --]

Re: standardizing i2c device ids

[Markus Armbruster]

Patrick Leis <venture@google.com> writes:

> Corey and Peter,
>
> My team builds lots of configurations for Qemu boards, and one pain point
> has been that the qom path for a device depends on the device insertion
> order, child[0], child[1] and the like.

Yes.

Discussed in my "Dynamic & heterogeneous machines, initial
configuration: problems" memo, under "Problem 4: The
/machine/unattached/ orphanage".

Copy of the "Problem 4" part appended for your your convenience.  Full
memo archived at
https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/

>                                          I noticed that the qdev paths for
> devices also exist by their device id property.  By default, this ends up
> being the device type name.

Which kind of devices?

There are onboard devices and user-created devices.

A user-created device's QOM path is "/machine/peripheral/ID" when it was
created with a qdev ID, and "/machine/peripheral-anon/device[N]" (where
N counts up from zero) when it was created without a qdev ID.  N depends
on creation order, which is under the user's control.  Users can and
should avoid relying on their order by supplying an ID.

An onboard device's QOM path is chosen by board code.  For instance, q35
puts the mch device at "/machine/q35/mch".  However, if the board code
neglects to put the device anywhere, the system puts it at
"/machine/unattached/device[N]" (where N counts up from zero).  N
depends on creation order.

N can change at the drop of a hat.  Whether "device[N]" is a stable
interface is unclear.  It would clearly be a bad one.

If (part of) your problem is "/machine/peripheral-anon/device[N]",
supply IDs to bypass it.

If (part of) your problem is "/machine/unattached/device[N]", all I can
offer is the proper solution: fix the board code to put the device in
its proper place instead of abandoning it to the "/machine/unattached/"
orphanage.

>                              I was wondering if it made sense to override
> this with the device type plus the smbus address?  I did something similar
> with the i2c mux device, to resolve part of this issue.

I doubt it.

Questions?



= Problem 4: The /machine/unattached/ orphanage =

Is it okay for a QOM object to have no parent?

An object without a parent is not part of the composition tree; it has
no canonical path, and object_get_canonical_path() returns null.

Such objects can behave in wonky ways.  For instance,
object_property_set_link() treats a target object without a parent as
null.  If a linked object somehow loses its parent,
object_property_get_link() will return null even though the underlying C
pointer still points to the poor orphan.

This strongly suggests QOM was designed with the assumption that objects
always have a parent, except during initialization (before they are
connected to anything) and finalization (when no longer connected to
anything).  object_property_try_add_child()'s contract seems to confirm
this:

 * Child properties form the composition tree.  All objects need to be a child
 * of another object.  Objects can only be a child of one object.

Some functions to create objects take the new object's parent as a
parameter.  Example: object_new_with_props(), object_new_with_propv(),
clock_new(), ...

Others set a fixed parent.  For instance, we always add character
backends to "/chardevs/", objects created with object-add in
"/objects/", devices created with device_add in "/machine/peripheral/"
(with ID) or "/machine/peripheral-anon/" (without ID), ...

There are also functions that don't set a parent: object_new(),
object_new_with_class(), qdev_new(), qdev_try_new(), ...  Setting a
parent is the callers job then.  Invites misuse.  I'm aware of one
instance: @current_migration remains without a parent forever.

Not all callers care to set a parent themselves.  Instead, they rely on
the "/machine/unattached/" orphanage:

* qdev_connect_gpio_out_named() needs the input pin to have a parent.
  If it lacks one, it gets added to "/machine/unattached/" with a
  made-up name.

* device_set_realized() ensures realized devices have a parent by adding
  devices lacking one to "/machine/unattached/" with a made-up name.

* portio_list_add() adds a memory region.  If the caller doesn't specify
  the parent, "/machine/unattached/" is assumed.

* memory_region_init() adds a memory region, and may set the parent.  If
  the caller requests setting a parent without specifying one,
  "/machine/unattached/" is assumed.

* qemu_create_machine() adds the main system bus to
  "/machine/unattached/".

Except for the last one, the child names depend on execution order.  For
instance, device_set_realized() uses "device[N]", where N counts up from
zero.

These brittle, made-up names are visible in QMP QOM introspection.
Whether that's a stable interface is unclear.  Better not.

We don't rely on these names in C.  We follow pointers instead.

When we replace C code by configuration, we switch from pointers to
names.  Brittle names become a problem.

Re: standardizing i2c device ids

[Patrick Leis]

[-- Attachment #1: Type: text/plain, Size: 5516 bytes --]

On Sat, Jun 22, 2024 at 1:43 AM Markus Armbruster <armbru@redhat.com> wrote:

> Patrick Leis <venture@google.com> writes:
>
> > Corey and Peter,
> >
> > My team builds lots of configurations for Qemu boards, and one pain point
> > has been that the qom path for a device depends on the device insertion
> > order, child[0], child[1] and the like.
>
> Yes.
>
> Discussed in my "Dynamic & heterogeneous machines, initial
> configuration: problems" memo, under "Problem 4: The
> /machine/unattached/ orphanage".
>
> Copy of the "Problem 4" part appended for your your convenience.  Full
> memo archived at
> https://lore.kernel.org/qemu-devel/87o7d1i7ky.fsf@pond.sub.org/
>
> >                                          I noticed that the qdev paths
> for
> > devices also exist by their device id property.  By default, this ends up
> > being the device type name.
>
> Which kind of devices?
>
> There are onboard devices and user-created devices.
>
> A user-created device's QOM path is "/machine/peripheral/ID" when it was
> created with a qdev ID, and "/machine/peripheral-anon/device[N]" (where
> N counts up from zero) when it was created without a qdev ID.  N depends
> on creation order, which is under the user's control.  Users can and
> should avoid relying on their order by supplying an ID.
>
> An onboard device's QOM path is chosen by board code.  For instance, q35
> puts the mch device at "/machine/q35/mch".  However, if the board code
> neglects to put the device anywhere, the system puts it at
> "/machine/unattached/device[N]" (where N counts up from zero).  N
> depends on creation order.
>
> N can change at the drop of a hat.  Whether "device[N]" is a stable
> interface is unclear.  It would clearly be a bad one.
>
> If (part of) your problem is "/machine/peripheral-anon/device[N]",
> supply IDs to bypass it.
>
> If (part of) your problem is "/machine/unattached/device[N]", all I can
> offer is the proper solution: fix the board code to put the device in
> its proper place instead of abandoning it to the "/machine/unattached/"
> orphanage.
>
> >                              I was wondering if it made sense to override
> > this with the device type plus the smbus address?  I did something
> similar
> > with the i2c mux device, to resolve part of this issue.
>
> I doubt it.
>
> Questions?
>
>
>
> = Problem 4: The /machine/unattached/ orphanage =
>
> Is it okay for a QOM object to have no parent?
>
> An object without a parent is not part of the composition tree; it has
> no canonical path, and object_get_canonical_path() returns null.
>
> Such objects can behave in wonky ways.  For instance,
> object_property_set_link() treats a target object without a parent as
> null.  If a linked object somehow loses its parent,
> object_property_get_link() will return null even though the underlying C
> pointer still points to the poor orphan.
>
> This strongly suggests QOM was designed with the assumption that objects
> always have a parent, except during initialization (before they are
> connected to anything) and finalization (when no longer connected to
> anything).  object_property_try_add_child()'s contract seems to confirm
> this:
>
>  * Child properties form the composition tree.  All objects need to be a
> child
>  * of another object.  Objects can only be a child of one object.
>
> Some functions to create objects take the new object's parent as a
> parameter.  Example: object_new_with_props(), object_new_with_propv(),
> clock_new(), ...
>
> Others set a fixed parent.  For instance, we always add character
> backends to "/chardevs/", objects created with object-add in
> "/objects/", devices created with device_add in "/machine/peripheral/"
> (with ID) or "/machine/peripheral-anon/" (without ID), ...
>
> There are also functions that don't set a parent: object_new(),
> object_new_with_class(), qdev_new(), qdev_try_new(), ...  Setting a
> parent is the callers job then.  Invites misuse.  I'm aware of one
> instance: @current_migration remains without a parent forever.
>
> Not all callers care to set a parent themselves.  Instead, they rely on
> the "/machine/unattached/" orphanage:
>
> * qdev_connect_gpio_out_named() needs the input pin to have a parent.
>   If it lacks one, it gets added to "/machine/unattached/" with a
>   made-up name.
>
> * device_set_realized() ensures realized devices have a parent by adding
>   devices lacking one to "/machine/unattached/" with a made-up name.
>
> * portio_list_add() adds a memory region.  If the caller doesn't specify
>   the parent, "/machine/unattached/" is assumed.
>
> * memory_region_init() adds a memory region, and may set the parent.  If
>   the caller requests setting a parent without specifying one,
>   "/machine/unattached/" is assumed.
>
> * qemu_create_machine() adds the main system bus to
>   "/machine/unattached/".
>
> Except for the last one, the child names depend on execution order.  For
> instance, device_set_realized() uses "device[N]", where N counts up from
> zero.
>
> These brittle, made-up names are visible in QMP QOM introspection.
> Whether that's a stable interface is unclear.  Better not.
>
> We don't rely on these names in C.  We follow pointers instead.
>
> When we replace C code by configuration, we switch from pointers to
> names.  Brittle names become a problem.
>

Thank you for the explanation and related documentation.

[-- Attachment #2: Type: text/html, Size: 6669 bytes --]