Philippe Mathieu-Daudé <f4...@amsat.org> writes: > On 6/26/20 7:49 AM, Markus Armbruster wrote: >> Philippe Mathieu-Daudé <f4...@amsat.org> writes: >> >>> On 6/25/20 10:12 AM, Philippe Mathieu-Daudé wrote: >>>> On 6/25/20 8:37 AM, Markus Armbruster wrote: >>>>> Cédric Le Goater <c...@kaod.org> writes: >>>>> >>>>>> On 6/22/20 10:31 AM, Philippe Mathieu-Daudé wrote: >>>>>>> On 6/22/20 8:27 AM, Cédric Le Goater wrote: >>>>>>>> On 6/21/20 12:58 AM, Philippe Mathieu-Daudé wrote: >>>>>>>>> Add a description field to distinguish between multiple devices. >>>>> >>>>> Pardon my lack of imagination: how does this help you with debugging? >>>> >>>> Ah, the patch subject is indeed incorrect, this should be: >>>> "... for *tracing* purpose" (I use tracing when debugging). >>>> >>>> In the next patch, we use the 'description' property: >>>> >>>> +# pca9552.c >>>> +pca9552_gpio_status(const char *description, const char *buf) "%s GPIOs >>>> 0-15 [%s]" >>>> >>>> So when the machine has multiple PCA9552 and guest accesses both, >>>> we can distinct which one is used. For me having "pca1" / "pca0" >>>> is easier to follow than the address of the QOM object. >>>> >>>>> >>>>>>>> Reviewed-by: Cédric Le Goater <c...@kaod.org> >>>>>>>> >>>>>>>> Could it be a QOM attribute ? >>>>>>> >>>>>>> What do you call a 'QOM attribute'? >>>>>>> Is it what qdev properties implement? >>>>>>> (in this case via DEFINE_PROP_STRING). >>>>>> >>>>>> I meant a default Object property, which would apply to all Objects. >>>>> >>>>> Good point. Many devices have multiple component objects of the same >>>>> type. >>>>> >>>>>> What you did is fine, so : >>>>>> >>>>>> Reviewed-by: Cédric Le Goater <c...@kaod.org> >>>>>> >>>>>> but, may be, a well defined child name is enough for the purpose. >>>>> >>>>> object_get_canonical_path() returns a distinct path for each (component) >>>>> object. The path components are the child property names. >>>>> >>>>> Properties can have descriptions: object_property_set_description(). >>>> >>>> TIL object_property_set_description :> >>>> >>>> Ah, there is no equivalent object_property_get_description(), >>>> we have to use object_get_canonical_path(). Hmm, not obvious. >>>> >>>>> >>>>> Sufficient? >>>> >>>> I don't know... This seems a complex way to do something simple... >>>> This is already a QDEV. Having to use QOM API seems going >>>> backward, since we have the DEFINE_PROP_STRING() macros available >>>> in "hw/qdev-properties.h". >>>> >>>> Maybe I'm not seeing the advantages clearly. I'll try later. >>> >>> The canonical path is not very helpful in trace log... >> >> Why? >> >> Okay, I checked the code. Since the devices in question don't get a >> composition tree parent assigned, realize puts them in the >> /machine/unattached orphanage. The canonical path is something like >> "/machine/unattached/device[6]", which is less than clear.
This is common for onboard devices. I hate it. Some boards do better than others. For instance, ast2600-evb has 33 sensibly named entries under /machine/soc/, and only 9 entries in the /machine/unattached/ orphanage. i440fx has two sensible named ones under /machine/, and 26 in the orphanage. >> The components of the canonical path are the names of the QOM child >> properties. object_get_canonical_path_component() returns the last one, >> in this case "device[6]". >> >> If we made the devices QOM children of some other device, we could name >> the child properties "pca0" and "pca1". >> object_get_canonical_path_component() would then return the strings you >> want to see. >> >> We make a device a QOM child of some QOM parent device only if the child >> is a component device of the parent (hence the name "composition >> tree"). >> >> Are these devices integral components of something else, or are they >> separate chips? > > Separate chips in the machine (actually not even on the machine mother > board where is the CPU, but on a daughter board card). > > So in the composition tree I expect to see them as > > /machine/pca0 > /machine/pca1 As long as the final canonical path component is something readable rather than auto-generated crap like "device[6]", object_get_canonical_path_component() is usable for tracing. >>> The description I set matches the hardware definitions >>> and is easier to follow, see patch #6 (where it is set) >>> where the description comes from: >>> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg714658.html >>> >>> Description name taken from: >>> https://github.com/open-power/witherspoon-xml/blob/master/witherspoon.xml >>> >>> So in this particular case I don't find the canonical pathname >>> practical (from an hardware debugging perspective). >> >> Personally, I'd be content with i2c bus and address for debugging >> purposes. >> >> The i2c buses *are* components: canonical paths look like >> "/machine/soc/i2c/aspeed.i2c.3". The combination of >> object_get_canonical_path_component(dev) and >> object_property_get_uint(dev, "address", &error_abort) identifies any >> i2c device on this machine, not just the two you decorate with a >> description string. > > The I2C busses is provided by Aspeed peripherals. I counted 19 different > I2C busses on this machine. > > "pca0" is connected to i2c bus #11, "pca1" to bus #3. > > I still don't think this will be practical, but I'll try your > suggestion. "bus=11 addr=0x60" isn't exactly wonderful, but it seems practical enough for me. Even "device[6]" seems workable, just bothersome, because it's needlessly hard to remember, and prone to change. Mind, I'm not recommending any particular solution for "want a more useful device ID in traces", I'm just throwing out ideas on how to solve the problem in a more general way. Working towards a a neater QOM composition tree might help with more than tracing.
