From: Felix Wu <flwu@google.com>
Added 32 bits property for ASPEED GPIO. Previously it can only be access in bitwise manner.
This change gives ASPEED similar behavior as Nuvoton.
Signed-off-by: Felix Wu <flwu@google.com>
---
hw/gpio/aspeed_gpio.c | 57 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 609a556908..2d78bf9515 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -1308,6 +1308,57 @@ static void aspeed_gpio_2700_write(void *opaque, hwaddr offset,
}
/* Setup functions */
+static void aspeed_gpio_set_set(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ uint32_t set_val = 0;
+ AspeedGPIOState *s = ASPEED_GPIO(obj);
+ AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
+ int set_idx = 0;
+
+ if (!visit_type_uint32(v, name, &set_val, errp)) {
+ return;
+ }
+
+ if (sscanf(name, "gpio-set[%d]", &set_idx) != 1) {
+ error_setg(errp, "%s: error reading %s", __func__, name);
+ return;
+ }
+
+ if (set_idx >= agc->nr_gpio_sets || set_idx < 0) {
+ error_setg(errp, "%s: invalid set_idx %s", __func__, name);
+ return;
+ }
+
+ aspeed_gpio_update(s, &s->sets[set_idx], set_val,
+ ~s->sets[set_idx].direction);
+}
+
+static void aspeed_gpio_get_set(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ uint32_t set_val = 0;
+ AspeedGPIOState *s = ASPEED_GPIO(obj);
+ AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
+ int set_idx = 0;
+
+ if (sscanf(name, "gpio-set[%d]", &set_idx) != 1) {
+ error_setg(errp, "%s: error reading %s", __func__, name);
+ return;
+ }
+
+ if (set_idx >= agc->nr_gpio_sets || set_idx < 0) {
+ error_setg(errp, "%s: invalid set_idx %s", __func__, name);
+ return;
+ }
+
+ set_val = s->sets[set_idx].data_value;
+ visit_type_uint32(v, name, &set_val, errp);
+}
+
+/****************** Setup functions ******************/
static const GPIOSetProperties ast2400_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
[0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
[1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} },
@@ -1435,6 +1486,12 @@ static void aspeed_gpio_init(Object *obj)
g_free(name);
}
}
+
+ for (int i = 0; i < agc->nr_gpio_sets; i++) {
+ char *name = g_strdup_printf("gpio-set[%d]", i);
+ object_property_add(obj, name, "uint32", aspeed_gpio_get_set,
+ aspeed_gpio_set_set, NULL, NULL);
+ }
}
static const VMStateDescription vmstate_gpio_regs = {
--
2.51.0.338.gd7d06c2dae-googOn Wed, 2025-09-03 at 21:38 +0000, Coco Li wrote: > From: Felix Wu <flwu@google.com> > > Added 32 bits property for ASPEED GPIO. Previously it can only be access in bitwise manner. > > This change gives ASPEED similar behavior as Nuvoton. Can you point me to the Nuvoton functionality? I had a quick look and couldn't immediately see an equivalent implementation. Regardless, I'd like to see more motivation in the commit message than "make it behave like Nuvoton". Can you provide a concrete use-case as an example? Andrew
Hello Andrew, I discussed with the original author of the patch and the Nuvoton functionality mentioned above has not been upstreamed since then, unfortunately. Aside from the comment above, we believe this patch series allows gpios to be accessed more dynamically/flexibly in complicated simulation, and also helps with accessing advanced gpio features, see the sgpio implementation later in this patch series. Best, Coco On Tue, Sep 9, 2025 at 8:52 PM Andrew Jeffery <andrew@codeconstruct.com.au> wrote: > On Wed, 2025-09-03 at 21:38 +0000, Coco Li wrote: > > From: Felix Wu <flwu@google.com> > > > > Added 32 bits property for ASPEED GPIO. Previously it can only be access > in bitwise manner. > > > > This change gives ASPEED similar behavior as Nuvoton. > > Can you point me to the Nuvoton functionality? I had a quick look and > couldn't immediately see an equivalent implementation. > > Regardless, I'd like to see more motivation in the commit message than > "make it behave like Nuvoton". Can you provide a concrete use-case as > an example? > > Andrew >
Hi Coco, On Fri, 2025-09-12 at 10:43 -0700, Coco Li wrote: > Hello Andrew, > > I discussed with the original author of the patch and the Nuvoton > functionality mentioned above has not been upstreamed since then, > unfortunately. Okay, so can you please update the commit message to reflect this? > Aside from the comment above, we believe this patch series allows gpios to > be accessed more dynamically/flexibly in complicated simulation, and also > helps with accessing advanced gpio features, see the sgpio implementation > later in this patch series. Can you please add a discussion of these complex simulations to the commit message? I'd like a description of what you're doing where the current pin-specific interface is insufficient. Motivations help a lot with the patch review. Thanks, Andrew
+Andrew for awareness
On 9/3/25 23:38, Coco Li wrote:
> From: Felix Wu <flwu@google.com>
>
> Added 32 bits property for ASPEED GPIO. Previously it can only be access in bitwise manner.
>
> This change gives ASPEED similar behavior as Nuvoton.
>
> Signed-off-by: Felix Wu <flwu@google.com>
> ---
> hw/gpio/aspeed_gpio.c | 57 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index 609a556908..2d78bf9515 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -1308,6 +1308,57 @@ static void aspeed_gpio_2700_write(void *opaque, hwaddr offset,
> }
>
> /* Setup functions */
> +static void aspeed_gpio_set_set(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + uint32_t set_val = 0;
> + AspeedGPIOState *s = ASPEED_GPIO(obj);
> + AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
> + int set_idx = 0;
> +
> + if (!visit_type_uint32(v, name, &set_val, errp)) {
> + return;
> + }
> +
> + if (sscanf(name, "gpio-set[%d]", &set_idx) != 1) {
> + error_setg(errp, "%s: error reading %s", __func__, name);
> + return;
> + }
> +
> + if (set_idx >= agc->nr_gpio_sets || set_idx < 0) {
> + error_setg(errp, "%s: invalid set_idx %s", __func__, name);
> + return;
> + }
> +
> + aspeed_gpio_update(s, &s->sets[set_idx], set_val,
> + ~s->sets[set_idx].direction);
> +}
> +
> +static void aspeed_gpio_get_set(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + uint32_t set_val = 0;
> + AspeedGPIOState *s = ASPEED_GPIO(obj);
> + AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
> + int set_idx = 0;
> +
> + if (sscanf(name, "gpio-set[%d]", &set_idx) != 1) {
> + error_setg(errp, "%s: error reading %s", __func__, name);
> + return;
> + }
> +
> + if (set_idx >= agc->nr_gpio_sets || set_idx < 0) {
> + error_setg(errp, "%s: invalid set_idx %s", __func__, name);
> + return;
> + }
> +
> + set_val = s->sets[set_idx].data_value;
> + visit_type_uint32(v, name, &set_val, errp);
> +}
> +
> +/****************** Setup functions ******************/
> static const GPIOSetProperties ast2400_set_props[ASPEED_GPIO_MAX_NR_SETS] = {
> [0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
> [1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} },
> @@ -1435,6 +1486,12 @@ static void aspeed_gpio_init(Object *obj)
> g_free(name);
> }
> }
> +
> + for (int i = 0; i < agc->nr_gpio_sets; i++) {
> + char *name = g_strdup_printf("gpio-set[%d]", i);
> + object_property_add(obj, name, "uint32", aspeed_gpio_get_set,
> + aspeed_gpio_set_set, NULL, NULL);
> + }
> }
>
> static const VMStateDescription vmstate_gpio_regs = {
© 2016 - 2026 Red Hat, Inc.