[PATCH v6 0/7] hw/misc: Add LED device

[PATCH v6 0/7] hw/misc: Add LED device

[Philippe Mathieu-Daudé]

Hello,

These patches are part of the GSoC unselected 'QEMU visualizer'
project.

This series introduce a LED device that can be easily connected
to a GPIO output.

Missing review:
- Patch #2: "hw/misc/led: Allow connecting from GPIO output"

Since v5:
- Rebased on "QOM boilerplate cleanup" (use DECLARE_INSTANCE_CHECKER)
- Addressed Luc and Richard comments
- Do not use confusing "polarity-inverted" property
- Reset LEDs in MPS2 devices
- Document GpioPolarity

Since v4:
- Fixed typos (Luc)
- Removed TYPE_TOSA_MISC_GPIO qdev conversion patch (Peter)

Since v3:
- Rebased (TYPE_TOSA_MISC_GPIO)
- Rebased (Meson)
- Addressed Richard's review comments
- Improved doc/comments

Since v2:
- Rebased on PCA9552
- Model intensity to be ready for PWM use (Dave)
- Remove QMP events until we get a UI visualizer (Peter)
- Remove microbit patch (Peter)

Since v1: addressed Eric Blake review comments
- Added QMP rate limit

Next steps planned:

- PoC visualizer...
- look at using a dbus backend (elmarco)
- look at LED array/matrix such 7segments.

Regards,

Phil.

$ git backport-diff -r v6.. -u v5
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respective=
ly

001/7:[0013] [FC] 'hw/misc/led: Add a LED device'
002/7:[0026] [FC] 'hw/misc/led: Allow connecting from GPIO output'
003/7:[----] [--] 'hw/misc/led: Emit a trace event when LED intensity has cha=
nged'
004/7:[----] [--] 'hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 =
#1'
005/7:[0004] [FC] 'hw/misc/mps2-fpgaio: Use the LED device'
006/7:[0004] [FC] 'hw/misc/mps2-scc: Use the LED device'
007/7:[----] [-C] 'hw/arm/tosa: Replace fprintf() calls by LED devices'

Philippe Mathieu-Daud=C3=A9 (7):
  hw/misc/led: Add a LED device
  hw/misc/led: Allow connecting from GPIO output
  hw/misc/led: Emit a trace event when LED intensity has changed
  hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1
  hw/misc/mps2-fpgaio: Use the LED device
  hw/misc/mps2-scc: Use the LED device
  hw/arm/tosa: Replace fprintf() calls by LED devices

 include/hw/misc/led.h         |  97 ++++++++++++++++++++
 include/hw/misc/mps2-fpgaio.h |   2 +
 include/hw/misc/mps2-scc.h    |   2 +
 include/hw/qdev-core.h        |  16 ++++
 hw/arm/aspeed.c               |  20 +++++
 hw/arm/tosa.c                 |  40 ++++-----
 hw/misc/led.c                 | 161 ++++++++++++++++++++++++++++++++++
 hw/misc/mps2-fpgaio.c         |  23 +++--
 hw/misc/mps2-scc.c            |  27 +++---
 MAINTAINERS                   |   6 ++
 hw/arm/Kconfig                |   2 +
 hw/misc/Kconfig               |   5 ++
 hw/misc/meson.build           |   1 +
 hw/misc/trace-events          |   6 +-
 14 files changed, 365 insertions(+), 43 deletions(-)
 create mode 100644 include/hw/misc/led.h
 create mode 100644 hw/misc/led.c

--=20
2.26.2

[PATCH v6 1/7] hw/misc/led: Add a LED device

[Philippe Mathieu-Daudé]

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.

Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/misc/led.h |  87 ++++++++++++++++++++++++++
 hw/misc/led.c         | 142 ++++++++++++++++++++++++++++++++++++++++++
 MAINTAINERS           |   6 ++
 hw/misc/Kconfig       |   3 +
 hw/misc/meson.build   |   1 +
 hw/misc/trace-events  |   3 +
 6 files changed, 242 insertions(+)
 create mode 100644 include/hw/misc/led.h
 create mode 100644 hw/misc/led.c

diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h
new file mode 100644
index 00000000000..286d37c75c1
--- /dev/null
+++ b/include/hw/misc/led.h
@@ -0,0 +1,87 @@
+/*
+ * QEMU single LED device
+ *
+ * Copyright (C) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef HW_MISC_LED_H
+#define HW_MISC_LED_H
+
+#include "qom/object.h"
+
+#define TYPE_LED "led"
+
+/**
+ * LEDColor: Color of a LED
+ *
+ * This set is restricted to physically available LED colors.
+ *
+ * LED colors from 'Table 1. Product performance of LUXEON Rebel Color
+ * Line' of the 'DS68 LUXEON Rebel Color Line' datasheet available at:
+ * https://www.lumileds.com/products/color-leds/luxeon-rebel-color/
+ */
+typedef enum {          /* Coarse wavelength range */
+    LED_COLOR_VIOLET,   /* 425 nm */
+    LED_COLOR_BLUE,     /* 475 nm */
+    LED_COLOR_CYAN,     /* 500 nm */
+    LED_COLOR_GREEN,    /* 535 nm */
+    LED_COLOR_AMBER,    /* 590 nm */
+    LED_COLOR_ORANGE,   /* 615 nm */
+    LED_COLOR_RED,      /* 630 nm */
+} LEDColor;
+
+struct LEDState {
+    /* Private */
+    DeviceState parent_obj;
+    /* Public */
+
+    uint8_t intensity_percent;
+
+    /* Properties */
+    char *description;
+    char *color;
+};
+typedef struct LEDState LEDState;
+DECLARE_INSTANCE_CHECKER(LEDState, LED, TYPE_LED)
+
+/**
+ * led_set_intensity: Set the intensity of a LED device
+ * @s: the LED object
+ * @intensity_percent: intensity as percentage in range 0 to 100.
+ */
+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_state: Set the state of a LED device
+ * @s: the LED object
+ * @is_emitting: boolean indicating whether the LED 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
+ * @parentobj: the parent object
+ * @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).
+ *
+ * Returns: The newly allocated and instantiated LED object.
+ */
+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..1e2f49c5710
--- /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 * const 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 (strcmp(color_name, led_color_name[i]) == 0) {
+            return true;
+        }
+    }
+    return false;
+}
+
+void led_set_intensity(LEDState *s, unsigned intensity_percent)
+{
+    if (intensity_percent > LED_INTENSITY_PERCENT_MAX) {
+        intensity_percent = LED_INTENSITY_PERCENT_MAX;
+    }
+    trace_led_set_intensity(s->description, s->color, intensity_percent);
+    s->intensity_percent = intensity_percent;
+}
+
+unsigned led_get_intensity(LEDState *s)
+{
+    return s->intensity_percent;
+}
+
+void led_set_state(LEDState *s, bool is_emitting)
+{
+    led_set_intensity(s, is_emitting ? LED_INTENSITY_PERCENT_MAX : 0);
+}
+
+static void led_reset(DeviceState *dev)
+{
+    LEDState *s = LED(dev);
+
+    led_set_state(s, false);
+}
+
+static const VMStateDescription vmstate_led = {
+    .name = TYPE_LED,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(intensity_percent, LEDState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void led_realize(DeviceState *dev, Error **errp)
+{
+    LEDState *s = LED(dev);
+
+    if (s->color == NULL) {
+        error_setg(errp, "property 'color' not specified");
+        return;
+    } else if (!led_color_name_is_valid(s->color)) {
+        error_setg(errp, "property 'color' invalid or not supported");
+        return;
+    }
+    if (s->description == NULL) {
+        s->description = g_strdup("n/a");
+    }
+}
+
+static Property led_properties[] = {
+    DEFINE_PROP_STRING("color", LEDState, color),
+    DEFINE_PROP_STRING("description", LEDState, description),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void led_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "LED";
+    dc->vmsd = &vmstate_led;
+    dc->reset = led_reset;
+    dc->realize = led_realize;
+    set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+    device_class_set_props(dc, led_properties);
+}
+
+static const TypeInfo led_info = {
+    .name = TYPE_LED,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(LEDState),
+    .class_init = led_class_init
+};
+
+static void led_register_types(void)
+{
+    type_register_static(&led_info);
+}
+
+type_init(led_register_types)
+
+LEDState *led_create_simple(Object *parentobj,
+                            LEDColor color,
+                            const char *description)
+{
+    g_autofree char *name = NULL;
+    DeviceState *dev;
+
+    dev = qdev_new(TYPE_LED);
+    qdev_prop_set_string(dev, "color", led_color_name[color]);
+    if (!description) {
+        static unsigned undescribed_led_id;
+        name = g_strdup_printf("undescribed-led-#%u", undescribed_led_id++);
+    } else {
+        qdev_prop_set_string(dev, "description", description);
+        name = g_ascii_strdown(description, -1);
+        name = g_strdelimit(name, " #", '-');
+    }
+    object_property_add_child(parentobj, name, OBJECT(dev));
+    qdev_realize_and_unref(dev, NULL, &error_fatal);
+
+    return LED(dev);
+}
diff --git a/MAINTAINERS b/MAINTAINERS
index cf96fa83795..8886e48b4e6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1951,6 +1951,12 @@ F: docs/specs/vmgenid.txt
 F: tests/qtest/vmgenid-test.c
 F: stubs/vmgenid.c
 
+LED
+M: Philippe Mathieu-Daudé <f4bug@amsat.org>
+S: Maintained
+F: include/hw/misc/led.h
+F: hw/misc/led.c
+
 Unimplemented device
 M: Peter Maydell <peter.maydell@linaro.org>
 R: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 92c397ca07a..5c151fa3a83 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -126,6 +126,9 @@ config AUX
 config UNIMP
     bool
 
+config LED
+    bool
+
 config MAC_VIA
     bool
     select MOS6522
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index e1576b81cf9..26f6dd037dc 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -11,6 +11,7 @@ softmmu_ss.add(when: 'CONFIG_TMP105', if_true: files('tmp105.c'))
 softmmu_ss.add(when: 'CONFIG_TMP421', if_true: files('tmp421.c'))
 softmmu_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c'))
 softmmu_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c'))
+softmmu_ss.add(when: 'CONFIG_LED', if_true: files('led.c'))
 
 # ARM devices
 softmmu_ss.add(when: 'CONFIG_PL310', if_true: files('arm_l2x0.c'))
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 56a622d1e97..211ff114694 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -215,6 +215,9 @@ via1_adb_poll(uint8_t data, const char *vadbint, int status, int index, int size
 grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read addr:0x%03"PRIx64" data:0x%08x"
 grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx64" data:0x%08x"
 
+# led.c
+led_set_intensity(const char *color, const char *desc, uint8_t intensity_percent) "LED desc:'%s' color:%s intensity: %u%%"
+
 # pca9552.c
 pca955x_gpio_status(const char *description, const char *buf) "%s GPIOs 0-15 [%s]"
 pca955x_gpio_change(const char *description, unsigned id, unsigned prev_state, unsigned current_state) "%s GPIO id:%u status: %u -> %u"
-- 
2.26.2

[PATCH v6 2/7] hw/misc/led: Allow connecting from GPIO output

[Philippe Mathieu-Daudé]

Some devices expose GPIO lines.

Add a GPIO qdev input to our LED device, so we can
connect a GPIO output using qdev_connect_gpio_out().

When used with GPIOs, the intensity can only be either
minium or maximum. This depends of the polarity of the
GPIO (which can be inverted).
Declare the GpioPolarity type to model the polarity.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/misc/led.h  | 10 ++++++++++
 include/hw/qdev-core.h | 16 ++++++++++++++++
 hw/misc/led.c          | 17 ++++++++++++++++-
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h
index 286d37c75c1..aa359b87c20 100644
--- a/include/hw/misc/led.h
+++ b/include/hw/misc/led.h
@@ -9,6 +9,7 @@
 #define HW_MISC_LED_H
 
 #include "qom/object.h"
+#include "hw/qdev-core.h"
 
 #define TYPE_LED "led"
 
@@ -37,10 +38,17 @@ struct LEDState {
     /* Public */
 
     uint8_t intensity_percent;
+    qemu_irq irq;
 
     /* Properties */
     char *description;
     char *color;
+    /*
+     * Determines whether a GPIO is using a positive (active-high)
+     * logic (when used with GPIO, the intensity at reset is related
+     * to the GPIO polarity).
+     */
+    bool gpio_active_high;
 };
 typedef struct LEDState LEDState;
 DECLARE_INSTANCE_CHECKER(LEDState, LED, TYPE_LED)
@@ -72,6 +80,7 @@ void led_set_state(LEDState *s, bool is_emitting);
 /**
  * led_create_simple: Create and realize a LED device
  * @parentobj: the parent object
+ * @gpio_polarity: GPIO polarity
  * @color: color of the LED
  * @description: description of the LED (optional)
  *
@@ -81,6 +90,7 @@ void led_set_state(LEDState *s, bool is_emitting);
  * Returns: The newly allocated and instantiated LED object.
  */
 LEDState *led_create_simple(Object *parentobj,
+                            GpioPolarity gpio_polarity,
                             LEDColor color,
                             const char *description);
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e025ba9653f..ec1f1efc37b 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -423,6 +423,22 @@ void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
 void qdev_machine_creation_done(void);
 bool qdev_machine_modified(void);
 
+/**
+ * GpioPolarity: Polarity of a GPIO line
+ *
+ * GPIO lines use either positive (active-high) logic,
+ * or negative (active-low) logic.
+ *
+ * In active-high logic (%GPIO_POLARITY_ACTIVE_HIGH), a pin is
+ * active when the voltage on the pin is high (relative to ground);
+ * whereas in active-low logic (%GPIO_POLARITY_ACTIVE_LOW), a pin
+ * is active when the voltage on the pin is low (or grounded).
+ */
+typedef enum {
+    GPIO_POLARITY_ACTIVE_LOW,
+    GPIO_POLARITY_ACTIVE_HIGH
+} GpioPolarity;
+
 /**
  * qdev_get_gpio_in: Get one of a device's anonymous input GPIO lines
  * @dev: Device whose GPIO we want
diff --git a/hw/misc/led.c b/hw/misc/led.c
index 1e2f49c5710..c5fa09a613a 100644
--- a/hw/misc/led.c
+++ b/hw/misc/led.c
@@ -10,6 +10,7 @@
 #include "migration/vmstate.h"
 #include "hw/qdev-properties.h"
 #include "hw/misc/led.h"
+#include "hw/irq.h"
 #include "trace.h"
 
 #define LED_INTENSITY_PERCENT_MAX   100
@@ -53,11 +54,19 @@ void led_set_state(LEDState *s, bool is_emitting)
     led_set_intensity(s, is_emitting ? LED_INTENSITY_PERCENT_MAX : 0);
 }
 
+static void led_set_state_gpio_handler(void *opaque, int line, int new_state)
+{
+    LEDState *s = LED(opaque);
+
+    assert(line == 0);
+    led_set_state(s, !!new_state != s->gpio_active_high);
+}
+
 static void led_reset(DeviceState *dev)
 {
     LEDState *s = LED(dev);
 
-    led_set_state(s, false);
+    led_set_state(s, s->gpio_active_high);
 }
 
 static const VMStateDescription vmstate_led = {
@@ -84,11 +93,14 @@ static void led_realize(DeviceState *dev, Error **errp)
     if (s->description == NULL) {
         s->description = g_strdup("n/a");
     }
+
+    qdev_init_gpio_in(DEVICE(s), led_set_state_gpio_handler, 1);
 }
 
 static Property led_properties[] = {
     DEFINE_PROP_STRING("color", LEDState, color),
     DEFINE_PROP_STRING("description", LEDState, description),
+    DEFINE_PROP_BOOL("gpio-active-high", LEDState, gpio_active_high, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -119,6 +131,7 @@ static void led_register_types(void)
 type_init(led_register_types)
 
 LEDState *led_create_simple(Object *parentobj,
+                            GpioPolarity gpio_polarity,
                             LEDColor color,
                             const char *description)
 {
@@ -126,6 +139,8 @@ LEDState *led_create_simple(Object *parentobj,
     DeviceState *dev;
 
     dev = qdev_new(TYPE_LED);
+    qdev_prop_set_bit(dev, "gpio-active-high",
+                      gpio_polarity == GPIO_POLARITY_ACTIVE_HIGH);
     qdev_prop_set_string(dev, "color", led_color_name[color]);
     if (!description) {
         static unsigned undescribed_led_id;
-- 
2.26.2

[PATCH v6 3/7] hw/misc/led: Emit a trace event when LED intensity has changed

[Philippe Mathieu-Daudé]

Track the LED intensity, and emit a trace event when it changes.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/led.c        | 4 ++++
 hw/misc/trace-events | 1 +
 2 files changed, 5 insertions(+)

diff --git a/hw/misc/led.c b/hw/misc/led.c
index c5fa09a613a..5266d026d0b 100644
--- a/hw/misc/led.c
+++ b/hw/misc/led.c
@@ -41,6 +41,10 @@ void led_set_intensity(LEDState *s, unsigned intensity_percent)
         intensity_percent = LED_INTENSITY_PERCENT_MAX;
     }
     trace_led_set_intensity(s->description, s->color, intensity_percent);
+    if (intensity_percent != s->intensity_percent) {
+        trace_led_change_intensity(s->description, s->color,
+                                   s->intensity_percent, intensity_percent);
+    }
     s->intensity_percent = intensity_percent;
 }
 
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 211ff114694..5f3f6121bc9 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -217,6 +217,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, uint8_t intensity_percent) "LED desc:'%s' color:%s intensity: %u%%"
+led_change_intensity(const char *color, const char *desc, uint8_t old_intensity_percent, uint8_t new_intensity_percent) "LED desc:'%s' color:%s intensity %u%% -> %u%%"
 
 # pca9552.c
 pca955x_gpio_status(const char *description, const char *buf) "%s GPIOs 0-15 [%s]"
-- 
2.26.2

[PATCH v6 4/7] hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1

[Philippe Mathieu-Daudé]

The Witherspoon has 3 LEDs connected to a PCA9552. Add them.
The names and reset values are taken from:
https://github.com/open-power/witherspoon-xml/blob/master/witherspoon.xml

Example booting obmc-phosphor-image:

  $ qemu-system-arm -M witherspoon-bmc -trace led_change_intensity
  1592693373.997015:led_change_intensity LED desc:'front-fault-4' color:green intensity 0% -> 100%
  1592693373.997632:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
  1592693373.998239:led_change_intensity LED desc:'front-id-5' color:green intensity 0% -> 100%
  1592693500.291805:led_change_intensity LED desc:'front-power-3' color:green intensity 100% -> 0%
  1592693500.312041:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
  1592693500.821254:led_change_intensity LED desc:'front-power-3' color:green intensity 100% -> 0%
  1592693501.331517:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
  1592693501.841367:led_change_intensity LED desc:'front-power-3' color:green intensity 100% -> 0%
  1592693502.350839:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
  1592693502.861134:led_change_intensity LED desc:'front-power-3' color:green intensity 100% -> 0%
  1592693503.371090:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%

We notice the front-power LED starts to blink at a ~2Hz rate.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/aspeed.c | 20 ++++++++++++++++++++
 hw/arm/Kconfig  |  1 +
 2 files changed, 21 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 8bfb1c79ddc..83e322ea983 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -20,6 +20,7 @@
 #include "hw/i2c/smbus_eeprom.h"
 #include "hw/misc/pca9552.h"
 #include "hw/misc/tmp105.h"
+#include "hw/misc/led.h"
 #include "hw/qdev-properties.h"
 #include "qemu/log.h"
 #include "sysemu/block-backend.h"
@@ -521,9 +522,20 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc)
 
 static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
 {
+    static const struct {
+        unsigned gpio_id;
+        LEDColor color;
+        const char *description;
+        bool gpio_polarity;
+    } pca1_leds[] = {
+        {13, LED_COLOR_GREEN, "front-fault-4",  GPIO_POLARITY_ACTIVE_LOW},
+        {14, LED_COLOR_GREEN, "front-power-3",  GPIO_POLARITY_ACTIVE_LOW},
+        {15, LED_COLOR_GREEN, "front-id-5",     GPIO_POLARITY_ACTIVE_LOW},
+    };
     AspeedSoCState *soc = &bmc->soc;
     uint8_t *eeprom_buf = g_malloc0(8 * 1024);
     DeviceState *dev;
+    LEDState *led;
 
     /* Bus 3: TODO bmp280@77 */
     /* Bus 3: TODO max31785@52 */
@@ -534,6 +546,14 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
                                 aspeed_i2c_get_bus(&soc->i2c, 3),
                                 &error_fatal);
 
+    for (size_t i = 0; i < ARRAY_SIZE(pca1_leds); i++) {
+        led = led_create_simple(OBJECT(bmc),
+                                pca1_leds[i].gpio_polarity,
+                                pca1_leds[i].color,
+                                pca1_leds[i].description);
+        qdev_connect_gpio_out(dev, pca1_leds[i].gpio_id,
+                              qdev_get_gpio_in(DEVICE(led), 0));
+    }
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), "tmp423", 0x4c);
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), "tmp423", 0x4c);
 
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index bc3a423940b..06ba1c355b1 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -394,6 +394,7 @@ config ASPEED_SOC
     select TMP105
     select TMP421
     select UNIMP
