On 9/14/20 9:27 AM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <f4...@amsat.org> writes: > >> Eduardo is already in Cc, adding Markus. >> >> On 9/12/20 12:44 AM, Richard Henderson wrote: >>> On 9/10/20 1:54 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é <f4...@amsat.org> >>>> --- >>>> include/hw/misc/led.h | 8 ++++++++ >>>> include/hw/qdev-core.h | 8 ++++++++ >>>> hw/misc/led.c | 17 ++++++++++++++++- >>>> 3 files changed, 32 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/hw/misc/led.h b/include/hw/misc/led.h >>>> index f5afaa34bfb..71c9b8c59bf 100644 >>>> --- a/include/hw/misc/led.h >>>> +++ b/include/hw/misc/led.h >>>> @@ -38,10 +38,16 @@ typedef struct LEDState { >>>> /* Public */ >>>> >>>> uint8_t intensity_percent; >>>> + qemu_irq irq; >>>> >>>> /* Properties */ >>>> char *description; >>>> char *color; >>>> + /* >>>> + * When used with GPIO, the intensity at reset is related >>>> + * to the GPIO polarity. >>>> + */ >>>> + bool inverted_polarity; >>> >>> Why are you not using the GpioPolarity enum that you added? >> >> Because it is migrated... >> >> Using DEFINE_PROP_BOOL() is simpler that adding hardware specific >> enum visitor in hw/core/qdev-properties.c (which is included in >> user-mode builds because pulled by the CPU type). > > Yes. > > You could also use DEFINE_PROP_UINT8(), and use it with your enumeration > constants. > > I'd be tempted to ditch the enum entirely. Matter of taste, no big deal > either way.
Done in v6! > >> A sane cleanup would be to make get_enum(), set_enum() >> and set_default_value_enum() public (prefixed with 'qdev_') >> in include/hw/qdev-properties.h. > > Purpose? To be able to define enum properties outside > qdev-properties.c? Yes, to avoid pulling in PCI and MAC address properties into qemu-storage-daemon and linux-user binaries...