| Message ID | 20200620230719.32139-4-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/misc: Add LED device | expand |
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? Otherwise, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
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~ >
On 6/21/20 3:25 PM, Philippe Mathieu-Daudé wrote: > Anyway I switched to a percent value. What is better to hold > it, an 'unsigned' or 'uint8_t' type? Might as well use unsigned; you'll not be saving space with uint8_t. r~
On 6/22/20 6:48 PM, Richard Henderson wrote: > On 6/21/20 3:25 PM, Philippe Mathieu-Daudé wrote: >> Anyway I switched to a percent value. What is better to hold >> it, an 'unsigned' or 'uint8_t' type? > > Might as well use unsigned; you'll not be saving space with uint8_t. Bah if we want to migrate the intensity, we can't use 'unsigned', so I'll keep uint8_t / VMSTATE_UINT8.
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; /* Properties */ diff --git a/hw/misc/led.c b/hw/misc/led.c index 8503dde777..37d9f1f3d2 100644 --- a/hw/misc/led.c +++ b/hw/misc/led.c @@ -32,6 +32,11 @@ void led_set_intensity(LEDState *s, uint16_t new_intensity) { trace_led_set_intensity(s->description ? s->description : "n/a", s->color, new_intensity); + if (new_intensity != s->current_intensity) { + trace_led_change_intensity(s->description ? s->description : "n/a", + s->color, + s->current_intensity, new_intensity); + } s->current_intensity = new_intensity; } diff --git a/hw/misc/trace-events b/hw/misc/trace-events index f58853d367..57d39bf9b9 100644 --- a/hw/misc/trace-events +++ b/hw/misc/trace-events @@ -209,6 +209,7 @@ grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx6 # led.c led_set_intensity(const char *color, const char *desc, uint16_t intensity) "LED desc:'%s' color:%s intensity: 0x%04"PRIx16 +led_change_intensity(const char *color, const char *desc, uint16_t old_intensity, uint16_t new_intensity) "LED desc:'%s' color:%s intensity 0x%04"PRIx16" -> 0x%04"PRIx16"" # pca9552.c pca9552_gpio_status(const char *description, const char *buf) "%s GPIOs 0-15 [%s]"
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(+)