+    select LED
 
 config MPS2
     bool
-- 
2.26.2

[PATCH v6 5/7] hw/misc/mps2-fpgaio: Use the LED device

[Philippe Mathieu-Daudé]

Per the 'ARM MPS2 and MPS2+ FPGA Prototyping Boards Technical
Reference Manual' (100112_0200_07_en):

  2.1  Overview of the MPS2 and MPS2+ hardware

       The MPS2 and MPS2+ FPGA Prototyping Boards contain the
       following components and interfaces:

       * User switches and user LEDs:

         - Two green LEDs and two push buttons that connect to
           the FPGA.
         - Eight green LEDs and one 8-way dip switch that connect
           to the MCC.

Add the 2 LEDs connected to the FPGA.

This replaces the 'mps2_fpgaio_leds' trace events by the generic
'led_set_intensity' event.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/misc/mps2-fpgaio.h |  2 ++
 hw/misc/mps2-fpgaio.c         | 23 ++++++++++++++++++-----
 hw/misc/Kconfig               |  1 +
 hw/misc/trace-events          |  1 -
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/include/hw/misc/mps2-fpgaio.h b/include/hw/misc/mps2-fpgaio.h
index 991f5b731e8..513e3be6f13 100644
--- a/include/hw/misc/mps2-fpgaio.h
+++ b/include/hw/misc/mps2-fpgaio.h
@@ -22,6 +22,7 @@
 #define MPS2_FPGAIO_H
 
 #include "hw/sysbus.h"
+#include "hw/misc/led.h"
 #include "qom/object.h"
 
 #define TYPE_MPS2_FPGAIO "mps2-fpgaio"
@@ -35,6 +36,7 @@ struct MPS2FPGAIO {
 
     /*< public >*/
     MemoryRegion iomem;
+    LEDState *led[2];
 
     uint32_t led0;
     uint32_t prescale;
diff --git a/hw/misc/mps2-fpgaio.c b/hw/misc/mps2-fpgaio.c
index 2f3fbeef348..6af0e8f837a 100644
--- a/hw/misc/mps2-fpgaio.c
+++ b/hw/misc/mps2-fpgaio.c
@@ -24,6 +24,7 @@
 #include "migration/vmstate.h"
 #include "hw/registerfields.h"
 #include "hw/misc/mps2-fpgaio.h"
+#include "hw/misc/led.h"
 #include "hw/qdev-properties.h"
 #include "qemu/timer.h"
 
@@ -176,12 +177,9 @@ static void mps2_fpgaio_write(void *opaque, hwaddr offset, uint64_t value,
 
     switch (offset) {
     case A_LED0:
-        /* LED bits [1:0] control board LEDs. We don't currently have
-         * a mechanism for displaying this graphically, so use a trace event.
-         */
-        trace_mps2_fpgaio_leds(value & 0x02 ? '*' : '.',
-                               value & 0x01 ? '*' : '.');
         s->led0 = value & 0x3;
+        led_set_state(s->led[0], value & 0x01);
+        led_set_state(s->led[1], value & 0x02);
         break;
     case A_PRESCALE:
         resync_counter(s);
@@ -239,6 +237,10 @@ static void mps2_fpgaio_reset(DeviceState *dev)
     s->counter = 0;
     s->pscntr = 0;
     s->pscntr_sync_ticks = now;
+
+    for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
+        device_cold_reset(DEVICE(s->led[i]));
+    }
 }
 
 static void mps2_fpgaio_init(Object *obj)
@@ -251,6 +253,16 @@ static void mps2_fpgaio_init(Object *obj)
     sysbus_init_mmio(sbd, &s->iomem);
 }
 
