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-googHi,
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)
>
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)
> >
>
>
© 2016 - 2025 Red Hat, Inc.