qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v5 3/9] qdev-monitor: print the device's clock w


From: Damien Hedde
Subject: Re: [Qemu-devel] [PATCH v5 3/9] qdev-monitor: print the device's clock with info qtree
Date: Fri, 12 Oct 2018 12:20:17 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0

Hi Philippe,

On 10/3/18 12:42 AM, Philippe Mathieu-Daudé wrote:
> Hi Damien,
> 
> On 10/2/18 4:24 PM, Damien Hedde wrote:
>> This prints the clocks attached to a DeviceState when using "info qtree" 
>> monitor
>> command. For every clock, it displays the direction, the name and if the
>> clock is forwarded. For input clock, it displays also the frequency.
> 
> What would also be really useful (during development mostly)
> is a "info clktree" monitor command.

What would be the output ? A list of all clock related devices and their
clocks ? Or something more like a clock tree starting from the source(s)
clock up to the last ones ? (or the opposite starting from a
clocked-device down to the sources)

Concerning development, it might also be useful to have access to the
frequency of a clock output. Maybe I should add the value in the object
for that purpose ?

> 
>>
>> This is based on the original work of Frederic Konrad.
>>
>> Signed-off-by: Damien Hedde <address@hidden>
>> ---
>>  qdev-monitor.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index 61e0300991..8c39a3a65b 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -682,6 +682,7 @@ static void qdev_print(Monitor *mon, DeviceState *dev, 
>> int indent)
>>      ObjectClass *class;
>>      BusState *child;
>>      NamedGPIOList *ngl;
>> +    NamedClockList *clk;
>>  
>>      qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
>>                  dev->id ? dev->id : "");
>> @@ -696,6 +697,17 @@ static void qdev_print(Monitor *mon, DeviceState *dev, 
>> int indent)
>>                          ngl->num_out);
>>          }
>>      }
>> +    QLIST_FOREACH(clk, &dev->clocks, node) {
>> +        if (clk->out) {
>> +            qdev_printf("clock-out%s \"%s\"\n",
>> +                        clk->forward ? " (fw)" : "",
>> +                        clk->name);
>> +        } else {
>> +            qdev_printf("clock-in%s \"%s\" freq=%" PRIu64 "Hz\n",
> 
> IMHO 'freq_hz=%" PRIu64 "\n"' is easier to read.

The frequency value in hertz is not easy to read in general (eg:
33333333Hz). Maybe we should print it with an apropriate unit (eg:
33.333333MHz). We can also use the "'" printf flag for thousand
separator but it's locale-dependent (and in my case it prints nothing).

> 
> However if we plan to add/use a qemu_strtohz() similar to
> qemu_strtosz_metric(), that would be fine.

You mean for test purpose ?

> 
>> +                        clk->forward ? " (fw)" : "",
>> +                        clk->name, clock_get_frequency(clk->in));
>> +        }
>> +    }
>>      class = object_get_class(OBJECT(dev));
>>      do {
>>          qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
>>
> 
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> Tested-by: Philippe Mathieu-Daudé <address@hidden>
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]