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 <[email protected]>
---
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)