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... 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). > > Thanks for your review, > > Phil. >