+static void mps2_fpgaio_realize(DeviceState *dev, Error **errp)
+{
+    MPS2FPGAIO *s = MPS2_FPGAIO(dev);
+
+    s->led[0] = led_create_simple(OBJECT(dev), GPIO_POLARITY_ACTIVE_HIGH,
+                                  LED_COLOR_GREEN, "USERLED0");
+    s->led[1] = led_create_simple(OBJECT(dev), GPIO_POLARITY_ACTIVE_HIGH,
+                                  LED_COLOR_GREEN, "USERLED1");
+}
+
 static bool mps2_fpgaio_counters_needed(void *opaque)
 {
     /* Currently vmstate.c insists all subsections have a 'needed' function */
@@ -299,6 +311,7 @@ static void mps2_fpgaio_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &mps2_fpgaio_vmstate;
+    dc->realize = mps2_fpgaio_realize;
     dc->reset = mps2_fpgaio_reset;
     device_class_set_props(dc, mps2_fpgaio_properties);
 }
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 5c151fa3a83..0cecad45aad 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -93,6 +93,7 @@ config MIPS_ITU
 
 config MPS2_FPGAIO
     bool
+    select LED
 
 config MPS2_SCC
     bool
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 5f3f6121bc9..908272e8593 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -92,7 +92,6 @@ mps2_scc_cfg_read(unsigned function, unsigned device, uint32_t value) "MPS2 SCC
 mps2_fpgaio_read(uint64_t offset, uint64_t data, unsigned size) "MPS2 FPGAIO read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 mps2_fpgaio_write(uint64_t offset, uint64_t data, unsigned size) "MPS2 FPGAIO write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 mps2_fpgaio_reset(void) "MPS2 FPGAIO: reset"
-mps2_fpgaio_leds(char led1, char led0) "MPS2 FPGAIO LEDs: %c%c"
 
 # msf2-sysreg.c
 msf2_sysreg_write(uint64_t offset, uint32_t val, uint32_t prev) "msf2-sysreg write: addr 0x%08" PRIx64 " data 0x%" PRIx32 " prev 0x%" PRIx32
-- 
2.26.2

[PATCH v6 6/7] hw/misc/mps2-scc: Use the LED device

[Philippe Mathieu-Daudé]

Per the 'ARM MPS2 and MPS2+ FPGA Prototyping Boards Technical
Reference Manual' (100112_0200_07_en):

  2.1  Overview of the MPS2 and MPS2+ hardware

       The MPS2 and MPS2+ FPGA Prototyping Boards contain the
       following components and interfaces:

       * User switches and user LEDs:

         - Two green LEDs and two push buttons that connect to
           the FPGA.
         - Eight green LEDs and one 8-way dip switch that connect
           to the MCC.

Add the 8 LEDs connected to the MCC.

This replaces the 'mps2_scc_leds' trace events by the generic
'led_set_intensity' event.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/hw/misc/mps2-scc.h |  2 ++
 hw/misc/mps2-scc.c         | 27 ++++++++++++++++-----------
 hw/misc/Kconfig            |  1 +
 hw/misc/trace-events       |  1 -
 4 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/include/hw/misc/mps2-scc.h b/include/hw/misc/mps2-scc.h
index 445e268b1fd..0009479ae83 100644
--- a/include/hw/misc/mps2-scc.h
+++ b/include/hw/misc/mps2-scc.h
@@ -13,6 +13,7 @@
 #define MPS2_SCC_H
 
 #include "hw/sysbus.h"
+#include "hw/misc/led.h"
 #include "qom/object.h"
 
 #define TYPE_MPS2_SCC "mps2-scc"
@@ -28,6 +29,7 @@ struct MPS2SCC {
 
     /*< public >*/
     MemoryRegion iomem;
+    LEDState *led[8];
 
     uint32_t cfg0;
     uint32_t cfg1;
diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
index 9d0909e7b35..ce1dfe93562 100644
--- a/hw/misc/mps2-scc.c
+++ b/hw/misc/mps2-scc.c
@@ -20,11 +20,13 @@
 #include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qemu/bitops.h"
 #include "trace.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "hw/registerfields.h"
 #include "hw/misc/mps2-scc.h"
+#include "hw/misc/led.h"
 #include "hw/qdev-properties.h"
 
 REG32(CFG0, 0)
@@ -152,18 +154,10 @@ static void mps2_scc_write(void *opaque, hwaddr offset, uint64_t value,
         s->cfg0 = value;
         break;
     case A_CFG1:
-        /* CFG1 bits [7:0] control the board LEDs. We don't currently have
-         * a mechanism for displaying this graphically, so use a trace event.
-         */
-        trace_mps2_scc_leds(value & 0x80 ? '*' : '.',
-                            value & 0x40 ? '*' : '.',
-                            value & 0x20 ? '*' : '.',
-                            value & 0x10 ? '*' : '.',
-                            value & 0x08 ? '*' : '.',
-                            value & 0x04 ? '*' : '.',
-                            value & 0x02 ? '*' : '.',
-                            value & 0x01 ? '*' : '.');
         s->cfg1 = value;
+        for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
+            led_set_state(s->led[i], extract32(value, i, 1));
+        }
         break;
     case A_CFGDATA_OUT:
         s->cfgdata_out = value;
@@ -236,6 +230,9 @@ static void mps2_scc_reset(DeviceState *dev)
     for (i = 0; i < NUM_OSCCLK; i++) {
         s->oscclk[i] = s->oscclk_reset[i];
     }
+    for (i = 0; i < ARRAY_SIZE(s->led); i++) {
+        device_cold_reset(DEVICE(s->led[i]));
+    }
 }
 
 static void mps2_scc_init(Object *obj)
