[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for deb
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose |
Date: |
Thu, 25 Jun 2020 16:23:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
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 <clg@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 <clg@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 <clg@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.
>
- [PATCH v4 3/8] hw/misc/pca9552: Use the PCA9552_PIN_COUNT definition, (continued)
- [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Philippe Mathieu-Daudé, 2020/06/20
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Cédric Le Goater, 2020/06/22
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Philippe Mathieu-Daudé, 2020/06/22
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Cédric Le Goater, 2020/06/22
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Markus Armbruster, 2020/06/25
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Philippe Mathieu-Daudé, 2020/06/25
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose,
Philippe Mathieu-Daudé <=
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Markus Armbruster, 2020/06/26
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Philippe Mathieu-Daudé, 2020/06/26
- Re: [PATCH v4 4/8] hw/misc/pca9552: Add a 'description' property for debugging purpose, Markus Armbruster, 2020/06/27
[PATCH v4 5/8] hw/misc/pca9552: Trace GPIO High/Low events, Philippe Mathieu-Daudé, 2020/06/20
[PATCH v4 6/8] hw/arm/aspeed: Describe each PCA9552 device, Philippe Mathieu-Daudé, 2020/06/20