[PATCH 0/2] hw/arm/npcm7xx_gpio: Add some pin state QOM

[PATCH 0/2] hw/arm/npcm7xx_gpio: Add some pin state QOM

[Joe Komlodi]

Hi all,

This series adds a couple QOM properties for retrieving and setting pin
state via qom-get and qom-get.

We ran into a situation in multi-SoC simulation where the BMC would need
to update its input pin state based on behavior from the other SoC. It
made the most sense to expose this over QMP, so this adds properties to
allow people to do so.

Since the NPCM7xx is typically used to help manage other SoCs, hopefully
other people will find this useful as well.

Thanks!
Joe

Joe Komlodi (2):
  hw/gpio/npcm7xx: Add GPIO DIN object property
  hw/gpio/npcm7xx: Support qom-get on GPIO pin level

 hw/gpio/npcm7xx_gpio.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

-- 
2.40.0.348.gf938b09366-goog

[PATCH 1/2] hw/gpio/npcm7xx: Add GPIO DIN object property

[Joe Komlodi]

In cases where the input pin is driven by an entity outside of the
machine, such as a machine the BMC is managing, we need a way to
update the pin state when the external machine drives it.

This allows us to do it via QMP.
For example, to set pin 20 on GPIO controller 0:
{"execute":"qom-set","arguments": {
   "path":"/machine/soc/gpio[0]",
   "property":"gpio-pins-in",
   "value":1048576
}}

1048576 == 0x100000, JSON does not support hex.

Signed-off-by: Joe Komlodi <komlodi@google.com>
---
 hw/gpio/npcm7xx_gpio.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/hw/gpio/npcm7xx_gpio.c b/hw/gpio/npcm7xx_gpio.c
index 3376901ab1..2a7be60d8d 100644
--- a/hw/gpio/npcm7xx_gpio.c
+++ b/hw/gpio/npcm7xx_gpio.c
@@ -20,6 +20,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qapi/error.h"
+#include "qapi/visitor.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qemu/units.h"
@@ -340,6 +341,29 @@ static void npcm7xx_gpio_set_input(void *opaque, int line, int level)
     npcm7xx_gpio_update_pins(s, BIT(line));
 }
 
+static void npcm7xx_gpio_get_pins_in(Object *obj, Visitor *v, const char *name,
+                                     void *opaque, Error **errp)
+{
+    uint32_t *pins_in = (uint32_t *)opaque;
+
+    visit_type_uint32(v, name, pins_in, errp);
+}
+
+static void npcm7xx_gpio_set_pins_in(Object *obj, Visitor *v, const char *name,
+                                     void *opaque, Error **errp)
+{
+    NPCM7xxGPIOState *s = NPCM7XX_GPIO(obj);
+    uint32_t new_pins_in;
+
+    if (!visit_type_uint32(v, name, &new_pins_in, errp)) {
+        return;
+    }
+
+    s->ext_driven = new_pins_in;
+    s->ext_level = new_pins_in;
+    npcm7xx_gpio_update_pins(s, new_pins_in);
+}
+
 static void npcm7xx_gpio_enter_reset(Object *obj, ResetType type)
 {
     NPCM7xxGPIOState *s = NPCM7XX_GPIO(obj);
@@ -371,6 +395,10 @@ static void npcm7xx_gpio_init(Object *obj)
 
     qdev_init_gpio_in(dev, npcm7xx_gpio_set_input, NPCM7XX_GPIO_NR_PINS);
     qdev_init_gpio_out(dev, s->output, NPCM7XX_GPIO_NR_PINS);
+
+    object_property_add(obj, "gpio-pins-in", "uint32",
+                        npcm7xx_gpio_get_pins_in,  npcm7xx_gpio_set_pins_in,
+                        NULL, &s->regs[NPCM7XX_GPIO_DIN]);
 }
 
 static const VMStateDescription vmstate_npcm7xx_gpio = {
-- 
2.40.0.348.gf938b09366-goog

[PATCH 2/2] hw/gpio/npcm7xx: Support qom-get on GPIO pin level

[Joe Komlodi]

This goes along with input pin modification. In some cases it's easier
to know the state of all pins on the GPIO controller before modifying
input pins, rather than knowing only the state of input pins.

For example over QMP:
{"execute":"qom-get","arguments":{
    "path":"/machine/soc/gpio[0]",
    "property":"gpio-pin-level"
}}

Signed-off-by: Joe Komlodi <komlodi@google.com>
---
 hw/gpio/npcm7xx_gpio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/gpio/npcm7xx_gpio.c b/hw/gpio/npcm7xx_gpio.c
index 2a7be60d8d..58db3a8d64 100644
--- a/hw/gpio/npcm7xx_gpio.c
+++ b/hw/gpio/npcm7xx_gpio.c
@@ -399,6 +399,9 @@ static void npcm7xx_gpio_init(Object *obj)
     object_property_add(obj, "gpio-pins-in", "uint32",
                         npcm7xx_gpio_get_pins_in,  npcm7xx_gpio_set_pins_in,
                         NULL, &s->regs[NPCM7XX_GPIO_DIN]);
+
+    object_property_add_uint32_ptr(obj, "gpio-pin-level", &s->pin_level,
+                                   OBJ_PROP_FLAG_READ);
 }
 
 static const VMStateDescription vmstate_npcm7xx_gpio = {
-- 
2.40.0.348.gf938b09366-goog

Re: [PATCH 0/2] hw/arm/npcm7xx_gpio: Add some pin state QOM

[Peter Maydell]

On Thu, 6 Apr 2023 at 01:25, Joe Komlodi <komlodi@google.com> wrote:
>
> Hi all,
>
> This series adds a couple QOM properties for retrieving and setting pin
> state via qom-get and qom-get.
>
> We ran into a situation in multi-SoC simulation where the BMC would need
> to update its input pin state based on behavior from the other SoC. It
> made the most sense to expose this over QMP, so this adds properties to
> allow people to do so.

This does leave the simulation in an odd situation if
the input GPIO was connected to some other device -- the
other device thinks it's put the GPIO line low, but then something
external has reached in and set it to 1, so the two ends of
what is conceptually a single signal line now disagree about
what voltage it's at...

It looks like the hw/gpio/aspeed_gpio device has been here before
you, only that device chose to use one bool property per GPIO
line. It would be nice to be consistent -- if we want to allow
QOM to set/get the GPIO line values, it should be the same
interface regardless of GPIO controller.

-- PMM