@@ -249,6 +246,14 @@ static void mps2_scc_init(Object *obj)
 
 static void mps2_scc_realize(DeviceState *dev, Error **errp)
 {
+    MPS2SCC *s = MPS2_SCC(dev);
+
+    for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
+        char *name = g_strdup_printf("SCC LED%zu", i);
+        s->led[i] = led_create_simple(OBJECT(dev), GPIO_POLARITY_ACTIVE_HIGH,
+                                      LED_COLOR_GREEN, name);
+        g_free(name);
+    }
 }
 
 static const VMStateDescription mps2_scc_vmstate = {
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 0cecad45aad..7557a3e7b46 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -97,6 +97,7 @@ config MPS2_FPGAIO
 
 config MPS2_SCC
     bool
+    select LED
 
 config TZ_MPC
     bool
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 908272e8593..97f511aa58c 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -84,7 +84,6 @@ aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64
 mps2_scc_read(uint64_t offset, uint64_t data, unsigned size) "MPS2 SCC read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 mps2_scc_write(uint64_t offset, uint64_t data, unsigned size) "MPS2 SCC write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 mps2_scc_reset(void) "MPS2 SCC: reset"
-mps2_scc_leds(char led7, char led6, char led5, char led4, char led3, char led2, char led1, char led0) "MPS2 SCC LEDs: %c%c%c%c%c%c%c%c"
 mps2_scc_cfg_write(unsigned function, unsigned device, uint32_t value) "MPS2 SCC config write: function %d device %d data 0x%" PRIx32
 mps2_scc_cfg_read(unsigned function, unsigned device, uint32_t value) "MPS2 SCC config read: function %d device %d data 0x%" PRIx32
 
-- 
2.26.2

[PATCH v6 7/7] hw/arm/tosa: Replace fprintf() calls by LED devices

[Philippe Mathieu-Daudé]

The recently added LED device reports LED status changes with
the 'led_set_intensity' trace event. It is less invasive than
the fprintf() calls. We need however to have a binary built
with tracing support.

Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/tosa.c  | 40 +++++++++++++++-------------------------
 hw/arm/Kconfig |  1 +
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index 2ef6c7b2883..ca998f05232 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -24,6 +24,7 @@
 #include "hw/irq.h"
 #include "hw/ssi/ssi.h"
 #include "hw/sysbus.h"
+#include "hw/misc/led.h"
 #include "exec/address-spaces.h"
 #include "qom/object.h"
 
@@ -83,26 +84,6 @@ struct TosaMiscGPIOState {
     SysBusDevice parent_obj;
 };
 
-static void tosa_gpio_leds(void *opaque, int line, int level)
-{
-    switch (line) {
-    case 0:
-        fprintf(stderr, "blue LED %s.\n", level ? "on" : "off");
-        break;
-    case 1:
-        fprintf(stderr, "green LED %s.\n", level ? "on" : "off");
-        break;
-    case 2:
-        fprintf(stderr, "amber LED %s.\n", level ? "on" : "off");
-        break;
-    case 3:
-        fprintf(stderr, "wlan LED %s.\n", level ? "on" : "off");
-        break;
-    default:
-        g_assert_not_reached();
-    }
-}
-
 static void tosa_reset(void *opaque, int line, int level)
 {
     if (level) {
@@ -114,7 +95,6 @@ static void tosa_misc_gpio_init(Object *obj)
 {
     DeviceState *dev = DEVICE(obj);
 
-    qdev_init_gpio_in_named(dev, tosa_gpio_leds, "leds", 4);
     qdev_init_gpio_in_named(dev, tosa_reset, "reset", 1);
 }
 
@@ -124,6 +104,7 @@ static void tosa_gpio_setup(PXA2xxState *cpu,
                 TC6393xbState *tmio)
 {
     DeviceState *misc_gpio;
+    LEDState *led[4];
 
     misc_gpio = sysbus_create_simple(TYPE_TOSA_MISC_GPIO, -1, NULL);
 
@@ -145,14 +126,23 @@ static void tosa_gpio_setup(PXA2xxState *cpu,
                         qdev_get_gpio_in(cpu->gpio, TOSA_GPIO_JC_CF_IRQ),
                         NULL);
 
+    led[0] = led_create_simple(OBJECT(misc_gpio), GPIO_POLARITY_ACTIVE_HIGH,
+                               LED_COLOR_BLUE, "bluetooth");
+    led[1] = led_create_simple(OBJECT(misc_gpio), GPIO_POLARITY_ACTIVE_HIGH,
+                               LED_COLOR_GREEN, "note");
+    led[2] = led_create_simple(OBJECT(misc_gpio), GPIO_POLARITY_ACTIVE_HIGH,
+                               LED_COLOR_AMBER, "charger-error");
+    led[3] = led_create_simple(OBJECT(misc_gpio), GPIO_POLARITY_ACTIVE_HIGH,
+                               LED_COLOR_GREEN, "wlan");
+
     qdev_connect_gpio_out(scp1, TOSA_GPIO_BT_LED,
-                          qdev_get_gpio_in_named(misc_gpio, "leds", 0));
+                          qdev_get_gpio_in(DEVICE(led[0]), 0));
     qdev_connect_gpio_out(scp1, TOSA_GPIO_NOTE_LED,
-                          qdev_get_gpio_in_named(misc_gpio, "leds", 1));
+                          qdev_get_gpio_in(DEVICE(led[1]), 0));
     qdev_connect_gpio_out(scp1, TOSA_GPIO_CHRG_ERR_LED,
-                          qdev_get_gpio_in_named(misc_gpio, "leds", 2));
+                          qdev_get_gpio_in(DEVICE(led[2]), 0));
     qdev_connect_gpio_out(scp1, TOSA_GPIO_WLAN_LED,
-                          qdev_get_gpio_in_named(misc_gpio, "leds", 3));
+                          qdev_get_gpio_in(DEVICE(led[3]), 0));
 
     qdev_connect_gpio_out(scp1, TOSA_GPIO_TC6393XB_L3V_ON, tc6393xb_l3v_get(tmio));
 
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 06ba1c355b1..bbcfa098ae2 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -150,6 +150,7 @@ config TOSA
     select ZAURUS  # scoop
     select MICRODRIVE
     select PXA2XX
+    select LED
 
 config SPITZ
     bool
-- 
2.26.2

Re: [PATCH v6 0/7] hw/misc: Add LED device

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200912134041.946260-1-f4bug@amsat.org/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.






The full log is available at
http://patchew.org/logs/20200912134041.946260-1-f4bug@amsat.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v6 2/7] hw/misc/led: Allow connecting from GPIO output

[Richard Henderson]

On 9/12/20 6:40 AM, Philippe Mathieu-Daudé wrote:
> Some devices expose GPIO lines.
> 
> Add a GPIO qdev input to our LED device, so we can
> connect a GPIO output using qdev_connect_gpio_out().
> 
> When used with GPIOs, the intensity can only be either
> minium or maximum. This depends of the polarity of the
> GPIO (which can be inverted).
> Declare the GpioPolarity type to model the polarity.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/misc/led.h  | 10 ++++++++++
>  include/hw/qdev-core.h | 16 ++++++++++++++++
>  hw/misc/led.c          | 17 ++++++++++++++++-
>  3 files changed, 42 insertions(+), 1 deletion(-)
git-submodule-update

Re: [PATCH v6 2/7] hw/misc/led: Allow connecting from GPIO output

[Richard Henderson]

On 9/12/20 6:40 AM, Philippe Mathieu-Daudé wrote:
> Some devices expose GPIO lines.
> 
> Add a GPIO qdev input to our LED device, so we can
> connect a GPIO output using qdev_connect_gpio_out().
> 
> When used with GPIOs, the intensity can only be either
> minium or maximum. This depends of the polarity of the
> GPIO (which can be inverted).
> Declare the GpioPolarity type to model the polarity.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/misc/led.h  | 10 ++++++++++
>  include/hw/qdev-core.h | 16 ++++++++++++++++
>  hw/misc/led.c          | 17 ++++++++++++++++-
>  3 files changed, 42 insertions(+), 1 deletion(-)

Oops.  Failed to have r-b in paste-buffer.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v6 2/7] hw/misc/led: Allow connecting from GPIO output

[Luc Michel]

On 9/12/20 3:40 PM, Philippe Mathieu-Daudé wrote:
> Some devices expose GPIO lines.
> 
> Add a GPIO qdev input to our LED device, so we can
> connect a GPIO output using qdev_connect_gpio_out().
> 
> When used with GPIOs, the intensity can only be either
> minium or maximum. This depends of the polarity of the
> GPIO (which can be inverted).
> Declare the GpioPolarity type to model the polarity.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Luc Michel <luc.michel@greensocs.com>

