> -----Original Message-----
> From: Yubin Zou <[email protected]>
> Sent: Wednesday, December 17, 2025 8:22 AM
> To: [email protected]
> Cc: Cédric Le Goater <[email protected]>; Peter Maydell
> <[email protected]>; Steven Lee <[email protected]>; Troy
> Lee <[email protected]>; Jamin Lin <[email protected]>; Andrew
> Jeffery <[email protected]>; Joel Stanley <[email protected]>;
> Fabiano Rosas <[email protected]>; Laurent Vivier <[email protected]>;
> Paolo Bonzini <[email protected]>; Kane Chen
> <[email protected]>; Nabih Estefan <[email protected]>;
> [email protected]; Yubin Zou <[email protected]>
> Subject: [PATCH v4 3/6] hw/gpio/aspeed_sgpio: Implement SGPIO interrupt
> handling
>
> The SGPIO controller can generate interrupts based on various pin state
> changes, such as rising/falling edges or high/low levels. This change adds the
> necessary logic to detect these events, update the interrupt status registers,
> and signal the interrupt to the SoC.
>
> Signed-off-by: Yubin Zou <[email protected]>
> ---
> include/hw/gpio/aspeed_sgpio.h | 2 +
> hw/gpio/aspeed_sgpio.c | 127
> ++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 127 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/gpio/aspeed_sgpio.h
> b/include/hw/gpio/aspeed_sgpio.h index
> 60279a597c722f94fba406d60cb30a52ef9544bc..8a11a9998c013cb2e4be996
> 90ecd7bcd9dcb5815 100644
> --- a/include/hw/gpio/aspeed_sgpio.h
> +++ b/include/hw/gpio/aspeed_sgpio.h
> @@ -58,7 +58,9 @@ struct AspeedSGPIOState {
>
> /*< public >*/
> MemoryRegion iomem;
> + int pending;
> qemu_irq irq;
> + qemu_irq sgpios[ASPEED_SGPIO_MAX_PIN_PAIR];
> uint32_t ctrl_regs[ASPEED_SGPIO_MAX_PIN_PAIR];
> uint32_t int_regs[ASPEED_SGPIO_MAX_INT]; }; diff --git
> a/hw/gpio/aspeed_sgpio.c b/hw/gpio/aspeed_sgpio.c index
> dc2df137db247c178adc05807bd0595fc0cb5c52..c6e6b3f52a9a982171a03cda
> 820a4573674ab67d 100644
> --- a/hw/gpio/aspeed_sgpio.c
> +++ b/hw/gpio/aspeed_sgpio.c
> @@ -12,9 +12,131 @@
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "qapi/visitor.h"
> +#include "hw/irq.h"
> #include "hw/qdev-properties.h"
> #include "hw/gpio/aspeed_sgpio.h"
>
> +/*
> + * For each set of gpios there are three sensitivity registers that
> +control
> + * the interrupt trigger mode.
> + *
> + * | 2 | 1 | 0 | trigger mode
> + * -----------------------------
> + * | 0 | 0 | 0 | falling-edge
> + * | 0 | 0 | 1 | rising-edge
> + * | 0 | 1 | 0 | level-low
> + * | 0 | 1 | 1 | level-high
> + * | 1 | X | X | dual-edge
> + */
> +
> +/* GPIO Interrupt Triggers */
> +#define ASPEED_FALLING_EDGE 0
> +#define ASPEED_RISING_EDGE 1
> +#define ASPEED_LEVEL_LOW 2
> +#define ASPEED_LEVEL_HIGH 3
> +#define ASPEED_DUAL_EDGE 4
> +
> +static void aspeed_clear_irq(AspeedSGPIOState *s, int idx) {
> + uint32_t reg_index = idx / 32;
> + uint32_t bit_index = idx % 32;
> + uint32_t pending = extract32(s->int_regs[reg_index], bit_index, 1);
> +
> + assert(s->pending >= pending);
> +
> + /* No change to s->pending if pending is 0 */
> + s->pending -= pending;
> +
> + /*
> + * The write acknowledged the interrupt regardless of whether it
> + * was pending or not. The post-condition is that it mustn't be
> + * pending. Unconditionally clear the status bit.
> + */
> + s->int_regs[reg_index] = deposit32(s->int_regs[reg_index],
> +bit_index, 1, 0); }
> +
> +static void aspeed_evaluate_irq(AspeedSGPIOState *s, int sgpio_prev_high,
> + int sgpio_curr_high, int idx) {
> + uint32_t ctrl = s->ctrl_regs[idx];
> + uint32_t falling_edge = 0, rising_edge = 0;
> + uint32_t int_trigger = SHARED_FIELD_EX32(ctrl, SGPIO_INT_TYPE);
> + uint32_t int_enabled = SHARED_FIELD_EX32(ctrl, SGPIO_INT_EN);
> + uint32_t reg_index = idx / 32;
> + uint32_t bit_index = idx % 32;
> +
> + if (!int_enabled) {
> + return;
> + }
> +
> + /* Detect edges */
> + if (sgpio_curr_high && !sgpio_prev_high) {
> + rising_edge = 1;
> + } else if (!sgpio_curr_high && sgpio_prev_high) {
> + falling_edge = 1;
> + }
> +
> + if (((int_trigger == ASPEED_FALLING_EDGE) && falling_edge) ||
> + ((int_trigger == ASPEED_RISING_EDGE) && rising_edge) ||
> + ((int_trigger == ASPEED_LEVEL_LOW) && !sgpio_curr_high) ||
> + ((int_trigger == ASPEED_LEVEL_HIGH) && sgpio_curr_high) ||
> + ((int_trigger >= ASPEED_DUAL_EDGE) && (rising_edge ||
> falling_edge)))
> + {
> + s->int_regs[reg_index] = deposit32(s->int_regs[reg_index],
> + bit_index, 1, 1);
> + /* Trigger the VIC IRQ */
> + s->pending++;
> + }
> +}
> +
> +static void aspeed_sgpio_update(AspeedSGPIOState *s, uint32_t idx,
> + uint32_t value) {
> + uint32_t old = s->ctrl_regs[idx];
> + uint32_t new = value;
> + uint32_t diff = (old ^ new);
> + if (diff) {
> + /* If the interrupt clear bit is set */
> + if (SHARED_FIELD_EX32(new, SGPIO_INT_STATUS)) {
> + aspeed_clear_irq(s, idx);
> + /* Clear the interrupt clear bit */
> + new &= ~SGPIO_INT_STATUS_MASK;
> + }
> +
> + /* Uppdate the control register. */
There is a small typo here ("Uppdate"). Please correct it.
> + s->ctrl_regs[idx] = new;
> +
> + /* If the output value is changed */
> + if (SHARED_FIELD_EX32(diff, SGPIO_SERIAL_OUT_VAL)) {
> + /* ...trigger the line-state IRQ */
> + qemu_set_irq(s->sgpios[idx], 1);
> + }
> +
> + /* If the input value is changed */
> + if (SHARED_FIELD_EX32(diff, SGPIO_SERIAL_IN_VAL)) {
> + aspeed_evaluate_irq(s,
> + SHARED_FIELD_EX32(old,
> SGPIO_SERIAL_IN_VAL),
> + SHARED_FIELD_EX32(new,
> SGPIO_SERIAL_IN_VAL),
> + idx);
> + }
> + }
> + qemu_set_irq(s->irq, !!(s->pending)); }
> +
> +static uint64_t aspeed_sgpio_2700_read_int_status_reg(AspeedSGPIOState
> *s,
> + uint32_t
> reg) {
> + uint32_t idx = reg - R_SGPIO_INT_STATUS_0;
> + if (idx >= ASPEED_SGPIO_MAX_INT) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: interrupt status index: %d, out of
> bounds\n",
> + __func__, idx);
> + return 0;
> + }
> + return s->int_regs[idx];
> +}
> +
> +
There seems to be an extra empty line here. It can be removed.
> static uint64_t aspeed_sgpio_2700_read_control_reg(AspeedSGPIOState *s,
> uint32_t reg) { @@ -38,7 +160,7
> @@ static void aspeed_sgpio_2700_write_control_reg(AspeedSGPIOState *s,
> __func__, idx);
> return;
> }
> - s->ctrl_regs[idx] = data;
> + aspeed_sgpio_update(s, idx, data);
> }
>
> static uint64_t aspeed_sgpio_2700_read(void *opaque, hwaddr offset, @@
> -52,6 +174,7 @@ static uint64_t aspeed_sgpio_2700_read(void *opaque,
> hwaddr offset,
>
> switch (reg) {
> case R_SGPIO_INT_STATUS_0 ... R_SGPIO_INT_STATUS_7:
> + value = aspeed_sgpio_2700_read_int_status_reg(s, reg);
> break;
> case R_SGPIO_0_CONTROL ... R_SGPIO_255_CONTROL:
> value = aspeed_sgpio_2700_read_control_reg(s, reg); @@ -116,7
> +239,7 @@ static void aspeed_sgpio_set_pin_level(AspeedSGPIOState *s, int
> pin, bool level)
> } else {
> value &= ~bit_mask;
> }
> - s->ctrl_regs[pin >> 1] = value;
> + aspeed_sgpio_update(s, pin >> 1, value);
> }
>
> static void aspeed_sgpio_get_pin(Object *obj, Visitor *v, const char *name,
>
> --
> 2.52.0.305.g3fc767764a-goog
Best Regards,
Kane
