[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output |
Date: |
Mon, 14 Sep 2020 09:27:26 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
Philippe Mathieu-Daudé <f4bug@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é <f4bug@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.
> 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?
> Out of the scope of this series, but might be worth it.
>
> Eduardo, Markus, what do you think?
>
> Thanks Richard for reviewing this series!
>
> Phil.
- Re: [PATCH v5 1/7] hw/misc/led: Add a LED device, (continued)
- [PATCH v5 3/7] hw/misc/led: Emit a trace event when LED intensity has changed, Philippe Mathieu-Daudé, 2020/09/10
- [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output, Philippe Mathieu-Daudé, 2020/09/10
- Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output, Richard Henderson, 2020/09/11
- Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output, Philippe Mathieu-Daudé, 2020/09/12
- Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output, Philippe Mathieu-Daudé, 2020/09/12
- Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output,
Markus Armbruster <=
- Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output, Philippe Mathieu-Daudé, 2020/09/14
- Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output, Eduardo Habkost, 2020/09/14
- Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output, Philippe Mathieu-Daudé, 2020/09/14
- Re: [PATCH v5 2/7] hw/misc/led: Allow connecting from GPIO output, Philippe Mathieu-Daudé, 2020/09/14
[PATCH v5 4/7] hw/arm/aspeed: Add the 3 front LEDs drived by the PCA9552 #1, Philippe Mathieu-Daudé, 2020/09/10
[PATCH v5 6/7] hw/misc/mps2-scc: Use the LED device, Philippe Mathieu-Daudé, 2020/09/10
[PATCH v5 5/7] hw/misc/mps2-fpgaio: Use the LED device, Philippe Mathieu-Daudé, 2020/09/10