[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 1/8] hw/misc/led: Add a LED device
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v4 1/8] hw/misc/led: Add a LED device |
Date: |
Mon, 7 Sep 2020 22:24:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
On 9/7/20 10:03 PM, Luc Michel wrote:
> Hi Philippe,
>
> On 9/7/20 6:32 PM, Philippe Mathieu-Daudé wrote:
>> Add a LED device which can be connected to a GPIO output.
>> They can also be dimmed with PWM devices. For now we do
>> not implement the dimmed mode, but in preparation of a
>> future implementation, we start using the LED intensity.
>>
>> LEDs are limited to a fixed set of colors.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> include/hw/misc/led.h | 84 +++++++++++++++++++++++++
>> hw/misc/led.c | 142 ++++++++++++++++++++++++++++++++++++++++++
>> MAINTAINERS | 6 ++
>> hw/misc/Kconfig | 3 +
>> hw/misc/meson.build | 1 +
>> hw/misc/trace-events | 3 +
>> 6 files changed, 239 insertions(+)
>> create mode 100644 include/hw/misc/led.h
>> create mode 100644 hw/misc/led.c
...
>> +/**
>> + * led_set_intensity: set the intensity of a LED device
>> + * @s: the LED object
>> + * @intensity: intensity as percentage in range 0 to 100.
> @intensity_percent
>
>> + */
>> +void led_set_intensity(LEDState *s, unsigned intensity_percent);
>> +
>> +/**
>> + * led_get_intensity:
>> + * @s: the LED object
>> + *
>> + * Returns: The LED intensity as percentage in range 0 to 100.
>> + */
>> +unsigned led_get_intensity(LEDState *s);
>> +
>> +/**
>> + * led_set_intensity: set the state of a LED device
> led_set_state
>
>> + * @s: the LED object
>> + * @is_on: boolean indicating whether the LED is emitting
> @is_emitting
>
>> + *
>> + * This utility is meant for LED connected to GPIO.
>> + */
>> +void led_set_state(LEDState *s, bool is_emitting);
>> +
>> +/**
>> + * led_create_simple: Create and realize a LED device
>> + * @parent: the parent object
> @parentobj
>
>> + * @color: color of the LED
>> + * @description: description of the LED (optional)
>> + *
>> + * Create the device state structure, initialize it, and
>> + * drop the reference to it (the device is realized).
>> + */
>> +LEDState *led_create_simple(Object *parentobj,
>> + LEDColor color,
>> + const char *description);
>> +
>> +#endif /* HW_MISC_LED_H */
>> diff --git a/hw/misc/led.c b/hw/misc/led.c
>> new file mode 100644
>> index 00000000000..f2140739b68
>> --- /dev/null
>> +++ b/hw/misc/led.c
>> @@ -0,0 +1,142 @@
>> +/*
>> + * QEMU single LED device
>> + *
>> + * Copyright (C) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "migration/vmstate.h"
>> +#include "hw/qdev-properties.h"
>> +#include "hw/misc/led.h"
>> +#include "trace.h"
>> +
>> +#define LED_INTENSITY_PERCENT_MAX 100
>> +
>> +static const char *led_color_name[] = {
>> + [LED_COLOR_VIOLET] = "violet",
>> + [LED_COLOR_BLUE] = "blue",
>> + [LED_COLOR_CYAN] = "cyan",
>> + [LED_COLOR_GREEN] = "green",
>> + [LED_COLOR_AMBER] = "amber",
>> + [LED_COLOR_ORANGE] = "orange",
>> + [LED_COLOR_RED] = "red",
>> +};
>> +
>> +static bool led_color_name_is_valid(const char *color_name)
>> +{
>> + for (size_t i = 0; i < ARRAY_SIZE(led_color_name); i++) {
>> + if (led_color_name[i] && strcmp(color_name,
>> led_color_name[i]) == 0) {
>
> Why are you checking led_color_name[i] here?
It could happen in v3 but not now, thanks for catching this :)
I'll address your comment and respin after few days.
>
> Otherwise, seems good to me.
>
> Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Thanks!
- [PATCH v4 0/8] hw/misc: Add LED device, Philippe Mathieu-Daudé, 2020/09/07
- [PATCH v4 1/8] hw/misc/led: Add a LED device, Philippe Mathieu-Daudé, 2020/09/07
- [PATCH v4 2/8] hw/misc/led: Allow connecting from GPIO output, Philippe Mathieu-Daudé, 2020/09/07
- [PATCH v4 3/8] hw/misc/led: Emit a trace event when LED intensity has changed, Philippe Mathieu-Daudé, 2020/09/07
- [PATCH v4 4/8] hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1, Philippe Mathieu-Daudé, 2020/09/07
- [PATCH v4 5/8] hw/misc/mps2-fpgaio: Use the LED device, Philippe Mathieu-Daudé, 2020/09/07
- [PATCH v4 6/8] hw/misc/mps2-scc: Use the LED device, Philippe Mathieu-Daudé, 2020/09/07
- [PATCH v4 7/8] hw/arm/tosa: Replace fprintf() calls by LED devices, Philippe Mathieu-Daudé, 2020/09/07
- [RFC PATCH v4 8/8] hw/arm/tosa: Make TYPE_TOSA_MISC_GPIO a plain QDev, Philippe Mathieu-Daudé, 2020/09/07