* [PATCH v2 0/6] hw/gpio/aspeed_sgpio: Add Aspeed Serial GPIO (SGPIO) controller model
@ 2025-12-09 0:01 Yubin Zou
2025-12-09 0:01 ` [PATCH v2 1/6] hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO Yubin Zou
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Yubin Zou @ 2025-12-09 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Jamin Lin, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Kane-Chen-AS, Nabih Estefan,
qemu-arm, Yubin Zou
This series introduces a model for the Aspeed Serial GPIO (SGPIO) controller,
commonly found on Aspeed SoCs such as the AST2700. The SGPIO peripheral
provides a large number of GPIO pins that can be controlled and monitored
serially.
Improvement to QEMU:
These patches enhance QEMU's hardware emulation capabilities for platforms
using Aspeed SoCs, particularly for BMC simulations. By modeling the SGPIO
controller, QEMU can more accurately represent the hardware, allowing for
better development and testing of firmware and software that relies on these
GPIOs for various functions like sensor monitoring, presence detect, and
system control signals.
Impact (Before/After):
Before:
QEMU lacked a model for the Aspeed SGPIO controller. Any guest software
attempting to interact with the SGPIO register space would find no device.
Firmware features depending on SGPIO pin states or interrupts could not be
tested in QEMU.
After:
QEMU emulates the Aspeed SGPIO controller on supported machines (e.g.,
ast2700-evb).
- Guest firmware can configure SGPIO pins, set output values, and read input
values through the memory-mapped registers.
- External entities (like test scripts or other QEMU components) can interact
with the pins via QOM properties (e.g., to simulate external signal changes).
Path example: /machine/soc/sgpio[0]/sgpio0
- The model generates interrupts based on input pin transitions, according to
the configured mode (level/edge), enabling testing of interrupt handlers.
Signed-off-by: Yubin Zou <yubinz@google.com>
---
Changes in v2:
Split the v1 into smaller commits and reorder it for better review:
- Link to v1: https://lore.kernel.org/qemu-devel/20251106-aspeed-sgpio-v1-0-b026093716fa@google.com
---
Yubin Zou (6):
hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO
hw/gpio/aspeed_sgpio: aspeed: Add QOM property accessors for SGPIO pins
hw/gpio/aspeed_sgpio: Implement SGPIO interrupt handling
hw/arm/aspeed_soc: Update Aspeed SoC to support two SGPIO controllers
hw/arm/aspeed_ast27x0: Wire SGPIO controller to AST2700 SoC
test/qtest: Add Unit test for Aspeed SGPIO
hw/arm/aspeed_ast10x0.c | 6 +-
hw/arm/aspeed_ast27x0.c | 26 +++
hw/gpio/aspeed_sgpio.c | 348 +++++++++++++++++++++++++++++++++++++++
hw/gpio/meson.build | 1 +
include/hw/arm/aspeed_soc.h | 8 +-
include/hw/gpio/aspeed_sgpio.h | 68 ++++++++
tests/qtest/ast2700-sgpio-test.c | 152 +++++++++++++++++
tests/qtest/meson.build | 1 +
8 files changed, 605 insertions(+), 5 deletions(-)
---
base-commit: 917ac07f9aef579b9538a81d45f45850aba42906
change-id: 20251105-aspeed-sgpio-1d49de6cea66
Best regards,
--
Yubin Zou <yubinz@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/6] hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO
2025-12-09 0:01 [PATCH v2 0/6] hw/gpio/aspeed_sgpio: Add Aspeed Serial GPIO (SGPIO) controller model Yubin Zou
@ 2025-12-09 0:01 ` Yubin Zou
2025-12-09 9:22 ` Cédric Le Goater
2025-12-09 0:01 ` [PATCH v2 2/6] hw/gpio/aspeed_sgpio: aspeed: Add QOM property accessors for SGPIO pins Yubin Zou
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Yubin Zou @ 2025-12-09 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Jamin Lin, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Kane-Chen-AS, Nabih Estefan,
qemu-arm, Yubin Zou
This initial implementation includes the basic device structure,
memory-mapped register definitions, and read/write handlers for the
SGPIO control registers.
Signed-off-by: Yubin Zou <yubinz@google.com>
---
hw/gpio/aspeed_sgpio.c | 154 +++++++++++++++++++++++++++++++++++++++++
hw/gpio/meson.build | 1 +
include/hw/gpio/aspeed_sgpio.h | 66 ++++++++++++++++++
3 files changed, 221 insertions(+)
diff --git a/hw/gpio/aspeed_sgpio.c b/hw/gpio/aspeed_sgpio.c
new file mode 100644
index 0000000000000000000000000000000000000000..8676fa7ced134f1f62dc9e30b42c5fe6db3de268
--- /dev/null
+++ b/hw/gpio/aspeed_sgpio.c
@@ -0,0 +1,154 @@
+/*
+ * ASPEED Serial GPIO Controller
+ *
+ * Copyright 2025 Google LLC.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/host-utils.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "hw/qdev-properties.h"
+#include "hw/gpio/aspeed_sgpio.h"
+
+static uint64_t aspeed_sgpio_2700_read_int_status_reg(AspeedSGPIOState *s,
+ uint32_t reg)
+{
+ return 0;
+}
+
+static uint64_t aspeed_sgpio_2700_read_control_reg(AspeedSGPIOState *s,
+ uint32_t reg)
+{
+ AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
+ uint32_t idx = reg - R_SGPIO_0_CONTROL;
+ if (idx >= agc->nr_sgpio_pin_pairs) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: pin index: %d, out of bounds\n",
+ __func__, idx);
+ return 0;
+ }
+ return s->ctrl_regs[idx];
+}
+
+static void aspeed_sgpio_2700_write_control_reg(AspeedSGPIOState *s,
+ uint32_t reg, uint64_t data)
+{
+ AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
+ uint32_t idx = reg - R_SGPIO_0_CONTROL;
+ if (idx >= agc->nr_sgpio_pin_pairs) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: pin index: %d, out of bounds\n",
+ __func__, idx);
+ return;
+ }
+ s->ctrl_regs[idx] = data;
+}
+
+static uint64_t aspeed_sgpio_2700_read(void *opaque, hwaddr offset,
+ uint32_t size)
+{
+ AspeedSGPIOState *s = ASPEED_SGPIO(opaque);
+ uint64_t value = 0;
+ uint64_t reg;
+
+ reg = offset >> 2;
+
+ switch (reg) {
+ case R_SGPIO_INT_STATUS_0 ... R_SGPIO_INT_STATUS_7:
+ aspeed_sgpio_2700_read_int_status_reg(s, reg);
+ break;
+ case R_SGPIO_0_CONTROL ... R_SGPIO_255_CONTROL:
+ value = aspeed_sgpio_2700_read_control_reg(s, reg);
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
+ PRIx64"\n", __func__, offset);
+ return 0;
+ }
+
+ return value;
+}
+
+static void aspeed_sgpio_2700_write(void *opaque, hwaddr offset, uint64_t data,
+ uint32_t size)
+{
+ AspeedSGPIOState *s = ASPEED_SGPIO(opaque);
+ uint64_t reg;
+
+ reg = offset >> 2;
+
+ switch (reg) {
+ case R_SGPIO_0_CONTROL ... R_SGPIO_255_CONTROL:
+ aspeed_sgpio_2700_write_control_reg(s, reg, data);
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
+ PRIx64"\n", __func__, offset);
+ return;
+ }
+}
+
+static const MemoryRegionOps aspeed_gpio_2700_ops = {
+ .read = aspeed_sgpio_2700_read,
+ .write = aspeed_sgpio_2700_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid.min_access_size = 4,
+ .valid.max_access_size = 4,
+};
+
+static void aspeed_sgpio_realize(DeviceState *dev, Error **errp)
+{
+ AspeedSGPIOState *s = ASPEED_SGPIO(dev);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
+
+ /* Interrupt parent line */
+ sysbus_init_irq(sbd, &s->irq);
+
+ memory_region_init_io(&s->iomem, OBJECT(s), agc->reg_ops, s,
+ TYPE_ASPEED_SGPIO, agc->mem_size);
+
+ sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void aspeed_sgpio_class_init(ObjectClass *klass, const void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = aspeed_sgpio_realize;
+ dc->desc = "Aspeed SGPIO Controller";
+}
+
+static void aspeed_sgpio_2700_class_init(ObjectClass *klass, const void *data)
+{
+ AspeedSGPIOClass *agc = ASPEED_SGPIO_CLASS(klass);
+ agc->nr_sgpio_pin_pairs = 256;
+ agc->mem_size = 0x1000;
+ agc->reg_ops = &aspeed_gpio_2700_ops;
+}
+
+static const TypeInfo aspeed_sgpio_info = {
+ .name = TYPE_ASPEED_SGPIO,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(AspeedSGPIOState),
+ .class_size = sizeof(AspeedSGPIOClass),
+ .class_init = aspeed_sgpio_class_init,
+ .abstract = true,
+};
+
+static const TypeInfo aspeed_sgpio_ast2700_info = {
+ .name = TYPE_ASPEED_SGPIO "-ast2700",
+ .parent = TYPE_ASPEED_SGPIO,
+ .class_init = aspeed_sgpio_2700_class_init,
+};
+
+static void aspeed_sgpio_register_types(void)
+{
+ type_register_static(&aspeed_sgpio_info);
+ type_register_static(&aspeed_sgpio_ast2700_info);
+}
+
+type_init(aspeed_sgpio_register_types);
diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
index 74840619c01bf4d9a02c058c434c3ec9d2a55bee..6a67ee958faace69ffd3fe08e8ade31ced0faf7e 100644
--- a/hw/gpio/meson.build
+++ b/hw/gpio/meson.build
@@ -16,5 +16,6 @@ system_ss.add(when: 'CONFIG_RASPI', if_true: files(
))
system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
+system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_sgpio.c'))
system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
diff --git a/include/hw/gpio/aspeed_sgpio.h b/include/hw/gpio/aspeed_sgpio.h
new file mode 100644
index 0000000000000000000000000000000000000000..ffdc54a112da8962a7bc5773d524f1d86eb85d39
--- /dev/null
+++ b/include/hw/gpio/aspeed_sgpio.h
@@ -0,0 +1,66 @@
+/*
+ * ASPEED Serial GPIO Controller
+ *
+ * Copyright 2025 Google LLC.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef ASPEED_SGPIO_H
+#define ASPEED_SGPIO_H
+
+#include "hw/sysbus.h"
+#include "qom/object.h"
+#include "hw/registerfields.h"
+
+#define TYPE_ASPEED_SGPIO "aspeed.sgpio"
+OBJECT_DECLARE_TYPE(AspeedSGPIOState, AspeedSGPIOClass, ASPEED_SGPIO)
+
+#define ASPEED_SGPIO_MAX_PIN_PAIR 256
+#define ASPEED_SGPIO_MAX_INT 8
+
+/* AST2700 SGPIO Register Address Offsets */
+REG32(SGPIO_INT_STATUS_0, 0x40)
+REG32(SGPIO_INT_STATUS_1, 0x44)
+REG32(SGPIO_INT_STATUS_2, 0x48)
+REG32(SGPIO_INT_STATUS_3, 0x4C)
+REG32(SGPIO_INT_STATUS_4, 0x50)
+REG32(SGPIO_INT_STATUS_5, 0x54)
+REG32(SGPIO_INT_STATUS_6, 0x58)
+REG32(SGPIO_INT_STATUS_7, 0x5C)
+/* AST2700 SGPIO_0 - SGPIO_255 Control Register */
+REG32(SGPIO_0_CONTROL, 0x80)
+ SHARED_FIELD(SGPIO_SERIAL_OUT_VAL, 0, 1)
+ SHARED_FIELD(SGPIO_PARALLEL_OUT_VAL, 1, 1)
+ SHARED_FIELD(SGPIO_INT_EN, 2, 1)
+ SHARED_FIELD(SGPIO_INT_TYPE, 3, 3)
+ SHARED_FIELD(SGPIO_RESET_POLARITY, 6, 1)
+ SHARED_FIELD(SGPIO_RESERVED_1, 7, 2)
+ SHARED_FIELD(SGPIO_INPUT_MASK, 9, 1)
+ SHARED_FIELD(SGPIO_PARALLEL_EN, 10, 1)
+ SHARED_FIELD(SGPIO_PARALLEL_IN_MODE, 11, 1)
+ SHARED_FIELD(SGPIO_INT_STATUS, 12, 1)
+ SHARED_FIELD(SGPIO_SERIAL_IN_VAL, 13, 1)
+ SHARED_FIELD(SGPIO_PARALLEL_IN_VAL, 14, 1)
+ SHARED_FIELD(SGPIO_RESERVED_2, 15, 12)
+ SHARED_FIELD(SGPIO_WRITE_PROTECT, 31, 1)
+REG32(SGPIO_255_CONTROL, 0x47C)
+
+struct AspeedSGPIOClass {
+ SysBusDevice parent_obj;
+ uint32_t nr_sgpio_pin_pairs;
+ uint64_t mem_size;
+ const MemoryRegionOps *reg_ops;
+};
+
+struct AspeedSGPIOState {
+ /* <private> */
+ SysBusDevice parent;
+
+ /*< public >*/
+ MemoryRegion iomem;
+ qemu_irq irq;
+ uint32_t ctrl_regs[ASPEED_SGPIO_MAX_PIN_PAIR];
+ uint32_t int_regs[ASPEED_SGPIO_MAX_INT];
+};
+
+#endif /* ASPEED_SGPIO_H */
--
2.52.0.223.gf5cc29aaa4-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/6] hw/gpio/aspeed_sgpio: aspeed: Add QOM property accessors for SGPIO pins
2025-12-09 0:01 [PATCH v2 0/6] hw/gpio/aspeed_sgpio: Add Aspeed Serial GPIO (SGPIO) controller model Yubin Zou
2025-12-09 0:01 ` [PATCH v2 1/6] hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO Yubin Zou
@ 2025-12-09 0:01 ` Yubin Zou
2025-12-09 14:52 ` Cédric Le Goater
2025-12-09 0:01 ` [PATCH v2 3/6] hw/gpio/aspeed_sgpio: Implement SGPIO interrupt handling Yubin Zou
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Yubin Zou @ 2025-12-09 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Jamin Lin, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Kane-Chen-AS, Nabih Estefan,
qemu-arm, Yubin Zou
This commit adds QOM property accessors for the Aspeed SGPIO pins.
The `aspeed_sgpio_get_pin` and `aspeed_sgpio_set_pin` functions are
implemented to get and set the level of individual SGPIO pins. These
are then exposed as boolean properties on the SGPIO device object.
Signed-off-by: Yubin Zou <yubinz@google.com>
---
hw/gpio/aspeed_sgpio.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/hw/gpio/aspeed_sgpio.c b/hw/gpio/aspeed_sgpio.c
index 8676fa7ced134f1f62dc9e30b42c5fe6db3de268..efa7e574abe87e33e58ac88dba5e3469c6702b83 100644
--- a/hw/gpio/aspeed_sgpio.c
+++ b/hw/gpio/aspeed_sgpio.c
@@ -91,6 +91,73 @@ static void aspeed_sgpio_2700_write(void *opaque, hwaddr offset, uint64_t data,
}
}
+static bool aspeed_sgpio_get_pin_level(AspeedSGPIOState *s, int pin)
+{
+ uint32_t value = s->ctrl_regs[pin >> 1];
+ bool is_input = !(pin % 2);
+ uint32_t bit_mask = 0;
+
+ if (is_input) {
+ bit_mask = SGPIO_SERIAL_IN_VAL_MASK;
+ } else {
+ bit_mask = SGPIO_SERIAL_OUT_VAL_MASK;
+ }
+
+ return value & bit_mask;
+}
+
+static void aspeed_sgpio_set_pin_level(AspeedSGPIOState *s, int pin, bool level)
+{
+ uint32_t value = s->ctrl_regs[pin >> 1];
+ bool is_input = !(pin % 2);
+ uint32_t bit_mask = 0;
+
+ if (is_input) {
+ bit_mask = SGPIO_SERIAL_IN_VAL_MASK;
+ } else {
+ bit_mask = SGPIO_SERIAL_OUT_VAL_MASK;
+ }
+
+ if (level) {
+ value |= bit_mask;
+ } else {
+ value &= ~bit_mask;
+ }
+ s->ctrl_regs[pin >> 1] = value;
+}
+
+static void aspeed_sgpio_get_pin(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ bool level = true;
+ int pin = 0xfff;
+ AspeedSGPIOState *s = ASPEED_SGPIO(obj);
+
+ if (sscanf(name, "sgpio%d", &pin) != 1) {
+ error_setg(errp, "%s: error reading %s", __func__, name);
+ return;
+ }
+ level = aspeed_sgpio_get_pin_level(s, pin);
+ visit_type_bool(v, name, &level, errp);
+}
+
+static void aspeed_sgpio_set_pin(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ bool level;
+ int pin = 0xfff;
+ AspeedSGPIOState *s = ASPEED_SGPIO(obj);
+
+ if (!visit_type_bool(v, name, &level, errp)) {
+ return;
+ }
+ if (sscanf(name, "sgpio%d", &pin) != 1) {
+ error_setg(errp, "%s: error reading %s", __func__, name);
+ return;
+ }
+ aspeed_sgpio_set_pin_level(s, pin, level);
+}
+
static const MemoryRegionOps aspeed_gpio_2700_ops = {
.read = aspeed_sgpio_2700_read,
.write = aspeed_sgpio_2700_write,
@@ -114,6 +181,16 @@ static void aspeed_sgpio_realize(DeviceState *dev, Error **errp)
sysbus_init_mmio(sbd, &s->iomem);
}
+static void aspeed_sgpio_init(Object *obj)
+{
+ for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR * 2; i++) {
+ char *name = g_strdup_printf("sgpio%d", i);
+ object_property_add(obj, name, "bool", aspeed_sgpio_get_pin,
+ aspeed_sgpio_set_pin, NULL, NULL);
+ g_free(name);
+ }
+}
+
static void aspeed_sgpio_class_init(ObjectClass *klass, const void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -143,6 +220,7 @@ static const TypeInfo aspeed_sgpio_ast2700_info = {
.name = TYPE_ASPEED_SGPIO "-ast2700",
.parent = TYPE_ASPEED_SGPIO,
.class_init = aspeed_sgpio_2700_class_init,
+ .instance_init = aspeed_sgpio_init,
};
static void aspeed_sgpio_register_types(void)
--
2.52.0.223.gf5cc29aaa4-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/6] hw/gpio/aspeed_sgpio: Implement SGPIO interrupt handling
2025-12-09 0:01 [PATCH v2 0/6] hw/gpio/aspeed_sgpio: Add Aspeed Serial GPIO (SGPIO) controller model Yubin Zou
2025-12-09 0:01 ` [PATCH v2 1/6] hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO Yubin Zou
2025-12-09 0:01 ` [PATCH v2 2/6] hw/gpio/aspeed_sgpio: aspeed: Add QOM property accessors for SGPIO pins Yubin Zou
@ 2025-12-09 0:01 ` Yubin Zou
2025-12-09 0:01 ` [PATCH v2 4/6] hw/arm/aspeed_soc: Update Aspeed SoC to support two SGPIO controllers Yubin Zou
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Yubin Zou @ 2025-12-09 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Jamin Lin, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Kane-Chen-AS, Nabih Estefan,
qemu-arm, Yubin Zou
The SGPIO controller can generate interrupts based on various pin state
changes, such as rising/falling edges or high/low levels. This change
adds the necessary logic to detect these events, update the interrupt
status registers, and signal the interrupt to the SoC.
Signed-off-by: Yubin Zou <yubinz@google.com>
---
hw/gpio/aspeed_sgpio.c | 126 +++++++++++++++++++++++++++++++++++++++--
include/hw/gpio/aspeed_sgpio.h | 2 +
2 files changed, 123 insertions(+), 5 deletions(-)
diff --git a/hw/gpio/aspeed_sgpio.c b/hw/gpio/aspeed_sgpio.c
index efa7e574abe87e33e58ac88dba5e3469c6702b83..ec5bc4e8351f89d86db0816fdd875ff9b3c99cc1 100644
--- a/hw/gpio/aspeed_sgpio.c
+++ b/hw/gpio/aspeed_sgpio.c
@@ -12,15 +12,131 @@
#include "qemu/error-report.h"
#include "qapi/error.h"
#include "qapi/visitor.h"
+#include "hw/irq.h"
#include "hw/qdev-properties.h"
#include "hw/gpio/aspeed_sgpio.h"
+/*
+ * For each set of gpios there are three sensitivity registers that control
+ * the interrupt trigger mode.
+ *
+ * | 2 | 1 | 0 | trigger mode
+ * -----------------------------
+ * | 0 | 0 | 0 | falling-edge
+ * | 0 | 0 | 1 | rising-edge
+ * | 0 | 1 | 0 | level-low
+ * | 0 | 1 | 1 | level-high
+ * | 1 | X | X | dual-edge
+ */
+
+/* GPIO Interrupt Triggers */
+#define ASPEED_FALLING_EDGE 0
+#define ASPEED_RISING_EDGE 1
+#define ASPEED_LEVEL_LOW 2
+#define ASPEED_LEVEL_HIGH 3
+#define ASPEED_DUAL_EDGE 4
+
+static void aspeed_clear_irq(AspeedSGPIOState *s, int idx)
+{
+ uint32_t reg_index = idx / 32;
+ uint32_t bit_index = idx % 32;
+ uint32_t pending = extract32(s->int_regs[reg_index], bit_index, 1);
+
+ assert(s->pending >= pending);
+
+ /* No change to s->pending if pending is 0 */
+ s->pending -= pending;
+
+ /*
+ * The write acknowledged the interrupt regardless of whether it
+ * was pending or not. The post-condition is that it mustn't be
+ * pending. Unconditionally clear the status bit.
+ */
+ s->int_regs[reg_index] = deposit32(s->int_regs[reg_index], bit_index, 1, 0);
+}
+
+static void aspeed_evaluate_irq(AspeedSGPIOState *s, int sgpio_prev_high,
+ int sgpio_curr_high, int idx)
+{
+ uint32_t ctrl = s->ctrl_regs[idx];
+ uint32_t falling_edge = 0, rising_edge = 0;
+ uint32_t int_trigger = SHARED_FIELD_EX32(ctrl, SGPIO_INT_TYPE);
+ uint32_t int_enabled = SHARED_FIELD_EX32(ctrl, SGPIO_INT_EN);
+ uint32_t reg_index = idx / 32;
+ uint32_t bit_index = idx % 32;
+
+ if (!int_enabled) {
+ return;
+ }
+
+ /* Detect edges */
+ if (sgpio_curr_high && !sgpio_prev_high) {
+ rising_edge = 1;
+ } else if (!sgpio_curr_high && sgpio_prev_high) {
+ falling_edge = 1;
+ }
+
+ if (((int_trigger == ASPEED_FALLING_EDGE) && falling_edge) ||
+ ((int_trigger == ASPEED_RISING_EDGE) && rising_edge) ||
+ ((int_trigger == ASPEED_LEVEL_LOW) && !sgpio_curr_high) ||
+ ((int_trigger == ASPEED_LEVEL_HIGH) && sgpio_curr_high) ||
+ ((int_trigger >= ASPEED_DUAL_EDGE) && (rising_edge || falling_edge)))
+ {
+ s->int_regs[reg_index] = deposit32(s->int_regs[reg_index],
+ bit_index, 1, 1);
+ /* Trigger the VIC IRQ */
+ s->pending++;
+ }
+}
+
+static void aspeed_sgpio_update(AspeedSGPIOState *s, uint32_t idx,
+ uint32_t value)
+{
+ uint32_t old = s->ctrl_regs[idx];
+ uint32_t new = value;
+ uint32_t diff = (old ^ new);
+ if (diff) {
+ /* If the interrupt clear bit is set */
+ if (SHARED_FIELD_EX32(new, SGPIO_INT_STATUS)) {
+ aspeed_clear_irq(s, idx);
+ /* Clear the interrupt clear bit */
+ new &= ~SGPIO_INT_STATUS_MASK;
+ }
+
+ /* Uppdate the control register. */
+ s->ctrl_regs[idx] = new;
+
+ /* If the output value is changed */
+ if (SHARED_FIELD_EX32(diff, SGPIO_SERIAL_OUT_VAL)) {
+ /* ...trigger the line-state IRQ */
+ qemu_set_irq(s->sgpios[idx], 1);
+ }
+
+ /* If the input value is changed */
+ if (SHARED_FIELD_EX32(diff, SGPIO_SERIAL_IN_VAL)) {
+ aspeed_evaluate_irq(s,
+ SHARED_FIELD_EX32(old, SGPIO_SERIAL_IN_VAL),
+ SHARED_FIELD_EX32(new, SGPIO_SERIAL_IN_VAL),
+ idx);
+ }
+ }
+ qemu_set_irq(s->irq, !!(s->pending));
+}
+
static uint64_t aspeed_sgpio_2700_read_int_status_reg(AspeedSGPIOState *s,
- uint32_t reg)
+ uint32_t reg)
{
- return 0;
+ uint32_t idx = reg - R_SGPIO_INT_STATUS_0;
+ if (idx >= ASPEED_SGPIO_MAX_INT) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: interrupt status index: %d, out of bounds\n",
+ __func__, idx);
+ return 0;
+ }
+ return s->int_regs[idx];
}
+
static uint64_t aspeed_sgpio_2700_read_control_reg(AspeedSGPIOState *s,
uint32_t reg)
{
@@ -44,7 +160,7 @@ static void aspeed_sgpio_2700_write_control_reg(AspeedSGPIOState *s,
__func__, idx);
return;
}
- s->ctrl_regs[idx] = data;
+ aspeed_sgpio_update(s, idx, data);
}
static uint64_t aspeed_sgpio_2700_read(void *opaque, hwaddr offset,
@@ -58,7 +174,7 @@ static uint64_t aspeed_sgpio_2700_read(void *opaque, hwaddr offset,
switch (reg) {
case R_SGPIO_INT_STATUS_0 ... R_SGPIO_INT_STATUS_7:
- aspeed_sgpio_2700_read_int_status_reg(s, reg);
+ value = aspeed_sgpio_2700_read_int_status_reg(s, reg);
break;
case R_SGPIO_0_CONTROL ... R_SGPIO_255_CONTROL:
value = aspeed_sgpio_2700_read_control_reg(s, reg);
@@ -123,7 +239,7 @@ static void aspeed_sgpio_set_pin_level(AspeedSGPIOState *s, int pin, bool level)
} else {
value &= ~bit_mask;
}
- s->ctrl_regs[pin >> 1] = value;
+ aspeed_sgpio_update(s, pin >> 1, value);
}
static void aspeed_sgpio_get_pin(Object *obj, Visitor *v, const char *name,
diff --git a/include/hw/gpio/aspeed_sgpio.h b/include/hw/gpio/aspeed_sgpio.h
index ffdc54a112da8962a7bc5773d524f1d86eb85d39..5dddab80848937417b5f99f1b52b44f62893bda9 100644
--- a/include/hw/gpio/aspeed_sgpio.h
+++ b/include/hw/gpio/aspeed_sgpio.h
@@ -58,7 +58,9 @@ struct AspeedSGPIOState {
/*< public >*/
MemoryRegion iomem;
+ int pending;
qemu_irq irq;
+ qemu_irq sgpios[ASPEED_SGPIO_MAX_PIN_PAIR];
uint32_t ctrl_regs[ASPEED_SGPIO_MAX_PIN_PAIR];
uint32_t int_regs[ASPEED_SGPIO_MAX_INT];
};
--
2.52.0.223.gf5cc29aaa4-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/6] hw/arm/aspeed_soc: Update Aspeed SoC to support two SGPIO controllers
2025-12-09 0:01 [PATCH v2 0/6] hw/gpio/aspeed_sgpio: Add Aspeed Serial GPIO (SGPIO) controller model Yubin Zou
` (2 preceding siblings ...)
2025-12-09 0:01 ` [PATCH v2 3/6] hw/gpio/aspeed_sgpio: Implement SGPIO interrupt handling Yubin Zou
@ 2025-12-09 0:01 ` Yubin Zou
2025-12-09 16:15 ` Cédric Le Goater
2025-12-09 0:01 ` [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Wire SGPIO controller to AST2700 SoC Yubin Zou
2025-12-09 0:01 ` [PATCH v2 6/6] test/qtest: Add Unit test for Aspeed SGPIO Yubin Zou
5 siblings, 1 reply; 13+ messages in thread
From: Yubin Zou @ 2025-12-09 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Jamin Lin, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Kane-Chen-AS, Nabih Estefan,
qemu-arm, Yubin Zou
This commit updates the Aspeed SoC model to support two SGPIO
controllers, reflecting the hardware capabilities of the AST2700
The memory map and interrupt map are updated to include entries for
two SGPIO controllers (SGPIOM0 and SGPIOM1). This change is a
prerequisite for the full implementation of the SGPIO device model.
Signed-off-by: Yubin Zou <yubinz@google.com>
---
hw/arm/aspeed_ast10x0.c | 6 +++---
hw/arm/aspeed_ast27x0.c | 10 ++++++++++
include/hw/arm/aspeed_soc.h | 8 ++++++--
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 7f49c13391be0b923e317409a0fccfa741f5e658..c141cc080422579ca6b6965369d84dfbe416247b 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -36,7 +36,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
[ASPEED_DEV_ESPI] = 0x7E6EE000,
[ASPEED_DEV_SBC] = 0x7E6F2000,
[ASPEED_DEV_GPIO] = 0x7E780000,
- [ASPEED_DEV_SGPIOM] = 0x7E780500,
+ [ASPEED_DEV_SGPIOM0] = 0x7E780500,
[ASPEED_DEV_TIMER1] = 0x7E782000,
[ASPEED_DEV_UART1] = 0x7E783000,
[ASPEED_DEV_UART2] = 0x7E78D000,
@@ -94,7 +94,7 @@ static const int aspeed_soc_ast1030_irqmap[] = {
[ASPEED_DEV_I2C] = 110, /* 110 ~ 123 */
[ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
[ASPEED_DEV_UDC] = 9,
- [ASPEED_DEV_SGPIOM] = 51,
+ [ASPEED_DEV_SGPIOM0] = 51,
[ASPEED_DEV_JTAG0] = 27,
[ASPEED_DEV_JTAG1] = 53,
};
@@ -427,7 +427,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
sc->memmap[ASPEED_DEV_UDC], 0x1000);
aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->sgpiom),
"aspeed.sgpiom",
- sc->memmap[ASPEED_DEV_SGPIOM], 0x100);
+ sc->memmap[ASPEED_DEV_SGPIOM0], 0x100);
aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->jtag[0]),
"aspeed.jtag",
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index c484bcd4e22fb49faf9c16992ae2cdfd6cd82da4..e5f04bd16e80696e41005d9062a6df6d060b8088 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -69,6 +69,8 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
[ASPEED_DEV_ADC] = 0x14C00000,
[ASPEED_DEV_SCUIO] = 0x14C02000,
[ASPEED_DEV_GPIO] = 0x14C0B000,
+ [ASPEED_DEV_SGPIOM0] = 0x14C0C000,
+ [ASPEED_DEV_SGPIOM1] = 0x14C0D000,
[ASPEED_DEV_I2C] = 0x14C0F000,
[ASPEED_DEV_INTCIO] = 0x14C18000,
[ASPEED_DEV_PCIE_PHY2] = 0x14C1C000,
@@ -122,6 +124,8 @@ static const int aspeed_soc_ast2700a0_irqmap[] = {
[ASPEED_DEV_KCS] = 128,
[ASPEED_DEV_ADC] = 130,
[ASPEED_DEV_GPIO] = 130,
+ [ASPEED_DEV_SGPIOM0] = 130,
+ [ASPEED_DEV_SGPIOM1] = 130,
[ASPEED_DEV_I2C] = 130,
[ASPEED_DEV_FMC] = 131,
[ASPEED_DEV_WDT] = 131,
@@ -173,6 +177,8 @@ static const int aspeed_soc_ast2700a1_irqmap[] = {
[ASPEED_DEV_I2C] = 194,
[ASPEED_DEV_ADC] = 194,
[ASPEED_DEV_GPIO] = 194,
+ [ASPEED_DEV_SGPIOM0] = 194,
+ [ASPEED_DEV_SGPIOM1] = 194,
[ASPEED_DEV_FMC] = 195,
[ASPEED_DEV_WDT] = 195,
[ASPEED_DEV_PWM] = 195,
@@ -214,6 +220,8 @@ static const int ast2700_gic130_gic194_intcmap[] = {
[ASPEED_DEV_I2C] = 0,
[ASPEED_DEV_ADC] = 16,
[ASPEED_DEV_GPIO] = 18,
+ [ASPEED_DEV_SGPIOM0] = 21,
+ [ASPEED_DEV_SGPIOM1] = 24,
};
/* GICINT 131 */
@@ -1061,6 +1069,7 @@ static void aspeed_soc_ast2700a0_class_init(ObjectClass *oc, const void *data)
sc->sram_size = 0x20000;
sc->pcie_num = 0;
sc->spis_num = 3;
+ sc->sgpio_num = 2;
sc->ehcis_num = 2;
sc->wdts_num = 8;
sc->macs_num = 1;
@@ -1089,6 +1098,7 @@ static void aspeed_soc_ast2700a1_class_init(ObjectClass *oc, const void *data)
sc->sram_size = 0x20000;
sc->pcie_num = 3;
sc->spis_num = 3;
+ sc->sgpio_num = 2;
sc->ehcis_num = 4;
sc->wdts_num = 8;
sc->macs_num = 3;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 4b8e599f1a53bfb2e4d3196d5495cd316f799354..18ff961a38508c5df83b46e187f732d736443f20 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -32,6 +32,7 @@
#include "hw/net/ftgmac100.h"
#include "target/arm/cpu.h"
#include "hw/gpio/aspeed_gpio.h"
+#include "hw/gpio/aspeed_sgpio.h"
#include "hw/sd/aspeed_sdhci.h"
#include "hw/usb/hcd-ehci.h"
#include "qom/object.h"
@@ -46,6 +47,7 @@
#define VBOOTROM_FILE_NAME "ast27x0_bootrom.bin"
#define ASPEED_SPIS_NUM 3
+#define ASPEED_SGPIO_NUM 2
#define ASPEED_EHCIS_NUM 4
#define ASPEED_WDTS_NUM 8
#define ASPEED_CPUS_NUM 4
@@ -89,6 +91,7 @@ struct AspeedSoCState {
AspeedMiiState mii[ASPEED_MACS_NUM];
AspeedGPIOState gpio;
AspeedGPIOState gpio_1_8v;
+ AspeedSGPIOState sgpiom[ASPEED_SGPIO_NUM];
AspeedSDHCIState sdhci;
AspeedSDHCIState emmc;
AspeedLPCState lpc;
@@ -106,7 +109,6 @@ struct AspeedSoCState {
UnimplementedDeviceState pwm;
UnimplementedDeviceState espi;
UnimplementedDeviceState udc;
- UnimplementedDeviceState sgpiom;
UnimplementedDeviceState ltpi;
UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
AspeedAPB2OPBState fsi[2];
@@ -166,6 +168,7 @@ struct AspeedSoCClass {
uint64_t secsram_size;
int pcie_num;
int spis_num;
+ int sgpio_num;
int ehcis_num;
int wdts_num;
int macs_num;
@@ -221,6 +224,8 @@ enum {
ASPEED_DEV_SDHCI,
ASPEED_DEV_GPIO,
ASPEED_DEV_GPIO_1_8V,
+ ASPEED_DEV_SGPIOM0,
+ ASPEED_DEV_SGPIOM1,
ASPEED_DEV_RTC,
ASPEED_DEV_TIMER1,
ASPEED_DEV_TIMER2,
@@ -263,7 +268,6 @@ enum {
ASPEED_DEV_I3C,
ASPEED_DEV_ESPI,
ASPEED_DEV_UDC,
- ASPEED_DEV_SGPIOM,
ASPEED_DEV_JTAG0,
ASPEED_DEV_JTAG1,
ASPEED_DEV_FSI1,
--
2.52.0.223.gf5cc29aaa4-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Wire SGPIO controller to AST2700 SoC
2025-12-09 0:01 [PATCH v2 0/6] hw/gpio/aspeed_sgpio: Add Aspeed Serial GPIO (SGPIO) controller model Yubin Zou
` (3 preceding siblings ...)
2025-12-09 0:01 ` [PATCH v2 4/6] hw/arm/aspeed_soc: Update Aspeed SoC to support two SGPIO controllers Yubin Zou
@ 2025-12-09 0:01 ` Yubin Zou
2025-12-09 16:15 ` Cédric Le Goater
2025-12-09 0:01 ` [PATCH v2 6/6] test/qtest: Add Unit test for Aspeed SGPIO Yubin Zou
5 siblings, 1 reply; 13+ messages in thread
From: Yubin Zou @ 2025-12-09 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Jamin Lin, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Kane-Chen-AS, Nabih Estefan,
qemu-arm, Yubin Zou
This commit integrates the Aspeed SGPIO controller into the AST2700
Signed-off-by: Yubin Zou <yubinz@google.com>
---
hw/arm/aspeed_ast27x0.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index e5f04bd16e80696e41005d9062a6df6d060b8088..787accadbecae376d0c747d054ec6372785375b1 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -519,6 +519,11 @@ static void aspeed_soc_ast2700_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
object_initialize_child(obj, "gpio", &s->gpio, typename);
+ snprintf(typename, sizeof(typename), "aspeed.sgpio-%s", socname);
+ for (i = 0; i < sc->sgpio_num; i++) {
+ object_initialize_child(obj, "sgpio[*]", &s->sgpiom[i], typename);
+ }
+
object_initialize_child(obj, "rtc", &s->rtc, TYPE_ASPEED_RTC);
snprintf(typename, sizeof(typename), "aspeed.sdhci-%s", socname);
@@ -973,6 +978,17 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
aspeed_soc_ast2700_get_irq(s, ASPEED_DEV_GPIO));
+ /* SGPIO */
+ for (i = 0; i < sc->sgpio_num; i++) {
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->sgpiom[i]), errp)) {
+ return;
+ }
+ aspeed_mmio_map(s->memory, SYS_BUS_DEVICE(&s->sgpiom[i]), 0,
+ sc->memmap[ASPEED_DEV_SGPIOM0 + i]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->sgpiom[i]), 0,
+ aspeed_soc_ast2700_get_irq(s, ASPEED_DEV_SGPIOM0 + i));
+ }
+
/* RTC */
if (!sysbus_realize(SYS_BUS_DEVICE(&s->rtc), errp)) {
return;
--
2.52.0.223.gf5cc29aaa4-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 6/6] test/qtest: Add Unit test for Aspeed SGPIO
2025-12-09 0:01 [PATCH v2 0/6] hw/gpio/aspeed_sgpio: Add Aspeed Serial GPIO (SGPIO) controller model Yubin Zou
` (4 preceding siblings ...)
2025-12-09 0:01 ` [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Wire SGPIO controller to AST2700 SoC Yubin Zou
@ 2025-12-09 0:01 ` Yubin Zou
5 siblings, 0 replies; 13+ messages in thread
From: Yubin Zou @ 2025-12-09 0:01 UTC (permalink / raw)
To: qemu-devel
Cc: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Jamin Lin, Andrew Jeffery, Joel Stanley, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini, Kane-Chen-AS, Nabih Estefan,
qemu-arm, Yubin Zou
This commit introduces a new qtest for the Aspeed SGPIO controller
The test covers the following:
- Setting and clearing SGPIO output pins and verifying the pin state.
- Setting and clearing SGPIO input pins and verifying the pin state.
- Verifying that level-high interrupts are correctly triggered and cleared.
Signed-off-by: Yubin Zou <yubinz@google.com>
---
tests/qtest/ast2700-sgpio-test.c | 152 +++++++++++++++++++++++++++++++++++++++
tests/qtest/meson.build | 1 +
2 files changed, 153 insertions(+)
diff --git a/tests/qtest/ast2700-sgpio-test.c b/tests/qtest/ast2700-sgpio-test.c
new file mode 100644
index 0000000000000000000000000000000000000000..fc52839c4b149a3010c6a035d8b29f9ad295930a
--- /dev/null
+++ b/tests/qtest/ast2700-sgpio-test.c
@@ -0,0 +1,152 @@
+/*
+ * QTest testcase for the ASPEED AST2700 GPIO Controller.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2025 Google LLC.
+ */
+
+#include "qemu/osdep.h"
+#include "qobject/qdict.h"
+#include "qemu/bitops.h"
+#include "qemu/timer.h"
+#include "qobject/qdict.h"
+#include "libqtest-single.h"
+#include "qemu/error-report.h"
+#include "hw/registerfields.h"
+#include "hw/gpio/aspeed_sgpio.h"
+
+#define ASPEED_SGPIO_MAX_PIN_PAIR 256
+#define AST2700_SGPIO0_BASE 0x14C0C000
+#define AST2700_SGPIO1_BASE 0x14C0D000
+
+static void test_output_pins(const char *machine, const uint32_t base, int idx)
+{
+ QTestState *s = qtest_init(machine);
+ char name[16];
+ char qom_path[64];
+ uint32_t offset = 0;
+ uint32_t value = 0;
+ for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR; i++) {
+ /* Odd index is output port */
+ sprintf(name, "sgpio%d", i * 2 + 1);
+ sprintf(qom_path, "/machine/soc/sgpio[%d]", idx);
+ offset = base + (R_SGPIO_0_CONTROL + i) * 4;
+ /* set serial output */
+ qtest_writel(s, offset, 0x00000001);
+ value = qtest_readl(s, offset);
+ g_assert_cmphex(SHARED_FIELD_EX32(value, SGPIO_SERIAL_OUT_VAL), ==, 1);
+ g_assert_cmphex(qtest_qom_get_bool(s, qom_path, name), ==, true);
+
+ /* clear serial output */
+ qtest_writel(s, offset, 0x00000000);
+ value = qtest_readl(s, offset);
+ g_assert_cmphex(SHARED_FIELD_EX32(value, SGPIO_SERIAL_OUT_VAL), ==, 0);
+ g_assert_cmphex(qtest_qom_get_bool(s, qom_path, name), ==, false);
+ }
+ qtest_quit(s);
+}
+
+static void test_input_pins(const char *machine, const uint32_t base, int idx)
+{
+ QTestState *s = qtest_init(machine);
+ char name[16];
+ char qom_path[64];
+ uint32_t offset = 0;
+ uint32_t value = 0;
+ for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR; i++) {
+ /* Even index is input port */
+ sprintf(name, "sgpio%d", i * 2);
+ sprintf(qom_path, "/machine/soc/sgpio[%d]", idx);
+ offset = base + (R_SGPIO_0_CONTROL + i) * 4;
+ /* set serial input */
+ qtest_qom_set_bool(s, qom_path, name, true);
+ value = qtest_readl(s, offset);
+ g_assert_cmphex(SHARED_FIELD_EX32(value, SGPIO_SERIAL_IN_VAL), ==, 1);
+ g_assert_cmphex(qtest_qom_get_bool(s, qom_path, name), ==, true);
+
+ /* clear serial input */
+ qtest_qom_set_bool(s, qom_path, name, false);
+ value = qtest_readl(s, offset);
+ g_assert_cmphex(SHARED_FIELD_EX32(value, SGPIO_SERIAL_IN_VAL), ==, 0);
+ g_assert_cmphex(qtest_qom_get_bool(s, qom_path, name), ==, false);
+ }
+ qtest_quit(s);
+}
+
+static void test_irq_level_high(const char *machine,
+ const uint32_t base, int idx)
+{
+ QTestState *s = qtest_init(machine);
+ char name[16];
+ char qom_path[64];
+ uint32_t ctrl_offset = 0;
+ uint32_t int_offset = 0;
+ uint32_t int_reg_idx = 0;
+ uint32_t int_bit_idx = 0;
+ uint32_t value = 0;
+ for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR; i++) {
+ /* Even index is input port */
+ sprintf(name, "sgpio%d", i * 2);
+ sprintf(qom_path, "/machine/soc/sgpio[%d]", idx);
+ int_reg_idx = i / 32;
+ int_bit_idx = i % 32;
+ int_offset = base + (R_SGPIO_INT_STATUS_0 + int_reg_idx) * 4;
+ ctrl_offset = base + (R_SGPIO_0_CONTROL + i) * 4;
+
+ /* Enable the interrupt */
+ value = SHARED_FIELD_DP32(value, SGPIO_INT_EN, 1);
+ qtest_writel(s, ctrl_offset, value);
+
+ /* Set the interrupt type to level-high trigger */
+ value = SHARED_FIELD_DP32(qtest_readl(s, ctrl_offset),
+ SGPIO_INT_TYPE, 3);
+ qtest_writel(s, ctrl_offset, value);
+
+ /* Set serial input high */
+ qtest_qom_set_bool(s, qom_path, name, true);
+ value = qtest_readl(s, ctrl_offset);
+ g_assert_cmphex(SHARED_FIELD_EX32(value, SGPIO_SERIAL_IN_VAL), ==, 1);
+
+ /* Interrupt status is set */
+ value = qtest_readl(s, int_offset);
+ g_assert_cmphex(extract32(value, int_bit_idx, 1), ==, 1);
+
+ /* Clear Interrupt */
+ value = SHARED_FIELD_DP32(qtest_readl(s, ctrl_offset),
+ SGPIO_INT_STATUS, 1);
+ qtest_writel(s, ctrl_offset, value);
+ value = qtest_readl(s, int_offset);
+ g_assert_cmphex(extract32(value, int_bit_idx, 1), ==, 0);
+
+ /* Clear serial input */
+ qtest_qom_set_bool(s, qom_path, name, false);
+ value = qtest_readl(s, ctrl_offset);
+ g_assert_cmphex(SHARED_FIELD_EX32(value, SGPIO_SERIAL_IN_VAL), ==, 0);
+ }
+ qtest_quit(s);
+}
+
+static void test_2700_input_pins(void)
+{
+ test_input_pins("-machine ast2700-evb",
+ AST2700_SGPIO0_BASE, 0);
+ test_input_pins("-machine ast2700-evb",
+ AST2700_SGPIO1_BASE, 1);
+ test_output_pins("-machine ast2700-evb",
+ AST2700_SGPIO0_BASE, 0);
+ test_output_pins("-machine ast2700-evb",
+ AST2700_SGPIO1_BASE, 1);
+ test_irq_level_high("-machine ast2700-evb",
+ AST2700_SGPIO0_BASE, 0);
+ test_irq_level_high("-machine ast2700-evb",
+ AST2700_SGPIO1_BASE, 1);
+}
+
+int main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/ast2700/sgpio/input_pins", test_2700_input_pins);
+
+ return g_test_run();
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 669d07c06bdedc6be0c69acadeba989dc15ddf3f..5c80b2ed6de1f453d2483db482c1b0e7801ba980 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -221,6 +221,7 @@ qtests_aspeed = \
qtests_aspeed64 = \
['ast2700-gpio-test',
'ast2700-hace-test',
+ 'ast2700-sgpio-test',
'ast2700-smc-test']
qtests_stm32l4x5 = \
--
2.52.0.223.gf5cc29aaa4-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/6] hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO
2025-12-09 0:01 ` [PATCH v2 1/6] hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO Yubin Zou
@ 2025-12-09 9:22 ` Cédric Le Goater
2025-12-10 8:02 ` Yubin Zou
0 siblings, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2025-12-09 9:22 UTC (permalink / raw)
To: Yubin Zou, qemu-devel
Cc: Peter Maydell, Steven Lee, Troy Lee, Jamin Lin, Andrew Jeffery,
Joel Stanley, Fabiano Rosas, Laurent Vivier, Paolo Bonzini,
Kane-Chen-AS, Nabih Estefan, qemu-arm
On 12/9/25 01:01, Yubin Zou wrote:
> This initial implementation includes the basic device structure,
> memory-mapped register definitions, and read/write handlers for the
> SGPIO control registers.
>
> Signed-off-by: Yubin Zou <yubinz@google.com>
> ---
> hw/gpio/aspeed_sgpio.c | 154 +++++++++++++++++++++++++++++++++++++++++
> hw/gpio/meson.build | 1 +
> include/hw/gpio/aspeed_sgpio.h | 66 ++++++++++++++++++
> 3 files changed, 221 insertions(+)
Please add to your .git/config :
[diff]
orderFile = /path/to/qemu/scripts/git.orderfile
It is easier to review if the changes in header files are first.
> diff --git a/hw/gpio/aspeed_sgpio.c b/hw/gpio/aspeed_sgpio.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..8676fa7ced134f1f62dc9e30b42c5fe6db3de268
> --- /dev/null
> +++ b/hw/gpio/aspeed_sgpio.c
> @@ -0,0 +1,154 @@
> +/*
> + * ASPEED Serial GPIO Controller
> + *
> + * Copyright 2025 Google LLC.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "qemu/log.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/gpio/aspeed_sgpio.h"
> +
> +static uint64_t aspeed_sgpio_2700_read_int_status_reg(AspeedSGPIOState *s,
> + uint32_t reg)
> +{
> + return 0;
> +}
> +
> +static uint64_t aspeed_sgpio_2700_read_control_reg(AspeedSGPIOState *s,
> + uint32_t reg)
> +{
> + AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
> + uint32_t idx = reg - R_SGPIO_0_CONTROL;
> + if (idx >= agc->nr_sgpio_pin_pairs) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: pin index: %d, out of bounds\n",
> + __func__, idx);
> + return 0;
> + }
> + return s->ctrl_regs[idx];
> +}
> +
> +static void aspeed_sgpio_2700_write_control_reg(AspeedSGPIOState *s,
> + uint32_t reg, uint64_t data)
> +{
> + AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
> + uint32_t idx = reg - R_SGPIO_0_CONTROL;
> + if (idx >= agc->nr_sgpio_pin_pairs) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: pin index: %d, out of bounds\n",
> + __func__, idx);
> + return;
> + }
> + s->ctrl_regs[idx] = data;
> +}
> +
> +static uint64_t aspeed_sgpio_2700_read(void *opaque, hwaddr offset,
> + uint32_t size)
> +{
> + AspeedSGPIOState *s = ASPEED_SGPIO(opaque);
> + uint64_t value = 0;
> + uint64_t reg;
> +
> + reg = offset >> 2;
> +
> + switch (reg) {
> + case R_SGPIO_INT_STATUS_0 ... R_SGPIO_INT_STATUS_7:
> + aspeed_sgpio_2700_read_int_status_reg(s, reg);
> + break;
> + case R_SGPIO_0_CONTROL ... R_SGPIO_255_CONTROL:
> + value = aspeed_sgpio_2700_read_control_reg(s, reg);
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
> + PRIx64"\n", __func__, offset);
> + return 0;
> + }
> +
> + return value;
> +}
> +
> +static void aspeed_sgpio_2700_write(void *opaque, hwaddr offset, uint64_t data,
> + uint32_t size)
> +{
> + AspeedSGPIOState *s = ASPEED_SGPIO(opaque);
> + uint64_t reg;
> +
> + reg = offset >> 2;
> +
> + switch (reg) {
> + case R_SGPIO_0_CONTROL ... R_SGPIO_255_CONTROL:
> + aspeed_sgpio_2700_write_control_reg(s, reg, data);
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
> + PRIx64"\n", __func__, offset);
> + return;
> + }
> +}
> +
> +static const MemoryRegionOps aspeed_gpio_2700_ops = {
> + .read = aspeed_sgpio_2700_read,
> + .write = aspeed_sgpio_2700_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid.min_access_size = 4,
> + .valid.max_access_size = 4,
> +};
> +
> +static void aspeed_sgpio_realize(DeviceState *dev, Error **errp)
> +{
> + AspeedSGPIOState *s = ASPEED_SGPIO(dev);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
> +
> + /* Interrupt parent line */
> + sysbus_init_irq(sbd, &s->irq);
> +
> + memory_region_init_io(&s->iomem, OBJECT(s), agc->reg_ops, s,
> + TYPE_ASPEED_SGPIO, agc->mem_size);
> +
> + sysbus_init_mmio(sbd, &s->iomem);
> +}
> +
> +static void aspeed_sgpio_class_init(ObjectClass *klass, const void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = aspeed_sgpio_realize;
> + dc->desc = "Aspeed SGPIO Controller";
> +}
> +
> +static void aspeed_sgpio_2700_class_init(ObjectClass *klass, const void *data)
> +{
> + AspeedSGPIOClass *agc = ASPEED_SGPIO_CLASS(klass);
> + agc->nr_sgpio_pin_pairs = 256;
> + agc->mem_size = 0x1000;
> + agc->reg_ops = &aspeed_gpio_2700_ops;
> +}
> +
> +static const TypeInfo aspeed_sgpio_info = {
> + .name = TYPE_ASPEED_SGPIO,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AspeedSGPIOState),
> + .class_size = sizeof(AspeedSGPIOClass),
> + .class_init = aspeed_sgpio_class_init,
> + .abstract = true,
> +};
> +
> +static const TypeInfo aspeed_sgpio_ast2700_info = {
> + .name = TYPE_ASPEED_SGPIO "-ast2700",
> + .parent = TYPE_ASPEED_SGPIO,
> + .class_init = aspeed_sgpio_2700_class_init,
> +};
> +
> +static void aspeed_sgpio_register_types(void)
> +{
> + type_register_static(&aspeed_sgpio_info);
> + type_register_static(&aspeed_sgpio_ast2700_info);
> +}
> +
> +type_init(aspeed_sgpio_register_types);
> diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> index 74840619c01bf4d9a02c058c434c3ec9d2a55bee..6a67ee958faace69ffd3fe08e8ade31ced0faf7e 100644
> --- a/hw/gpio/meson.build
> +++ b/hw/gpio/meson.build
> @@ -16,5 +16,6 @@ system_ss.add(when: 'CONFIG_RASPI', if_true: files(
> ))
> system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true: files('stm32l4x5_gpio.c'))
> system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_gpio.c'))
> +system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_sgpio.c'))
> system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
> system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
> diff --git a/include/hw/gpio/aspeed_sgpio.h b/include/hw/gpio/aspeed_sgpio.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..ffdc54a112da8962a7bc5773d524f1d86eb85d39
> --- /dev/null
> +++ b/include/hw/gpio/aspeed_sgpio.h
> @@ -0,0 +1,66 @@
> +/*
> + * ASPEED Serial GPIO Controller
> + *
> + * Copyright 2025 Google LLC.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef ASPEED_SGPIO_H
> +#define ASPEED_SGPIO_H
> +
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
> +#include "hw/registerfields.h"
> +
> +#define TYPE_ASPEED_SGPIO "aspeed.sgpio"
> +OBJECT_DECLARE_TYPE(AspeedSGPIOState, AspeedSGPIOClass, ASPEED_SGPIO)
> +
> +#define ASPEED_SGPIO_MAX_PIN_PAIR 256
> +#define ASPEED_SGPIO_MAX_INT 8
> +
> +/* AST2700 SGPIO Register Address Offsets */
> +REG32(SGPIO_INT_STATUS_0, 0x40)
> +REG32(SGPIO_INT_STATUS_1, 0x44)
> +REG32(SGPIO_INT_STATUS_2, 0x48)
> +REG32(SGPIO_INT_STATUS_3, 0x4C)
> +REG32(SGPIO_INT_STATUS_4, 0x50)
> +REG32(SGPIO_INT_STATUS_5, 0x54)
> +REG32(SGPIO_INT_STATUS_6, 0x58)
> +REG32(SGPIO_INT_STATUS_7, 0x5C)
> +/* AST2700 SGPIO_0 - SGPIO_255 Control Register */
> +REG32(SGPIO_0_CONTROL, 0x80)
> + SHARED_FIELD(SGPIO_SERIAL_OUT_VAL, 0, 1)
> + SHARED_FIELD(SGPIO_PARALLEL_OUT_VAL, 1, 1)
> + SHARED_FIELD(SGPIO_INT_EN, 2, 1)
> + SHARED_FIELD(SGPIO_INT_TYPE, 3, 3)
> + SHARED_FIELD(SGPIO_RESET_POLARITY, 6, 1)
> + SHARED_FIELD(SGPIO_RESERVED_1, 7, 2)
> + SHARED_FIELD(SGPIO_INPUT_MASK, 9, 1)
> + SHARED_FIELD(SGPIO_PARALLEL_EN, 10, 1)
> + SHARED_FIELD(SGPIO_PARALLEL_IN_MODE, 11, 1)
> + SHARED_FIELD(SGPIO_INT_STATUS, 12, 1)
> + SHARED_FIELD(SGPIO_SERIAL_IN_VAL, 13, 1)
> + SHARED_FIELD(SGPIO_PARALLEL_IN_VAL, 14, 1)
> + SHARED_FIELD(SGPIO_RESERVED_2, 15, 12)
> + SHARED_FIELD(SGPIO_WRITE_PROTECT, 31, 1)
> +REG32(SGPIO_255_CONTROL, 0x47C)
> +
> +struct AspeedSGPIOClass {
> + SysBusDevice parent_obj;
> + uint32_t nr_sgpio_pin_pairs;
> + uint64_t mem_size;
> + const MemoryRegionOps *reg_ops;
> +};
I don't see any need of an AspeedSGPIOClass for now. Do you have plans
for future models ?
Thanks,
C.
> +
> +struct AspeedSGPIOState {
> + /* <private> */
> + SysBusDevice parent;
> +
> + /*< public >*/
> + MemoryRegion iomem;
> + qemu_irq irq;
> + uint32_t ctrl_regs[ASPEED_SGPIO_MAX_PIN_PAIR];
> + uint32_t int_regs[ASPEED_SGPIO_MAX_INT];
> +};
> +
> +#endif /* ASPEED_SGPIO_H */
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/6] hw/gpio/aspeed_sgpio: aspeed: Add QOM property accessors for SGPIO pins
2025-12-09 0:01 ` [PATCH v2 2/6] hw/gpio/aspeed_sgpio: aspeed: Add QOM property accessors for SGPIO pins Yubin Zou
@ 2025-12-09 14:52 ` Cédric Le Goater
2025-12-10 8:08 ` Yubin Zou
0 siblings, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2025-12-09 14:52 UTC (permalink / raw)
To: Yubin Zou, qemu-devel
Cc: Peter Maydell, Steven Lee, Troy Lee, Jamin Lin, Andrew Jeffery,
Joel Stanley, Fabiano Rosas, Laurent Vivier, Paolo Bonzini,
Kane-Chen-AS, Nabih Estefan, qemu-arm
Hi,
The subject needs a fix. Please remove the extra 'aspeed: '.
On 12/9/25 01:01, Yubin Zou wrote:
> This commit adds QOM property accessors for the Aspeed SGPIO pins.
I think you can drop the above sentence from the commit log. It's
redundant with the subject.
> The `aspeed_sgpio_get_pin` and `aspeed_sgpio_set_pin` functions are
> implemented to get and set the level of individual SGPIO pins. These
> are then exposed as boolean properties on the SGPIO device object.
>
> Signed-off-by: Yubin Zou <yubinz@google.com>
> ---
> hw/gpio/aspeed_sgpio.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/hw/gpio/aspeed_sgpio.c b/hw/gpio/aspeed_sgpio.c
> index 8676fa7ced134f1f62dc9e30b42c5fe6db3de268..efa7e574abe87e33e58ac88dba5e3469c6702b83 100644
> --- a/hw/gpio/aspeed_sgpio.c
> +++ b/hw/gpio/aspeed_sgpio.c
> @@ -91,6 +91,73 @@ static void aspeed_sgpio_2700_write(void *opaque, hwaddr offset, uint64_t data,
> }
> }
>
> +static bool aspeed_sgpio_get_pin_level(AspeedSGPIOState *s, int pin)
> +{
> + uint32_t value = s->ctrl_regs[pin >> 1];
> + bool is_input = !(pin % 2);
> + uint32_t bit_mask = 0;
> +
> + if (is_input) {
> + bit_mask = SGPIO_SERIAL_IN_VAL_MASK;
> + } else {
> + bit_mask = SGPIO_SERIAL_OUT_VAL_MASK;
> + }
> +
> + return value & bit_mask;
> +}
> +
> +static void aspeed_sgpio_set_pin_level(AspeedSGPIOState *s, int pin, bool level)
> +{
> + uint32_t value = s->ctrl_regs[pin >> 1];
> + bool is_input = !(pin % 2);
> + uint32_t bit_mask = 0;
> +
> + if (is_input) {
> + bit_mask = SGPIO_SERIAL_IN_VAL_MASK;
> + } else {
> + bit_mask = SGPIO_SERIAL_OUT_VAL_MASK;
> + }
> +
> + if (level) {
> + value |= bit_mask;
> + } else {
> + value &= ~bit_mask;
> + }
> + s->ctrl_regs[pin >> 1] = value;
> +}
> +
> +static void aspeed_sgpio_get_pin(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + bool level = true;
> + int pin = 0xfff;
> + AspeedSGPIOState *s = ASPEED_SGPIO(obj);
> +
> + if (sscanf(name, "sgpio%d", &pin) != 1) {
> + error_setg(errp, "%s: error reading %s", __func__, name);
> + return;
> + }
> + level = aspeed_sgpio_get_pin_level(s, pin);
> + visit_type_bool(v, name, &level, errp);
> +}
> +
> +static void aspeed_sgpio_set_pin(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + bool level;
> + int pin = 0xfff;
> + AspeedSGPIOState *s = ASPEED_SGPIO(obj);
> +
> + if (!visit_type_bool(v, name, &level, errp)) {
> + return;
> + }
> + if (sscanf(name, "sgpio%d", &pin) != 1) {
> + error_setg(errp, "%s: error reading %s", __func__, name);
> + return;
> + }
> + aspeed_sgpio_set_pin_level(s, pin, level);
> +}
> +
> static const MemoryRegionOps aspeed_gpio_2700_ops = {
> .read = aspeed_sgpio_2700_read,
> .write = aspeed_sgpio_2700_write,
> @@ -114,6 +181,16 @@ static void aspeed_sgpio_realize(DeviceState *dev, Error **errp)
> sysbus_init_mmio(sbd, &s->iomem);
> }
>
> +static void aspeed_sgpio_init(Object *obj)
> +{
> + for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR * 2; i++) {
> + char *name = g_strdup_printf("sgpio%d", i);
You could use a g_autofree variable and drop the g_free below.
How about using a "%03d" format in the printf and sscanf too ?
C.
> + object_property_add(obj, name, "bool", aspeed_sgpio_get_pin,
> + aspeed_sgpio_set_pin, NULL, NULL);
> + g_free(name);
> + }
> +}
> +
> static void aspeed_sgpio_class_init(ObjectClass *klass, const void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -143,6 +220,7 @@ static const TypeInfo aspeed_sgpio_ast2700_info = {
> .name = TYPE_ASPEED_SGPIO "-ast2700",
> .parent = TYPE_ASPEED_SGPIO,
> .class_init = aspeed_sgpio_2700_class_init,
> + .instance_init = aspeed_sgpio_init,
> };
>
> static void aspeed_sgpio_register_types(void)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] hw/arm/aspeed_soc: Update Aspeed SoC to support two SGPIO controllers
2025-12-09 0:01 ` [PATCH v2 4/6] hw/arm/aspeed_soc: Update Aspeed SoC to support two SGPIO controllers Yubin Zou
@ 2025-12-09 16:15 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2025-12-09 16:15 UTC (permalink / raw)
To: Yubin Zou, qemu-devel
Cc: Peter Maydell, Steven Lee, Troy Lee, Jamin Lin, Andrew Jeffery,
Joel Stanley, Fabiano Rosas, Laurent Vivier, Paolo Bonzini,
Kane-Chen-AS, Nabih Estefan, qemu-arm
On 12/9/25 01:01, Yubin Zou wrote:
> This commit updates the Aspeed SoC model to support two SGPIO
> controllers, reflecting the hardware capabilities of the AST2700
>
> The memory map and interrupt map are updated to include entries for
> two SGPIO controllers (SGPIOM0 and SGPIOM1). This change is a
> prerequisite for the full implementation of the SGPIO device model.
>
> Signed-off-by: Yubin Zou <yubinz@google.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
> ---
> hw/arm/aspeed_ast10x0.c | 6 +++---
> hw/arm/aspeed_ast27x0.c | 10 ++++++++++
> include/hw/arm/aspeed_soc.h | 8 ++++++--
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
> index 7f49c13391be0b923e317409a0fccfa741f5e658..c141cc080422579ca6b6965369d84dfbe416247b 100644
> --- a/hw/arm/aspeed_ast10x0.c
> +++ b/hw/arm/aspeed_ast10x0.c
> @@ -36,7 +36,7 @@ static const hwaddr aspeed_soc_ast1030_memmap[] = {
> [ASPEED_DEV_ESPI] = 0x7E6EE000,
> [ASPEED_DEV_SBC] = 0x7E6F2000,
> [ASPEED_DEV_GPIO] = 0x7E780000,
> - [ASPEED_DEV_SGPIOM] = 0x7E780500,
> + [ASPEED_DEV_SGPIOM0] = 0x7E780500,
> [ASPEED_DEV_TIMER1] = 0x7E782000,
> [ASPEED_DEV_UART1] = 0x7E783000,
> [ASPEED_DEV_UART2] = 0x7E78D000,
> @@ -94,7 +94,7 @@ static const int aspeed_soc_ast1030_irqmap[] = {
> [ASPEED_DEV_I2C] = 110, /* 110 ~ 123 */
> [ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
> [ASPEED_DEV_UDC] = 9,
> - [ASPEED_DEV_SGPIOM] = 51,
> + [ASPEED_DEV_SGPIOM0] = 51,
> [ASPEED_DEV_JTAG0] = 27,
> [ASPEED_DEV_JTAG1] = 53,
> };
> @@ -427,7 +427,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
> sc->memmap[ASPEED_DEV_UDC], 0x1000);
> aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->sgpiom),
> "aspeed.sgpiom",
> - sc->memmap[ASPEED_DEV_SGPIOM], 0x100);
> + sc->memmap[ASPEED_DEV_SGPIOM0], 0x100);
>
> aspeed_mmio_map_unimplemented(s->memory, SYS_BUS_DEVICE(&s->jtag[0]),
> "aspeed.jtag",
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index c484bcd4e22fb49faf9c16992ae2cdfd6cd82da4..e5f04bd16e80696e41005d9062a6df6d060b8088 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -69,6 +69,8 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
> [ASPEED_DEV_ADC] = 0x14C00000,
> [ASPEED_DEV_SCUIO] = 0x14C02000,
> [ASPEED_DEV_GPIO] = 0x14C0B000,
> + [ASPEED_DEV_SGPIOM0] = 0x14C0C000,
> + [ASPEED_DEV_SGPIOM1] = 0x14C0D000,
> [ASPEED_DEV_I2C] = 0x14C0F000,
> [ASPEED_DEV_INTCIO] = 0x14C18000,
> [ASPEED_DEV_PCIE_PHY2] = 0x14C1C000,
> @@ -122,6 +124,8 @@ static const int aspeed_soc_ast2700a0_irqmap[] = {
> [ASPEED_DEV_KCS] = 128,
> [ASPEED_DEV_ADC] = 130,
> [ASPEED_DEV_GPIO] = 130,
> + [ASPEED_DEV_SGPIOM0] = 130,
> + [ASPEED_DEV_SGPIOM1] = 130,
> [ASPEED_DEV_I2C] = 130,
> [ASPEED_DEV_FMC] = 131,
> [ASPEED_DEV_WDT] = 131,
> @@ -173,6 +177,8 @@ static const int aspeed_soc_ast2700a1_irqmap[] = {
> [ASPEED_DEV_I2C] = 194,
> [ASPEED_DEV_ADC] = 194,
> [ASPEED_DEV_GPIO] = 194,
> + [ASPEED_DEV_SGPIOM0] = 194,
> + [ASPEED_DEV_SGPIOM1] = 194,
> [ASPEED_DEV_FMC] = 195,
> [ASPEED_DEV_WDT] = 195,
> [ASPEED_DEV_PWM] = 195,
> @@ -214,6 +220,8 @@ static const int ast2700_gic130_gic194_intcmap[] = {
> [ASPEED_DEV_I2C] = 0,
> [ASPEED_DEV_ADC] = 16,
> [ASPEED_DEV_GPIO] = 18,
> + [ASPEED_DEV_SGPIOM0] = 21,
> + [ASPEED_DEV_SGPIOM1] = 24,
> };
>
> /* GICINT 131 */
> @@ -1061,6 +1069,7 @@ static void aspeed_soc_ast2700a0_class_init(ObjectClass *oc, const void *data)
> sc->sram_size = 0x20000;
> sc->pcie_num = 0;
> sc->spis_num = 3;
> + sc->sgpio_num = 2;
> sc->ehcis_num = 2;
> sc->wdts_num = 8;
> sc->macs_num = 1;
> @@ -1089,6 +1098,7 @@ static void aspeed_soc_ast2700a1_class_init(ObjectClass *oc, const void *data)
> sc->sram_size = 0x20000;
> sc->pcie_num = 3;
> sc->spis_num = 3;
> + sc->sgpio_num = 2;
> sc->ehcis_num = 4;
> sc->wdts_num = 8;
> sc->macs_num = 3;
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 4b8e599f1a53bfb2e4d3196d5495cd316f799354..18ff961a38508c5df83b46e187f732d736443f20 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -32,6 +32,7 @@
> #include "hw/net/ftgmac100.h"
> #include "target/arm/cpu.h"
> #include "hw/gpio/aspeed_gpio.h"
> +#include "hw/gpio/aspeed_sgpio.h"
> #include "hw/sd/aspeed_sdhci.h"
> #include "hw/usb/hcd-ehci.h"
> #include "qom/object.h"
> @@ -46,6 +47,7 @@
> #define VBOOTROM_FILE_NAME "ast27x0_bootrom.bin"
>
> #define ASPEED_SPIS_NUM 3
> +#define ASPEED_SGPIO_NUM 2
> #define ASPEED_EHCIS_NUM 4
> #define ASPEED_WDTS_NUM 8
> #define ASPEED_CPUS_NUM 4
> @@ -89,6 +91,7 @@ struct AspeedSoCState {
> AspeedMiiState mii[ASPEED_MACS_NUM];
> AspeedGPIOState gpio;
> AspeedGPIOState gpio_1_8v;
> + AspeedSGPIOState sgpiom[ASPEED_SGPIO_NUM];
> AspeedSDHCIState sdhci;
> AspeedSDHCIState emmc;
> AspeedLPCState lpc;
> @@ -106,7 +109,6 @@ struct AspeedSoCState {
> UnimplementedDeviceState pwm;
> UnimplementedDeviceState espi;
> UnimplementedDeviceState udc;
> - UnimplementedDeviceState sgpiom;
> UnimplementedDeviceState ltpi;
> UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
> AspeedAPB2OPBState fsi[2];
> @@ -166,6 +168,7 @@ struct AspeedSoCClass {
> uint64_t secsram_size;
> int pcie_num;
> int spis_num;
> + int sgpio_num;
> int ehcis_num;
> int wdts_num;
> int macs_num;
> @@ -221,6 +224,8 @@ enum {
> ASPEED_DEV_SDHCI,
> ASPEED_DEV_GPIO,
> ASPEED_DEV_GPIO_1_8V,
> + ASPEED_DEV_SGPIOM0,
> + ASPEED_DEV_SGPIOM1,
> ASPEED_DEV_RTC,
> ASPEED_DEV_TIMER1,
> ASPEED_DEV_TIMER2,
> @@ -263,7 +268,6 @@ enum {
> ASPEED_DEV_I3C,
> ASPEED_DEV_ESPI,
> ASPEED_DEV_UDC,
> - ASPEED_DEV_SGPIOM,
> ASPEED_DEV_JTAG0,
> ASPEED_DEV_JTAG1,
> ASPEED_DEV_FSI1,
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Wire SGPIO controller to AST2700 SoC
2025-12-09 0:01 ` [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Wire SGPIO controller to AST2700 SoC Yubin Zou
@ 2025-12-09 16:15 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2025-12-09 16:15 UTC (permalink / raw)
To: Yubin Zou, qemu-devel
Cc: Peter Maydell, Steven Lee, Troy Lee, Jamin Lin, Andrew Jeffery,
Joel Stanley, Fabiano Rosas, Laurent Vivier, Paolo Bonzini,
Kane-Chen-AS, Nabih Estefan, qemu-arm
On 12/9/25 01:01, Yubin Zou wrote:
> This commit integrates the Aspeed SGPIO controller into the AST2700
>
> Signed-off-by: Yubin Zou <yubinz@google.com>
> ---
> hw/arm/aspeed_ast27x0.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index e5f04bd16e80696e41005d9062a6df6d060b8088..787accadbecae376d0c747d054ec6372785375b1 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -519,6 +519,11 @@ static void aspeed_soc_ast2700_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
> object_initialize_child(obj, "gpio", &s->gpio, typename);
>
> + snprintf(typename, sizeof(typename), "aspeed.sgpio-%s", socname);
> + for (i = 0; i < sc->sgpio_num; i++) {
> + object_initialize_child(obj, "sgpio[*]", &s->sgpiom[i], typename);
> + }
> +
> object_initialize_child(obj, "rtc", &s->rtc, TYPE_ASPEED_RTC);
>
> snprintf(typename, sizeof(typename), "aspeed.sdhci-%s", socname);
> @@ -973,6 +978,17 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
> aspeed_soc_ast2700_get_irq(s, ASPEED_DEV_GPIO));
>
> + /* SGPIO */
> + for (i = 0; i < sc->sgpio_num; i++) {
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->sgpiom[i]), errp)) {
> + return;
> + }
> + aspeed_mmio_map(s->memory, SYS_BUS_DEVICE(&s->sgpiom[i]), 0,
> + sc->memmap[ASPEED_DEV_SGPIOM0 + i]);
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->sgpiom[i]), 0,
> + aspeed_soc_ast2700_get_irq(s, ASPEED_DEV_SGPIOM0 + i));
> + }
> +
> /* RTC */
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->rtc), errp)) {
> return;
>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/6] hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO
2025-12-09 9:22 ` Cédric Le Goater
@ 2025-12-10 8:02 ` Yubin Zou
0 siblings, 0 replies; 13+ messages in thread
From: Yubin Zou @ 2025-12-10 8:02 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-devel, Peter Maydell, Steven Lee, Troy Lee, Jamin Lin,
Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
Paolo Bonzini, Kane-Chen-AS, Nabih Estefan, qemu-arm
[-- Attachment #1: Type: text/plain, Size: 10066 bytes --]
Hi Cédric
The Aspeed SGPIO has two versions, one is associated with AST2700 and an
older version before and includes AST2600.
I don't have an impending plan to implement the older version, but the
class structure is for future extensibility.
Thanks
Yubin
On Tue, Dec 9, 2025 at 1:22 AM Cédric Le Goater <clg@kaod.org> wrote:
> On 12/9/25 01:01, Yubin Zou wrote:
> > This initial implementation includes the basic device structure,
> > memory-mapped register definitions, and read/write handlers for the
> > SGPIO control registers.
> >
> > Signed-off-by: Yubin Zou <yubinz@google.com>
> > ---
> > hw/gpio/aspeed_sgpio.c | 154
> +++++++++++++++++++++++++++++++++++++++++
> > hw/gpio/meson.build | 1 +
> > include/hw/gpio/aspeed_sgpio.h | 66 ++++++++++++++++++
> > 3 files changed, 221 insertions(+)
>
> Please add to your .git/config :
>
> [diff]
> orderFile = /path/to/qemu/scripts/git.orderfile
>
> It is easier to review if the changes in header files are first.
>
>
> > diff --git a/hw/gpio/aspeed_sgpio.c b/hw/gpio/aspeed_sgpio.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..8676fa7ced134f1f62dc9e30b42c5fe6db3de268
> > --- /dev/null
> > +++ b/hw/gpio/aspeed_sgpio.c
> > @@ -0,0 +1,154 @@
> > +/*
> > + * ASPEED Serial GPIO Controller
> > + *
> > + * Copyright 2025 Google LLC.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/host-utils.h"
> > +#include "qemu/log.h"
> > +#include "qemu/error-report.h"
> > +#include "qapi/error.h"
> > +#include "qapi/visitor.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/gpio/aspeed_sgpio.h"
> > +
> > +static uint64_t aspeed_sgpio_2700_read_int_status_reg(AspeedSGPIOState
> *s,
> > + uint32_t reg)
> > +{
> > + return 0;
> > +}
> > +
> > +static uint64_t aspeed_sgpio_2700_read_control_reg(AspeedSGPIOState *s,
> > + uint32_t reg)
> > +{
> > + AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
> > + uint32_t idx = reg - R_SGPIO_0_CONTROL;
> > + if (idx >= agc->nr_sgpio_pin_pairs) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: pin index: %d, out of
> bounds\n",
> > + __func__, idx);
> > + return 0;
> > + }
> > + return s->ctrl_regs[idx];
> > +}
> > +
> > +static void aspeed_sgpio_2700_write_control_reg(AspeedSGPIOState *s,
> > + uint32_t reg, uint64_t data)
> > +{
> > + AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
> > + uint32_t idx = reg - R_SGPIO_0_CONTROL;
> > + if (idx >= agc->nr_sgpio_pin_pairs) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: pin index: %d, out of
> bounds\n",
> > + __func__, idx);
> > + return;
> > + }
> > + s->ctrl_regs[idx] = data;
> > +}
> > +
> > +static uint64_t aspeed_sgpio_2700_read(void *opaque, hwaddr offset,
> > + uint32_t size)
> > +{
> > + AspeedSGPIOState *s = ASPEED_SGPIO(opaque);
> > + uint64_t value = 0;
> > + uint64_t reg;
> > +
> > + reg = offset >> 2;
> > +
> > + switch (reg) {
> > + case R_SGPIO_INT_STATUS_0 ... R_SGPIO_INT_STATUS_7:
> > + aspeed_sgpio_2700_read_int_status_reg(s, reg);
> > + break;
> > + case R_SGPIO_0_CONTROL ... R_SGPIO_255_CONTROL:
> > + value = aspeed_sgpio_2700_read_control_reg(s, reg);
> > + break;
> > + default:
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
> > + PRIx64"\n", __func__, offset);
> > + return 0;
> > + }
> > +
> > + return value;
> > +}
> > +
> > +static void aspeed_sgpio_2700_write(void *opaque, hwaddr offset,
> uint64_t data,
> > + uint32_t size)
> > +{
> > + AspeedSGPIOState *s = ASPEED_SGPIO(opaque);
> > + uint64_t reg;
> > +
> > + reg = offset >> 2;
> > +
> > + switch (reg) {
> > + case R_SGPIO_0_CONTROL ... R_SGPIO_255_CONTROL:
> > + aspeed_sgpio_2700_write_control_reg(s, reg, data);
> > + break;
> > + default:
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
> > + PRIx64"\n", __func__, offset);
> > + return;
> > + }
> > +}
> > +
> > +static const MemoryRegionOps aspeed_gpio_2700_ops = {
> > + .read = aspeed_sgpio_2700_read,
> > + .write = aspeed_sgpio_2700_write,
> > + .endianness = DEVICE_LITTLE_ENDIAN,
> > + .valid.min_access_size = 4,
> > + .valid.max_access_size = 4,
> > +};
> > +
> > +static void aspeed_sgpio_realize(DeviceState *dev, Error **errp)
> > +{
> > + AspeedSGPIOState *s = ASPEED_SGPIO(dev);
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > + AspeedSGPIOClass *agc = ASPEED_SGPIO_GET_CLASS(s);
> > +
> > + /* Interrupt parent line */
> > + sysbus_init_irq(sbd, &s->irq);
> > +
> > + memory_region_init_io(&s->iomem, OBJECT(s), agc->reg_ops, s,
> > + TYPE_ASPEED_SGPIO, agc->mem_size);
> > +
> > + sysbus_init_mmio(sbd, &s->iomem);
> > +}
> > +
> > +static void aspeed_sgpio_class_init(ObjectClass *klass, const void
> *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > + dc->realize = aspeed_sgpio_realize;
> > + dc->desc = "Aspeed SGPIO Controller";
> > +}
> > +
> > +static void aspeed_sgpio_2700_class_init(ObjectClass *klass, const void
> *data)
> > +{
> > + AspeedSGPIOClass *agc = ASPEED_SGPIO_CLASS(klass);
> > + agc->nr_sgpio_pin_pairs = 256;
> > + agc->mem_size = 0x1000;
> > + agc->reg_ops = &aspeed_gpio_2700_ops;
> > +}
> > +
> > +static const TypeInfo aspeed_sgpio_info = {
> > + .name = TYPE_ASPEED_SGPIO,
> > + .parent = TYPE_SYS_BUS_DEVICE,
> > + .instance_size = sizeof(AspeedSGPIOState),
> > + .class_size = sizeof(AspeedSGPIOClass),
> > + .class_init = aspeed_sgpio_class_init,
> > + .abstract = true,
> > +};
> > +
> > +static const TypeInfo aspeed_sgpio_ast2700_info = {
> > + .name = TYPE_ASPEED_SGPIO "-ast2700",
> > + .parent = TYPE_ASPEED_SGPIO,
> > + .class_init = aspeed_sgpio_2700_class_init,
> > +};
> > +
> > +static void aspeed_sgpio_register_types(void)
> > +{
> > + type_register_static(&aspeed_sgpio_info);
> > + type_register_static(&aspeed_sgpio_ast2700_info);
> > +}
> > +
> > +type_init(aspeed_sgpio_register_types);
> > diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
> > index
> 74840619c01bf4d9a02c058c434c3ec9d2a55bee..6a67ee958faace69ffd3fe08e8ade31ced0faf7e
> 100644
> > --- a/hw/gpio/meson.build
> > +++ b/hw/gpio/meson.build
> > @@ -16,5 +16,6 @@ system_ss.add(when: 'CONFIG_RASPI', if_true: files(
> > ))
> > system_ss.add(when: 'CONFIG_STM32L4X5_SOC', if_true:
> files('stm32l4x5_gpio.c'))
> > system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true:
> files('aspeed_gpio.c'))
> > +system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true:
> files('aspeed_sgpio.c'))
> > system_ss.add(when: 'CONFIG_SIFIVE_GPIO', if_true:
> files('sifive_gpio.c'))
> > system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
> > diff --git a/include/hw/gpio/aspeed_sgpio.h
> b/include/hw/gpio/aspeed_sgpio.h
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..ffdc54a112da8962a7bc5773d524f1d86eb85d39
> > --- /dev/null
> > +++ b/include/hw/gpio/aspeed_sgpio.h
> > @@ -0,0 +1,66 @@
> > +/*
> > + * ASPEED Serial GPIO Controller
> > + *
> > + * Copyright 2025 Google LLC.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#ifndef ASPEED_SGPIO_H
> > +#define ASPEED_SGPIO_H
> > +
> > +#include "hw/sysbus.h"
> > +#include "qom/object.h"
> > +#include "hw/registerfields.h"
> > +
> > +#define TYPE_ASPEED_SGPIO "aspeed.sgpio"
> > +OBJECT_DECLARE_TYPE(AspeedSGPIOState, AspeedSGPIOClass, ASPEED_SGPIO)
> > +
> > +#define ASPEED_SGPIO_MAX_PIN_PAIR 256
> > +#define ASPEED_SGPIO_MAX_INT 8
> > +
> > +/* AST2700 SGPIO Register Address Offsets */
> > +REG32(SGPIO_INT_STATUS_0, 0x40)
> > +REG32(SGPIO_INT_STATUS_1, 0x44)
> > +REG32(SGPIO_INT_STATUS_2, 0x48)
> > +REG32(SGPIO_INT_STATUS_3, 0x4C)
> > +REG32(SGPIO_INT_STATUS_4, 0x50)
> > +REG32(SGPIO_INT_STATUS_5, 0x54)
> > +REG32(SGPIO_INT_STATUS_6, 0x58)
> > +REG32(SGPIO_INT_STATUS_7, 0x5C)
> > +/* AST2700 SGPIO_0 - SGPIO_255 Control Register */
> > +REG32(SGPIO_0_CONTROL, 0x80)
> > + SHARED_FIELD(SGPIO_SERIAL_OUT_VAL, 0, 1)
> > + SHARED_FIELD(SGPIO_PARALLEL_OUT_VAL, 1, 1)
> > + SHARED_FIELD(SGPIO_INT_EN, 2, 1)
> > + SHARED_FIELD(SGPIO_INT_TYPE, 3, 3)
> > + SHARED_FIELD(SGPIO_RESET_POLARITY, 6, 1)
> > + SHARED_FIELD(SGPIO_RESERVED_1, 7, 2)
> > + SHARED_FIELD(SGPIO_INPUT_MASK, 9, 1)
> > + SHARED_FIELD(SGPIO_PARALLEL_EN, 10, 1)
> > + SHARED_FIELD(SGPIO_PARALLEL_IN_MODE, 11, 1)
> > + SHARED_FIELD(SGPIO_INT_STATUS, 12, 1)
> > + SHARED_FIELD(SGPIO_SERIAL_IN_VAL, 13, 1)
> > + SHARED_FIELD(SGPIO_PARALLEL_IN_VAL, 14, 1)
> > + SHARED_FIELD(SGPIO_RESERVED_2, 15, 12)
> > + SHARED_FIELD(SGPIO_WRITE_PROTECT, 31, 1)
> > +REG32(SGPIO_255_CONTROL, 0x47C)
> > +
> > +struct AspeedSGPIOClass {
> > + SysBusDevice parent_obj;
> > + uint32_t nr_sgpio_pin_pairs;
> > + uint64_t mem_size;
> > + const MemoryRegionOps *reg_ops;
> > +};
>
>
> I don't see any need of an AspeedSGPIOClass for now. Do you have plans
> for future models ?
>
> Thanks,
>
> C.
>
>
> > +
> > +struct AspeedSGPIOState {
> > + /* <private> */
> > + SysBusDevice parent;
> > +
> > + /*< public >*/
> > + MemoryRegion iomem;
> > + qemu_irq irq;
> > + uint32_t ctrl_regs[ASPEED_SGPIO_MAX_PIN_PAIR];
> > + uint32_t int_regs[ASPEED_SGPIO_MAX_INT];
> > +};
> > +
> > +#endif /* ASPEED_SGPIO_H */
> >
>
>
[-- Attachment #2: Type: text/html, Size: 12700 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/6] hw/gpio/aspeed_sgpio: aspeed: Add QOM property accessors for SGPIO pins
2025-12-09 14:52 ` Cédric Le Goater
@ 2025-12-10 8:08 ` Yubin Zou
0 siblings, 0 replies; 13+ messages in thread
From: Yubin Zou @ 2025-12-10 8:08 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-devel, Peter Maydell, Steven Lee, Troy Lee, Jamin Lin,
Andrew Jeffery, Joel Stanley, Fabiano Rosas, Laurent Vivier,
Paolo Bonzini, Kane-Chen-AS, Nabih Estefan, qemu-arm
[-- Attachment #1: Type: text/plain, Size: 4705 bytes --]
Hi Cédric
Good catch. I will fix these issues.
Yubin
On Tue, Dec 9, 2025 at 6:52 AM Cédric Le Goater <clg@redhat.com> wrote:
> Hi,
>
> The subject needs a fix. Please remove the extra 'aspeed: '.
>
> On 12/9/25 01:01, Yubin Zou wrote:
> > This commit adds QOM property accessors for the Aspeed SGPIO pins.
>
> I think you can drop the above sentence from the commit log. It's
> redundant with the subject.
>
> > The `aspeed_sgpio_get_pin` and `aspeed_sgpio_set_pin` functions are
> > implemented to get and set the level of individual SGPIO pins. These
> > are then exposed as boolean properties on the SGPIO device object.
> >
> > Signed-off-by: Yubin Zou <yubinz@google.com>
> > ---
> > hw/gpio/aspeed_sgpio.c | 78
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 78 insertions(+)
> >
> > diff --git a/hw/gpio/aspeed_sgpio.c b/hw/gpio/aspeed_sgpio.c
> > index
> 8676fa7ced134f1f62dc9e30b42c5fe6db3de268..efa7e574abe87e33e58ac88dba5e3469c6702b83
> 100644
> > --- a/hw/gpio/aspeed_sgpio.c
> > +++ b/hw/gpio/aspeed_sgpio.c
> > @@ -91,6 +91,73 @@ static void aspeed_sgpio_2700_write(void *opaque,
> hwaddr offset, uint64_t data,
> > }
> > }
> >
> > +static bool aspeed_sgpio_get_pin_level(AspeedSGPIOState *s, int pin)
> > +{
> > + uint32_t value = s->ctrl_regs[pin >> 1];
> > + bool is_input = !(pin % 2);
> > + uint32_t bit_mask = 0;
> > +
> > + if (is_input) {
> > + bit_mask = SGPIO_SERIAL_IN_VAL_MASK;
> > + } else {
> > + bit_mask = SGPIO_SERIAL_OUT_VAL_MASK;
> > + }
> > +
> > + return value & bit_mask;
> > +}
> > +
> > +static void aspeed_sgpio_set_pin_level(AspeedSGPIOState *s, int pin,
> bool level)
> > +{
> > + uint32_t value = s->ctrl_regs[pin >> 1];
> > + bool is_input = !(pin % 2);
> > + uint32_t bit_mask = 0;
> > +
> > + if (is_input) {
> > + bit_mask = SGPIO_SERIAL_IN_VAL_MASK;
> > + } else {
> > + bit_mask = SGPIO_SERIAL_OUT_VAL_MASK;
> > + }
> > +
> > + if (level) {
> > + value |= bit_mask;
> > + } else {
> > + value &= ~bit_mask;
> > + }
> > + s->ctrl_regs[pin >> 1] = value;
> > +}
> > +
> > +static void aspeed_sgpio_get_pin(Object *obj, Visitor *v, const char
> *name,
> > + void *opaque, Error **errp)
> > +{
> > + bool level = true;
> > + int pin = 0xfff;
> > + AspeedSGPIOState *s = ASPEED_SGPIO(obj);
> > +
> > + if (sscanf(name, "sgpio%d", &pin) != 1) {
> > + error_setg(errp, "%s: error reading %s", __func__, name);
> > + return;
> > + }
> > + level = aspeed_sgpio_get_pin_level(s, pin);
> > + visit_type_bool(v, name, &level, errp);
> > +}
> > +
> > +static void aspeed_sgpio_set_pin(Object *obj, Visitor *v, const char
> *name,
> > + void *opaque, Error **errp)
> > +{
> > + bool level;
> > + int pin = 0xfff;
> > + AspeedSGPIOState *s = ASPEED_SGPIO(obj);
> > +
> > + if (!visit_type_bool(v, name, &level, errp)) {
> > + return;
> > + }
> > + if (sscanf(name, "sgpio%d", &pin) != 1) {
> > + error_setg(errp, "%s: error reading %s", __func__, name);
> > + return;
> > + }
> > + aspeed_sgpio_set_pin_level(s, pin, level);
> > +}
> > +
> > static const MemoryRegionOps aspeed_gpio_2700_ops = {
> > .read = aspeed_sgpio_2700_read,
> > .write = aspeed_sgpio_2700_write,
> > @@ -114,6 +181,16 @@ static void aspeed_sgpio_realize(DeviceState *dev,
> Error **errp)
> > sysbus_init_mmio(sbd, &s->iomem);
> > }
> >
> > +static void aspeed_sgpio_init(Object *obj)
> > +{
> > + for (int i = 0; i < ASPEED_SGPIO_MAX_PIN_PAIR * 2; i++) {
> > + char *name = g_strdup_printf("sgpio%d", i);
>
> You could use a g_autofree variable and drop the g_free below.
>
> How about using a "%03d" format in the printf and sscanf too ?
>
> C.
>
> > + object_property_add(obj, name, "bool", aspeed_sgpio_get_pin,
> > + aspeed_sgpio_set_pin, NULL, NULL);
> > + g_free(name);
> > + }
> > +}
> > +
> > static void aspeed_sgpio_class_init(ObjectClass *klass, const void
> *data)
> > {
> > DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -143,6 +220,7 @@ static const TypeInfo aspeed_sgpio_ast2700_info = {
> > .name = TYPE_ASPEED_SGPIO "-ast2700",
> > .parent = TYPE_ASPEED_SGPIO,
> > .class_init = aspeed_sgpio_2700_class_init,
> > + .instance_init = aspeed_sgpio_init,
> > };
> >
> > static void aspeed_sgpio_register_types(void)
> >
>
>
[-- Attachment #2: Type: text/html, Size: 6095 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-12-10 8:09 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-09 0:01 [PATCH v2 0/6] hw/gpio/aspeed_sgpio: Add Aspeed Serial GPIO (SGPIO) controller model Yubin Zou
2025-12-09 0:01 ` [PATCH v2 1/6] hw/gpio/aspeed_sgpio: Add basic device model for Aspeed SGPIO Yubin Zou
2025-12-09 9:22 ` Cédric Le Goater
2025-12-10 8:02 ` Yubin Zou
2025-12-09 0:01 ` [PATCH v2 2/6] hw/gpio/aspeed_sgpio: aspeed: Add QOM property accessors for SGPIO pins Yubin Zou
2025-12-09 14:52 ` Cédric Le Goater
2025-12-10 8:08 ` Yubin Zou
2025-12-09 0:01 ` [PATCH v2 3/6] hw/gpio/aspeed_sgpio: Implement SGPIO interrupt handling Yubin Zou
2025-12-09 0:01 ` [PATCH v2 4/6] hw/arm/aspeed_soc: Update Aspeed SoC to support two SGPIO controllers Yubin Zou
2025-12-09 16:15 ` Cédric Le Goater
2025-12-09 0:01 ` [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Wire SGPIO controller to AST2700 SoC Yubin Zou
2025-12-09 16:15 ` Cédric Le Goater
2025-12-09 0:01 ` [PATCH v2 6/6] test/qtest: Add Unit test for Aspeed SGPIO Yubin Zou
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).