> ---
>   include/hw/misc/led.h  | 10 ++++++++++
>   include/hw/qdev-core.h | 16 ++++++++++++++++
>   hw/misc/led.c          | 17 ++++++++++++++++-
>   3 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h
> index 286d37c75c1..aa359b87c20 100644
> --- a/include/hw/misc/led.h
> +++ b/include/hw/misc/led.h
> @@ -9,6 +9,7 @@
>   #define HW_MISC_LED_H
>   
>   #include "qom/object.h"
> +#include "hw/qdev-core.h"
>   
>   #define TYPE_LED "led"
>   
> @@ -37,10 +38,17 @@ struct LEDState {
>       /* Public */
>   
>       uint8_t intensity_percent;
> +    qemu_irq irq;
>   
>       /* Properties */
>       char *description;
>       char *color;
> +    /*
> +     * Determines whether a GPIO is using a positive (active-high)
> +     * logic (when used with GPIO, the intensity at reset is related
> +     * to the GPIO polarity).
> +     */
> +    bool gpio_active_high;
>   };
>   typedef struct LEDState LEDState;
>   DECLARE_INSTANCE_CHECKER(LEDState, LED, TYPE_LED)
> @@ -72,6 +80,7 @@ void led_set_state(LEDState *s, bool is_emitting);
>   /**
>    * led_create_simple: Create and realize a LED device
>    * @parentobj: the parent object
> + * @gpio_polarity: GPIO polarity
>    * @color: color of the LED
>    * @description: description of the LED (optional)
>    *
> @@ -81,6 +90,7 @@ void led_set_state(LEDState *s, bool is_emitting);
>    * Returns: The newly allocated and instantiated LED object.
>    */
>   LEDState *led_create_simple(Object *parentobj,
> +                            GpioPolarity gpio_polarity,
>                               LEDColor color,
>                               const char *description);
>   
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index e025ba9653f..ec1f1efc37b 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -423,6 +423,22 @@ void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
>   void qdev_machine_creation_done(void);
>   bool qdev_machine_modified(void);
>   
> +/**
> + * GpioPolarity: Polarity of a GPIO line
> + *
> + * GPIO lines use either positive (active-high) logic,
> + * or negative (active-low) logic.
> + *
> + * In active-high logic (%GPIO_POLARITY_ACTIVE_HIGH), a pin is
> + * active when the voltage on the pin is high (relative to ground);
> + * whereas in active-low logic (%GPIO_POLARITY_ACTIVE_LOW), a pin
> + * is active when the voltage on the pin is low (or grounded).
> + */
> +typedef enum {
> +    GPIO_POLARITY_ACTIVE_LOW,
> +    GPIO_POLARITY_ACTIVE_HIGH
> +} GpioPolarity;
> +
>   /**
>    * qdev_get_gpio_in: Get one of a device's anonymous input GPIO lines
>    * @dev: Device whose GPIO we want
> diff --git a/hw/misc/led.c b/hw/misc/led.c
> index 1e2f49c5710..c5fa09a613a 100644
> --- a/hw/misc/led.c
> +++ b/hw/misc/led.c
> @@ -10,6 +10,7 @@
>   #include "migration/vmstate.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/misc/led.h"
> +#include "hw/irq.h"
>   #include "trace.h"
>   
>   #define LED_INTENSITY_PERCENT_MAX   100
> @@ -53,11 +54,19 @@ void led_set_state(LEDState *s, bool is_emitting)
>       led_set_intensity(s, is_emitting ? LED_INTENSITY_PERCENT_MAX : 0);
>   }
>   
> +static void led_set_state_gpio_handler(void *opaque, int line, int new_state)
> +{
> +    LEDState *s = LED(opaque);
> +
> +    assert(line == 0);
> +    led_set_state(s, !!new_state != s->gpio_active_high);
> +}
> +
>   static void led_reset(DeviceState *dev)
>   {
>       LEDState *s = LED(dev);
>   
> -    led_set_state(s, false);
> +    led_set_state(s, s->gpio_active_high);
>   }
>   
>   static const VMStateDescription vmstate_led = {
> @@ -84,11 +93,14 @@ static void led_realize(DeviceState *dev, Error **errp)
>       if (s->description == NULL) {
>           s->description = g_strdup("n/a");
>       }
> +
> +    qdev_init_gpio_in(DEVICE(s), led_set_state_gpio_handler, 1);
>   }
>   
>   static Property led_properties[] = {
>       DEFINE_PROP_STRING("color", LEDState, color),
>       DEFINE_PROP_STRING("description", LEDState, description),
> +    DEFINE_PROP_BOOL("gpio-active-high", LEDState, gpio_active_high, true),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> @@ -119,6 +131,7 @@ static void led_register_types(void)
>   type_init(led_register_types)
>   
>   LEDState *led_create_simple(Object *parentobj,
> +                            GpioPolarity gpio_polarity,
>                               LEDColor color,
>                               const char *description)
>   {
> @@ -126,6 +139,8 @@ LEDState *led_create_simple(Object *parentobj,
>       DeviceState *dev;
>   
>       dev = qdev_new(TYPE_LED);
> +    qdev_prop_set_bit(dev, "gpio-active-high",
> +                      gpio_polarity == GPIO_POLARITY_ACTIVE_HIGH);
>       qdev_prop_set_string(dev, "color", led_color_name[color]);
>       if (!description) {
>           static unsigned undescribed_led_id;
> 

Re: [PATCH v6 5/7] hw/misc/mps2-fpgaio: Use the LED device

[Luc Michel]

On 9/12/20 3:40 PM, Philippe Mathieu-Daudé wrote:
> Per the 'ARM MPS2 and MPS2+ FPGA Prototyping Boards Technical
> Reference Manual' (100112_0200_07_en):
> 
>    2.1  Overview of the MPS2 and MPS2+ hardware
> 
>         The MPS2 and MPS2+ FPGA Prototyping Boards contain the
>         following components and interfaces:
> 
>         * User switches and user LEDs:
> 
>           - Two green LEDs and two push buttons that connect to
>             the FPGA.
>           - Eight green LEDs and one 8-way dip switch that connect
>             to the MCC.
> 
> Add the 2 LEDs connected to the FPGA.
> 
> This replaces the 'mps2_fpgaio_leds' trace events by the generic
> 'led_set_intensity' event.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Luc Michel <luc.michel@greensocs.com>

> ---
>   include/hw/misc/mps2-fpgaio.h |  2 ++
>   hw/misc/mps2-fpgaio.c         | 23 ++++++++++++++++++-----
>   hw/misc/Kconfig               |  1 +
>   hw/misc/trace-events          |  1 -
>   4 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/misc/mps2-fpgaio.h b/include/hw/misc/mps2-fpgaio.h
> index 991f5b731e8..513e3be6f13 100644
> --- a/include/hw/misc/mps2-fpgaio.h
> +++ b/include/hw/misc/mps2-fpgaio.h
> @@ -22,6 +22,7 @@
>   #define MPS2_FPGAIO_H
>   
>   #include "hw/sysbus.h"
> +#include "hw/misc/led.h"
>   #include "qom/object.h"
>   
>   #define TYPE_MPS2_FPGAIO "mps2-fpgaio"
> @@ -35,6 +36,7 @@ struct MPS2FPGAIO {
>   
>       /*< public >*/
>       MemoryRegion iomem;
> +    LEDState *led[2];
>   
>       uint32_t led0;
>       uint32_t prescale;
> diff --git a/hw/misc/mps2-fpgaio.c b/hw/misc/mps2-fpgaio.c
> index 2f3fbeef348..6af0e8f837a 100644
> --- a/hw/misc/mps2-fpgaio.c
> +++ b/hw/misc/mps2-fpgaio.c
> @@ -24,6 +24,7 @@
>   #include "migration/vmstate.h"
>   #include "hw/registerfields.h"
>   #include "hw/misc/mps2-fpgaio.h"
> +#include "hw/misc/led.h"
>   #include "hw/qdev-properties.h"
>   #include "qemu/timer.h"
>   
> @@ -176,12 +177,9 @@ static void mps2_fpgaio_write(void *opaque, hwaddr offset, uint64_t value,
>   
>       switch (offset) {
>       case A_LED0:
> -        /* LED bits [1:0] control board LEDs. We don't currently have
> -         * a mechanism for displaying this graphically, so use a trace event.
> -         */
> -        trace_mps2_fpgaio_leds(value & 0x02 ? '*' : '.',
> -                               value & 0x01 ? '*' : '.');
>           s->led0 = value & 0x3;
> +        led_set_state(s->led[0], value & 0x01);
> +        led_set_state(s->led[1], value & 0x02);
>           break;
>       case A_PRESCALE:
>           resync_counter(s);
> @@ -239,6 +237,10 @@ static void mps2_fpgaio_reset(DeviceState *dev)
>       s->counter = 0;
>       s->pscntr = 0;
>       s->pscntr_sync_ticks = now;
> +
> +    for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
> +        device_cold_reset(DEVICE(s->led[i]));
> +    }
>   }
>   
>   static void mps2_fpgaio_init(Object *obj)
> @@ -251,6 +253,16 @@ static void mps2_fpgaio_init(Object *obj)
>       sysbus_init_mmio(sbd, &s->iomem);
>   }
>   
> +static void mps2_fpgaio_realize(DeviceState *dev, Error **errp)
> +{
> +    MPS2FPGAIO *s = MPS2_FPGAIO(dev);
> +
> +    s->led[0] = led_create_simple(OBJECT(dev), GPIO_POLARITY_ACTIVE_HIGH,
> +                                  LED_COLOR_GREEN, "USERLED0");
> +    s->led[1] = led_create_simple(OBJECT(dev), GPIO_POLARITY_ACTIVE_HIGH,
> +                                  LED_COLOR_GREEN, "USERLED1");
> +}
> +
>   static bool mps2_fpgaio_counters_needed(void *opaque)
>   {
>       /* Currently vmstate.c insists all subsections have a 'needed' function */
> @@ -299,6 +311,7 @@ static void mps2_fpgaio_class_init(ObjectClass *klass, void *data)
>       DeviceClass *dc = DEVICE_CLASS(klass);
>   
>       dc->vmsd = &mps2_fpgaio_vmstate;
> +    dc->realize = mps2_fpgaio_realize;
>       dc->reset = mps2_fpgaio_reset;
>       device_class_set_props(dc, mps2_fpgaio_properties);
>   }
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 5c151fa3a83..0cecad45aad 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -93,6 +93,7 @@ config MIPS_ITU
>   
>   config MPS2_FPGAIO
>       bool
> +    select LED
>   
>   config MPS2_SCC
>       bool
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 5f3f6121bc9..908272e8593 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -92,7 +92,6 @@ mps2_scc_cfg_read(unsigned function, unsigned device, uint32_t value) "MPS2 SCC
>   mps2_fpgaio_read(uint64_t offset, uint64_t data, unsigned size) "MPS2 FPGAIO read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>   mps2_fpgaio_write(uint64_t offset, uint64_t data, unsigned size) "MPS2 FPGAIO write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>   mps2_fpgaio_reset(void) "MPS2 FPGAIO: reset"
> -mps2_fpgaio_leds(char led1, char led0) "MPS2 FPGAIO LEDs: %c%c"
>   
>   # msf2-sysreg.c
>   msf2_sysreg_write(uint64_t offset, uint32_t val, uint32_t prev) "msf2-sysreg write: addr 0x%08" PRIx64 " data 0x%" PRIx32 " prev 0x%" PRIx32
> 

