[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 1/5] hw/misc: Add a LED device
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [RFC PATCH v2 1/5] hw/misc: Add a LED device |
Date: |
Mon, 15 Jun 2020 11:55:44 +0100 |
User-agent: |
Mutt/1.14.0 (2020-05-02) |
* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> A LED device can be connected to a GPIO output.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/misc/led.h | 30 ++++++++++++++++
> hw/misc/led.c | 84 +++++++++++++++++++++++++++++++++++++++++++
> MAINTAINERS | 6 ++++
> hw/misc/Kconfig | 3 ++
> hw/misc/Makefile.objs | 1 +
> hw/misc/trace-events | 3 ++
> 6 files changed, 127 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 0000000000..427ca1418e
> --- /dev/null
> +++ b/include/hw/misc/led.h
> @@ -0,0 +1,30 @@
> +/*
> + * 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 "hw/qdev-core.h"
> +#include "hw/sysbus.h" /* FIXME remove */
> +
> +#define TYPE_LED "led"
> +#define LED(obj) OBJECT_CHECK(LEDState, (obj), TYPE_LED)
> +
> +typedef struct LEDState {
> + /* Private */
> + SysBusDevice parent_obj; /* FIXME DeviceState */
> + /* Public */
> +
> + qemu_irq irq;
Why an irq?
> + uint8_t current_state;
Is the state of this device boolean or is it a 0..255 0=off, 255=full
on, analog thing?
Can an LED device be connected to a PWM device driving a GPIO - what
happens?
Dave
> + /* Properties */
> + char *name;
> + uint8_t reset_state; /* TODO [GPIO_ACTIVE_LOW, GPIO_ACTIVE_HIGH] */
> +} LEDState;
> +
> +#endif /* HW_MISC_LED_H */
> diff --git a/hw/misc/led.c b/hw/misc/led.c
> new file mode 100644
> index 0000000000..1bae1a34c0
> --- /dev/null
> +++ b/hw/misc/led.c
> @@ -0,0 +1,84 @@
> +/*
> + * 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 "hw/irq.h"
> +#include "trace.h"
> +
> +static void led_set(void *opaque, int line, int new_state)
> +{
> + LEDState *s = LED(opaque);
> +
> + trace_led_set(s->name, s->current_state, new_state);
> +
> + s->current_state = new_state;
> +}
> +
> +static void led_reset(DeviceState *dev)
> +{
> + LEDState *s = LED(dev);
> +
> + led_set(dev, 0, s->reset_state);
> +}
> +
> +static const VMStateDescription vmstate_led = {
> + .name = TYPE_LED,
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT8(reset_state, LEDState),
I'm not sure you need to migrate this - this is a property that's set on
the device, not a dynamic state of the device
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void led_realize(DeviceState *dev, Error **errp)
> +{
> + LEDState *s = LED(dev);
> +
> + if (s->name == NULL) {
> + error_setg(errp, "property 'name' not specified");
> + return;
> + }
> +
> + qdev_init_gpio_in(DEVICE(s), led_set, 1);
> +}
> +
> +static Property led_properties[] = {
> + DEFINE_PROP_STRING("name", LEDState, name),
> + DEFINE_PROP_UINT8("reset_state", LEDState, reset_state, 0),
I suggest you add a property for the notional colour; that way any UIs
that are built can use that as a hint.
Dave
> + 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_SYS_BUS_DEVICE, /* FIXME 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)
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3abe3faa4e..10593863dc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1857,6 +1857,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 bdd77d8020..f60dce694d 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/Makefile.objs b/hw/misc/Makefile.objs
> index 5aaca8a039..9efa3c941c 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -91,3 +91,4 @@ common-obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
> obj-$(CONFIG_MAC_VIA) += mac_via.o
>
> common-obj-$(CONFIG_GRLIB) += grlib_ahb_apb_pnp.o
> +common-obj-$(CONFIG_LED) += led.o
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 5561746866..e15b7f7c81 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -206,3 +206,6 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int
> addr, int value) "secto
> # grlib_ahb_apb_pnp.c
> 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(const char *name, uint8_t old_state, uint8_t new_state) "led
> name:'%s' state %d -> %d"
> --
> 2.21.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- [RFC PATCH v2 0/5] hw/misc: Add LED device, Philippe Mathieu-Daudé, 2020/06/12
- [RFC PATCH v2 2/5] hw/misc/led: Add LED_STATUS_CHANGED QAPI event, Philippe Mathieu-Daudé, 2020/06/12
- [RFC PATCH v2 3/5] hw/misc/led: Add create_led_by_gpio_id() helper, Philippe Mathieu-Daudé, 2020/06/12
- [RFC PATCH v2 4/5] hw/arm/microbit: Add a fake LED to use as proof-of-concept with Zephyr, Philippe Mathieu-Daudé, 2020/06/12
- [RFC PATCH v2 5/5] hw/arm/tosa: Use LED device for the Bluetooth led, Philippe Mathieu-Daudé, 2020/06/12