[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 3/7] hw/misc/led: Emit a trace event when LED intensity ha
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v3 3/7] hw/misc/led: Emit a trace event when LED intensity has changed |
Date: |
Mon, 22 Jun 2020 00:25:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 6/21/20 9:23 PM, Richard Henderson wrote:
> On 6/20/20 4:07 PM, Philippe Mathieu-Daudé wrote:
>> Track the LED intensity, and emit a trace event when it changes.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> include/hw/misc/led.h | 1 +
>> hw/misc/led.c | 5 +++++
>> hw/misc/trace-events | 1 +
>> 3 files changed, 7 insertions(+)
>>
>> diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h
>> index 883006bb8f..df5b32a2db 100644
>> --- a/include/hw/misc/led.h
>> +++ b/include/hw/misc/led.h
>> @@ -35,6 +35,7 @@ typedef struct LEDState {
>> DeviceState parent_obj;
>> /* Public */
>>
>> + uint16_t current_intensity;
>> qemu_irq irq;
>
> Why not sort this new field next to the other uint16_t and (partially) fill
> the
> hole?
>From a reviewer point of view, I prefer to keep the state fields
separated from the properties fields, wasting few bits of RAM.
Anyway I switched to a percent value. What is better to hold
it, an 'unsigned' or 'uint8_t' type?
>
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Thanks!
>
>
> r~
>
[PATCH v3 4/7] hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1, Philippe Mathieu-Daudé, 2020/06/20
[PATCH v3 5/7] hw/misc/mps2-fpgaio: Use the LED device, Philippe Mathieu-Daudé, 2020/06/20
[PATCH v3 6/7] hw/misc/mps2-scc: Use the LED device, Philippe Mathieu-Daudé, 2020/06/20