* [PATCH v1 1/5] hw/gpio: Add property for ASPEED GPIO in 32 bits basis
2025-09-25 0:58 [PATCH v1 0/5] Add Aspeed GPIO test and Support Nuvoton Serial GPIO Expansion (SGPIO) device Coco Li
@ 2025-09-25 0:58 ` Coco Li
2025-10-01 23:24 ` Andrew Jeffery
2025-09-25 0:58 ` [PATCH v1 2/5] tests/qtest: Add qtest for for ASPEED GPIO gpio-set property Coco Li
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Coco Li @ 2025-09-25 0:58 UTC (permalink / raw)
To: peter.maydell, clg; +Cc: qemu-arm, qemu-devel, lixiaoyan, flwu, andrew
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.536.g15c5d4f767-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/5] hw/gpio: Add property for ASPEED GPIO in 32 bits basis
2025-09-25 0:58 ` [PATCH v1 1/5] hw/gpio: Add property for ASPEED GPIO in 32 bits basis Coco Li
@ 2025-10-01 23:24 ` Andrew Jeffery
2025-10-03 17:44 ` Coco Li
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Jeffery @ 2025-10-01 23:24 UTC (permalink / raw)
To: Coco Li, clg; +Cc: qemu-arm, qemu-devel, flwu, Peter Maydell
On Thu, 2025-09-25 at 00:58 +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.
I don't think this has adequately addressed my request on the prior
series:
https://lore.kernel.org/all/e244fdb5d2d889674a583df8f8b9bc4bf8d476f4.camel@codeconstruct.com.au/
Can you please improve the commit message?
I don't have any particular concern with the implementation, other than
understanding whether it's something that's reasonable to add to begin
with. The "sets" and their indexes are somewhat an implementation
detail. Exposing them might preclude a different implementation design,
though I'm also not sure why we'd change at this point.
Andrew
>
> 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 = {
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/5] hw/gpio: Add property for ASPEED GPIO in 32 bits basis
2025-10-01 23:24 ` Andrew Jeffery
@ 2025-10-03 17:44 ` Coco Li
2025-10-13 0:28 ` Andrew Jeffery
0 siblings, 1 reply; 14+ messages in thread
From: Coco Li @ 2025-10-03 17:44 UTC (permalink / raw)
To: Andrew Jeffery; +Cc: clg, qemu-arm, qemu-devel, flwu, Peter Maydell
On Wed, Oct 1, 2025 at 4:24 PM Andrew Jeffery
<andrew@codeconstruct.com.au> wrote:
>
> On Thu, 2025-09-25 at 00:58 +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.
>
> I don't think this has adequately addressed my request on the prior
> series:
>
> https://lore.kernel.org/all/e244fdb5d2d889674a583df8f8b9bc4bf8d476f4.camel@codeconstruct.com.au/
>
> Can you please improve the commit message?
>
> I don't have any particular concern with the implementation, other than
> understanding whether it's something that's reasonable to add to begin
> with. The "sets" and their indexes are somewhat an implementation
> detail. Exposing them might preclude a different implementation design,
> though I'm also not sure why we'd change at this point.
>
> Andrew
>
Hello Andrew,
To confirm that I understand your request, I should do the following:
1) remove the reference to Nuvoton behavior in the ASPEED patches
(will do in follow up)
2) you asked for discussion of complex simulations, is the addition in
the cover patch sufficient? Otherwise, could you elaborate on your
comment here on what I can help provide please?
Best,
Coco
> >
> > 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 = {
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 1/5] hw/gpio: Add property for ASPEED GPIO in 32 bits basis
2025-10-03 17:44 ` Coco Li
@ 2025-10-13 0:28 ` Andrew Jeffery
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Jeffery @ 2025-10-13 0:28 UTC (permalink / raw)
To: Coco Li; +Cc: clg, qemu-arm, qemu-devel, flwu, Peter Maydell
On Fri, 2025-10-03 at 10:44 -0700, Coco Li wrote:
> On Wed, Oct 1, 2025 at 4:24 PM Andrew Jeffery
> <andrew@codeconstruct.com.au> wrote:
> >
> > On Thu, 2025-09-25 at 00:58 +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.
> >
> > I don't think this has adequately addressed my request on the prior
> > series:
> >
> > https://lore.kernel.org/all/e244fdb5d2d889674a583df8f8b9bc4bf8d476f4.camel@codeconstruct.com.au/
> >
> > Can you please improve the commit message?
> >
> > I don't have any particular concern with the implementation, other than
> > understanding whether it's something that's reasonable to add to begin
> > with. The "sets" and their indexes are somewhat an implementation
> > detail. Exposing them might preclude a different implementation design,
> > though I'm also not sure why we'd change at this point.
> >
> > Andrew
> >
>
> Hello Andrew,
>
> To confirm that I understand your request, I should do the following:
>
> 1) remove the reference to Nuvoton behavior in the ASPEED patches
> (will do in follow up)
> 2) you asked for discussion of complex simulations, is the addition in
> the cover patch sufficient? Otherwise, could you elaborate on your
> comment here on what I can help provide please?
Can you please integrate the description from the cover letter into the
commit message? It's not always the case that the series cover letter
is tracked in the git history.
Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 2/5] tests/qtest: Add qtest for for ASPEED GPIO gpio-set property
2025-09-25 0:58 [PATCH v1 0/5] Add Aspeed GPIO test and Support Nuvoton Serial GPIO Expansion (SGPIO) device Coco Li
2025-09-25 0:58 ` [PATCH v1 1/5] hw/gpio: Add property for ASPEED GPIO in 32 bits basis Coco Li
@ 2025-09-25 0:58 ` Coco Li
2025-09-25 0:58 ` [PATCH v1 3/5] hw/arm/npcm8xx.c: Add all IRQ ENUMs Coco Li
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Coco Li @ 2025-09-25 0:58 UTC (permalink / raw)
To: peter.maydell, clg; +Cc: qemu-arm, qemu-devel, lixiaoyan, flwu, andrew
From: Felix Wu <flwu@google.com>
- Added qtests to test gpio-set property for ASPEED.
- Added function to get uint in qdict.
Signed-off-by: Felix Wu <flwu@google.com>
---
include/qobject/qdict.h | 1 +
qobject/qdict.c | 13 ++++
tests/qtest/aspeed_gpio-test.c | 105 ++++++++++++++++++++++++++++++---
3 files changed, 110 insertions(+), 9 deletions(-)
diff --git a/include/qobject/qdict.h b/include/qobject/qdict.h
index 903e6e5462..861996f08d 100644
--- a/include/qobject/qdict.h
+++ b/include/qobject/qdict.h
@@ -57,6 +57,7 @@ void qdict_put_str(QDict *qdict, const char *key, const char *value);
double qdict_get_double(const QDict *qdict, const char *key);
int64_t qdict_get_int(const QDict *qdict, const char *key);
+uint64_t qdict_get_uint(const QDict *qdict, const char *key);
bool qdict_get_bool(const QDict *qdict, const char *key);
QList *qdict_get_qlist(const QDict *qdict, const char *key);
QDict *qdict_get_qdict(const QDict *qdict, const char *key);
diff --git a/qobject/qdict.c b/qobject/qdict.c
index a90ac9ae2f..0dafe6d421 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -209,6 +209,19 @@ int64_t qdict_get_int(const QDict *qdict, const char *key)
return qnum_get_int(qobject_to(QNum, qdict_get(qdict, key)));
}
+/**
+ * qdict_get_uint(): Get an unsigned integer mapped by 'key'
+ *
+ * This function assumes that 'key' exists and it stores a
+ * QNum representable as uint.
+ *
+ * Return unsigned integer mapped by 'key'.
+ */
+uint64_t qdict_get_uint(const QDict *qdict, const char *key)
+{
+ return qnum_get_uint(qobject_to(QNum, qdict_get(qdict, key)));
+}
+
/**
* qdict_get_bool(): Get a bool mapped by 'key'
*
diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
index 12675d4cbb..c2f9ca2298 100644
--- a/tests/qtest/aspeed_gpio-test.c
+++ b/tests/qtest/aspeed_gpio-test.c
@@ -27,28 +27,115 @@
#include "qemu/timer.h"
#include "qobject/qdict.h"
#include "libqtest-single.h"
+#include "qemu/typedefs.h"
#define AST2600_GPIO_BASE 0x1E780000
#define GPIO_ABCD_DATA_VALUE 0x000
#define GPIO_ABCD_DIRECTION 0x004
+static uint32_t qtest_qom_get_uint32(QTestState *s, const char *path,
+ const char *property)
+{
+ QDict *r;
+
+ uint32_t res;
+ r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
+ "{ 'path': %s, 'property': %s } }", path, property);
+ res = qdict_get_uint(r, "return");
+ qobject_unref(r);
+
+ return res;
+}
+
+static void qtest_qom_set_uint32(QTestState *s, const char *path,
+ const char *property, uint32_t value)
+{
+ QDict *r;
+
+ r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
+ "{ 'path': %s, 'property': %s, 'value': %" PRIu32 " } }",
+ path, property, value);
+ qobject_unref(r);
+}
+
+static const char *resp_get_error(QDict *r, const char* error_key)
+{
+ QDict *qdict;
+
+ g_assert(r);
+
+ qdict = qdict_get_qdict(r, "error");
+ if (qdict) {
+ return qdict_get_str(qdict, error_key);
+ }
+
+ return NULL;
+}
+
+static bool qtest_qom_check_error(QTestState *s, const char *path,
+ const char *property, const char *error_msg,
+ const char *error_msg_key)
+{
+ QDict *r;
+ bool b;
+
+ r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
+ "{ 'path': %s, 'property': %s } }", path, property);
+ b = g_str_equal(resp_get_error(r, error_msg_key), error_msg);
+ qobject_unref(r);
+
+ return b;
+}
+
static void test_set_colocated_pins(const void *data)
{
QTestState *s = (QTestState *)data;
-
+ const char path[] = "/machine/soc/gpio";
/*
* gpioV4-7 occupy bits within a single 32-bit value, so we want to make
* sure that modifying one doesn't affect the other.
*/
- qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
- qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
- qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
- qtest_qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
- g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
- g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
- g_assert(qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
- g_assert(!qtest_qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
+ qtest_qom_set_bool(s, path, "gpioV4", true);
+ qtest_qom_set_bool(s, path, "gpioV5", false);
+ qtest_qom_set_bool(s, path, "gpioV6", true);
+ qtest_qom_set_bool(s, path, "gpioV7", false);
+ g_assert(qtest_qom_get_bool(s, path, "gpioV4"));
+ g_assert(!qtest_qom_get_bool(s, path, "gpioV5"));
+ g_assert(qtest_qom_get_bool(s, path, "gpioV6"));
+ g_assert(!qtest_qom_get_bool(s, path, "gpioV7"));
+
+ /*
+ * Testing the gpio-set[%d] properties, using individual gpio boolean
+ * properties to do cross check.
+ * We use gpioR4-7 for test, Setting them to be 0b1010.
+ */
+ qtest_qom_set_uint32(s, path, "gpio-set[4]", 0x0);
+ g_assert(qtest_qom_get_uint32(s, path, "gpio-set[4]") == 0x0);
+ qtest_qom_set_uint32(s, path, "gpio-set[4]", 0xa000);
+ g_assert(qtest_qom_get_uint32(s, path, "gpio-set[4]") == 0xa000);
+
+ g_assert(!qtest_qom_get_bool(s, path, "gpioR4"));
+ g_assert(qtest_qom_get_bool(s, path, "gpioR5"));
+ g_assert(!qtest_qom_get_bool(s, path, "gpioR6"));
+ g_assert(qtest_qom_get_bool(s, path, "gpioR7"));
+
+ /*
+ * Testing the invalid indexing, the response info should contain following
+ * info:
+ * {key: "class", value: "GenericError"}
+ *
+ * For pins, it should follow "gpio%2[A-Z]%1d" or "gpio%3[18A-E]%1d" format.
+ */
+ const char error_msg[] = "GenericError";
+ const char error_msg_key[] = "class";
+
+ g_assert(qtest_qom_check_error(s, path, "gpioR+1", error_msg,
+ error_msg_key));
+ g_assert(qtest_qom_check_error(s, path, "gpio-set[99]", error_msg,
+ error_msg_key));
+ g_assert(qtest_qom_check_error(s, path, "gpio-set[-3]", error_msg,
+ error_msg_key));
}
static void test_set_input_pins(const void *data)
--
2.51.0.536.g15c5d4f767-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v1 3/5] hw/arm/npcm8xx.c: Add all IRQ ENUMs
2025-09-25 0:58 [PATCH v1 0/5] Add Aspeed GPIO test and Support Nuvoton Serial GPIO Expansion (SGPIO) device Coco Li
2025-09-25 0:58 ` [PATCH v1 1/5] hw/gpio: Add property for ASPEED GPIO in 32 bits basis Coco Li
2025-09-25 0:58 ` [PATCH v1 2/5] tests/qtest: Add qtest for for ASPEED GPIO gpio-set property Coco Li
@ 2025-09-25 0:58 ` Coco Li
2025-09-25 1:08 ` Philippe Mathieu-Daudé
2025-09-25 0:58 ` [PATCH v1 4/5] hw/gpio/npcm8xx: Implement SIOX (SPGIO) device for NPCM without input pin logic Coco Li
2025-09-25 0:58 ` [PATCH v1 5/5] hw/gpio/npcm8xx: Implement npcm sgpio device " Coco Li
4 siblings, 1 reply; 14+ messages in thread
From: Coco Li @ 2025-09-25 0:58 UTC (permalink / raw)
To: peter.maydell, clg; +Cc: qemu-arm, qemu-devel, lixiaoyan, flwu, andrew, Hao Wu
In the process of implementing serial gpio and adding the corresponding
ENUMs, also complete the list for npcm8xx.
Signed-off-by: Coco Li <lixiaoyan@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
hw/arm/npcm8xx.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/hw/arm/npcm8xx.c b/hw/arm/npcm8xx.c
index a276fea698..10887d07fa 100644
--- a/hw/arm/npcm8xx.c
+++ b/hw/arm/npcm8xx.c
@@ -92,8 +92,14 @@ enum NPCM8xxInterrupt {
NPCM8XX_GMAC2_IRQ,
NPCM8XX_GMAC3_IRQ,
NPCM8XX_GMAC4_IRQ,
- NPCM8XX_MMC_IRQ = 26,
+ NPCM8XX_ESPI_IRQ,
+ NPCM8XX_SIOX0_IRQ,
+ NPCM8XX_SIOX1_IRQ,
+ NPCM8XX_MC_IRQ = 25,
+ NPCM8XX_MMC_IRQ,
NPCM8XX_PSPI_IRQ = 28,
+ NPCM8XX_VDMA_IRQ,
+ NPCM8XX_MCTP_IRQ,
NPCM8XX_TIMER0_IRQ = 32, /* Timer Module 0 */
NPCM8XX_TIMER1_IRQ,
NPCM8XX_TIMER2_IRQ,
@@ -116,6 +122,14 @@ enum NPCM8xxInterrupt {
NPCM8XX_OHCI1_IRQ,
NPCM8XX_EHCI2_IRQ,
NPCM8XX_OHCI2_IRQ,
+ NPCM8XX_SPI1_IRQ = 82,
+ NPCM8XX_RNG_IRQ = 84,
+ NPCM8XX_SPI0_IRQ = 85,
+ NPCM8XX_SPI3_IRQ = 87,
+ NPCM8XX_GDMA0_IRQ = 88,
+ NPCM8XX_GDMA1_IRQ,
+ NPCM8XX_GDMA2_IRQ,
+ NPCM8XX_OTP_IRQ = 92,
NPCM8XX_PWM0_IRQ = 93, /* PWM module 0 */
NPCM8XX_PWM1_IRQ, /* PWM module 1 */
NPCM8XX_MFT0_IRQ = 96, /* MFT module 0 */
@@ -128,6 +142,11 @@ enum NPCM8xxInterrupt {
NPCM8XX_MFT7_IRQ, /* MFT module 7 */
NPCM8XX_PCI_MBOX1_IRQ = 105,
NPCM8XX_PCI_MBOX2_IRQ,
+ NPCM8XX_GPIO231_IRQ = 108,
+ NPCM8XX_GPIO233_IRQ,
+ NPCM8XX_GPIO234_IRQ,
+ NPCM8XX_GPIO93_IRQ,
+ NPCM8XX_GPIO94_IRQ,
NPCM8XX_GPIO0_IRQ = 116,
NPCM8XX_GPIO1_IRQ,
NPCM8XX_GPIO2_IRQ,
@@ -163,6 +182,12 @@ enum NPCM8xxInterrupt {
NPCM8XX_SMBUS24_IRQ,
NPCM8XX_SMBUS25_IRQ,
NPCM8XX_SMBUS26_IRQ,
+ NPCM8XX_FLM0_IRQ = 160,
+ NPCM8XX_FLM1_IRQ,
+ NPCM8XX_FLM2_IRQ,
+ NPCM8XX_FLM3_IRQ,
+ NPCM8XX_JMT1_IRQ = 188,
+ NPCM8XX_JMT2_IRQ,
NPCM8XX_UART0_IRQ = 192,
NPCM8XX_UART1_IRQ,
NPCM8XX_UART2_IRQ,
@@ -170,6 +195,22 @@ enum NPCM8xxInterrupt {
NPCM8XX_UART4_IRQ,
NPCM8XX_UART5_IRQ,
NPCM8XX_UART6_IRQ,
+ NPCM8XX_I3C0_IRQ = 224,
+ NPCM8XX_I3C1_IRQ,
+ NPCM8XX_I3C2_IRQ,
+ NPCM8XX_I3C3_IRQ,
+ NPCM8XX_I3C4_IRQ,
+ NPCM8XX_I3C5_IRQ,
+ NPCM8XX_A35INTERR_IRQ = 240,
+ NPCM8XX_A35EXTERR_IRQ,
+ NPCM8XX_PMU0_IRQ,
+ NPCM8XX_PMU1_IRQ,
+ NPCM8XX_PMU2_IRQ,
+ NPCM8XX_PMU3_IRQ,
+ NPCM8XX_CTI0_IRQ,
+ NPCM8XX_CTI1_IRQ,
+ NPCM8XX_CTI2_IRQ,
+ NPCM8XX_CTI3_IRQ,
};
/* Total number of GIC interrupts, including internal Cortex-A35 interrupts. */
--
2.51.0.536.g15c5d4f767-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 3/5] hw/arm/npcm8xx.c: Add all IRQ ENUMs
2025-09-25 0:58 ` [PATCH v1 3/5] hw/arm/npcm8xx.c: Add all IRQ ENUMs Coco Li
@ 2025-09-25 1:08 ` Philippe Mathieu-Daudé
2025-09-26 21:48 ` Coco Li
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-25 1:08 UTC (permalink / raw)
To: Coco Li, peter.maydell, clg; +Cc: qemu-arm, qemu-devel, flwu, andrew, Hao Wu
Hi,
On 25/9/25 02:58, Coco Li wrote:
> In the process of implementing serial gpio and adding the corresponding
> ENUMs, also complete the list for npcm8xx.
>
> Signed-off-by: Coco Li <lixiaoyan@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> ---
> hw/arm/npcm8xx.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/npcm8xx.c b/hw/arm/npcm8xx.c
> index a276fea698..10887d07fa 100644
> --- a/hw/arm/npcm8xx.c
> +++ b/hw/arm/npcm8xx.c
> @@ -92,8 +92,14 @@ enum NPCM8xxInterrupt {
> NPCM8XX_GMAC2_IRQ,
> NPCM8XX_GMAC3_IRQ,
> NPCM8XX_GMAC4_IRQ,
> - NPCM8XX_MMC_IRQ = 26,
> + NPCM8XX_ESPI_IRQ,
> + NPCM8XX_SIOX0_IRQ,
> + NPCM8XX_SIOX1_IRQ,
> + NPCM8XX_MC_IRQ = 25,
> + NPCM8XX_MMC_IRQ,
> NPCM8XX_PSPI_IRQ = 28,
> + NPCM8XX_VDMA_IRQ,
> + NPCM8XX_MCTP_IRQ,
> NPCM8XX_TIMER0_IRQ = 32, /* Timer Module 0 */
> NPCM8XX_TIMER1_IRQ,
> NPCM8XX_TIMER2_IRQ,
> @@ -116,6 +122,14 @@ enum NPCM8xxInterrupt {
> NPCM8XX_OHCI1_IRQ,
> NPCM8XX_EHCI2_IRQ,
> NPCM8XX_OHCI2_IRQ,
> + NPCM8XX_SPI1_IRQ = 82,
> + NPCM8XX_RNG_IRQ = 84,
> + NPCM8XX_SPI0_IRQ = 85,
> + NPCM8XX_SPI3_IRQ = 87,
> + NPCM8XX_GDMA0_IRQ = 88,
> + NPCM8XX_GDMA1_IRQ,
> + NPCM8XX_GDMA2_IRQ,
> + NPCM8XX_OTP_IRQ = 92,
> NPCM8XX_PWM0_IRQ = 93, /* PWM module 0 */
> NPCM8XX_PWM1_IRQ, /* PWM module 1 */
> NPCM8XX_MFT0_IRQ = 96, /* MFT module 0 */
> @@ -128,6 +142,11 @@ enum NPCM8xxInterrupt {
> NPCM8XX_MFT7_IRQ, /* MFT module 7 */
> NPCM8XX_PCI_MBOX1_IRQ = 105,
> NPCM8XX_PCI_MBOX2_IRQ,
> + NPCM8XX_GPIO231_IRQ = 108,
> + NPCM8XX_GPIO233_IRQ,
> + NPCM8XX_GPIO234_IRQ,
> + NPCM8XX_GPIO93_IRQ,
> + NPCM8XX_GPIO94_IRQ,
> NPCM8XX_GPIO0_IRQ = 116,
> NPCM8XX_GPIO1_IRQ,
> NPCM8XX_GPIO2_IRQ,
> @@ -163,6 +182,12 @@ enum NPCM8xxInterrupt {
> NPCM8XX_SMBUS24_IRQ,
> NPCM8XX_SMBUS25_IRQ,
> NPCM8XX_SMBUS26_IRQ,
> + NPCM8XX_FLM0_IRQ = 160,
> + NPCM8XX_FLM1_IRQ,
> + NPCM8XX_FLM2_IRQ,
> + NPCM8XX_FLM3_IRQ,
Minor style comment, maybe worth adding a new line when the
following enum is not contiguous.
Regards,
Phil.
> + NPCM8XX_JMT1_IRQ = 188,
> + NPCM8XX_JMT2_IRQ,
> NPCM8XX_UART0_IRQ = 192,
> NPCM8XX_UART1_IRQ,
> NPCM8XX_UART2_IRQ,
> @@ -170,6 +195,22 @@ enum NPCM8xxInterrupt {
> NPCM8XX_UART4_IRQ,
> NPCM8XX_UART5_IRQ,
> NPCM8XX_UART6_IRQ,
> + NPCM8XX_I3C0_IRQ = 224,
> + NPCM8XX_I3C1_IRQ,
> + NPCM8XX_I3C2_IRQ,
> + NPCM8XX_I3C3_IRQ,
> + NPCM8XX_I3C4_IRQ,
> + NPCM8XX_I3C5_IRQ,
> + NPCM8XX_A35INTERR_IRQ = 240,
> + NPCM8XX_A35EXTERR_IRQ,
> + NPCM8XX_PMU0_IRQ,
> + NPCM8XX_PMU1_IRQ,
> + NPCM8XX_PMU2_IRQ,
> + NPCM8XX_PMU3_IRQ,
> + NPCM8XX_CTI0_IRQ,
> + NPCM8XX_CTI1_IRQ,
> + NPCM8XX_CTI2_IRQ,
> + NPCM8XX_CTI3_IRQ,
> };
>
> /* Total number of GIC interrupts, including internal Cortex-A35 interrupts. */
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 3/5] hw/arm/npcm8xx.c: Add all IRQ ENUMs
2025-09-25 1:08 ` Philippe Mathieu-Daudé
@ 2025-09-26 21:48 ` Coco Li
2025-09-29 9:46 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 14+ messages in thread
From: Coco Li @ 2025-09-26 21:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: peter.maydell, clg, qemu-arm, qemu-devel, flwu, andrew, Hao Wu
[-- Attachment #1: Type: text/plain, Size: 3877 bytes --]
Hi Phil,
Thanks for the review!
It looks like IRQ mapping enums on other boards also generally do not have
line breaks, is it ok if I keep it like this for consistency sake?
Best,
Coco
On Wed, Sep 24, 2025 at 6:08 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:
> Hi,
>
> On 25/9/25 02:58, Coco Li wrote:
> > In the process of implementing serial gpio and adding the corresponding
> > ENUMs, also complete the list for npcm8xx.
> >
> > Signed-off-by: Coco Li <lixiaoyan@google.com>
> > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > ---
> > hw/arm/npcm8xx.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/npcm8xx.c b/hw/arm/npcm8xx.c
> > index a276fea698..10887d07fa 100644
> > --- a/hw/arm/npcm8xx.c
> > +++ b/hw/arm/npcm8xx.c
> > @@ -92,8 +92,14 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_GMAC2_IRQ,
> > NPCM8XX_GMAC3_IRQ,
> > NPCM8XX_GMAC4_IRQ,
> > - NPCM8XX_MMC_IRQ = 26,
> > + NPCM8XX_ESPI_IRQ,
> > + NPCM8XX_SIOX0_IRQ,
> > + NPCM8XX_SIOX1_IRQ,
> > + NPCM8XX_MC_IRQ = 25,
> > + NPCM8XX_MMC_IRQ,
> > NPCM8XX_PSPI_IRQ = 28,
> > + NPCM8XX_VDMA_IRQ,
> > + NPCM8XX_MCTP_IRQ,
> > NPCM8XX_TIMER0_IRQ = 32, /* Timer Module 0 */
> > NPCM8XX_TIMER1_IRQ,
> > NPCM8XX_TIMER2_IRQ,
> > @@ -116,6 +122,14 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_OHCI1_IRQ,
> > NPCM8XX_EHCI2_IRQ,
> > NPCM8XX_OHCI2_IRQ,
> > + NPCM8XX_SPI1_IRQ = 82,
> > + NPCM8XX_RNG_IRQ = 84,
> > + NPCM8XX_SPI0_IRQ = 85,
> > + NPCM8XX_SPI3_IRQ = 87,
> > + NPCM8XX_GDMA0_IRQ = 88,
> > + NPCM8XX_GDMA1_IRQ,
> > + NPCM8XX_GDMA2_IRQ,
> > + NPCM8XX_OTP_IRQ = 92,
> > NPCM8XX_PWM0_IRQ = 93, /* PWM module 0 */
> > NPCM8XX_PWM1_IRQ, /* PWM module 1 */
> > NPCM8XX_MFT0_IRQ = 96, /* MFT module 0 */
> > @@ -128,6 +142,11 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_MFT7_IRQ, /* MFT module 7 */
> > NPCM8XX_PCI_MBOX1_IRQ = 105,
> > NPCM8XX_PCI_MBOX2_IRQ,
> > + NPCM8XX_GPIO231_IRQ = 108,
> > + NPCM8XX_GPIO233_IRQ,
> > + NPCM8XX_GPIO234_IRQ,
> > + NPCM8XX_GPIO93_IRQ,
> > + NPCM8XX_GPIO94_IRQ,
> > NPCM8XX_GPIO0_IRQ = 116,
> > NPCM8XX_GPIO1_IRQ,
> > NPCM8XX_GPIO2_IRQ,
> > @@ -163,6 +182,12 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_SMBUS24_IRQ,
> > NPCM8XX_SMBUS25_IRQ,
> > NPCM8XX_SMBUS26_IRQ,
> > + NPCM8XX_FLM0_IRQ = 160,
> > + NPCM8XX_FLM1_IRQ,
> > + NPCM8XX_FLM2_IRQ,
> > + NPCM8XX_FLM3_IRQ,
>
> Minor style comment, maybe worth adding a new line when the
> following enum is not contiguous.
>
> Regards,
>
> Phil.
>
> > + NPCM8XX_JMT1_IRQ = 188,
> > + NPCM8XX_JMT2_IRQ,
> > NPCM8XX_UART0_IRQ = 192,
> > NPCM8XX_UART1_IRQ,
> > NPCM8XX_UART2_IRQ,
> > @@ -170,6 +195,22 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_UART4_IRQ,
> > NPCM8XX_UART5_IRQ,
> > NPCM8XX_UART6_IRQ,
> > + NPCM8XX_I3C0_IRQ = 224,
> > + NPCM8XX_I3C1_IRQ,
> > + NPCM8XX_I3C2_IRQ,
> > + NPCM8XX_I3C3_IRQ,
> > + NPCM8XX_I3C4_IRQ,
> > + NPCM8XX_I3C5_IRQ,
> > + NPCM8XX_A35INTERR_IRQ = 240,
> > + NPCM8XX_A35EXTERR_IRQ,
> > + NPCM8XX_PMU0_IRQ,
> > + NPCM8XX_PMU1_IRQ,
> > + NPCM8XX_PMU2_IRQ,
> > + NPCM8XX_PMU3_IRQ,
> > + NPCM8XX_CTI0_IRQ,
> > + NPCM8XX_CTI1_IRQ,
> > + NPCM8XX_CTI2_IRQ,
> > + NPCM8XX_CTI3_IRQ,
> > };
> >
> > /* Total number of GIC interrupts, including internal Cortex-A35
> interrupts. */
>
>
[-- Attachment #2: Type: text/html, Size: 5202 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 3/5] hw/arm/npcm8xx.c: Add all IRQ ENUMs
2025-09-26 21:48 ` Coco Li
@ 2025-09-29 9:46 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-29 9:46 UTC (permalink / raw)
To: Coco Li; +Cc: peter.maydell, clg, qemu-arm, qemu-devel, flwu, andrew, Hao Wu
On 26/9/25 23:48, Coco Li wrote:
> Hi Phil,
>
> Thanks for the review!
> It looks like IRQ mapping enums on other boards also generally do not
> have line breaks, is it ok if I keep it like this for consistency sake?
This is just a suggestion, since you are modifying these lines. I
won't object if you keep the old style, and I don't mind if the
other boards as not changed (also, can be done as future cleanup).
(please avoid top-posting on technical mailing lists).
>
> Best,
> Coco
>
> On Wed, Sep 24, 2025 at 6:08 PM Philippe Mathieu-Daudé
> <philmd@linaro.org <mailto:philmd@linaro.org>> wrote:
>
> Hi,
>
> On 25/9/25 02:58, Coco Li wrote:
> > In the process of implementing serial gpio and adding the
> corresponding
> > ENUMs, also complete the list for npcm8xx.
> >
> > Signed-off-by: Coco Li <lixiaoyan@google.com
> <mailto:lixiaoyan@google.com>>
> > Reviewed-by: Hao Wu <wuhaotsh@google.com
> <mailto:wuhaotsh@google.com>>
> > ---
> > hw/arm/npcm8xx.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/npcm8xx.c b/hw/arm/npcm8xx.c
> > index a276fea698..10887d07fa 100644
> > --- a/hw/arm/npcm8xx.c
> > +++ b/hw/arm/npcm8xx.c
> > @@ -92,8 +92,14 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_GMAC2_IRQ,
> > NPCM8XX_GMAC3_IRQ,
> > NPCM8XX_GMAC4_IRQ,
> > - NPCM8XX_MMC_IRQ = 26,
> > + NPCM8XX_ESPI_IRQ,
> > + NPCM8XX_SIOX0_IRQ,
> > + NPCM8XX_SIOX1_IRQ,
> > + NPCM8XX_MC_IRQ = 25,
> > + NPCM8XX_MMC_IRQ,
> > NPCM8XX_PSPI_IRQ = 28,
> > + NPCM8XX_VDMA_IRQ,
> > + NPCM8XX_MCTP_IRQ,
> > NPCM8XX_TIMER0_IRQ = 32, /* Timer Module 0 */
> > NPCM8XX_TIMER1_IRQ,
> > NPCM8XX_TIMER2_IRQ,
> > @@ -116,6 +122,14 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_OHCI1_IRQ,
> > NPCM8XX_EHCI2_IRQ,
> > NPCM8XX_OHCI2_IRQ,
> > + NPCM8XX_SPI1_IRQ = 82,
> > + NPCM8XX_RNG_IRQ = 84,
> > + NPCM8XX_SPI0_IRQ = 85,
> > + NPCM8XX_SPI3_IRQ = 87,
> > + NPCM8XX_GDMA0_IRQ = 88,
> > + NPCM8XX_GDMA1_IRQ,
> > + NPCM8XX_GDMA2_IRQ,
> > + NPCM8XX_OTP_IRQ = 92,
> > NPCM8XX_PWM0_IRQ = 93, /* PWM module 0 */
> > NPCM8XX_PWM1_IRQ, /* PWM module 1 */
> > NPCM8XX_MFT0_IRQ = 96, /* MFT module 0 */
> > @@ -128,6 +142,11 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_MFT7_IRQ, /* MFT module 7 */
> > NPCM8XX_PCI_MBOX1_IRQ = 105,
> > NPCM8XX_PCI_MBOX2_IRQ,
> > + NPCM8XX_GPIO231_IRQ = 108,
> > + NPCM8XX_GPIO233_IRQ,
> > + NPCM8XX_GPIO234_IRQ,
> > + NPCM8XX_GPIO93_IRQ,
> > + NPCM8XX_GPIO94_IRQ,
> > NPCM8XX_GPIO0_IRQ = 116,
> > NPCM8XX_GPIO1_IRQ,
> > NPCM8XX_GPIO2_IRQ,
> > @@ -163,6 +182,12 @@ enum NPCM8xxInterrupt {
> > NPCM8XX_SMBUS24_IRQ,
> > NPCM8XX_SMBUS25_IRQ,
> > NPCM8XX_SMBUS26_IRQ,
> > + NPCM8XX_FLM0_IRQ = 160,
> > + NPCM8XX_FLM1_IRQ,
> > + NPCM8XX_FLM2_IRQ,
> > + NPCM8XX_FLM3_IRQ,
>
> Minor style comment, maybe worth adding a new line when the
> following enum is not contiguous.
>
> Regards,
>
> Phil.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 4/5] hw/gpio/npcm8xx: Implement SIOX (SPGIO) device for NPCM without input pin logic
2025-09-25 0:58 [PATCH v1 0/5] Add Aspeed GPIO test and Support Nuvoton Serial GPIO Expansion (SGPIO) device Coco Li
` (2 preceding siblings ...)
2025-09-25 0:58 ` [PATCH v1 3/5] hw/arm/npcm8xx.c: Add all IRQ ENUMs Coco Li
@ 2025-09-25 0:58 ` Coco Li
2025-09-25 0:58 ` [PATCH v1 5/5] hw/gpio/npcm8xx: Implement npcm sgpio device " Coco Li
4 siblings, 0 replies; 14+ messages in thread
From: Coco Li @ 2025-09-25 0:58 UTC (permalink / raw)
To: peter.maydell, clg; +Cc: qemu-arm, qemu-devel, lixiaoyan, flwu, andrew, Hao Wu
Signed-off-by: Coco Li <lixiaoyan@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
hw/arm/npcm8xx.c | 23 +-
hw/gpio/meson.build | 1 +
hw/gpio/npcm8xx_sgpio.c | 425 +++++++++++++++++++++++++++++++
hw/gpio/trace-events | 4 +
include/hw/arm/npcm8xx.h | 2 +
include/hw/gpio/npcm8xx_sgpio.h | 45 ++++
tests/qtest/meson.build | 3 +-
tests/qtest/npcm8xx_sgpio-test.c | 100 ++++++++
8 files changed, 600 insertions(+), 3 deletions(-)
create mode 100644 hw/gpio/npcm8xx_sgpio.c
create mode 100644 include/hw/gpio/npcm8xx_sgpio.h
create mode 100644 tests/qtest/npcm8xx_sgpio-test.c
diff --git a/hw/arm/npcm8xx.c b/hw/arm/npcm8xx.c
index 10887d07fa..ae72c6d54b 100644
--- a/hw/arm/npcm8xx.c
+++ b/hw/arm/npcm8xx.c
@@ -369,6 +369,11 @@ static const struct {
},
};
+static const hwaddr npcm8xx_sgpio_addr[] = {
+ 0xf0101000,
+ 0xf0102000,
+};
+
static const struct {
const char *name;
hwaddr regs_addr;
@@ -474,6 +479,11 @@ static void npcm8xx_init(Object *obj)
}
+ for (i = 0; i < ARRAY_SIZE(s->sgpio); i++) {
+ object_initialize_child(obj, "sgpio[*]",
+ &s->sgpio[i], TYPE_NPCM8XX_SGPIO);
+ }
+
for (i = 0; i < ARRAY_SIZE(s->smbus); i++) {
object_initialize_child(obj, "smbus[*]", &s->smbus[i],
TYPE_NPCM7XX_SMBUS);
@@ -671,6 +681,17 @@ static void npcm8xx_realize(DeviceState *dev, Error **errp)
npcm8xx_irq(s, NPCM8XX_GPIO0_IRQ + i));
}
+ /* Serial SIOX modules. Cannot fail. */
+ QEMU_BUILD_BUG_ON(ARRAY_SIZE(npcm8xx_sgpio_addr) != ARRAY_SIZE(s->sgpio));
+ for (i = 0; i < ARRAY_SIZE(s->sgpio); i++) {
+ Object *obj = OBJECT(&s->sgpio[i]);
+
+ sysbus_realize(SYS_BUS_DEVICE(obj), &error_abort);
+ sysbus_mmio_map(SYS_BUS_DEVICE(obj), 0, npcm8xx_sgpio_addr[i]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(obj), 0,
+ npcm8xx_irq(s, NPCM8XX_SIOX0_IRQ + i));
+ }
+
/* SMBus modules. Cannot fail. */
QEMU_BUILD_BUG_ON(ARRAY_SIZE(npcm8xx_smbus_addr) != ARRAY_SIZE(s->smbus));
for (i = 0; i < ARRAY_SIZE(s->smbus); i++) {
@@ -818,8 +839,6 @@ static void npcm8xx_realize(DeviceState *dev, Error **errp)
create_unimplemented_device("npcm8xx.bt", 0xf0030000, 4 * KiB);
create_unimplemented_device("npcm8xx.espi", 0xf009f000, 4 * KiB);
create_unimplemented_device("npcm8xx.peci", 0xf0100000, 4 * KiB);
- create_unimplemented_device("npcm8xx.siox[1]", 0xf0101000, 4 * KiB);
- create_unimplemented_device("npcm8xx.siox[2]", 0xf0102000, 4 * KiB);
create_unimplemented_device("npcm8xx.tmps", 0xf0188000, 4 * KiB);
create_unimplemented_device("npcm8xx.viru1", 0xf0204000, 4 * KiB);
create_unimplemented_device("npcm8xx.viru2", 0xf0205000, 4 * KiB);
diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build
index 74840619c0..8a3879983c 100644
--- a/hw/gpio/meson.build
+++ b/hw/gpio/meson.build
@@ -18,3 +18,4 @@ 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_SIFIVE_GPIO', if_true: files('sifive_gpio.c'))
system_ss.add(when: 'CONFIG_PCF8574', if_true: files('pcf8574.c'))
+system_ss.add(when: 'CONFIG_NPCM8XX', if_true: files('npcm8xx_sgpio.c'))
diff --git a/hw/gpio/npcm8xx_sgpio.c b/hw/gpio/npcm8xx_sgpio.c
new file mode 100644
index 0000000000..f7d6bbf672
--- /dev/null
+++ b/hw/gpio/npcm8xx_sgpio.c
@@ -0,0 +1,425 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Nuvoton Serial I/O EXPANSION INTERFACE (SOIX)
+ *
+ * Copyright 2025 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/gpio/npcm8xx_sgpio.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/units.h"
+#include "trace.h"
+
+#define NPCM8XX_SGPIO_RD_MODE_MASK 0x6
+#define NPCM8XX_SGPIO_RD_MODE_PERIODIC 0x4
+#define NPCM8XX_SGPIO_RD_MODE_ON_DEMAND 0x0
+#define NPCM8XX_SGPIO_IOXCTS_IOXIF_EN BIT(7)
+#define NPCM8XX_SGPIO_IOXCTS_WR_PEND BIT(6)
+#define NPCM8XX_SGPIO_IOXCTS_DATA16W BIT(3)
+#define NPCM8XX_SGPIO_REGS_SIZE (4 * KiB)
+
+#define NPCM8XX_SGPIO_IXOEVCFG_FALLING BIT(1)
+#define NPCM8XX_SGPIO_IXOEVCFG_RISING BIT(0)
+#define NPCM8XX_SGPIO_IXOEVCFG_BOTH (NPCM8XX_SGPIO_IXOEVCFG_FALLING | \
+ NPCM8XX_SGPIO_IXOEVCFG_RISING)
+
+#define IXOEVCFG_MASK 0x3
+
+/* 8-bit register indices, with the event config registers being 16-bit */
+enum NPCM8xxSGPIORegister {
+ NPCM8XX_SGPIO_XDOUT0,
+ NPCM8XX_SGPIO_XDOUT1,
+ NPCM8XX_SGPIO_XDOUT2,
+ NPCM8XX_SGPIO_XDOUT3,
+ NPCM8XX_SGPIO_XDOUT4,
+ NPCM8XX_SGPIO_XDOUT5,
+ NPCM8XX_SGPIO_XDOUT6,
+ NPCM8XX_SGPIO_XDOUT7,
+ NPCM8XX_SGPIO_XDIN0,
+ NPCM8XX_SGPIO_XDIN1,
+ NPCM8XX_SGPIO_XDIN2,
+ NPCM8XX_SGPIO_XDIN3,
+ NPCM8XX_SGPIO_XDIN4,
+ NPCM8XX_SGPIO_XDIN5,
+ NPCM8XX_SGPIO_XDIN6,
+ NPCM8XX_SGPIO_XDIN7,
+ NPCM8XX_SGPIO_XEVCFG0 = 0x10,
+ NPCM8XX_SGPIO_XEVCFG1 = 0x12,
+ NPCM8XX_SGPIO_XEVCFG2 = 0x14,
+ NPCM8XX_SGPIO_XEVCFG3 = 0x16,
+ NPCM8XX_SGPIO_XEVCFG4 = 0x18,
+ NPCM8XX_SGPIO_XEVCFG5 = 0x1a,
+ NPCM8XX_SGPIO_XEVCFG6 = 0x1c,
+ NPCM8XX_SGPIO_XEVCFG7 = 0x1e,
+ NPCM8XX_SGPIO_XEVSTS0 = 0x20,
+ NPCM8XX_SGPIO_XEVSTS1,
+ NPCM8XX_SGPIO_XEVSTS2,
+ NPCM8XX_SGPIO_XEVSTS3,
+ NPCM8XX_SGPIO_XEVSTS4,
+ NPCM8XX_SGPIO_XEVSTS5,
+ NPCM8XX_SGPIO_XEVSTS6,
+ NPCM8XX_SGPIO_XEVSTS7,
+ NPCM8XX_SGPIO_IOXCTS,
+ NPCM8XX_SGPIO_IOXINDR,
+ NPCM8XX_SGPIO_IOXCFG1,
+ NPCM8XX_SGPIO_IOXCFG2,
+ NPCM8XX_SGPIO_IOXDATR = 0x2d,
+ NPCM8XX_SGPIO_REGS_END,
+};
+
+static uint8_t npcm8xx_sgpio_get_in_port(NPCM8xxSGPIOState *s)
+{
+ uint8_t p;
+
+ p = s->regs[NPCM8XX_SGPIO_IOXCFG2] & 0xf;
+ if (p > 8) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Trying to set more the allowed input ports %d\n",
+ DEVICE(s)->canonical_path, p);
+ }
+
+ return p;
+}
+
+static uint8_t npcm8xx_sgpio_get_out_port(NPCM8xxSGPIOState *s)
+{
+ uint8_t p;
+
+ p = (s->regs[NPCM8XX_SGPIO_IOXCFG2] >> 4) & 0xf;
+ if (p > 8) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Trying to set more the allowed output ports %d\n",
+ DEVICE(s)->canonical_path, p);
+ }
+
+ return p;
+}
+
+static bool npcm8xx_sgpio_is_16bit(NPCM8xxSGPIOState *s)
+{
+ return s->regs[NPCM8XX_SGPIO_IOXCTS] & NPCM8XX_SGPIO_IOXCTS_DATA16W;
+}
+
+static uint64_t npcm8xx_sgpio_regs_read_with_cfg(NPCM8xxSGPIOState *s,
+ hwaddr reg)
+{
+ bool rd_word = npcm8xx_sgpio_is_16bit(s);
+ uint64_t value;
+
+ if (rd_word) {
+ value = ((uint16_t)s->regs[reg] << 8) | s->regs[reg + 1];
+ } else {
+ value = s->regs[reg];
+ }
+
+ return value;
+}
+
+static void npcm8xx_sgpio_update_event(NPCM8xxSGPIOState *s, uint64_t diff)
+{
+ /* TODO in upcoming patch */
+}
+
+static void npcm8xx_sgpio_update_pins_in(NPCM8xxSGPIOState *s, uint64_t diff)
+{
+ /* TODO in upcoming patch */
+}
+
+static void npcm8xx_sgpio_update_pins_out(NPCM8xxSGPIOState *s, hwaddr reg)
+{
+ uint8_t nout, dout;
+
+ if (~(s->regs[NPCM8XX_SGPIO_IOXCTS] & NPCM8XX_SGPIO_IOXCTS_IOXIF_EN)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Device disabled, transaction out aborted\n",
+ DEVICE(s)->canonical_path);
+ }
+
+ nout = npcm8xx_sgpio_get_out_port(s);
+ dout = reg - NPCM8XX_SGPIO_XDOUT0;
+ if (dout >= nout) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Accessing XDOUT%d when NOUT is %d\n",
+ DEVICE(s)->canonical_path, dout, nout);
+ }
+ s->pin_out_level[dout] = s->regs[NPCM8XX_SGPIO_XDOUT0 + dout];
+ /* unset WR_PEND */
+ s->regs[NPCM8XX_SGPIO_IOXCTS] &= ~0x40;
+}
+
+static uint64_t npcm8xx_sgpio_regs_read(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+ NPCM8xxSGPIOState *s = opaque;
+ uint8_t rd_mode = s->regs[NPCM8XX_SGPIO_IOXCTS] &
+ NPCM8XX_SGPIO_RD_MODE_MASK;
+ hwaddr reg = addr / sizeof(uint8_t);
+ uint8_t nout, nin, din, dout;
+ uint64_t value = 0;
+
+ switch (reg) {
+ case NPCM8XX_SGPIO_XDOUT0 ... NPCM8XX_SGPIO_XDOUT7:
+ nout = npcm8xx_sgpio_get_out_port(s);
+ dout = reg - NPCM8XX_SGPIO_XDOUT0;
+
+ if (dout >= nout) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Accessing XDOUT%d when NOUT is %d\n",
+ DEVICE(s)->canonical_path, dout, nout);
+ break;
+ }
+
+ value = npcm8xx_sgpio_regs_read_with_cfg(s, reg);
+ break;
+
+ case NPCM8XX_SGPIO_XDIN0 ... NPCM8XX_SGPIO_XDIN7:
+ nin = npcm8xx_sgpio_get_in_port(s);
+ din = reg - NPCM8XX_SGPIO_XDIN0;
+
+ if (din >= nin) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Accessing XDIN%d when NIN is %d\n",
+ DEVICE(s)->canonical_path, din, nin);
+ break;
+ }
+
+ switch (rd_mode) {
+ case NPCM8XX_SGPIO_RD_MODE_PERIODIC:
+ /* XDIN are up-to-date from scanning, return directly. */
+ value = npcm8xx_sgpio_regs_read_with_cfg(s, reg);
+ break;
+ case NPCM8XX_SGPIO_RD_MODE_ON_DEMAND:
+ /*
+ * IOX_SCAN write behavior is unimplemented.
+ * Event generation is also umimplemented.
+ */
+ qemu_log_mask(LOG_UNIMP,
+ "%s: On Demand with Polling reading mode is not implemented.\n",
+ __func__);
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown read mode\n", __func__);
+ }
+ break;
+
+ case NPCM8XX_SGPIO_XEVCFG0 ... NPCM8XX_SGPIO_XEVCFG7:
+ value = ((uint64_t)s->regs[reg] << 8) | s->regs[reg + 1];
+ break;
+
+ case NPCM8XX_SGPIO_XEVSTS0 ... NPCM8XX_SGPIO_XEVSTS7:
+ value = npcm8xx_sgpio_regs_read_with_cfg(s, reg);
+ break;
+
+ case NPCM8XX_SGPIO_IOXCTS ... NPCM8XX_SGPIO_IOXDATR:
+ value = s->regs[reg];
+ break;
+
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: read from invalid offset 0x%" HWADDR_PRIx "\n",
+ DEVICE(s)->canonical_path, addr);
+ break;
+ }
+
+ trace_npcm8xx_sgpio_read(DEVICE(s)->canonical_path, addr, value);
+
+ return value;
+}
+
+static void npcm8xx_sgpio_regs_write(void *opaque, hwaddr addr, uint64_t v,
+ unsigned int size)
+{
+ hwaddr reg = addr / sizeof(uint8_t);
+ uint8_t hi_val = (uint8_t)(v >> 8);
+ NPCM8xxSGPIOState *s = opaque;
+ uint8_t value = (uint8_t) v;
+ uint8_t diff;
+
+ trace_npcm8xx_sgpio_write(DEVICE(s)->canonical_path, addr, v);
+
+ switch (reg) {
+ case NPCM8XX_SGPIO_XDOUT0 ... NPCM8XX_SGPIO_XDOUT7:
+ /* Set WR_PEND bit */
+ s->regs[NPCM8XX_SGPIO_IOXCTS] |= 0x40;
+ if (npcm8xx_sgpio_is_16bit(s)) {
+ if ((reg - NPCM8XX_SGPIO_XDOUT0) % 2) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: write unaligned 16 bit register @ 0x%"
+ HWADDR_PRIx "\n",
+ DEVICE(s)->canonical_path, addr);
+ break;
+ }
+ s->regs[reg] = hi_val;
+ s->regs[reg + 1] = value;
+ npcm8xx_sgpio_update_pins_out(s, reg + 1);
+ } else {
+ s->regs[reg] = value;
+ }
+ npcm8xx_sgpio_update_pins_out(s, reg);
+ break;
+
+ /* 2 byte long regs */
+ case NPCM8XX_SGPIO_XEVCFG0 ... NPCM8XX_SGPIO_XEVCFG7:
+ if (~(s->regs[NPCM8XX_SGPIO_IOXCTS] & NPCM8XX_SGPIO_IOXCTS_IOXIF_EN)) {
+ s->regs[reg] = hi_val;
+ s->regs[reg + 1] = value;
+ }
+ break;
+
+ case NPCM8XX_SGPIO_XEVSTS0 ... NPCM8XX_SGPIO_XEVSTS7:
+ if (npcm8xx_sgpio_is_16bit(s)) {
+ if ((reg - NPCM8XX_SGPIO_XEVSTS0) % 2) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: write unaligned 16 bit register @ 0x%"
+ HWADDR_PRIx "\n",
+ DEVICE(s)->canonical_path, addr);
+ break;
+ }
+ s->regs[reg] ^= hi_val;
+ s->regs[reg + 1] ^= value;
+ npcm8xx_sgpio_update_event(s, 0);
+ } else {
+ s->regs[reg] ^= value;
+ npcm8xx_sgpio_update_event(s, 0);
+ }
+ break;
+
+ case NPCM8XX_SGPIO_IOXCTS:
+ /* Make sure RO bit WR_PEND is not written to */
+ value &= ~NPCM8XX_SGPIO_IOXCTS_WR_PEND;
+ diff = s->regs[reg] ^ value;
+ s->regs[reg] = value;
+ if ((s->regs[NPCM8XX_SGPIO_IOXCTS] & NPCM8XX_SGPIO_IOXCTS_IOXIF_EN) &&
+ (diff & NPCM8XX_SGPIO_RD_MODE_MASK)) {
+ /* reset RD_MODE if attempting to write with IOXIF_EN enabled */
+ s->regs[reg] ^= (diff & NPCM8XX_SGPIO_RD_MODE_MASK);
+ }
+ break;
+
+ case NPCM8XX_SGPIO_IOXINDR:
+ /*
+ * Only relevant to SIOX1.
+ * HSIOX unimplemented for both, set value and do nothing.
+ */
+ s->regs[reg] = value;
+ break;
+
+ case NPCM8XX_SGPIO_IOXCFG1:
+ case NPCM8XX_SGPIO_IOXCFG2:
+ if (~(s->regs[NPCM8XX_SGPIO_IOXCTS] & NPCM8XX_SGPIO_IOXCTS_IOXIF_EN)) {
+ s->regs[reg] = value;
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: trying to write to register @ 0x%"
+ HWADDR_PRIx "while IOXIF_EN is enabled\n",
+ DEVICE(s)->canonical_path, addr);
+ }
+ break;
+
+ case NPCM8XX_SGPIO_XDIN0 ... NPCM8XX_SGPIO_XDIN7:
+ case NPCM8XX_SGPIO_IOXDATR:
+ /* IOX_SCAN is unimplemented given no on-demand mode */
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: write to read-only register @ 0x%" HWADDR_PRIx "\n",
+ DEVICE(s)->canonical_path, addr);
+ break;
+
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: write to invalid offset 0x%" HWADDR_PRIx "\n",
+ DEVICE(s)->canonical_path, addr);
+ break;
+ }
+}
+
+static const MemoryRegionOps npcm8xx_sgpio_regs_ops = {
+ .read = npcm8xx_sgpio_regs_read,
+ .write = npcm8xx_sgpio_regs_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 2,
+ .unaligned = false,
+ },
+};
+
+static void npcm8xx_sgpio_enter_reset(Object *obj, ResetType type)
+{
+ NPCM8xxSGPIOState *s = NPCM8XX_SGPIO(obj);
+
+ memset(s->regs, 0, sizeof(s->regs));
+}
+
+static void npcm8xx_sgpio_hold_reset(Object *obj, ResetType type)
+{
+ NPCM8xxSGPIOState *s = NPCM8XX_SGPIO(obj);
+
+ npcm8xx_sgpio_update_pins_in(s, -1);
+}
+
+static void npcm8xx_sgpio_init(Object *obj)
+{
+ NPCM8xxSGPIOState *s = NPCM8XX_SGPIO(obj);
+
+ memory_region_init_io(&s->mmio, obj, &npcm8xx_sgpio_regs_ops, s,
+ "regs", NPCM8XX_SGPIO_REGS_SIZE);
+ sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+ sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
+
+ /* TODO: Add input GPIO pins */
+}
+
+static const VMStateDescription vmstate_npcm8xx_sgpio = {
+ .name = "npcm8xx-sgpio",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = (const VMStateField[]) {
+ VMSTATE_UINT8_ARRAY(pin_in_level, NPCM8xxSGPIOState,
+ NPCM8XX_SGPIO_NR_PINS / NPCM8XX_SGPIO_MAX_PORTS),
+ VMSTATE_UINT8_ARRAY(pin_out_level, NPCM8xxSGPIOState,
+ NPCM8XX_SGPIO_NR_PINS / NPCM8XX_SGPIO_MAX_PORTS),
+ VMSTATE_UINT8_ARRAY(regs, NPCM8xxSGPIOState, NPCM8XX_SGPIO_NR_REGS),
+ VMSTATE_END_OF_LIST(),
+ },
+};
+
+static void npcm8xx_sgpio_class_init(ObjectClass *klass, const void *data)
+{
+ ResettableClass *reset = RESETTABLE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ QEMU_BUILD_BUG_ON(NPCM8XX_SGPIO_REGS_END > NPCM8XX_SGPIO_NR_REGS);
+
+ dc->desc = "NPCM8xx SIOX Controller";
+ dc->vmsd = &vmstate_npcm8xx_sgpio;
+ reset->phases.enter = npcm8xx_sgpio_enter_reset;
+ reset->phases.hold = npcm8xx_sgpio_hold_reset;
+}
+
+static const TypeInfo npcm8xx_sgpio_types[] = {
+ {
+ .name = TYPE_NPCM8XX_SGPIO,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(NPCM8xxSGPIOState),
+ .class_init = npcm8xx_sgpio_class_init,
+ .instance_init = npcm8xx_sgpio_init,
+ },
+};
+DEFINE_TYPES(npcm8xx_sgpio_types);
diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
index cea896b28f..5a3625cabc 100644
--- a/hw/gpio/trace-events
+++ b/hw/gpio/trace-events
@@ -12,6 +12,10 @@ npcm7xx_gpio_set_input(const char *id, int32_t line, int32_t level) "%s line: %"
npcm7xx_gpio_set_output(const char *id, int32_t line, int32_t level) "%s line: %" PRIi32 " level: %" PRIi32
npcm7xx_gpio_update_events(const char *id, uint32_t evst, uint32_t even) "%s evst: 0x%08" PRIx32 " even: 0x%08" PRIx32
+# npcm8xx_sgpio.c
+npcm8xx_sgpio_read(const char *id, uint64_t offset, uint64_t value) " %s offset: 0x%04" PRIx64 " value 0x%08" PRIx64
+npcm8xx_sgpio_write(const char *id, uint64_t offset, uint64_t value) " %s offset: 0x%04" PRIx64 " value 0x%08" PRIx64
+
# nrf51_gpio.c
nrf51_gpio_read(uint64_t offset, uint64_t r) "offset 0x%" PRIx64 " value 0x%" PRIx64
nrf51_gpio_write(uint64_t offset, uint64_t value) "offset 0x%" PRIx64 " value 0x%" PRIx64
diff --git a/include/hw/arm/npcm8xx.h b/include/hw/arm/npcm8xx.h
index a8377db490..2d177329b8 100644
--- a/include/hw/arm/npcm8xx.h
+++ b/include/hw/arm/npcm8xx.h
@@ -21,6 +21,7 @@
#include "hw/cpu/cluster.h"
#include "hw/gpio/npcm7xx_gpio.h"
#include "hw/i2c/npcm7xx_smbus.h"
+#include "hw/gpio/npcm8xx_sgpio.h"
#include "hw/intc/arm_gic_common.h"
#include "hw/mem/npcm7xx_mc.h"
#include "hw/misc/npcm_clk.h"
@@ -104,6 +105,7 @@ struct NPCM8xxState {
NPCMPCSState pcs;
NPCM7xxSDHCIState mmc;
NPCMPSPIState pspi;
+ NPCM8xxSGPIOState sgpio[2];
};
struct NPCM8xxClass {
diff --git a/include/hw/gpio/npcm8xx_sgpio.h b/include/hw/gpio/npcm8xx_sgpio.h
new file mode 100644
index 0000000000..cce844951e
--- /dev/null
+++ b/include/hw/gpio/npcm8xx_sgpio.h
@@ -0,0 +1,45 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Nuvoton NPCM8xx General Purpose Input / Output (GPIO)
+ *
+ * Copyright 2025 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#ifndef NPCM8XX_SGPIO_H
+#define NPCM8XX_SGPIO_H
+
+#include "hw/sysbus.h"
+
+/* Number of pins managed by each controller. */
+#define NPCM8XX_SGPIO_NR_PINS (64)
+
+/*
+ * Number of registers in our device state structure. Don't change this without
+ * incrementing the version_id in the vmstate.
+ */
+#define NPCM8XX_SGPIO_NR_REGS (0x2e)
+#define NPCM8XX_SGPIO_MAX_PORTS 8
+
+typedef struct NPCM8xxSGPIOState {
+ SysBusDevice parent;
+
+ MemoryRegion mmio;
+ qemu_irq irq;
+
+ uint8_t pin_in_level[NPCM8XX_SGPIO_MAX_PORTS];
+ uint8_t pin_out_level[NPCM8XX_SGPIO_MAX_PORTS];
+ uint8_t regs[NPCM8XX_SGPIO_NR_REGS];
+} NPCM8xxSGPIOState;
+
+#define TYPE_NPCM8XX_SGPIO "npcm8xx-sgpio"
+OBJECT_DECLARE_SIMPLE_TYPE(NPCM8xxSGPIOState, NPCM8XX_SGPIO)
+
+#endif /* NPCM8XX_SGPIO_H */
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 669d07c06b..12737ad21f 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -212,7 +212,8 @@ qtests_npcm7xx = \
'npcm7xx_watchdog_timer-test'] + \
(slirp.found() ? ['npcm7xx_emc-test'] : [])
qtests_npcm8xx = \
- ['npcm_gmac-test']
+ ['npcm_gmac-test',
+ 'npcm8xx_sgpio-test',]
qtests_aspeed = \
['aspeed_gpio-test',
'aspeed_hace-test',
diff --git a/tests/qtest/npcm8xx_sgpio-test.c b/tests/qtest/npcm8xx_sgpio-test.c
new file mode 100644
index 0000000000..b0b11b3481
--- /dev/null
+++ b/tests/qtest/npcm8xx_sgpio-test.c
@@ -0,0 +1,100 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * QTest testcase for the Nuvoton NPCM8xx I/O EXPANSION INTERFACE (SOIX)
+ * modules.
+ *
+ * Copyright 2025 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+
+#define NR_SGPIO_DEVICES 8
+#define SGPIO(x) (0xf0101000 + (x) * 0x1000)
+#define SGPIO_IRQ(x) (19 + (x))
+
+/* SGPIO registers */
+#define GP_N_XDOUT(x) (0x00 + x)
+#define GP_N_XDIN(x) (0x08 + x)
+#define GP_N_XEVCFG(x) (0x10 + (x) * 0x2)
+#define GP_N_XEVSTS(x) (0x20 + x)
+#define GP_N_IOXCTS 0x28
+#define GP_N_IOXINDR 0x29
+#define GP_N_IOXCFG1 0x2a
+#define GP_N_IOXCFG2 0x2b
+#define GP_N_RD_MODE_PERIODIC 0x4
+#define GP_N_IOXIF_EN 0x80
+
+
+/* Restore the SGPIO controller to a sensible default state. */
+static void sgpio_reset(int n)
+{
+ int i;
+
+ for (i = 0; i < NR_SGPIO_DEVICES; ++i) {
+ writel(SGPIO(n) + GP_N_XDOUT(i), 0x00000000);
+ writel(SGPIO(n) + GP_N_XEVCFG(i), 0x00000000);
+ writel(SGPIO(n) + GP_N_XEVSTS(i), 0x00000000);
+ }
+ writel(SGPIO(n) + GP_N_IOXCTS, 0x00000000);
+ writel(SGPIO(n) + GP_N_IOXINDR, 0x00000000);
+ writel(SGPIO(n) + GP_N_IOXCFG1, 0x00000000);
+ writel(SGPIO(n) + GP_N_IOXCFG2, 0x00000000);
+}
+
+static void test_read_dout_byte(void)
+{
+ int i;
+
+ sgpio_reset(0);
+
+ /* set all 8 output devices */
+ writel(SGPIO(0) + GP_N_IOXCFG2, NR_SGPIO_DEVICES << 4);
+ for (i = 0; i < NR_SGPIO_DEVICES; ++i) {
+ writel(SGPIO(0) + GP_N_XDOUT(i), 0xff);
+ g_assert_cmphex(readb(SGPIO(0) + GP_N_XDOUT(i)), ==, 0xff);
+ }
+}
+
+static void test_read_dout_word(void)
+{
+ int i;
+
+ sgpio_reset(0);
+ /* set all 8 output devices */
+ writel(SGPIO(0) + GP_N_IOXCFG2, NR_SGPIO_DEVICES << 4);
+ /* set 16 bit aligned access */
+ writel(SGPIO(0) + GP_N_IOXCTS, 1 << 3);
+ for (i = 0; i < NR_SGPIO_DEVICES / 2; ++i) {
+ writel(SGPIO(0) + GP_N_XDOUT(i * 2), 0xf0f0);
+ g_assert_cmphex(readw(SGPIO(0) + GP_N_XDOUT(i * 2)), ==, 0xf0f0);
+ }
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+ g_test_set_nonfatal_assertions();
+
+ qtest_add_func("/npcm8xx_sgpio/read_dout_byte", test_read_dout_byte);
+ qtest_add_func("/npcm8xx_sgpio/read_dout_word", test_read_dout_word);
+
+ qtest_start("-machine npcm845-evb");
+ qtest_irq_intercept_in(global_qtest, "/machine/soc/sgpio");
+ ret = g_test_run();
+ qtest_end();
+
+ return ret;
+}
--
2.51.0.536.g15c5d4f767-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v1 5/5] hw/gpio/npcm8xx: Implement npcm sgpio device input pin logic
2025-09-25 0:58 [PATCH v1 0/5] Add Aspeed GPIO test and Support Nuvoton Serial GPIO Expansion (SGPIO) device Coco Li
` (3 preceding siblings ...)
2025-09-25 0:58 ` [PATCH v1 4/5] hw/gpio/npcm8xx: Implement SIOX (SPGIO) device for NPCM without input pin logic Coco Li
@ 2025-09-25 0:58 ` Coco Li
2025-09-25 1:10 ` Philippe Mathieu-Daudé
4 siblings, 1 reply; 14+ messages in thread
From: Coco Li @ 2025-09-25 0:58 UTC (permalink / raw)
To: peter.maydell, clg; +Cc: qemu-arm, qemu-devel, lixiaoyan, flwu, andrew, Hao Wu
Signed-off-by: Coco Li <lixiaoyan@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
hw/gpio/npcm8xx_sgpio.c | 134 ++++++++++++++++++++---
include/hw/gpio/npcm8xx_sgpio.h | 4 +-
tests/qtest/npcm8xx_sgpio-test.c | 180 ++++++++++++++++++++++++++-----
3 files changed, 274 insertions(+), 44 deletions(-)
diff --git a/hw/gpio/npcm8xx_sgpio.c b/hw/gpio/npcm8xx_sgpio.c
index f7d6bbf672..3b626a44e5 100644
--- a/hw/gpio/npcm8xx_sgpio.c
+++ b/hw/gpio/npcm8xx_sgpio.c
@@ -27,6 +27,8 @@
#include "qemu/units.h"
#include "trace.h"
+#include <limits.h>
+
#define NPCM8XX_SGPIO_RD_MODE_MASK 0x6
#define NPCM8XX_SGPIO_RD_MODE_PERIODIC 0x4
#define NPCM8XX_SGPIO_RD_MODE_ON_DEMAND 0x0
@@ -126,24 +128,83 @@ static uint64_t npcm8xx_sgpio_regs_read_with_cfg(NPCM8xxSGPIOState *s,
if (rd_word) {
value = ((uint16_t)s->regs[reg] << 8) | s->regs[reg + 1];
} else {
- value = s->regs[reg];
+ value = (uint8_t) s->regs[reg];
}
return value;
}
+static uint8_t get_even_bits(uint16_t n)
+{
+ n &= 0x5555;
+
+ n = (n | (n >> 1)) & 0x3333;
+ n = (n | (n >> 2)) & 0x0F0F;
+ n = (n | (n >> 4)) & 0x00FF;
+
+ return (uint8_t)n;
+}
+
+static uint8_t get_odd_bits(uint16_t n)
+{
+ return get_even_bits(n >> 1);
+}
+
+/*
+ * For each pin, event can be generated from one of 3 conditions.
+ *
+ * | 1 | 0 | event configuration
+ * -----------------------------
+ * | 0 | 0 | disabled
+ * | 0 | 1 | 0-1 transition
+ * | 1 | 0 | 1-0 transition
+ * | 1 | 1 | even by any transition
+ */
+
static void npcm8xx_sgpio_update_event(NPCM8xxSGPIOState *s, uint64_t diff)
{
- /* TODO in upcoming patch */
+ uint8_t *d = (uint8_t *)&(diff);
+ uint8_t *p = (uint8_t *)&s->pin_in_level;
+ uint16_t type;
+ uint8_t sts;
+ int i;
+
+ for (i = 0; i < npcm8xx_sgpio_get_in_port(s); ++i) {
+ type = ((uint16_t)s->regs[NPCM8XX_SGPIO_XEVCFG0 + 2 * i] << 8) |
+ s->regs[NPCM8XX_SGPIO_XEVCFG0 + 2 * i + 1];
+
+ /* 0-1 transitions */
+ sts = p[i] & d[i] & get_even_bits(type);
+ /* 1-0 transitions */
+ sts |= (~p[i]) & (d[i] & get_odd_bits(type));
+
+ s->regs[NPCM8XX_SGPIO_XEVSTS0 + i] = sts;
+
+ /* Generate event if the event status register tells us so */
+ qemu_set_irq(s->irq, !!(s->regs[NPCM8XX_SGPIO_XEVSTS0 + i]));
+ }
}
-static void npcm8xx_sgpio_update_pins_in(NPCM8xxSGPIOState *s, uint64_t diff)
+static void npcm8xx_sgpio_update_pins_in(NPCM8xxSGPIOState *s, uint64_t value)
{
- /* TODO in upcoming patch */
+ uint8_t *nv = (uint8_t *)&value;
+ uint8_t *ov = (uint8_t *)&s->pin_in_level;
+ uint64_t diff = s->pin_in_level ^ value;
+ int i;
+
+ for (i = 0; i < npcm8xx_sgpio_get_in_port(s); ++i) {
+ if (ov[i] == nv[i]) {
+ continue;
+ }
+ s->regs[NPCM8XX_SGPIO_XDIN0 + i] = nv[i];
+ }
+ s->pin_in_level = value;
+ npcm8xx_sgpio_update_event(s, diff);
}
static void npcm8xx_sgpio_update_pins_out(NPCM8xxSGPIOState *s, hwaddr reg)
{
+ uint8_t *p = (uint8_t *)&s->pin_out_level;
uint8_t nout, dout;
if (~(s->regs[NPCM8XX_SGPIO_IOXCTS] & NPCM8XX_SGPIO_IOXCTS_IOXIF_EN)) {
@@ -159,7 +220,7 @@ static void npcm8xx_sgpio_update_pins_out(NPCM8xxSGPIOState *s, hwaddr reg)
"%s: Accessing XDOUT%d when NOUT is %d\n",
DEVICE(s)->canonical_path, dout, nout);
}
- s->pin_out_level[dout] = s->regs[NPCM8XX_SGPIO_XDOUT0 + dout];
+ p[dout] = s->regs[reg];
/* unset WR_PEND */
s->regs[NPCM8XX_SGPIO_IOXCTS] &= ~0x40;
}
@@ -294,10 +355,8 @@ static void npcm8xx_sgpio_regs_write(void *opaque, hwaddr addr, uint64_t v,
}
s->regs[reg] ^= hi_val;
s->regs[reg + 1] ^= value;
- npcm8xx_sgpio_update_event(s, 0);
} else {
s->regs[reg] ^= value;
- npcm8xx_sgpio_update_event(s, 0);
}
break;
@@ -371,19 +430,70 @@ static void npcm8xx_sgpio_hold_reset(Object *obj, ResetType type)
{
NPCM8xxSGPIOState *s = NPCM8XX_SGPIO(obj);
- npcm8xx_sgpio_update_pins_in(s, -1);
+ npcm8xx_sgpio_update_pins_in(s, 0);
+}
+
+static void npcm8xx_sgpio_set_input_lo(void *opaque, int line, int level)
+{
+ NPCM8xxSGPIOState *s = opaque;
+
+ g_assert(line >= 0 && line < NPCM8XX_SGPIO_NR_PINS / 2);
+
+ npcm8xx_sgpio_update_pins_in(s, BIT(line) && level);
+}
+
+static void npcm8xx_sgpio_set_input_hi(void *opaque, int line, int level)
+{
+ NPCM8xxSGPIOState *s = opaque;
+ uint64_t line_ull = line;
+
+ g_assert(line >= NPCM8XX_SGPIO_NR_PINS / 2 && line < NPCM8XX_SGPIO_NR_PINS);
+
+ npcm8xx_sgpio_update_pins_in(s, BIT(line_ull << 32) && level);
+}
+
+static void npcm8xx_sgpio_get_pins_in(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ NPCM8xxSGPIOState *s = NPCM8XX_SGPIO(obj);
+
+ visit_type_uint64(v, name, &s->pin_in_level, errp);
+}
+
+static void npcm8xx_sgpio_set_pins_in(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ NPCM8xxSGPIOState *s = NPCM8XX_SGPIO(obj);
+ uint64_t new_pins_in;
+
+ if (!visit_type_uint64(v, name, &new_pins_in, errp)) {
+ return;
+ }
+
+ npcm8xx_sgpio_update_pins_in(s, new_pins_in);
}
static void npcm8xx_sgpio_init(Object *obj)
{
NPCM8xxSGPIOState *s = NPCM8XX_SGPIO(obj);
+ DeviceState *dev = DEVICE(obj);
memory_region_init_io(&s->mmio, obj, &npcm8xx_sgpio_regs_ops, s,
"regs", NPCM8XX_SGPIO_REGS_SIZE);
sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
- /* TODO: Add input GPIO pins */
+ /* There are total 64 input pins that can be set */
+ QEMU_BUILD_BUG_ON(NPCM8XX_SGPIO_NR_PINS >
+ sizeof(s->pin_in_level) * CHAR_BIT);
+ qdev_init_gpio_in(dev, npcm8xx_sgpio_set_input_hi,
+ NPCM8XX_SGPIO_NR_PINS / 2);
+ qdev_init_gpio_in(dev, npcm8xx_sgpio_set_input_lo,
+ NPCM8XX_SGPIO_NR_PINS / 2);
+
+ object_property_add(obj, "sgpio-pins-in", "uint64",
+ npcm8xx_sgpio_get_pins_in, npcm8xx_sgpio_set_pins_in,
+ NULL, NULL);
}
static const VMStateDescription vmstate_npcm8xx_sgpio = {
@@ -391,10 +501,8 @@ static const VMStateDescription vmstate_npcm8xx_sgpio = {
.version_id = 0,
.minimum_version_id = 0,
.fields = (const VMStateField[]) {
- VMSTATE_UINT8_ARRAY(pin_in_level, NPCM8xxSGPIOState,
- NPCM8XX_SGPIO_NR_PINS / NPCM8XX_SGPIO_MAX_PORTS),
- VMSTATE_UINT8_ARRAY(pin_out_level, NPCM8xxSGPIOState,
- NPCM8XX_SGPIO_NR_PINS / NPCM8XX_SGPIO_MAX_PORTS),
+ VMSTATE_UINT64(pin_in_level, NPCM8xxSGPIOState),
+ VMSTATE_UINT64(pin_out_level, NPCM8xxSGPIOState),
VMSTATE_UINT8_ARRAY(regs, NPCM8xxSGPIOState, NPCM8XX_SGPIO_NR_REGS),
VMSTATE_END_OF_LIST(),
},
diff --git a/include/hw/gpio/npcm8xx_sgpio.h b/include/hw/gpio/npcm8xx_sgpio.h
index cce844951e..05dfafcb5e 100644
--- a/include/hw/gpio/npcm8xx_sgpio.h
+++ b/include/hw/gpio/npcm8xx_sgpio.h
@@ -34,8 +34,8 @@ typedef struct NPCM8xxSGPIOState {
MemoryRegion mmio;
qemu_irq irq;
- uint8_t pin_in_level[NPCM8XX_SGPIO_MAX_PORTS];
- uint8_t pin_out_level[NPCM8XX_SGPIO_MAX_PORTS];
+ uint64_t pin_in_level;
+ uint64_t pin_out_level;
uint8_t regs[NPCM8XX_SGPIO_NR_REGS];
} NPCM8xxSGPIOState;
diff --git a/tests/qtest/npcm8xx_sgpio-test.c b/tests/qtest/npcm8xx_sgpio-test.c
index b0b11b3481..b26109600c 100644
--- a/tests/qtest/npcm8xx_sgpio-test.c
+++ b/tests/qtest/npcm8xx_sgpio-test.c
@@ -36,65 +36,187 @@
#define GP_N_IOXIF_EN 0x80
+static void qtest_qom_set_uint64(QTestState *s, const char *path,
+ const char *property, uint64_t value)
+{
+ QDict *r;
+ QDict *qdict;
+
+ r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
+ "{ 'path': %s, 'property': %s, 'value': %" PRIu64 " } }",
+ path, property, value);
+
+ qdict = qdict_get_qdict(r, "error");
+ if (qdict) {
+ printf("DEBUG: set error: %s\n", qdict_get_try_str(qdict, "desc"));
+ }
+
+ qobject_unref(r);
+}
+
+
+static uint64_t qtest_qom_get_uint64(QTestState *s, const char *path,
+ const char *property)
+{
+ QDict *r;
+
+ uint64_t res;
+ r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
+ "{ 'path': %s, 'property': %s } }", path, property);
+
+ res = qdict_get_uint(r, "return");
+ qobject_unref(r);
+
+ return res;
+}
+
/* Restore the SGPIO controller to a sensible default state. */
-static void sgpio_reset(int n)
+static void sgpio_reset(QTestState *s, int n)
{
int i;
for (i = 0; i < NR_SGPIO_DEVICES; ++i) {
- writel(SGPIO(n) + GP_N_XDOUT(i), 0x00000000);
- writel(SGPIO(n) + GP_N_XEVCFG(i), 0x00000000);
- writel(SGPIO(n) + GP_N_XEVSTS(i), 0x00000000);
+ qtest_writeq(s, SGPIO(n) + GP_N_XDOUT(i), 0x0);
+ qtest_writeq(s, SGPIO(n) + GP_N_XEVCFG(i), 0x0);
+ qtest_writeq(s, SGPIO(n) + GP_N_XEVSTS(i), 0x0);
}
- writel(SGPIO(n) + GP_N_IOXCTS, 0x00000000);
- writel(SGPIO(n) + GP_N_IOXINDR, 0x00000000);
- writel(SGPIO(n) + GP_N_IOXCFG1, 0x00000000);
- writel(SGPIO(n) + GP_N_IOXCFG2, 0x00000000);
+ qtest_writeq(s, SGPIO(n) + GP_N_IOXCTS, 0x0);
+ qtest_writeq(s, SGPIO(n) + GP_N_IOXINDR, 0x0);
+ qtest_writeq(s, SGPIO(n) + GP_N_IOXCFG1, 0x0);
+ qtest_writeq(s, SGPIO(n) + GP_N_IOXCFG2, 0x0);
}
-static void test_read_dout_byte(void)
+static void test_read_dout_byte(const char *machine)
{
+ QTestState *s = qtest_init(machine);
int i;
- sgpio_reset(0);
+ sgpio_reset(s, 0);
/* set all 8 output devices */
- writel(SGPIO(0) + GP_N_IOXCFG2, NR_SGPIO_DEVICES << 4);
+ qtest_writeq(s, SGPIO(0) + GP_N_IOXCFG2, NR_SGPIO_DEVICES << 4);
for (i = 0; i < NR_SGPIO_DEVICES; ++i) {
- writel(SGPIO(0) + GP_N_XDOUT(i), 0xff);
- g_assert_cmphex(readb(SGPIO(0) + GP_N_XDOUT(i)), ==, 0xff);
+ qtest_writeq(s, SGPIO(0) + GP_N_XDOUT(i), 0xff);
+ g_assert_cmphex(qtest_readb(s, SGPIO(0) + GP_N_XDOUT(i)), ==, 0xff);
}
+ qtest_quit(s);
}
-static void test_read_dout_word(void)
+static void test_read_dout_word(const char *machine)
{
+ QTestState *s = qtest_init(machine);
int i;
- sgpio_reset(0);
+ sgpio_reset(s, 0);
/* set all 8 output devices */
- writel(SGPIO(0) + GP_N_IOXCFG2, NR_SGPIO_DEVICES << 4);
+ qtest_writeq(s, SGPIO(0) + GP_N_IOXCFG2, NR_SGPIO_DEVICES << 4);
/* set 16 bit aligned access */
- writel(SGPIO(0) + GP_N_IOXCTS, 1 << 3);
+ qtest_writeq(s, SGPIO(0) + GP_N_IOXCTS, 1 << 3);
for (i = 0; i < NR_SGPIO_DEVICES / 2; ++i) {
- writel(SGPIO(0) + GP_N_XDOUT(i * 2), 0xf0f0);
- g_assert_cmphex(readw(SGPIO(0) + GP_N_XDOUT(i * 2)), ==, 0xf0f0);
+ qtest_writeq(s, SGPIO(0) + GP_N_XDOUT(i * 2), 0xf0f0);
+ g_assert_cmphex(qtest_readw(s, SGPIO(0) + GP_N_XDOUT(i * 2)),
+ ==, 0xf0f0);
}
+ qtest_quit(s);
}
-int main(int argc, char **argv)
+static void test_events_din_rising_edge(const char *machine)
{
- int ret;
+ QTestState *s = qtest_init(machine);
+ const char path[] = "/machine/soc/sgpio[0]";
+ int i;
+
+ /* clear all inputs */
+ sgpio_reset(s, 0);
+
+ /* set all 8 input devices */
+ qtest_writel(s, SGPIO(0) + GP_N_IOXCFG2, NR_SGPIO_DEVICES);
+
+ /* set event detection type to be on the rising edge*/
+ for (i = 0; i < NR_SGPIO_DEVICES; ++i) {
+ qtest_writel(s, SGPIO(0) + GP_N_XEVCFG(i), 0x5555);
+ }
+ /* Set periodic reading mode, the only accepted mode */
+ qtest_writel(s, SGPIO(0) + GP_N_IOXCTS, GP_N_RD_MODE_PERIODIC);
+ /* enable device, set IOXIF_EN */
+ qtest_writel(s, SGPIO(0) + GP_N_IOXCTS,
+ GP_N_IOXIF_EN | GP_N_RD_MODE_PERIODIC);
+
+ qtest_irq_intercept_in(s, "/machine/soc/gic");
+
+ /* raise all input pin values */
+ qtest_qom_set_uint64(s, path, "sgpio-pins-in", 0xffffffffffffffff);
+ g_assert(qtest_qom_get_uint64(s, path, "sgpio-pins-in")
+ == 0xffffffffffffffff);
+
+ /* set event status to implicitly change pins */
+ for (i = 0; i < NR_SGPIO_DEVICES; ++i) {
+ g_assert_cmphex(qtest_readb(s, SGPIO(0) + GP_N_XDIN(i)), ==, 0xff);
+ g_assert_cmphex(qtest_readb(s, SGPIO(0) + GP_N_XEVSTS(i)), ==, 0xff);
+ g_assert_true(qtest_get_irq(s, SGPIO_IRQ(0)));
+ }
+ qtest_quit(s);
+}
+
+static void test_events_din_falling_edge(const char *machine)
+{
+ QTestState *s = qtest_init(machine);
+ const char path[] = "/machine/soc/sgpio[0]";
+ int i;
+
+ /* clear all inputs */
+ sgpio_reset(s, 0);
+
+ /* set all 8 input devices */
+ qtest_writel(s, SGPIO(0) + GP_N_IOXCFG2, NR_SGPIO_DEVICES);
+
+ /* set event detection type to be on the falling edge*/
+ for (i = 0; i < NR_SGPIO_DEVICES; ++i) {
+ qtest_writel(s, SGPIO(0) + GP_N_XEVCFG(i), 0xaaaa);
+ }
+ /* Set periodic reading mode, the only accepted mode */
+ qtest_writel(s, SGPIO(0) + GP_N_IOXCTS, GP_N_RD_MODE_PERIODIC);
+ /* enable device, set IOXIF_EN */
+ qtest_writel(s, SGPIO(0) + GP_N_IOXCTS,
+ GP_N_IOXIF_EN | GP_N_RD_MODE_PERIODIC);
+
+ qtest_irq_intercept_in(s, "/machine/soc/gic");
+
+ /* raise all input pin values */
+ qtest_qom_set_uint64(s, path, "sgpio-pins-in", 0xffffffffffffffff);
+ g_assert(qtest_qom_get_uint64(s, path, "sgpio-pins-in")
+ == 0xffffffffffffffff);
+
+ /* reset all input pin values */
+ qtest_qom_set_uint64(s, path, "sgpio-pins-in", 0x0);
+ g_assert(qtest_qom_get_uint64(s, path, "sgpio-pins-in") == 0x0);
+
+ /* set event status to implicitly change pins */
+ for (i = 0; i < NR_SGPIO_DEVICES; ++i) {
+ g_assert_cmphex(qtest_readb(s, SGPIO(0) + GP_N_XDIN(i)), ==, 0x00);
+ g_assert_cmphex(qtest_readb(s, SGPIO(0) + GP_N_XEVSTS(i)), ==, 0xff);
+ g_assert_true(qtest_get_irq(s, SGPIO_IRQ(0)));
+ }
+
+ qtest_quit(s);
+}
+
+
+static void test_npcm8xx(void)
+{
+ test_read_dout_byte("-machine npcm845-evb");
+ test_read_dout_word("-machine npcm845-evb");
+ test_events_din_rising_edge("-machine npcm845-evb");
+ test_events_din_falling_edge("-machine npcm845-evb");
+}
+
+int main(int argc, char **argv)
+{
g_test_init(&argc, &argv, NULL);
g_test_set_nonfatal_assertions();
- qtest_add_func("/npcm8xx_sgpio/read_dout_byte", test_read_dout_byte);
- qtest_add_func("/npcm8xx_sgpio/read_dout_word", test_read_dout_word);
-
- qtest_start("-machine npcm845-evb");
- qtest_irq_intercept_in(global_qtest, "/machine/soc/sgpio");
- ret = g_test_run();
- qtest_end();
+ qtest_add_func("/npcm8xx/sgpio", test_npcm8xx);
- return ret;
+ return g_test_run();
}
--
2.51.0.536.g15c5d4f767-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v1 5/5] hw/gpio/npcm8xx: Implement npcm sgpio device input pin logic
2025-09-25 0:58 ` [PATCH v1 5/5] hw/gpio/npcm8xx: Implement npcm sgpio device " Coco Li
@ 2025-09-25 1:10 ` Philippe Mathieu-Daudé
2025-09-26 21:49 ` Coco Li
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-25 1:10 UTC (permalink / raw)
To: Coco Li, peter.maydell, clg, Richard Henderson
Cc: qemu-arm, qemu-devel, flwu, andrew, Hao Wu
On 25/9/25 02:58, Coco Li wrote:
> Signed-off-by: Coco Li <lixiaoyan@google.com>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> ---
> hw/gpio/npcm8xx_sgpio.c | 134 ++++++++++++++++++++---
> include/hw/gpio/npcm8xx_sgpio.h | 4 +-
> tests/qtest/npcm8xx_sgpio-test.c | 180 ++++++++++++++++++++++++++-----
> 3 files changed, 274 insertions(+), 44 deletions(-)
>
> diff --git a/hw/gpio/npcm8xx_sgpio.c b/hw/gpio/npcm8xx_sgpio.c
> +static uint8_t get_even_bits(uint16_t n)
> +{
> + n &= 0x5555;
> +
> + n = (n | (n >> 1)) & 0x3333;
> + n = (n | (n >> 2)) & 0x0F0F;
> + n = (n | (n >> 4)) & 0x00FF;
> +
> + return (uint8_t)n;
> +}
> +
> +static uint8_t get_odd_bits(uint16_t n)
> +{
> + return get_even_bits(n >> 1);
> +}
Candidates for "include/qemu/bitops.h"?
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v1 5/5] hw/gpio/npcm8xx: Implement npcm sgpio device input pin logic
2025-09-25 1:10 ` Philippe Mathieu-Daudé
@ 2025-09-26 21:49 ` Coco Li
0 siblings, 0 replies; 14+ messages in thread
From: Coco Li @ 2025-09-26 21:49 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: peter.maydell, clg, Richard Henderson, qemu-arm, qemu-devel, flwu,
andrew, Hao Wu
[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]
Good call, I can put it in the next version.
On Wed, Sep 24, 2025 at 6:11 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:
> On 25/9/25 02:58, Coco Li wrote:
> > Signed-off-by: Coco Li <lixiaoyan@google.com>
> > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > ---
> > hw/gpio/npcm8xx_sgpio.c | 134 ++++++++++++++++++++---
> > include/hw/gpio/npcm8xx_sgpio.h | 4 +-
> > tests/qtest/npcm8xx_sgpio-test.c | 180 ++++++++++++++++++++++++++-----
> > 3 files changed, 274 insertions(+), 44 deletions(-)
> >
> > diff --git a/hw/gpio/npcm8xx_sgpio.c b/hw/gpio/npcm8xx_sgpio.c
>
>
> > +static uint8_t get_even_bits(uint16_t n)
> > +{
> > + n &= 0x5555;
> > +
> > + n = (n | (n >> 1)) & 0x3333;
> > + n = (n | (n >> 2)) & 0x0F0F;
> > + n = (n | (n >> 4)) & 0x00FF;
> > +
> > + return (uint8_t)n;
> > +}
> > +
> > +static uint8_t get_odd_bits(uint16_t n)
> > +{
> > + return get_even_bits(n >> 1);
> > +}
>
> Candidates for "include/qemu/bitops.h"?
>
[-- Attachment #2: Type: text/html, Size: 1649 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread