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é <f4...@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.hender...@linaro.org> Thanks! > > > r~ >