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. > > 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 >> 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. Regards, Phil.
