* [PATCH v2 0/3] Add a simple latching_switch
@ 2022-09-20 18:46 Jian Zhang
2022-09-20 18:46 ` [PATCH v2 1/3] hw/misc/latching_switch: Add a simple latching_switch device Jian Zhang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jian Zhang @ 2022-09-20 18:46 UTC (permalink / raw)
To: f4bug, peter.maydell, clg, andrew, joel, qemu-devel, qemu-arm
Cc: yulei.sh, tangyiwei.2022, chentingting.2150, yuhao.17,
wangxiaohua.1217, xiening.xll
This patchset adds a simple latching_switch, which is a switch that
can be turned on/off by a trigger.
The latching switch is a switch that can be turned on and off.
When the input new state and match the trigger edge, the switch
state will be toggled.
This device privide 2 properties `state(bool)` and `trigger-edge(string)`,
and 2 gpios `input` and `output`.
The `state` property is the initial state of the switch, and the `trigger-edge`
property is the edge that will trigger the switch state to be toggled,
the value can be `rising`, `falling` or `both`.
V2:
- Rename `host-power` to `latching-switch`
- Add `trigger-edge` property
- Add `state` property
- Using anonymous gpio
- Rename power button to input, and power good to output
- Reorder the patchset
- Rebase to latest master
Jian Zhang (3):
hw/misc/latching_switch: Add a simple latching_switch device
hw/gpio/aspeed_gpio: Add gpios in/out init
hw/arm/aspeed: g220a: Add a latching switch device
MAINTAINERS | 2 +
hw/arm/Kconfig | 1 +
hw/arm/aspeed.c | 20 +++
hw/gpio/aspeed_gpio.c | 17 +++
hw/misc/Kconfig | 3 +
hw/misc/latching_switch.c | 212 ++++++++++++++++++++++++++++++
hw/misc/meson.build | 1 +
include/hw/misc/latching_switch.h | 56 ++++++++
8 files changed, 312 insertions(+)
create mode 100644 hw/misc/latching_switch.c
create mode 100644 include/hw/misc/latching_switch.h
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/3] hw/misc/latching_switch: Add a simple latching_switch device
2022-09-20 18:46 [PATCH v2 0/3] Add a simple latching_switch Jian Zhang
@ 2022-09-20 18:46 ` Jian Zhang
2022-10-10 14:24 ` Peter Maydell
2022-09-20 18:46 ` [PATCH v2 2/3] hw/gpio/aspeed_gpio: Add gpios in/out init Jian Zhang
2022-09-20 18:46 ` [PATCH v2 3/3] hw/arm/aspeed: g220a: Add a latching switch device Jian Zhang
2 siblings, 1 reply; 5+ messages in thread
From: Jian Zhang @ 2022-09-20 18:46 UTC (permalink / raw)
To: f4bug, peter.maydell, clg, andrew, joel, qemu-devel, qemu-arm
Cc: yulei.sh, tangyiwei.2022, chentingting.2150, yuhao.17,
wangxiaohua.1217, xiening.xll
Implement a simple latching switch device.
The latching switch is a switch that can be turned on and off.
When the input new state and match the trigger edge, the switch
state will be toggled.
This device privide 2 properties `state(bool)` and `trigger-edge(string)`,
and 2 gpios `input` and `output`.
The `state` property is the initial state of the switch, and the `trigger-edge`
property is the edge that will trigger the switch state to be toggled,
the value can be `rising`, `falling` or `both`.
Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
---
MAINTAINERS | 2 +
hw/misc/Kconfig | 3 +
hw/misc/latching_switch.c | 212 ++++++++++++++++++++++++++++++
hw/misc/meson.build | 1 +
include/hw/misc/latching_switch.h | 56 ++++++++
5 files changed, 274 insertions(+)
create mode 100644 hw/misc/latching_switch.c
create mode 100644 include/hw/misc/latching_switch.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 1729c0901c..0b252a9339 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1066,6 +1066,8 @@ F: include/hw/net/ftgmac100.h
F: docs/system/arm/aspeed.rst
F: tests/qtest/*aspeed*
F: hw/arm/fby35.c
+F: include/hw/misc/latching_switch.h
+F: hw/misc/latching_switch.c
NRF51
M: Joel Stanley <joel@jms.id.au>
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index cbabe9f78c..96345c6456 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -139,6 +139,9 @@ config UNIMP
config LED
bool
+config LATCHING_SWITCH
+ bool
+
config MAC_VIA
bool
select MOS6522
diff --git a/hw/misc/latching_switch.c b/hw/misc/latching_switch.c
new file mode 100644
index 0000000000..03ce40b77c
--- /dev/null
+++ b/hw/misc/latching_switch.c
@@ -0,0 +1,212 @@
+/*
+ * QEMU single Latching Switch device
+ *
+ * Copyright (C) 2022 Jian Zhang <zhangjian.3032@bytedance.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
+#include "hw/irq.h"
+#include "hw/misc/latching_switch.h"
+#include "trace.h"
+
+static void toggle_output(LatchingSwitchState *s)
+{
+ s->state = !(s->state);
+ qemu_set_irq(s->output, !!(s->state));
+}
+
+static void input_handler(void *opaque, int line, int new_state)
+{
+ LatchingSwitchState *s = LATCHING_SWITCH(opaque);
+
+ assert(line == 0);
+
+ if (s->trigger_edge == TRIGGER_EDGE_FALLING && new_state == 0) {
+ toggle_output(s);
+ } else if (s->trigger_edge == TRIGGER_EDGE_RISING && new_state == 1) {
+ toggle_output(s);
+ } else if (s->trigger_edge == TRIGGER_EDGE_BOTH) {
+ toggle_output(s);
+ }
+}
+
+static void latching_switch_reset(DeviceState *dev)
+{
+ LatchingSwitchState *s = LATCHING_SWITCH(dev);
+ /* reset to off */
+ s->state = false;
+ /* reset to falling edge trigger */
+ s->trigger_edge = TRIGGER_EDGE_FALLING;
+}
+
+static const VMStateDescription vmstate_latching_switch = {
+ .name = TYPE_LATCHING_SWITCH,
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_BOOL(state, LatchingSwitchState),
+ VMSTATE_U8(trigger_edge, LatchingSwitchState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void latching_switch_realize(DeviceState *dev, Error **errp)
+{
+ LatchingSwitchState *s = LATCHING_SWITCH(dev);
+
+ /* init a input io */
+ qdev_init_gpio_in(dev, input_handler, 1);
+
+ /* init a output io */
+ qdev_init_gpio_out(dev, &(s->output), 1);
+}
+
+static void latching_switch_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->desc = "Latching Switch";
+ dc->vmsd = &vmstate_latching_switch;
+ dc->reset = latching_switch_reset;
+ dc->realize = latching_switch_realize;
+ set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+}
+
+static void latching_switch_get_state(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ LatchingSwitchState *s = LATCHING_SWITCH(obj);
+ bool value = s->state;
+
+ visit_type_bool(v, name, &value, errp);
+}
+
+static void latching_switch_set_state(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ LatchingSwitchState *s = LATCHING_SWITCH(obj);
+ bool value;
+ Error *err = NULL;
+
+ visit_type_bool(v, name, &value, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ if (value != s->state) {
+ toggle_output(s);
+ }
+}
+static void latching_switch_get_trigger_edge(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ LatchingSwitchState *s = LATCHING_SWITCH(obj);
+ int value = s->trigger_edge;
+ char *p = NULL;
+
+ if (value == TRIGGER_EDGE_FALLING) {
+ p = g_strdup("falling");
+ visit_type_str(v, name, &p, errp);
+ } else if (value == TRIGGER_EDGE_RISING) {
+ p = g_strdup("rising");
+ visit_type_str(v, name, &p, errp);
+ } else if (value == TRIGGER_EDGE_BOTH) {
+ p = g_strdup("both");
+ visit_type_str(v, name, &p, errp);
+ } else {
+ error_setg(errp, "Invalid trigger edge value");
+ }
+ g_free(p);
+}
+
+static void latching_switch_set_trigger_edge(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ LatchingSwitchState *s = LATCHING_SWITCH(obj);
+ char *value;
+ Error *err = NULL;
+
+ visit_type_str(v, name, &value, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ if (strcmp(value, "falling") == 0) {
+ s->trigger_edge = TRIGGER_EDGE_FALLING;
+ } else if (strcmp(value, "rising") == 0) {
+ s->trigger_edge = TRIGGER_EDGE_RISING;
+ } else if (strcmp(value, "both") == 0) {
+ s->trigger_edge = TRIGGER_EDGE_BOTH;
+ } else {
+ error_setg(errp, "Invalid trigger edge type: %s", value);
+ }
+}
+
+static void latching_switch_init(Object *obj)
+{
+ LatchingSwitchState *s = LATCHING_SWITCH(obj);
+
+ s->state = false;
+ s->trigger_edge = TRIGGER_EDGE_FALLING;
+
+ object_property_add(obj, "state", "bool",
+ latching_switch_get_state,
+ latching_switch_set_state,
+ NULL, NULL);
+ object_property_add(obj, "trigger-edge", "string",
+ latching_switch_get_trigger_edge,
+ latching_switch_set_trigger_edge,
+ NULL, NULL);
+}
+
+static const TypeInfo latching_switch_info = {
+ .name = TYPE_LATCHING_SWITCH,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(LatchingSwitchState),
+ .class_init = latching_switch_class_init,
+ .instance_init = latching_switch_init,
+};
+
+static void latching_switch_register_types(void)
+{
+ type_register_static(&latching_switch_info);
+}
+
+type_init(latching_switch_register_types);
+
+LatchingSwitchState *latching_switch_create_simple(Object *parentobj,
+ bool state,
+ uint8_t trigger_edge)
+{
+ static const char *name = "latching-switch";
+ DeviceState *dev;
+
+ dev = qdev_new(TYPE_LATCHING_SWITCH);
+
+ qdev_prop_set_bit(dev, "state", state);
+
+ if (trigger_edge == TRIGGER_EDGE_FALLING) {
+ qdev_prop_set_string(dev, "trigger-edge", "falling");
+ } else if (trigger_edge == TRIGGER_EDGE_RISING) {
+ qdev_prop_set_string(dev, "trigger-edge", "rising");
+ } else if (trigger_edge == TRIGGER_EDGE_BOTH) {
+ qdev_prop_set_string(dev, "trigger-edge", "both");
+ } else {
+ error_report("Invalid trigger edge value");
+ exit(1);
+ }
+
+ object_property_add_child(parentobj, name, OBJECT(dev));
+ qdev_realize_and_unref(dev, NULL, &error_fatal);
+
+ return LATCHING_SWITCH(dev);
+}
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 95268eddc0..23b5c7f69e 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -9,6 +9,7 @@ softmmu_ss.add(when: 'CONFIG_SGA', if_true: files('sga.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'))
+softmmu_ss.add(when: 'CONFIG_LATCHING_SWITCH', if_true: files('latching_switch.c'))
softmmu_ss.add(when: 'CONFIG_PVPANIC_COMMON', if_true: files('pvpanic.c'))
# ARM devices
diff --git a/include/hw/misc/latching_switch.h b/include/hw/misc/latching_switch.h
new file mode 100644
index 0000000000..de1d9d27a4
--- /dev/null
+++ b/include/hw/misc/latching_switch.h
@@ -0,0 +1,56 @@
+/*
+ * QEMU single Latching Switch device
+ *
+ * Copyright (C) Jian Zhang <zhangjian.3032@bytedance.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef HW_MISC_LATCHING_SWITCH_H
+#define HW_MISC_LATCHING_SWITCH_H
+
+#include "qom/object.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_LATCHING_SWITCH "latching-switch"
+
+enum TriggerEdge {
+ TRIGGER_EDGE_FALLING,
+ TRIGGER_EDGE_RISING,
+ TRIGGER_EDGE_BOTH,
+};
+
+struct LatchingSwitchState {
+ /* Private */
+ DeviceState parent_obj;
+
+ /* Public */
+ qemu_irq input;
+ qemu_irq output;
+
+ /* Properties */
+ bool state;
+ uint8_t trigger_edge;
+ char *description;
+};
+typedef struct LatchingSwitchState LatchingSwitchState;
+DECLARE_INSTANCE_CHECKER(LatchingSwitchState, LATCHING_SWITCH,
+ TYPE_LATCHING_SWITCH)
+
+/**
+ * latching_switch_create_simple: Create and realize a device
+ * @parentobj: the parent object
+ * @state: the initial state of the switch
+ * @trigger_edge: the trigger edge of the switch
+ * 0: falling edge
+ * 1: rising edge
+ * 2: both edge
+ *
+ * Create the device state structure, initialize it, and
+ * drop the reference to it (the device is realized).
+ *
+ */
+LatchingSwitchState *latching_switch_create_simple(Object *parentobj,
+ bool state,
+ uint8_t trigger_edge);
+
+#endif /* HW_MISC_LATCHING_SWITCH_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/3] hw/gpio/aspeed_gpio: Add gpios in/out init
2022-09-20 18:46 [PATCH v2 0/3] Add a simple latching_switch Jian Zhang
2022-09-20 18:46 ` [PATCH v2 1/3] hw/misc/latching_switch: Add a simple latching_switch device Jian Zhang
@ 2022-09-20 18:46 ` Jian Zhang
2022-09-20 18:46 ` [PATCH v2 3/3] hw/arm/aspeed: g220a: Add a latching switch device Jian Zhang
2 siblings, 0 replies; 5+ messages in thread
From: Jian Zhang @ 2022-09-20 18:46 UTC (permalink / raw)
To: f4bug, peter.maydell, clg, andrew, joel, qemu-devel, qemu-arm
Cc: yulei.sh, tangyiwei.2022, chentingting.2150, yuhao.17,
wangxiaohua.1217, xiening.xll
Add gpios in/out init for aspeed gpio to add the ability to connect
to other gpio devices.
Based the qdev-core.h comments, If you want to connect a GPIO to other
devices, you need to call qdev_init_gpio_in() or qdev_init_gpio_out().
```
For input gpios:
*
* Outbound GPIO lines can be connected to any qemu_irq, but the common
* case is connecting them to another device's inbound GPIO line, using
* the qemu_irq returned by qdev_get_gpio_in() or qdev_get_gpio_in_named().
For output gpios:
* This function is intended to be used by board code or SoC "container"
* device models to wire up the GPIO lines; usually the return value
* will be passed to qdev_connect_gpio_out() or a similar function to
* connect another device's output GPIO line to this input.
```
Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
---
hw/gpio/aspeed_gpio.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 1e267dd482..51f5f1c386 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -1013,6 +1013,17 @@ static void aspeed_gpio_reset(DeviceState *dev)
memset(s->sets, 0, sizeof(s->sets));
}
+static void aspeed_gpio_set(void *opaque, int line, int new_state)
+{
+ AspeedGPIOState *s = ASPEED_GPIO(opaque);
+ uint32_t set_idx, pin;
+
+ set_idx = line / ASPEED_GPIOS_PER_SET;
+ pin = line % ASPEED_GPIOS_PER_SET;
+
+ aspeed_gpio_set_pin_level(s, set_idx, pin, new_state);
+}
+
static void aspeed_gpio_realize(DeviceState *dev, Error **errp)
{
AspeedGPIOState *s = ASPEED_GPIO(dev);
@@ -1037,6 +1048,12 @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s,
TYPE_ASPEED_GPIO, 0x800);
+ /* TODO: Maybe could in named, not anonymous is better */
+ qdev_init_gpio_out(dev, &s->gpios[0][0],
+ ASPEED_GPIO_MAX_NR_SETS * ASPEED_GPIOS_PER_SET);
+ qdev_init_gpio_in(dev, aspeed_gpio_set,
+ ASPEED_GPIO_MAX_NR_SETS * ASPEED_GPIOS_PER_SET);
+
sysbus_init_mmio(sbd, &s->iomem);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/3] hw/arm/aspeed: g220a: Add a latching switch device
2022-09-20 18:46 [PATCH v2 0/3] Add a simple latching_switch Jian Zhang
2022-09-20 18:46 ` [PATCH v2 1/3] hw/misc/latching_switch: Add a simple latching_switch device Jian Zhang
2022-09-20 18:46 ` [PATCH v2 2/3] hw/gpio/aspeed_gpio: Add gpios in/out init Jian Zhang
@ 2022-09-20 18:46 ` Jian Zhang
2 siblings, 0 replies; 5+ messages in thread
From: Jian Zhang @ 2022-09-20 18:46 UTC (permalink / raw)
To: f4bug, peter.maydell, clg, andrew, joel, qemu-devel, qemu-arm
Cc: yulei.sh, tangyiwei.2022, chentingting.2150, yuhao.17,
wangxiaohua.1217, xiening.xll
Add a latching switch device connect between g220a BMC machind(soc
gpio) as host-power.
The latching switch device default state is off and trigger edge is
falling edge.
Tested:
In qemu, use g220a image
~# ipmitool power status
Chassis Power is off
~# ipmitool power on
Chassis Power Control: Up/On
~# ipmitool power status
Chassis Power is on
~# ipmitool power off
Chassis Power Control: Down/Off
~# ipmitool power status
Chassis Power is off
Signed-off-by: Jian Zhang <zhangjian.3032@bytedance.com>
---
hw/arm/Kconfig | 1 +
hw/arm/aspeed.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 15fa79afd3..f2455db5a0 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -457,6 +457,7 @@ config ASPEED_SOC
select LED
select PMBUS
select MAX31785
+ select LATCHING_SWITCH
config MPS2
bool
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index bc3ecdb619..070de3aeff 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -27,6 +27,7 @@
#include "qemu/units.h"
#include "hw/qdev-clock.h"
#include "sysemu/sysemu.h"
+#include "hw/misc/latching_switch.h"
static struct arm_boot_info aspeed_board_binfo = {
.board_id = -1, /* device-tree-only board */
@@ -666,6 +667,25 @@ static void g220a_bmc_i2c_init(AspeedMachineState *bmc)
};
smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 4), 0x57,
eeprom_buf);
+
+ /* Add a host-power device */
+ LatchingSwitchState *power =
+ latching_switch_create_simple(OBJECT(bmc),
+ false, TRIGGER_EDGE_FALLING);
+
+ /*
+ * connect the input to soc(out, power button)
+ * the power button in g220a is 215
+ */
+ qdev_connect_gpio_out(DEVICE(&bmc->soc.gpio), 215,
+ qdev_get_gpio_in(DEVICE(power), 0));
+
+ /*
+ * connect the output to soc(in, power good signal)
+ * the power good in g220a is 209
+ */
+ qdev_connect_gpio_out(DEVICE(power), 0,
+ qdev_get_gpio_in(DEVICE(&bmc->soc.gpio), 209));
}
static void aspeed_eeprom_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/3] hw/misc/latching_switch: Add a simple latching_switch device
2022-09-20 18:46 ` [PATCH v2 1/3] hw/misc/latching_switch: Add a simple latching_switch device Jian Zhang
@ 2022-10-10 14:24 ` Peter Maydell
0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2022-10-10 14:24 UTC (permalink / raw)
To: Jian Zhang
Cc: f4bug, clg, andrew, joel, qemu-devel, qemu-arm, yulei.sh,
tangyiwei.2022, chentingting.2150, yuhao.17, wangxiaohua.1217,
xiening.xll
On Tue, 20 Sept 2022 at 19:46, Jian Zhang <zhangjian.3032@bytedance.com> wrote:
>
> Implement a simple latching switch device.
>
> The latching switch is a switch that can be turned on and off.
> When the input new state and match the trigger edge, the switch
> state will be toggled.
>
> This device privide 2 properties `state(bool)` and `trigger-edge(string)`,
> and 2 gpios `input` and `output`.
>
> The `state` property is the initial state of the switch, and the `trigger-edge`
> property is the edge that will trigger the switch state to be toggled,
> the value can be `rising`, `falling` or `both`.
> +static void toggle_output(LatchingSwitchState *s)
> +{
> + s->state = !(s->state);
> + qemu_set_irq(s->output, !!(s->state));
s->state is a bool so this !! is unnecessary.
> +}
> +
> +static void input_handler(void *opaque, int line, int new_state)
> +{
> + LatchingSwitchState *s = LATCHING_SWITCH(opaque);
> +
> + assert(line == 0);
> +
> + if (s->trigger_edge == TRIGGER_EDGE_FALLING && new_state == 0) {
> + toggle_output(s);
This won't have the correct behaviour, because the device
on the other end of this input line might call
qemu_set_irq() on it twice in a row with new_state == 0 both times.
It's only a falling edge if new_state is 0 and the old
input line state was not 0.
If you need to detect edges then you need to record the state
of the input line within LatchingSwitchState.
> + } else if (s->trigger_edge == TRIGGER_EDGE_RISING && new_state == 1) {
> + toggle_output(s);
> + } else if (s->trigger_edge == TRIGGER_EDGE_BOTH) {
> + toggle_output(s);
> + }
> +}
> +
> +static void latching_switch_reset(DeviceState *dev)
> +{
> + LatchingSwitchState *s = LATCHING_SWITCH(dev);
> + /* reset to off */
> + s->state = false;
> + /* reset to falling edge trigger */
> + s->trigger_edge = TRIGGER_EDGE_FALLING;
trigger_edge isn't guest-visible runtime modifiable state, it's how
the device or board configures the latch when it creates it, right?
Configuration shouldn't be reset, because if the board creates a
rising-edge switch, it should stay a rising edge switch even if the
guest power-cycles itself.
(If the QOM property can be changed at runtime things get more
complex, but in this case it can't be.)
> +}
> +
> +static const VMStateDescription vmstate_latching_switch = {
> + .name = TYPE_LATCHING_SWITCH,
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_BOOL(state, LatchingSwitchState),
> + VMSTATE_U8(trigger_edge, LatchingSwitchState),
trigger_edge isn't runtime changeable so it doesn't need to be
saved in the vmstate.
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void latching_switch_realize(DeviceState *dev, Error **errp)
> +{
> + LatchingSwitchState *s = LATCHING_SWITCH(dev);
> +
> + /* init a input io */
> + qdev_init_gpio_in(dev, input_handler, 1);
> +
> + /* init a output io */
> + qdev_init_gpio_out(dev, &(s->output), 1);
> +}
> +
> +static void latching_switch_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->desc = "Latching Switch";
> + dc->vmsd = &vmstate_latching_switch;
> + dc->reset = latching_switch_reset;
> + dc->realize = latching_switch_realize;
> + set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
This is definitely not a display device :-)
> +}
> +
> +static void latching_switch_get_state(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + LatchingSwitchState *s = LATCHING_SWITCH(obj);
> + bool value = s->state;
> +
> + visit_type_bool(v, name, &value, errp);
> +}
> +
> +static void latching_switch_set_state(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
What's the requirement for being able to set the output state via a
QOM property ?
> +{
> + LatchingSwitchState *s = LATCHING_SWITCH(obj);
> + bool value;
> + Error *err = NULL;
> +
> + visit_type_bool(v, name, &value, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + if (value != s->state) {
> + toggle_output(s);
This will call qemu_set_irq() on the output, which isn't a valid thing
to do during board creation or in certain stages of reset.
If you need to handle "the board can create a switch which starts off
with its output asserted", that can be done, but it's a little more
complicated (it involves implementing your reset handler as 3-phase-reset).
It looks from patch 3 like your use case in the aspeed board doesn't
require this, so my suggestion would be simply to not implement it:
make the switch always start with the output state being 0.
> + }
> +}
> +static void latching_switch_get_trigger_edge(Object *obj, Visitor *v,
Missing newline between the two functions.
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + LatchingSwitchState *s = LATCHING_SWITCH(obj);
> + int value = s->trigger_edge;
> + char *p = NULL;
> +
> + if (value == TRIGGER_EDGE_FALLING) {
> + p = g_strdup("falling");
> + visit_type_str(v, name, &p, errp);
> + } else if (value == TRIGGER_EDGE_RISING) {
> + p = g_strdup("rising");
> + visit_type_str(v, name, &p, errp);
> + } else if (value == TRIGGER_EDGE_BOTH) {
> + p = g_strdup("both");
> + visit_type_str(v, name, &p, errp);
> + } else {
> + error_setg(errp, "Invalid trigger edge value");
> + }
> + g_free(p);
> +}
> +
> +static void latching_switch_set_trigger_edge(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + LatchingSwitchState *s = LATCHING_SWITCH(obj);
> + char *value;
> + Error *err = NULL;
> +
> + visit_type_str(v, name, &value, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + if (strcmp(value, "falling") == 0) {
> + s->trigger_edge = TRIGGER_EDGE_FALLING;
> + } else if (strcmp(value, "rising") == 0) {
> + s->trigger_edge = TRIGGER_EDGE_RISING;
> + } else if (strcmp(value, "both") == 0) {
> + s->trigger_edge = TRIGGER_EDGE_BOTH;
> + } else {
> + error_setg(errp, "Invalid trigger edge type: %s", value);
> + }
> +}
> +
> +static void latching_switch_init(Object *obj)
> +{
> + LatchingSwitchState *s = LATCHING_SWITCH(obj);
> +
> + s->state = false;
> + s->trigger_edge = TRIGGER_EDGE_FALLING;
> +
> + object_property_add(obj, "state", "bool",
> + latching_switch_get_state,
> + latching_switch_set_state,
> + NULL, NULL);
> + object_property_add(obj, "trigger-edge", "string",
> + latching_switch_get_trigger_edge,
> + latching_switch_set_trigger_edge,
> + NULL, NULL);
> +}
> +
> +static const TypeInfo latching_switch_info = {
> + .name = TYPE_LATCHING_SWITCH,
> + .parent = TYPE_DEVICE,
Don't implement new devices as direct child types of TYPE_DEVICE.
At the moment that makes them never get reset. Make it a child
of TYPE_SYS_BUS_DEVICE instead.
> + .instance_size = sizeof(LatchingSwitchState),
> + .class_init = latching_switch_class_init,
> + .instance_init = latching_switch_init,
> +};
> +
> +static void latching_switch_register_types(void)
> +{
> + type_register_static(&latching_switch_info);
> +}
> +
> +type_init(latching_switch_register_types);
> +
> +LatchingSwitchState *latching_switch_create_simple(Object *parentobj,
> + bool state,
> + uint8_t trigger_edge)
> +{
> + static const char *name = "latching-switch";
> + DeviceState *dev;
> +
> + dev = qdev_new(TYPE_LATCHING_SWITCH);
> +
> + qdev_prop_set_bit(dev, "state", state);
> +
> + if (trigger_edge == TRIGGER_EDGE_FALLING) {
> + qdev_prop_set_string(dev, "trigger-edge", "falling");
> + } else if (trigger_edge == TRIGGER_EDGE_RISING) {
> + qdev_prop_set_string(dev, "trigger-edge", "rising");
> + } else if (trigger_edge == TRIGGER_EDGE_BOTH) {
> + qdev_prop_set_string(dev, "trigger-edge", "both");
> + } else {
> + error_report("Invalid trigger edge value");
> + exit(1);
> + }
> +
> + object_property_add_child(parentobj, name, OBJECT(dev));
> + qdev_realize_and_unref(dev, NULL, &error_fatal);
Current QEMU style doesn't favour providing this sort of
create-and-configure-the-device wrapper function. Instead just
create and set the properties on the device in the board code.
> +
> + return LATCHING_SWITCH(dev);
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-10 14:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-20 18:46 [PATCH v2 0/3] Add a simple latching_switch Jian Zhang
2022-09-20 18:46 ` [PATCH v2 1/3] hw/misc/latching_switch: Add a simple latching_switch device Jian Zhang
2022-10-10 14:24 ` Peter Maydell
2022-09-20 18:46 ` [PATCH v2 2/3] hw/gpio/aspeed_gpio: Add gpios in/out init Jian Zhang
2022-09-20 18:46 ` [PATCH v2 3/3] hw/arm/aspeed: g220a: Add a latching switch device Jian Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).