openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600
@ 2022-02-07 15:04 Andrew Jeffery
  2022-02-07 15:04 ` [PATCH 1/3] hw: aspeed_gpio: Cleanup stray semicolon after switch Andrew Jeffery
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Andrew Jeffery @ 2022-02-07 15:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, openbmc, qemu-arm, clg

Hello,

This series adds support for a new register interface supported by the
Aspeed GPIO controller, present in at least the AST2600.

The new interface is a single register implementing an indirect command
protocol that allows us to manipulate up to (at least) 208 GPIOs. This
makes it possible to write very simple drivers for e.g. u-boot and
jettison the need for the tedious data model required to deal with the
old register layout.

I've lightly tested the device consistency under Linux. The Linux
driver is implemented in terms of the old interface, so data model
consistency can be tested one way by poking the driver using the
libgpiod tools and then the other using devmem to drive the new
interface.

Please review!

Andrew

Andrew Jeffery (3):
  hw: aspeed_gpio: Cleanup stray semicolon after switch
  hw: aspeed_gpio: Split GPIOSet handling from accessors
  hw: aspeed_gpio: Support the AST2600's indexed register interface

 hw/gpio/aspeed_gpio.c         | 305 ++++++++++++++++++++++++++++------
 include/hw/gpio/aspeed_gpio.h |   3 +
 2 files changed, 261 insertions(+), 47 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] hw: aspeed_gpio: Cleanup stray semicolon after switch
  2022-02-07 15:04 [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600 Andrew Jeffery
@ 2022-02-07 15:04 ` Andrew Jeffery
  2022-02-07 15:04 ` [PATCH 2/3] hw: aspeed_gpio: Split GPIOSet handling from accessors Andrew Jeffery
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Jeffery @ 2022-02-07 15:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, openbmc, qemu-arm, clg

Not sure how that got there.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 hw/gpio/aspeed_gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 911d21c8cfbe..c63634d3d3e2 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -571,7 +571,7 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size)
         qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
                       HWADDR_PRIx"\n", __func__, offset);
         return 0;
-    };
+    }
 }
 
 static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] hw: aspeed_gpio: Split GPIOSet handling from accessors
  2022-02-07 15:04 [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600 Andrew Jeffery
  2022-02-07 15:04 ` [PATCH 1/3] hw: aspeed_gpio: Cleanup stray semicolon after switch Andrew Jeffery
@ 2022-02-07 15:04 ` Andrew Jeffery
  2022-02-07 15:04 ` [PATCH 3/3] hw: aspeed_gpio: Support the AST2600's indexed register interface Andrew Jeffery
  2022-02-09 11:09 ` [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600 Andrew Jeffery
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Jeffery @ 2022-02-07 15:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, openbmc, qemu-arm, clg

Pave the way for implementing the new register interface for GPIO
control provided by the AST2600. We need a consistent data model, so
do some work to enable use of the AspeedGPIOReg / GPIOSets data
structures for both.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 hw/gpio/aspeed_gpio.c | 105 ++++++++++++++++++++++++------------------
 1 file changed, 60 insertions(+), 45 deletions(-)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index c63634d3d3e2..1d4d1aedc4b5 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -516,28 +516,11 @@ static const AspeedGPIOReg aspeed_1_8v_gpios[GPIO_1_8V_REG_ARRAY_SIZE] = {
     [GPIO_1_8V_E_INPUT_MASK] =     {1, gpio_reg_input_mask},
 };
 
-static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size)
+static uint64_t
+aspeed_gpio_set_read(const AspeedGPIOState *s, const AspeedGPIOReg *reg)
 {
-    AspeedGPIOState *s = ASPEED_GPIO(opaque);
-    AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
-    uint64_t idx = -1;
-    const AspeedGPIOReg *reg;
-    GPIOSets *set;
+    const GPIOSets *set = &s->sets[reg->set_idx];
 
-    idx = offset >> 2;
-    if (idx >= GPIO_DEBOUNCE_TIME_1 && idx <= GPIO_DEBOUNCE_TIME_3) {
-        idx -= GPIO_DEBOUNCE_TIME_1;
-        return (uint64_t) s->debounce_regs[idx];
-    }
-
-    reg = &agc->reg_table[idx];
-    if (reg->set_idx >= agc->nr_gpio_sets) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
-                      HWADDR_PRIx"\n", __func__, offset);
-        return 0;
-    }
-
-    set = &s->sets[reg->set_idx];
     switch (reg->type) {
     case gpio_reg_data_value:
         return set->data_value;
@@ -567,37 +550,44 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size)
         return set->data_read;
     case gpio_reg_input_mask:
         return set->input_mask;
-    default:
+    case gpio_not_a_reg:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid register: %d\n", __func__,
+                      reg->type);
+    }
+
+    return 0;
+}
+
+static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size)
+{
+    AspeedGPIOState *s = ASPEED_GPIO(opaque);
+    AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
+    const AspeedGPIOReg *reg;
+    uint64_t idx = -1;
+
+    idx = offset >> 2;
+    if (idx >= GPIO_DEBOUNCE_TIME_1 && idx <= GPIO_DEBOUNCE_TIME_3) {
+        idx -= GPIO_DEBOUNCE_TIME_1;
+        return (uint64_t) s->debounce_regs[idx];
+    }
+
+    reg = &agc->reg_table[idx];
+    if (reg->set_idx >= agc->nr_gpio_sets) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
                       HWADDR_PRIx"\n", __func__, offset);
         return 0;
     }
+
+    return aspeed_gpio_set_read(s, reg);
 }
 
-static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
-                              uint32_t size)
+static void aspeed_gpio_set_write(AspeedGPIOState *s, const AspeedGPIOReg *reg,
+                                  uint32_t data)
 {
-    AspeedGPIOState *s = ASPEED_GPIO(opaque);
     AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
     const GPIOSetProperties *props;
-    uint64_t idx = -1;
-    const AspeedGPIOReg *reg;
-    GPIOSets *set;
     uint32_t cleared;
-
-    idx = offset >> 2;
-    if (idx >= GPIO_DEBOUNCE_TIME_1 && idx <= GPIO_DEBOUNCE_TIME_3) {
-        idx -= GPIO_DEBOUNCE_TIME_1;
-        s->debounce_regs[idx] = (uint32_t) data;
-        return;
-    }
-
-    reg = &agc->reg_table[idx];
-    if (reg->set_idx >= agc->nr_gpio_sets) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
-                      HWADDR_PRIx"\n", __func__, offset);
-        return;
-    }
+    GPIOSets *set;
 
     set = &s->sets[reg->set_idx];
     props = &agc->props[reg->set_idx];
@@ -678,13 +668,38 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
          */
          set->input_mask = data & props->input;
         break;
-    default:
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
-                      HWADDR_PRIx"\n", __func__, offset);
+    case gpio_not_a_reg:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid register: %d\n", __func__,
+                      reg->type);
         return;
     }
+
     aspeed_gpio_update(s, set, set->data_value);
-    return;
+}
+
+static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
+                              uint32_t size)
+{
+    AspeedGPIOState *s = ASPEED_GPIO(opaque);
+    AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
+    const AspeedGPIOReg *reg;
+    uint64_t idx = -1;
+
+    idx = offset >> 2;
+    if (idx >= GPIO_DEBOUNCE_TIME_1 && idx <= GPIO_DEBOUNCE_TIME_3) {
+        idx -= GPIO_DEBOUNCE_TIME_1;
+        s->debounce_regs[idx] = (uint32_t) data;
+        return;
+    }
+
+    reg = &agc->reg_table[idx];
+    if (reg->set_idx >= agc->nr_gpio_sets) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
+                      HWADDR_PRIx"\n", __func__, offset);
+        return;
+    }
+
+    aspeed_gpio_set_write(s, reg, data);
 }
 
 static int get_set_idx(AspeedGPIOState *s, const char *group, int *group_idx)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] hw: aspeed_gpio: Support the AST2600's indexed register interface
  2022-02-07 15:04 [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600 Andrew Jeffery
  2022-02-07 15:04 ` [PATCH 1/3] hw: aspeed_gpio: Cleanup stray semicolon after switch Andrew Jeffery
  2022-02-07 15:04 ` [PATCH 2/3] hw: aspeed_gpio: Split GPIOSet handling from accessors Andrew Jeffery
@ 2022-02-07 15:04 ` Andrew Jeffery
  2022-02-09 11:09 ` [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600 Andrew Jeffery
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Jeffery @ 2022-02-07 15:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, openbmc, qemu-arm, clg

A new register interface was added to the AST2600 GPIO controller that
allows a single 32 bit register to drive configuration of up to 208
GPIOs. This makes way for a very simple driver implementation in
early-boot firmware such as u-boot. The old register interface required
drivers implement a tedious data model, but allowed efficient multi-line
bit-banging.

Either way, the hardware model in qemu becomes quite complex, though it
would have been less so had the new interface been the only one
available.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 hw/gpio/aspeed_gpio.c         | 202 +++++++++++++++++++++++++++++++++-
 include/hw/gpio/aspeed_gpio.h |   3 +
 2 files changed, 202 insertions(+), 3 deletions(-)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 1d4d1aedc4b5..cee1a9a2e065 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -160,7 +160,42 @@
 #define GPIO_YZAAAB_DIRECTION      (0x1E4 >> 2)
 #define GPIO_AC_DATA_VALUE         (0x1E8 >> 2)
 #define GPIO_AC_DIRECTION          (0x1EC >> 2)
-#define GPIO_3_3V_MEM_SIZE         0x1F0
+#define GPIO_INDEX                 (0x2AC >> 2)
+#define  GPIO_INDEX_DATA_SHIFT     20
+#define  GPIO_INDEX_DATA_LEN       12
+#define   GPIO_INDEX_DATA_DATA     20
+#define   GPIO_INDEX_DATA_DIR      20
+#define   GPIO_INDEX_DATA_IRQ_EN   20
+#define   GPIO_INDEX_DATA_IRQ_TY0  21
+#define   GPIO_INDEX_DATA_IRQ_TY1  22
+#define   GPIO_INDEX_DATA_IRQ_TY2  23
+#define   GPIO_INDEX_DATA_IRQ_STS  24
+#define   GPIO_INDEX_DATA_DB1      20
+#define   GPIO_INDEX_DATA_DB2      21
+#define   GPIO_INDEX_DATA_TOL      20
+#define   GPIO_INDEX_DATA_SRC0     20
+#define   GPIO_INDEX_DATA_SRC1     20
+#define   GPIO_INDEX_DATA_INPUT    20
+#define   GPIO_INDEX_DATA_WR_SRC   20
+#define  GPIO_INDEX_TYPE_SHIFT     16
+#define  GPIO_INDEX_TYPE_LEN       4
+#define   GPIO_INDEX_TYPE_DATA     0
+#define   GPIO_INDEX_TYPE_DIR      1
+#define   GPIO_INDEX_TYPE_IRQ      2
+#define   GPIO_INDEX_TYPE_DEBOUNCE 3
+#define   GPIO_INDEX_TYPE_TOL      4
+#define   GPIO_INDEX_TYPE_SRC      5
+#define   GPIO_INDEX_TYPE_INPUT    6
+#define   GPIO_INDEX_TYPE_RSVD     7
+#define   GPIO_INDEX_TYPE_WR_SRC   8
+#define   GPIO_INDEX_TYPE_RD_SRC   9
+#define  GPIO_INDEX_CMD_SHIFT      12
+#define  GPIO_INDEX_CMD_LEN        1
+#define   GPIO_INDEX_CMD_WRITE     0
+#define   GPIO_INDEX_CMD_READ      1
+#define  GPIO_INDEX_NR_SHIFT       0
+#define  GPIO_INDEX_NR_LEN         8
+#define GPIO_3_3V_MEM_SIZE         0x2B0
 #define GPIO_3_3V_REG_ARRAY_SIZE   (GPIO_3_3V_MEM_SIZE >> 2)
 
 /* AST2600 only - 1.8V gpios */
@@ -571,6 +606,11 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size)
         return (uint64_t) s->debounce_regs[idx];
     }
 
+    /* This is a (new, indirect) register interface for configuring GPIOs */
+    if (agc->have_index_reg && idx == GPIO_INDEX) {
+        return (uint64_t) s->index;
+    }
+
     reg = &agc->reg_table[idx];
     if (reg->set_idx >= agc->nr_gpio_sets) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: no getter for offset 0x%"
@@ -581,8 +621,73 @@ static uint64_t aspeed_gpio_read(void *opaque, hwaddr offset, uint32_t size)
     return aspeed_gpio_set_read(s, reg);
 }
 
-static void aspeed_gpio_set_write(AspeedGPIOState *s, const AspeedGPIOReg *reg,
-                                  uint32_t data)
+static int aspeed_gpio_set_offset_read(AspeedGPIOState *s, int set, enum GPIORegType reg,
+                                       int offset)
+{
+    return !!(aspeed_gpio_set_read(s, &(AspeedGPIOReg){set, reg}) & BIT(offset));
+}
+
+static const enum GPIORegType aspeed_gpio_index_type_map[] = {
+   [GPIO_INDEX_TYPE_DATA] = gpio_reg_data_value,
+   [GPIO_INDEX_TYPE_DIR] = gpio_reg_direction,
+   [GPIO_INDEX_TYPE_TOL] = gpio_reg_reset_tolerant,
+   [GPIO_INDEX_TYPE_INPUT] = gpio_reg_input_mask,
+   [GPIO_INDEX_TYPE_WR_SRC] = gpio_reg_input_mask /* See GPIO2AC doc */
+};
+
+static void
+aspeed_gpio_index_read(AspeedGPIOState *s, uint32_t type, uint32_t number)
+{
+    int pin = number % 32;
+    int set = number / 32;
+
+    /* Clear the data field so we can OR into it without further data dependencies */
+    s->index = deposit32(s->index, GPIO_INDEX_DATA_SHIFT, GPIO_INDEX_DATA_LEN, 0);
+
+    switch (type) {
+    case GPIO_INDEX_TYPE_DATA:
+    case GPIO_INDEX_TYPE_DIR:
+    case GPIO_INDEX_TYPE_TOL:
+    case GPIO_INDEX_TYPE_INPUT:
+    case GPIO_INDEX_TYPE_WR_SRC:
+    {
+        enum GPIORegType reg = aspeed_gpio_index_type_map[type];
+        s->index |= deposit32(0, GPIO_INDEX_DATA_SHIFT, GPIO_INDEX_DATA_LEN,
+                              aspeed_gpio_set_offset_read(s, set, reg, pin));
+        break;
+    }
+    case GPIO_INDEX_TYPE_IRQ:
+        s->index |= deposit32(0, GPIO_INDEX_DATA_IRQ_EN, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_int_enable, pin));
+        s->index |= deposit32(0, GPIO_INDEX_DATA_IRQ_TY0, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_int_sens_0, pin));
+        s->index |= deposit32(0, GPIO_INDEX_DATA_IRQ_TY1, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_int_sens_1, pin));
+        s->index |= deposit32(0, GPIO_INDEX_DATA_IRQ_TY2, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_int_sens_2, pin));
+        s->index |= deposit32(0, GPIO_INDEX_DATA_IRQ_STS, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_int_status, pin));
+        break;
+    case GPIO_INDEX_TYPE_DEBOUNCE:
+        s->index |= deposit32(0, GPIO_INDEX_DATA_DB1, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_debounce_1, pin));
+        s->index |= deposit32(0, GPIO_INDEX_DATA_DB2, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_debounce_2, pin));
+        break;
+    case GPIO_INDEX_TYPE_SRC:
+        s->index |= deposit32(0, GPIO_INDEX_DATA_SRC0, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_cmd_source_0, pin));
+        s->index |= deposit32(0, GPIO_INDEX_DATA_SRC1, 1,
+                        aspeed_gpio_set_offset_read(s, set, gpio_reg_cmd_source_1, pin));
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: no such command type: %" PRIu32 "\n",
+                      __func__, type);
+    }
+}
+
+static void
+aspeed_gpio_set_write(AspeedGPIOState *s, const AspeedGPIOReg *reg, uint32_t data)
 {
     AspeedGPIOClass *agc = ASPEED_GPIO_GET_CLASS(s);
     const GPIOSetProperties *props;
@@ -677,6 +782,87 @@ static void aspeed_gpio_set_write(AspeedGPIOState *s, const AspeedGPIOReg *reg,
     aspeed_gpio_update(s, set, set->data_value);
 }
 
+static void
+aspeed_gpio_set_offset_write(AspeedGPIOState *s, int set, enum GPIORegType reg,
+                             int offset, int val)
+{
+    AspeedGPIOReg agr = { set, reg };
+    uint32_t data;
+
+    data = aspeed_gpio_set_read(s, &agr);
+    data = deposit32(data, offset, 1, val);
+    aspeed_gpio_set_write(s, &agr, data);
+}
+
+static void
+aspeed_gpio_index_write(AspeedGPIOState *s, uint32_t type, uint32_t number, uint32_t data)
+{
+    int pin = number % 32;
+    int set = number / 32;
+
+    switch (type) {
+    case GPIO_INDEX_TYPE_DATA:
+    case GPIO_INDEX_TYPE_DIR:
+    case GPIO_INDEX_TYPE_TOL:
+    case GPIO_INDEX_TYPE_INPUT:
+    case GPIO_INDEX_TYPE_WR_SRC:
+    {
+        enum GPIORegType reg = aspeed_gpio_index_type_map[type];
+        aspeed_gpio_set_offset_write(s, set, reg, pin, data);
+        break;
+    }
+    case GPIO_INDEX_TYPE_IRQ:
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_int_enable, pin,
+                                     extract32(data, GPIO_INDEX_DATA_IRQ_EN, 1));
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_int_sens_0, pin,
+                                     extract32(data, GPIO_INDEX_DATA_IRQ_TY0, 1));
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_int_sens_1, pin,
+                                     extract32(data, GPIO_INDEX_DATA_IRQ_TY1, 1));
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_int_sens_2, pin,
+                                     extract32(data, GPIO_INDEX_DATA_IRQ_TY2, 1));
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_int_status, pin,
+                                     extract32(data, GPIO_INDEX_DATA_IRQ_STS, 1));
+        break;
+    case GPIO_INDEX_TYPE_DEBOUNCE:
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_debounce_1, pin,
+                                     extract32(data, GPIO_INDEX_DATA_DB1, 1));
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_debounce_2, pin,
+                                     extract32(data, GPIO_INDEX_DATA_DB2, 1));
+        break;
+    case GPIO_INDEX_TYPE_SRC:
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_cmd_source_0, pin,
+                                     extract32(data, GPIO_INDEX_DATA_SRC0, 1));
+        aspeed_gpio_set_offset_write(s, set, gpio_reg_cmd_source_1, pin,
+                                     extract32(data, GPIO_INDEX_DATA_SRC1, 1));
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: no such command type: %" PRIu32 "\n",
+                      __func__, type);
+    };
+}
+
+static void aspeed_gpio_index_command(AspeedGPIOState *s, uint32_t index)
+{
+    uint32_t command, number, type;
+
+    s->index = index;
+
+    command = extract32(index, GPIO_INDEX_CMD_SHIFT, GPIO_INDEX_CMD_LEN);
+    number = extract32(index, GPIO_INDEX_NR_SHIFT, GPIO_INDEX_NR_LEN);
+    type = extract32(index, GPIO_INDEX_TYPE_SHIFT, GPIO_INDEX_TYPE_LEN);
+
+    if (command == GPIO_INDEX_CMD_WRITE) {
+        uint32_t data;
+
+        data = extract32(index, GPIO_INDEX_DATA_SHIFT, GPIO_INDEX_DATA_LEN);
+        aspeed_gpio_index_write(s, type, number, data);
+
+        return;
+    }
+
+    aspeed_gpio_index_read(s, type, number);
+}
+
 static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
                               uint32_t size)
 {
@@ -692,6 +878,12 @@ static void aspeed_gpio_write(void *opaque, hwaddr offset, uint64_t data,
         return;
     }
 
+    /* This is a (new, indirect) register interface for configuring GPIOs */
+    if (agc->have_index_reg && idx == GPIO_INDEX) {
+        aspeed_gpio_index_command(s, data);
+        return;
+    }
+
     reg = &agc->reg_table[idx];
     if (reg->set_idx >= agc->nr_gpio_sets) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: no setter for offset 0x%"
@@ -930,6 +1122,7 @@ static void aspeed_gpio_ast2400_class_init(ObjectClass *klass, void *data)
     agc->nr_gpio_pins = 216;
     agc->nr_gpio_sets = 7;
     agc->reg_table = aspeed_3_3v_gpios;
+    agc->have_index_reg = false;
 }
 
 static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
@@ -940,6 +1133,7 @@ static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
     agc->nr_gpio_pins = 228;
     agc->nr_gpio_sets = 8;
     agc->reg_table = aspeed_3_3v_gpios;
+    agc->have_index_reg = false;
 }
 
 static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
@@ -950,6 +1144,7 @@ static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
     agc->nr_gpio_pins = 208;
     agc->nr_gpio_sets = 7;
     agc->reg_table = aspeed_3_3v_gpios;
+    agc->have_index_reg = true;
 }
 
 static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
@@ -960,6 +1155,7 @@ static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
     agc->nr_gpio_pins = 36;
     agc->nr_gpio_sets = 2;
     agc->reg_table = aspeed_1_8v_gpios;
+    agc->have_index_reg = true;
 }
 
 static const TypeInfo aspeed_gpio_info = {
diff --git a/include/hw/gpio/aspeed_gpio.h b/include/hw/gpio/aspeed_gpio.h
index 801846befb3b..57188fcb4098 100644
--- a/include/hw/gpio/aspeed_gpio.h
+++ b/include/hw/gpio/aspeed_gpio.h
@@ -61,6 +61,7 @@ struct AspeedGPIOClass {
     uint32_t nr_gpio_pins;
     uint32_t nr_gpio_sets;
     const AspeedGPIOReg *reg_table;
+    bool have_index_reg;
 };
 
 struct AspeedGPIOState {
@@ -91,6 +92,8 @@ struct AspeedGPIOState {
         uint32_t debounce_2;
         uint32_t input_mask;
     } sets[ASPEED_GPIO_MAX_NR_SETS];
+
+    uint32_t index;
 };
 
 #endif /* _ASPEED_GPIO_H_ */
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600
  2022-02-07 15:04 [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600 Andrew Jeffery
                   ` (2 preceding siblings ...)
  2022-02-07 15:04 ` [PATCH 3/3] hw: aspeed_gpio: Support the AST2600's indexed register interface Andrew Jeffery
@ 2022-02-09 11:09 ` Andrew Jeffery
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Jeffery @ 2022-02-09 11:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, openbmc, Cédric Le Goater



On Tue, 8 Feb 2022, at 01:34, Andrew Jeffery wrote:
> Hello,
>
> This series adds support for a new register interface supported by the
> Aspeed GPIO controller, present in at least the AST2600.
>
> The new interface is a single register implementing an indirect command
> protocol that allows us to manipulate up to (at least) 208 GPIOs. This
> makes it possible to write very simple drivers for e.g. u-boot and
> jettison the need for the tedious data model required to deal with the
> old register layout.
>
> I've lightly tested the device consistency under Linux. The Linux
> driver is implemented in terms of the old interface, so data model
> consistency can be tested one way by poking the driver using the
> libgpiod tools and then the other using devmem to drive the new
> interface.
>
> Please review!

Naturally further testing revealed some quirks that require further 
enhancements to the modelling.

Hold off on doing anything with this series for the moment.

Cheers,

Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-02-09 11:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-07 15:04 [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600 Andrew Jeffery
2022-02-07 15:04 ` [PATCH 1/3] hw: aspeed_gpio: Cleanup stray semicolon after switch Andrew Jeffery
2022-02-07 15:04 ` [PATCH 2/3] hw: aspeed_gpio: Split GPIOSet handling from accessors Andrew Jeffery
2022-02-07 15:04 ` [PATCH 3/3] hw: aspeed_gpio: Support the AST2600's indexed register interface Andrew Jeffery
2022-02-09 11:09 ` [PATCH 0/3] hw: aspeed_gpio: Model new interface for the AST2600 Andrew Jeffery

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).