Re: [PATCH v6 6/7] hw/misc/mps2-scc: Use the LED device

[Luc Michel]

On 9/12/20 3:40 PM, Philippe Mathieu-Daudé wrote:
> Per the 'ARM MPS2 and MPS2+ FPGA Prototyping Boards Technical
> Reference Manual' (100112_0200_07_en):
> 
>    2.1  Overview of the MPS2 and MPS2+ hardware
> 
>         The MPS2 and MPS2+ FPGA Prototyping Boards contain the
>         following components and interfaces:
> 
>         * User switches and user LEDs:
> 
>           - Two green LEDs and two push buttons that connect to
>             the FPGA.
>           - Eight green LEDs and one 8-way dip switch that connect
>             to the MCC.
> 
> Add the 8 LEDs connected to the MCC.
> 
> This replaces the 'mps2_scc_leds' trace events by the generic
> 'led_set_intensity' event.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Luc Michel <luc.michel@greensocs.com>

> ---
>   include/hw/misc/mps2-scc.h |  2 ++
>   hw/misc/mps2-scc.c         | 27 ++++++++++++++++-----------
>   hw/misc/Kconfig            |  1 +
>   hw/misc/trace-events       |  1 -
>   4 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/misc/mps2-scc.h b/include/hw/misc/mps2-scc.h
> index 445e268b1fd..0009479ae83 100644
> --- a/include/hw/misc/mps2-scc.h
> +++ b/include/hw/misc/mps2-scc.h
> @@ -13,6 +13,7 @@
>   #define MPS2_SCC_H
>   
>   #include "hw/sysbus.h"
> +#include "hw/misc/led.h"
>   #include "qom/object.h"
>   
>   #define TYPE_MPS2_SCC "mps2-scc"
> @@ -28,6 +29,7 @@ struct MPS2SCC {
>   
>       /*< public >*/
>       MemoryRegion iomem;
> +    LEDState *led[8];
>   
>       uint32_t cfg0;
>       uint32_t cfg1;
> diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
> index 9d0909e7b35..ce1dfe93562 100644
> --- a/hw/misc/mps2-scc.c
> +++ b/hw/misc/mps2-scc.c
> @@ -20,11 +20,13 @@
>   #include "qemu/osdep.h"
>   #include "qemu/log.h"
>   #include "qemu/module.h"
> +#include "qemu/bitops.h"
>   #include "trace.h"
>   #include "hw/sysbus.h"
>   #include "migration/vmstate.h"
>   #include "hw/registerfields.h"
>   #include "hw/misc/mps2-scc.h"
> +#include "hw/misc/led.h"
>   #include "hw/qdev-properties.h"
>   
>   REG32(CFG0, 0)
> @@ -152,18 +154,10 @@ static void mps2_scc_write(void *opaque, hwaddr offset, uint64_t value,
>           s->cfg0 = value;
>           break;
>       case A_CFG1:
> -        /* CFG1 bits [7:0] control the board LEDs. We don't currently have
> -         * a mechanism for displaying this graphically, so use a trace event.
> -         */
> -        trace_mps2_scc_leds(value & 0x80 ? '*' : '.',
> -                            value & 0x40 ? '*' : '.',
> -                            value & 0x20 ? '*' : '.',
> -                            value & 0x10 ? '*' : '.',
> -                            value & 0x08 ? '*' : '.',
> -                            value & 0x04 ? '*' : '.',
> -                            value & 0x02 ? '*' : '.',
> -                            value & 0x01 ? '*' : '.');
>           s->cfg1 = value;
> +        for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
> +            led_set_state(s->led[i], extract32(value, i, 1));
> +        }
>           break;
>       case A_CFGDATA_OUT:
>           s->cfgdata_out = value;
> @@ -236,6 +230,9 @@ static void mps2_scc_reset(DeviceState *dev)
>       for (i = 0; i < NUM_OSCCLK; i++) {
>           s->oscclk[i] = s->oscclk_reset[i];
>       }
> +    for (i = 0; i < ARRAY_SIZE(s->led); i++) {
> +        device_cold_reset(DEVICE(s->led[i]));
> +    }
>   }
>   
>   static void mps2_scc_init(Object *obj)
> @@ -249,6 +246,14 @@ static void mps2_scc_init(Object *obj)
>   
>   static void mps2_scc_realize(DeviceState *dev, Error **errp)
>   {
> +    MPS2SCC *s = MPS2_SCC(dev);
> +
> +    for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
> +        char *name = g_strdup_printf("SCC LED%zu", i);
> +        s->led[i] = led_create_simple(OBJECT(dev), GPIO_POLARITY_ACTIVE_HIGH,
> +                                      LED_COLOR_GREEN, name);
> +        g_free(name);
> +    }
>   }
>   
>   static const VMStateDescription mps2_scc_vmstate = {
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 0cecad45aad..7557a3e7b46 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -97,6 +97,7 @@ config MPS2_FPGAIO
>   
>   config MPS2_SCC
>       bool
> +    select LED
>   
>   config TZ_MPC
>       bool
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 908272e8593..97f511aa58c 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -84,7 +84,6 @@ aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64
>   mps2_scc_read(uint64_t offset, uint64_t data, unsigned size) "MPS2 SCC read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>   mps2_scc_write(uint64_t offset, uint64_t data, unsigned size) "MPS2 SCC write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
>   mps2_scc_reset(void) "MPS2 SCC: reset"
> -mps2_scc_leds(char led7, char led6, char led5, char led4, char led3, char led2, char led1, char led0) "MPS2 SCC LEDs: %c%c%c%c%c%c%c%c"
>   mps2_scc_cfg_write(unsigned function, unsigned device, uint32_t value) "MPS2 SCC config write: function %d device %d data 0x%" PRIx32
>   mps2_scc_cfg_read(unsigned function, unsigned device, uint32_t value) "MPS2 SCC config read: function %d device %d data 0x%" PRIx32
>   
> 

Re: [PATCH v6 4/7] hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1

[Cédric Le Goater]

On 9/12/20 3:40 PM, Philippe Mathieu-Daudé wrote:
> The Witherspoon has 3 LEDs connected to a PCA9552. Add them.
> The names and reset values are taken from:
> https://github.com/open-power/witherspoon-xml/blob/master/witherspoon.xml
> 
> Example booting obmc-phosphor-image:
> 
>   $ qemu-system-arm -M witherspoon-bmc -trace led_change_intensity
>   1592693373.997015:led_change_intensity LED desc:'front-fault-4' color:green intensity 0% -> 100%
>   1592693373.997632:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
>   1592693373.998239:led_change_intensity LED desc:'front-id-5' color:green intensity 0% -> 100%
>   1592693500.291805:led_change_intensity LED desc:'front-power-3' color:green intensity 100% -> 0%
>   1592693500.312041:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
>   1592693500.821254:led_change_intensity LED desc:'front-power-3' color:green intensity 100% -> 0%
>   1592693501.331517:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
>   1592693501.841367:led_change_intensity LED desc:'front-power-3' color:green intensity 100% -> 0%
>   1592693502.350839:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
>   1592693502.861134:led_change_intensity LED desc:'front-power-3' color:green intensity 100% -> 0%
>   1592693503.371090:led_change_intensity LED desc:'front-power-3' color:green intensity 0% -> 100%
> 
> We notice the front-power LED starts to blink at a ~2Hz rate.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Luc Michel <luc.michel@greensocs.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/arm/aspeed.c | 20 ++++++++++++++++++++
>  hw/arm/Kconfig  |  1 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 8bfb1c79ddc..83e322ea983 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -20,6 +20,7 @@
>  #include "hw/i2c/smbus_eeprom.h"
>  #include "hw/misc/pca9552.h"
>  #include "hw/misc/tmp105.h"
> +#include "hw/misc/led.h"
>  #include "hw/qdev-properties.h"
>  #include "qemu/log.h"
>  #include "sysemu/block-backend.h"
> @@ -521,9 +522,20 @@ static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc)
>  
>  static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>  {
> +    static const struct {
> +        unsigned gpio_id;
> +        LEDColor color;
> +        const char *description;
> +        bool gpio_polarity;
> +    } pca1_leds[] = {
> +        {13, LED_COLOR_GREEN, "front-fault-4",  GPIO_POLARITY_ACTIVE_LOW},
> +        {14, LED_COLOR_GREEN, "front-power-3",  GPIO_POLARITY_ACTIVE_LOW},
> +        {15, LED_COLOR_GREEN, "front-id-5",     GPIO_POLARITY_ACTIVE_LOW},
> +    };
>      AspeedSoCState *soc = &bmc->soc;
>      uint8_t *eeprom_buf = g_malloc0(8 * 1024);
>      DeviceState *dev;
> +    LEDState *led;
>  
>      /* Bus 3: TODO bmp280@77 */
>      /* Bus 3: TODO max31785@52 */
> @@ -534,6 +546,14 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
>                                  aspeed_i2c_get_bus(&soc->i2c, 3),
>                                  &error_fatal);
>  
> +    for (size_t i = 0; i < ARRAY_SIZE(pca1_leds); i++) {
> +        led = led_create_simple(OBJECT(bmc),
> +                                pca1_leds[i].gpio_polarity,
> +                                pca1_leds[i].color,
> +                                pca1_leds[i].description);
> +        qdev_connect_gpio_out(dev, pca1_leds[i].gpio_id,
> +                              qdev_get_gpio_in(DEVICE(led), 0));
> +    }
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), "tmp423", 0x4c);
>      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 5), "tmp423", 0x4c);
>  
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index bc3a423940b..06ba1c355b1 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -394,6 +394,7 @@ config ASPEED_SOC
>      select TMP105
>      select TMP421
>      select UNIMP
> +    select LED
>  
>  config MPS2
>      bool
> 

Re: [PATCH v6 0/7] hw/misc: Add LED device

[Philippe Mathieu-Daudé]

On 9/12/20 3:40 PM, Philippe Mathieu-Daudé wrote:
> Hello,
> 
> These patches are part of the GSoC unselected 'QEMU visualizer'
> project.
> 
> This series introduce a LED device that can be easily connected
> to a GPIO output.
[...]
> Philippe Mathieu-Daud=C3=A9 (7):
>   hw/misc/led: Add a LED device
>   hw/misc/led: Allow connecting from GPIO output
>   hw/misc/led: Emit a trace event when LED intensity has changed
>   hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1
>   hw/misc/mps2-fpgaio: Use the LED device
>   hw/misc/mps2-scc: Use the LED device
>   hw/arm/tosa: Replace fprintf() calls by LED devices

This series is now fully reviewed.

Peter, since so far only ARM boards use the LED device,
do you mind queuing it via your ARM tree?

Alternatively I can ask Paolo or send a pull request
myself is both of you think it is out of the scope of
your maintenance area.

Thanks,

Phil.

Re: [PATCH v6 0/7] hw/misc: Add LED device

[Philippe Mathieu-Daudé]

Hi Cédric,

On 9/19/20 2:30 PM, Philippe Mathieu-Daudé wrote:
> On 9/12/20 3:40 PM, Philippe Mathieu-Daudé wrote:
>> Hello,
>>
>> These patches are part of the GSoC unselected 'QEMU visualizer'
>> project.
>>
>> This series introduce a LED device that can be easily connected
>> to a GPIO output.
> [...]
>> Philippe Mathieu-Daud=C3=A9 (7):
>>    hw/misc/led: Add a LED device
>>    hw/misc/led: Allow connecting from GPIO output
>>    hw/misc/led: Emit a trace event when LED intensity has changed
>>    hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1
>>    hw/misc/mps2-fpgaio: Use the LED device
>>    hw/misc/mps2-scc: Use the LED device
>>    hw/arm/tosa: Replace fprintf() calls by LED devices
> 
> This series is now fully reviewed.

Since the Aspeed board is the main user, do you mind
taking this series via your aspeed tree?

> 
> Peter, since so far only ARM boards use the LED device,
> do you mind queuing it via your ARM tree?
> 
> Alternatively I can ask Paolo or send a pull request
> myself is both of you think it is out of the scope of
> your maintenance area.
> 
> Thanks,
> 
> Phil.
> 

Re: [PATCH v6 0/7] hw/misc: Add LED device

[Philippe Mathieu-Daudé]

On 10/16/20 5:29 PM, Philippe Mathieu-Daudé wrote:
> On 9/19/20 2:30 PM, Philippe Mathieu-Daudé wrote:
>> On 9/12/20 3:40 PM, Philippe Mathieu-Daudé wrote:
>>> Hello,
>>>
>>> These patches are part of the GSoC unselected 'QEMU visualizer'
>>> project.
>>>
>>> This series introduce a LED device that can be easily connected
>>> to a GPIO output.
>> [...]
>>> Philippe Mathieu-Daud=C3=A9 (7):
>>>    hw/misc/led: Add a LED device
>>>    hw/misc/led: Allow connecting from GPIO output
>>>    hw/misc/led: Emit a trace event when LED intensity has changed
>>>    hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1
>>>    hw/misc/mps2-fpgaio: Use the LED device
>>>    hw/misc/mps2-scc: Use the LED device
>>>    hw/arm/tosa: Replace fprintf() calls by LED devices
>>
>> This series is now fully reviewed.

As soft-freeze is tomorrow, I'll go ahead and send a pull request.

Thanks,

Phil.

Re: [PATCH v6 0/7] hw/misc: Add LED device

[Cédric Le Goater]

On 10/26/20 11:00 PM, Philippe Mathieu-Daudé wrote:
> On 10/16/20 5:29 PM, Philippe Mathieu-Daudé wrote:
>> On 9/19/20 2:30 PM, Philippe Mathieu-Daudé wrote:
>>> On 9/12/20 3:40 PM, Philippe Mathieu-Daudé wrote:
>>>> Hello,
>>>>
>>>> These patches are part of the GSoC unselected 'QEMU visualizer'
>>>> project.
>>>>
>>>> This series introduce a LED device that can be easily connected
>>>> to a GPIO output.
>>> [...]
>>>> Philippe Mathieu-Daud=C3=A9 (7):
>>>>    hw/misc/led: Add a LED device
>>>>    hw/misc/led: Allow connecting from GPIO output
>>>>    hw/misc/led: Emit a trace event when LED intensity has changed
>>>>    hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1
>>>>    hw/misc/mps2-fpgaio: Use the LED device
>>>>    hw/misc/mps2-scc: Use the LED device
>>>>    hw/arm/tosa: Replace fprintf() calls by LED devices
>>>
>>> This series is now fully reviewed.
> 
> As soft-freeze is tomorrow, I'll go ahead and send a pull request.

You did well. I was out for a couple of weeks.

Thanks,

C.