* [PATCH v2 0/3] hw: aspeed_gpio: MMIO region fix and cleanups
@ 2021-07-13 6:58 Joel Stanley
2021-07-13 6:58 ` [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size Joel Stanley
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Joel Stanley @ 2021-07-13 6:58 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, qemu-devel
This fixes the MMIO region so that the rtc works again on the ast2600.
The two patches after that are cleanups.
Joel Stanley (3):
hw: aspeed_gpio: Fix memory size
hw: aspeed_gpio: Simplify 1.8V defines
hw: aspeed_gpio: Clarify GPIO controller name
hw/gpio/aspeed_gpio.c | 100 +++++++++++++++++++++---------------------
1 file changed, 49 insertions(+), 51 deletions(-)
--
2.32.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size
2021-07-13 6:58 [PATCH v2 0/3] hw: aspeed_gpio: MMIO region fix and cleanups Joel Stanley
@ 2021-07-13 6:58 ` Joel Stanley
2021-07-13 7:41 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
2021-07-13 6:58 ` [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines Joel Stanley
2021-07-13 6:58 ` [PATCH v2 3/3] hw: aspeed_gpio: Clarify GPIO controller name Joel Stanley
2 siblings, 2 replies; 13+ messages in thread
From: Joel Stanley @ 2021-07-13 6:58 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, qemu-devel
The macro used to calculate the maximum memory size of the MMIO region
had a mistake, causing all GPIO models to create a mapping of 0x9D8.
The intent was to have it be 0x9D8 - 0x800.
This extra size doesn't matter on ast2400 and ast2500, which have a 4KB
region set aside for the GPIO controller.
On the ast2600 the 3.3V and 1.8V GPIO controllers are 2KB apart, so the
regions would overlap. Worse was the 1.8V controller would map over the
top of the following perianal, which happens to be the RTC.
The mmio region used by each device is a maximum of 2KB, so avoid the
calculations and hard code this as the maximum.
Fixes: 36d737ee82b2 ("hw/gpio: Add in AST2600 specific implementation")
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
hw/gpio/aspeed_gpio.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 6ae0116be70b..b3dec4448009 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -207,7 +207,6 @@
#define GPIO_1_8V_MEM_SIZE 0x9D8
#define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_MAX_MEM_SIZE MAX(GPIO_3_6V_MEM_SIZE, GPIO_1_8V_MEM_SIZE)
static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio)
{
@@ -849,7 +848,7 @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp)
}
memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s,
- TYPE_ASPEED_GPIO, GPIO_MAX_MEM_SIZE);
+ TYPE_ASPEED_GPIO, 0x800);
sysbus_init_mmio(sbd, &s->iomem);
}
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines
2021-07-13 6:58 [PATCH v2 0/3] hw: aspeed_gpio: MMIO region fix and cleanups Joel Stanley
2021-07-13 6:58 ` [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size Joel Stanley
@ 2021-07-13 6:58 ` Joel Stanley
2021-07-13 7:46 ` Rashmica Gupta
` (2 more replies)
2021-07-13 6:58 ` [PATCH v2 3/3] hw: aspeed_gpio: Clarify GPIO controller name Joel Stanley
2 siblings, 3 replies; 13+ messages in thread
From: Joel Stanley @ 2021-07-13 6:58 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, qemu-devel
There's no need to define the registers relative to the 0x800 offset
where the controller is mapped, as the device is instantiated as it's
own model at the correct memory address.
Simplify the defines and remove the offset to save future confusion.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
hw/gpio/aspeed_gpio.c | 73 +++++++++++++++++++++----------------------
1 file changed, 36 insertions(+), 37 deletions(-)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index b3dec4448009..dc721aec5da7 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -169,44 +169,43 @@
/* AST2600 only - 1.8V gpios */
/*
- * The AST2600 has same 3.6V gpios as the AST2400 (memory offsets 0x0-0x198)
- * and additional 1.8V gpios (memory offsets 0x800-0x9D4).
+ * The AST2600 two copies of the GPIO controller: the same 3.6V gpios as the
+ * AST2400 (memory offsets 0x0-0x198) and a second controller with 1.8V gpios
+ * (memory offsets 0x800-0x9D4).
*/
-#define GPIO_1_8V_REG_OFFSET 0x800
-#define GPIO_1_8V_ABCD_DATA_VALUE ((0x800 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_DIRECTION ((0x804 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_INT_ENABLE ((0x808 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_INT_SENS_0 ((0x80C - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_INT_SENS_1 ((0x810 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_INT_SENS_2 ((0x814 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_INT_STATUS ((0x818 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_RESET_TOLERANT ((0x81C - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_DATA_VALUE ((0x820 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_DIRECTION ((0x824 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_INT_ENABLE ((0x828 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_INT_SENS_0 ((0x82C - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_INT_SENS_1 ((0x830 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_INT_SENS_2 ((0x834 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_INT_STATUS ((0x838 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_RESET_TOLERANT ((0x83C - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_DEBOUNCE_1 ((0x840 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_DEBOUNCE_2 ((0x844 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_DEBOUNCE_1 ((0x848 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_DEBOUNCE_2 ((0x84C - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_DEBOUNCE_TIME_1 ((0x850 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_DEBOUNCE_TIME_2 ((0x854 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_DEBOUNCE_TIME_3 ((0x858 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_COMMAND_SRC_0 ((0x860 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_COMMAND_SRC_1 ((0x864 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_COMMAND_SRC_0 ((0x868 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_COMMAND_SRC_1 ((0x86C - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_DATA_READ ((0x8C0 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_DATA_READ ((0x8C4 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_ABCD_INPUT_MASK ((0x9D0 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_E_INPUT_MASK ((0x9D4 - GPIO_1_8V_REG_OFFSET) >> 2)
-#define GPIO_1_8V_MEM_SIZE 0x9D8
-#define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
- GPIO_1_8V_REG_OFFSET) >> 2)
+#define GPIO_1_8V_ABCD_DATA_VALUE (0x000 >> 2)
+#define GPIO_1_8V_ABCD_DIRECTION (0x004 >> 2)
+#define GPIO_1_8V_ABCD_INT_ENABLE (0x008 >> 2)
+#define GPIO_1_8V_ABCD_INT_SENS_0 (0x00C >> 2)
+#define GPIO_1_8V_ABCD_INT_SENS_1 (0x010 >> 2)
+#define GPIO_1_8V_ABCD_INT_SENS_2 (0x014 >> 2)
+#define GPIO_1_8V_ABCD_INT_STATUS (0x018 >> 2)
+#define GPIO_1_8V_ABCD_RESET_TOLERANT (0x01C >> 2)
+#define GPIO_1_8V_E_DATA_VALUE (0x020 >> 2)
+#define GPIO_1_8V_E_DIRECTION (0x024 >> 2)
+#define GPIO_1_8V_E_INT_ENABLE (0x028 >> 2)
+#define GPIO_1_8V_E_INT_SENS_0 (0x02C >> 2)
+#define GPIO_1_8V_E_INT_SENS_1 (0x030 >> 2)
+#define GPIO_1_8V_E_INT_SENS_2 (0x034 >> 2)
+#define GPIO_1_8V_E_INT_STATUS (0x038 >> 2)
+#define GPIO_1_8V_E_RESET_TOLERANT (0x03C >> 2)
+#define GPIO_1_8V_ABCD_DEBOUNCE_1 (0x040 >> 2)
+#define GPIO_1_8V_ABCD_DEBOUNCE_2 (0x044 >> 2)
+#define GPIO_1_8V_E_DEBOUNCE_1 (0x048 >> 2)
+#define GPIO_1_8V_E_DEBOUNCE_2 (0x04C >> 2)
+#define GPIO_1_8V_DEBOUNCE_TIME_1 (0x050 >> 2)
+#define GPIO_1_8V_DEBOUNCE_TIME_2 (0x054 >> 2)
+#define GPIO_1_8V_DEBOUNCE_TIME_3 (0x058 >> 2)
+#define GPIO_1_8V_ABCD_COMMAND_SRC_0 (0x060 >> 2)
+#define GPIO_1_8V_ABCD_COMMAND_SRC_1 (0x064 >> 2)
+#define GPIO_1_8V_E_COMMAND_SRC_0 (0x068 >> 2)
+#define GPIO_1_8V_E_COMMAND_SRC_1 (0x06C >> 2)
+#define GPIO_1_8V_ABCD_DATA_READ (0x0C0 >> 2)
+#define GPIO_1_8V_E_DATA_READ (0x0C4 >> 2)
+#define GPIO_1_8V_ABCD_INPUT_MASK (0x1D0 >> 2)
+#define GPIO_1_8V_E_INPUT_MASK (0x1D4 >> 2)
+#define GPIO_1_8V_MEM_SIZE 0x1D8
+#define GPIO_1_8V_REG_ARRAY_SIZE (GPIO_1_8V_MEM_SIZE >> 2)
static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio)
{
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] hw: aspeed_gpio: Clarify GPIO controller name
2021-07-13 6:58 [PATCH v2 0/3] hw: aspeed_gpio: MMIO region fix and cleanups Joel Stanley
2021-07-13 6:58 ` [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size Joel Stanley
2021-07-13 6:58 ` [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines Joel Stanley
@ 2021-07-13 6:58 ` Joel Stanley
2021-07-13 7:48 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
2 siblings, 2 replies; 13+ messages in thread
From: Joel Stanley @ 2021-07-13 6:58 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, qemu-devel
There are two GPIO controllers in the ast2600; one is 3.3V and the other
is 1.8V.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
hw/gpio/aspeed_gpio.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index dc721aec5da7..dfa6d6cb40a9 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -164,12 +164,12 @@
#define GPIO_YZAAAB_DIRECTION (0x1E4 >> 2)
#define GPIO_AC_DATA_VALUE (0x1E8 >> 2)
#define GPIO_AC_DIRECTION (0x1EC >> 2)
-#define GPIO_3_6V_MEM_SIZE 0x1F0
-#define GPIO_3_6V_REG_ARRAY_SIZE (GPIO_3_6V_MEM_SIZE >> 2)
+#define GPIO_3_3V_MEM_SIZE 0x1F0
+#define GPIO_3_3V_REG_ARRAY_SIZE (GPIO_3_3V_MEM_SIZE >> 2)
/* AST2600 only - 1.8V gpios */
/*
- * The AST2600 two copies of the GPIO controller: the same 3.6V gpios as the
+ * The AST2600 two copies of the GPIO controller: the same 3.3V gpios as the
* AST2400 (memory offsets 0x0-0x198) and a second controller with 1.8V gpios
* (memory offsets 0x800-0x9D4).
*/
@@ -380,7 +380,7 @@ static uint32_t update_value_control_source(GPIOSets *regs, uint32_t old_value,
return new_value;
}
-static const AspeedGPIOReg aspeed_3_6v_gpios[GPIO_3_6V_REG_ARRAY_SIZE] = {
+static const AspeedGPIOReg aspeed_3_3v_gpios[GPIO_3_3V_REG_ARRAY_SIZE] = {
/* Set ABCD */
[GPIO_ABCD_DATA_VALUE] = { 0, gpio_reg_data_value },
[GPIO_ABCD_DIRECTION] = { 0, gpio_reg_direction },
@@ -800,7 +800,7 @@ static const GPIOSetProperties ast2500_set_props[] = {
[7] = {0x000000ff, 0x000000ff, {"AC"} },
};
-static GPIOSetProperties ast2600_3_6v_set_props[] = {
+static GPIOSetProperties ast2600_3_3v_set_props[] = {
[0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
[1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} },
[2] = {0xffffffff, 0xffffffff, {"I", "J", "K", "L"} },
@@ -927,7 +927,7 @@ static void aspeed_gpio_ast2400_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_pins = 216;
agc->nr_gpio_sets = 7;
agc->gap = 196;
- agc->reg_table = aspeed_3_6v_gpios;
+ agc->reg_table = aspeed_3_3v_gpios;
}
static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
@@ -938,17 +938,17 @@ static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
agc->nr_gpio_pins = 228;
agc->nr_gpio_sets = 8;
agc->gap = 220;
- agc->reg_table = aspeed_3_6v_gpios;
+ agc->reg_table = aspeed_3_3v_gpios;
}
-static void aspeed_gpio_ast2600_3_6v_class_init(ObjectClass *klass, void *data)
+static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
{
AspeedGPIOClass *agc = ASPEED_GPIO_CLASS(klass);
- agc->props = ast2600_3_6v_set_props;
+ agc->props = ast2600_3_3v_set_props;
agc->nr_gpio_pins = 208;
agc->nr_gpio_sets = 7;
- agc->reg_table = aspeed_3_6v_gpios;
+ agc->reg_table = aspeed_3_3v_gpios;
}
static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
@@ -984,10 +984,10 @@ static const TypeInfo aspeed_gpio_ast2500_info = {
.instance_init = aspeed_gpio_init,
};
-static const TypeInfo aspeed_gpio_ast2600_3_6v_info = {
+static const TypeInfo aspeed_gpio_ast2600_3_3v_info = {
.name = TYPE_ASPEED_GPIO "-ast2600",
.parent = TYPE_ASPEED_GPIO,
- .class_init = aspeed_gpio_ast2600_3_6v_class_init,
+ .class_init = aspeed_gpio_ast2600_3_3v_class_init,
.instance_init = aspeed_gpio_init,
};
@@ -1003,7 +1003,7 @@ static void aspeed_gpio_register_types(void)
type_register_static(&aspeed_gpio_info);
type_register_static(&aspeed_gpio_ast2400_info);
type_register_static(&aspeed_gpio_ast2500_info);
- type_register_static(&aspeed_gpio_ast2600_3_6v_info);
+ type_register_static(&aspeed_gpio_ast2600_3_3v_info);
type_register_static(&aspeed_gpio_ast2600_1_8v_info);
}
--
2.32.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size
2021-07-13 6:58 ` [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size Joel Stanley
@ 2021-07-13 7:41 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
1 sibling, 0 replies; 13+ messages in thread
From: Rashmica Gupta @ 2021-07-13 7:41 UTC (permalink / raw)
To: Joel Stanley, Cédric Le Goater; +Cc: Andrew Jeffery, qemu-arm, qemu-devel
On Tue, 2021-07-13 at 16:28 +0930, Joel Stanley wrote:
> The macro used to calculate the maximum memory size of the MMIO
> region
> had a mistake, causing all GPIO models to create a mapping of 0x9D8.
> The intent was to have it be 0x9D8 - 0x800.
>
> This extra size doesn't matter on ast2400 and ast2500, which have a
> 4KB
> region set aside for the GPIO controller.
>
> On the ast2600 the 3.3V and 1.8V GPIO controllers are 2KB apart, so
> the
> regions would overlap. Worse was the 1.8V controller would map over
> the
> top of the following perianal, which happens to be the RTC.
>
> The mmio region used by each device is a maximum of 2KB, so avoid the
> calculations and hard code this as the maximum.
>
> Fixes: 36d737ee82b2 ("hw/gpio: Add in AST2600 specific
> implementation")
> Signed-off-by: Joel Stanley <joel@jms.id.au>
derp. Sorry about that. This looks correct.
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
> hw/gpio/aspeed_gpio.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index 6ae0116be70b..b3dec4448009 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -207,7 +207,6 @@
> #define GPIO_1_8V_MEM_SIZE 0x9D8
> #define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_MAX_MEM_SIZE MAX(GPIO_3_6V_MEM_SIZE,
> GPIO_1_8V_MEM_SIZE)
>
> static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high,
> int gpio)
> {
> @@ -849,7 +848,7 @@ static void aspeed_gpio_realize(DeviceState *dev,
> Error **errp)
> }
>
> memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s,
> - TYPE_ASPEED_GPIO, GPIO_MAX_MEM_SIZE);
> + TYPE_ASPEED_GPIO, 0x800);
>
> sysbus_init_mmio(sbd, &s->iomem);
> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines
2021-07-13 6:58 ` [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines Joel Stanley
@ 2021-07-13 7:46 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
2021-07-19 22:05 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 13+ messages in thread
From: Rashmica Gupta @ 2021-07-13 7:46 UTC (permalink / raw)
To: Joel Stanley, Cédric Le Goater; +Cc: Andrew Jeffery, qemu-arm, qemu-devel
On Tue, 2021-07-13 at 16:28 +0930, Joel Stanley wrote:
> There's no need to define the registers relative to the 0x800 offset
> where the controller is mapped, as the device is instantiated as it's
> own model at the correct memory address.
>
> Simplify the defines and remove the offset to save future confusion.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Makes sense, and it is cleaner.
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
> hw/gpio/aspeed_gpio.c | 73 +++++++++++++++++++++--------------------
> --
> 1 file changed, 36 insertions(+), 37 deletions(-)
>
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index b3dec4448009..dc721aec5da7 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -169,44 +169,43 @@
>
> /* AST2600 only - 1.8V gpios */
> /*
> - * The AST2600 has same 3.6V gpios as the AST2400 (memory offsets
> 0x0-0x198)
> - * and additional 1.8V gpios (memory offsets 0x800-0x9D4).
> + * The AST2600 two copies of the GPIO controller: the same 3.6V
> gpios as the
> + * AST2400 (memory offsets 0x0-0x198) and a second controller with
> 1.8V gpios
> + * (memory offsets 0x800-0x9D4).
> */
> -#define GPIO_1_8V_REG_OFFSET 0x800
> -#define GPIO_1_8V_ABCD_DATA_VALUE ((0x800 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_DIRECTION ((0x804 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_ENABLE ((0x808 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_SENS_0 ((0x80C -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_SENS_1 ((0x810 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_SENS_2 ((0x814 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_STATUS ((0x818 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_RESET_TOLERANT ((0x81C -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DATA_VALUE ((0x820 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DIRECTION ((0x824 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_ENABLE ((0x828 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_SENS_0 ((0x82C -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_SENS_1 ((0x830 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_SENS_2 ((0x834 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_STATUS ((0x838 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_RESET_TOLERANT ((0x83C -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_DEBOUNCE_1 ((0x840 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_DEBOUNCE_2 ((0x844 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DEBOUNCE_1 ((0x848 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DEBOUNCE_2 ((0x84C -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_DEBOUNCE_TIME_1 ((0x850 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_DEBOUNCE_TIME_2 ((0x854 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_DEBOUNCE_TIME_3 ((0x858 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_COMMAND_SRC_0 ((0x860 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_COMMAND_SRC_1 ((0x864 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_COMMAND_SRC_0 ((0x868 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_COMMAND_SRC_1 ((0x86C -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_DATA_READ ((0x8C0 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DATA_READ ((0x8C4 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INPUT_MASK ((0x9D0 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INPUT_MASK ((0x9D4 -
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_MEM_SIZE 0x9D8
> -#define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
> - GPIO_1_8V_REG_OFFSET) >> 2)
> +#define GPIO_1_8V_ABCD_DATA_VALUE (0x000 >> 2)
> +#define GPIO_1_8V_ABCD_DIRECTION (0x004 >> 2)
> +#define GPIO_1_8V_ABCD_INT_ENABLE (0x008 >> 2)
> +#define GPIO_1_8V_ABCD_INT_SENS_0 (0x00C >> 2)
> +#define GPIO_1_8V_ABCD_INT_SENS_1 (0x010 >> 2)
> +#define GPIO_1_8V_ABCD_INT_SENS_2 (0x014 >> 2)
> +#define GPIO_1_8V_ABCD_INT_STATUS (0x018 >> 2)
> +#define GPIO_1_8V_ABCD_RESET_TOLERANT (0x01C >> 2)
> +#define GPIO_1_8V_E_DATA_VALUE (0x020 >> 2)
> +#define GPIO_1_8V_E_DIRECTION (0x024 >> 2)
> +#define GPIO_1_8V_E_INT_ENABLE (0x028 >> 2)
> +#define GPIO_1_8V_E_INT_SENS_0 (0x02C >> 2)
> +#define GPIO_1_8V_E_INT_SENS_1 (0x030 >> 2)
> +#define GPIO_1_8V_E_INT_SENS_2 (0x034 >> 2)
> +#define GPIO_1_8V_E_INT_STATUS (0x038 >> 2)
> +#define GPIO_1_8V_E_RESET_TOLERANT (0x03C >> 2)
> +#define GPIO_1_8V_ABCD_DEBOUNCE_1 (0x040 >> 2)
> +#define GPIO_1_8V_ABCD_DEBOUNCE_2 (0x044 >> 2)
> +#define GPIO_1_8V_E_DEBOUNCE_1 (0x048 >> 2)
> +#define GPIO_1_8V_E_DEBOUNCE_2 (0x04C >> 2)
> +#define GPIO_1_8V_DEBOUNCE_TIME_1 (0x050 >> 2)
> +#define GPIO_1_8V_DEBOUNCE_TIME_2 (0x054 >> 2)
> +#define GPIO_1_8V_DEBOUNCE_TIME_3 (0x058 >> 2)
> +#define GPIO_1_8V_ABCD_COMMAND_SRC_0 (0x060 >> 2)
> +#define GPIO_1_8V_ABCD_COMMAND_SRC_1 (0x064 >> 2)
> +#define GPIO_1_8V_E_COMMAND_SRC_0 (0x068 >> 2)
> +#define GPIO_1_8V_E_COMMAND_SRC_1 (0x06C >> 2)
> +#define GPIO_1_8V_ABCD_DATA_READ (0x0C0 >> 2)
> +#define GPIO_1_8V_E_DATA_READ (0x0C4 >> 2)
> +#define GPIO_1_8V_ABCD_INPUT_MASK (0x1D0 >> 2)
> +#define GPIO_1_8V_E_INPUT_MASK (0x1D4 >> 2)
> +#define GPIO_1_8V_MEM_SIZE 0x1D8
> +#define GPIO_1_8V_REG_ARRAY_SIZE (GPIO_1_8V_MEM_SIZE >> 2)
>
> static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high,
> int gpio)
> {
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/3] hw: aspeed_gpio: Clarify GPIO controller name
2021-07-13 6:58 ` [PATCH v2 3/3] hw: aspeed_gpio: Clarify GPIO controller name Joel Stanley
@ 2021-07-13 7:48 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
1 sibling, 0 replies; 13+ messages in thread
From: Rashmica Gupta @ 2021-07-13 7:48 UTC (permalink / raw)
To: Joel Stanley, Cédric Le Goater; +Cc: Andrew Jeffery, qemu-arm, qemu-devel
On Tue, 2021-07-13 at 16:28 +0930, Joel Stanley wrote:
> There are two GPIO controllers in the ast2600; one is 3.3V and the
> other
> is 1.8V.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Thanks for picking this up.
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
> hw/gpio/aspeed_gpio.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index dc721aec5da7..dfa6d6cb40a9 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -164,12 +164,12 @@
> #define GPIO_YZAAAB_DIRECTION (0x1E4 >> 2)
> #define GPIO_AC_DATA_VALUE (0x1E8 >> 2)
> #define GPIO_AC_DIRECTION (0x1EC >> 2)
> -#define GPIO_3_6V_MEM_SIZE 0x1F0
> -#define GPIO_3_6V_REG_ARRAY_SIZE (GPIO_3_6V_MEM_SIZE >> 2)
> +#define GPIO_3_3V_MEM_SIZE 0x1F0
> +#define GPIO_3_3V_REG_ARRAY_SIZE (GPIO_3_3V_MEM_SIZE >> 2)
>
> /* AST2600 only - 1.8V gpios */
> /*
> - * The AST2600 two copies of the GPIO controller: the same 3.6V
> gpios as the
> + * The AST2600 two copies of the GPIO controller: the same 3.3V
> gpios as the
> * AST2400 (memory offsets 0x0-0x198) and a second controller with
> 1.8V gpios
> * (memory offsets 0x800-0x9D4).
> */
> @@ -380,7 +380,7 @@ static uint32_t
> update_value_control_source(GPIOSets *regs, uint32_t old_value,
> return new_value;
> }
>
> -static const AspeedGPIOReg
> aspeed_3_6v_gpios[GPIO_3_6V_REG_ARRAY_SIZE] = {
> +static const AspeedGPIOReg
> aspeed_3_3v_gpios[GPIO_3_3V_REG_ARRAY_SIZE] = {
> /* Set ABCD */
> [GPIO_ABCD_DATA_VALUE] = { 0, gpio_reg_data_value },
> [GPIO_ABCD_DIRECTION] = { 0, gpio_reg_direction },
> @@ -800,7 +800,7 @@ static const GPIOSetProperties
> ast2500_set_props[] = {
> [7] = {0x000000ff, 0x000000ff, {"AC"} },
> };
>
> -static GPIOSetProperties ast2600_3_6v_set_props[] = {
> +static GPIOSetProperties ast2600_3_3v_set_props[] = {
> [0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
> [1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} },
> [2] = {0xffffffff, 0xffffffff, {"I", "J", "K", "L"} },
> @@ -927,7 +927,7 @@ static void
> aspeed_gpio_ast2400_class_init(ObjectClass *klass, void *data)
> agc->nr_gpio_pins = 216;
> agc->nr_gpio_sets = 7;
> agc->gap = 196;
> - agc->reg_table = aspeed_3_6v_gpios;
> + agc->reg_table = aspeed_3_3v_gpios;
> }
>
> static void aspeed_gpio_2500_class_init(ObjectClass *klass, void
> *data)
> @@ -938,17 +938,17 @@ static void
> aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
> agc->nr_gpio_pins = 228;
> agc->nr_gpio_sets = 8;
> agc->gap = 220;
> - agc->reg_table = aspeed_3_6v_gpios;
> + agc->reg_table = aspeed_3_3v_gpios;
> }
>
> -static void aspeed_gpio_ast2600_3_6v_class_init(ObjectClass *klass,
> void *data)
> +static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass,
> void *data)
> {
> AspeedGPIOClass *agc = ASPEED_GPIO_CLASS(klass);
>
> - agc->props = ast2600_3_6v_set_props;
> + agc->props = ast2600_3_3v_set_props;
> agc->nr_gpio_pins = 208;
> agc->nr_gpio_sets = 7;
> - agc->reg_table = aspeed_3_6v_gpios;
> + agc->reg_table = aspeed_3_3v_gpios;
> }
>
> static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass,
> void *data)
> @@ -984,10 +984,10 @@ static const TypeInfo aspeed_gpio_ast2500_info
> = {
> .instance_init = aspeed_gpio_init,
> };
>
> -static const TypeInfo aspeed_gpio_ast2600_3_6v_info = {
> +static const TypeInfo aspeed_gpio_ast2600_3_3v_info = {
> .name = TYPE_ASPEED_GPIO "-ast2600",
> .parent = TYPE_ASPEED_GPIO,
> - .class_init = aspeed_gpio_ast2600_3_6v_class_init,
> + .class_init = aspeed_gpio_ast2600_3_3v_class_init,
> .instance_init = aspeed_gpio_init,
> };
>
> @@ -1003,7 +1003,7 @@ static void aspeed_gpio_register_types(void)
> type_register_static(&aspeed_gpio_info);
> type_register_static(&aspeed_gpio_ast2400_info);
> type_register_static(&aspeed_gpio_ast2500_info);
> - type_register_static(&aspeed_gpio_ast2600_3_6v_info);
> + type_register_static(&aspeed_gpio_ast2600_3_3v_info);
> type_register_static(&aspeed_gpio_ast2600_1_8v_info);
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [SPAM] [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size
2021-07-13 6:58 ` [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size Joel Stanley
2021-07-13 7:41 ` Rashmica Gupta
@ 2021-07-19 16:02 ` Cédric Le Goater
2021-07-27 8:02 ` Joel Stanley
1 sibling, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2021-07-19 16:02 UTC (permalink / raw)
To: Joel Stanley; +Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, qemu-devel
On 7/13/21 8:58 AM, Joel Stanley wrote:
> The macro used to calculate the maximum memory size of the MMIO region
> had a mistake, causing all GPIO models to create a mapping of 0x9D8.
> The intent was to have it be 0x9D8 - 0x800.
>
> This extra size doesn't matter on ast2400 and ast2500, which have a 4KB
> region set aside for the GPIO controller.
>
> On the ast2600 the 3.3V and 1.8V GPIO controllers are 2KB apart, so the
> regions would overlap. Worse was the 1.8V controller would map over the
> top of the following perianal, which happens to be the RTC.
>
> The mmio region used by each device is a maximum of 2KB, so avoid the
> calculations and hard code this as the maximum.
>
> Fixes: 36d737ee82b2 ("hw/gpio: Add in AST2600 specific implementation")
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/gpio/aspeed_gpio.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index 6ae0116be70b..b3dec4448009 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -207,7 +207,6 @@
> #define GPIO_1_8V_MEM_SIZE 0x9D8
> #define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
> GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_MAX_MEM_SIZE MAX(GPIO_3_6V_MEM_SIZE, GPIO_1_8V_MEM_SIZE)
>
> static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio)
> {
> @@ -849,7 +848,7 @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp)
> }
>
> memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s,
> - TYPE_ASPEED_GPIO, GPIO_MAX_MEM_SIZE);
> + TYPE_ASPEED_GPIO, 0x800);
>
> sysbus_init_mmio(sbd, &s->iomem);
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [SPAM] [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines
2021-07-13 6:58 ` [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines Joel Stanley
2021-07-13 7:46 ` Rashmica Gupta
@ 2021-07-19 16:02 ` Cédric Le Goater
2021-07-19 22:05 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2021-07-19 16:02 UTC (permalink / raw)
To: Joel Stanley; +Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, qemu-devel
On 7/13/21 8:58 AM, Joel Stanley wrote:
> There's no need to define the registers relative to the 0x800 offset
> where the controller is mapped, as the device is instantiated as it's
> own model at the correct memory address.
>
> Simplify the defines and remove the offset to save future confusion.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/gpio/aspeed_gpio.c | 73 +++++++++++++++++++++----------------------
> 1 file changed, 36 insertions(+), 37 deletions(-)
>
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index b3dec4448009..dc721aec5da7 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -169,44 +169,43 @@
>
> /* AST2600 only - 1.8V gpios */
> /*
> - * The AST2600 has same 3.6V gpios as the AST2400 (memory offsets 0x0-0x198)
> - * and additional 1.8V gpios (memory offsets 0x800-0x9D4).
> + * The AST2600 two copies of the GPIO controller: the same 3.6V gpios as the
> + * AST2400 (memory offsets 0x0-0x198) and a second controller with 1.8V gpios
> + * (memory offsets 0x800-0x9D4).
> */
> -#define GPIO_1_8V_REG_OFFSET 0x800
> -#define GPIO_1_8V_ABCD_DATA_VALUE ((0x800 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_DIRECTION ((0x804 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_ENABLE ((0x808 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_SENS_0 ((0x80C - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_SENS_1 ((0x810 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_SENS_2 ((0x814 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INT_STATUS ((0x818 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_RESET_TOLERANT ((0x81C - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DATA_VALUE ((0x820 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DIRECTION ((0x824 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_ENABLE ((0x828 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_SENS_0 ((0x82C - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_SENS_1 ((0x830 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_SENS_2 ((0x834 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INT_STATUS ((0x838 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_RESET_TOLERANT ((0x83C - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_DEBOUNCE_1 ((0x840 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_DEBOUNCE_2 ((0x844 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DEBOUNCE_1 ((0x848 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DEBOUNCE_2 ((0x84C - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_DEBOUNCE_TIME_1 ((0x850 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_DEBOUNCE_TIME_2 ((0x854 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_DEBOUNCE_TIME_3 ((0x858 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_COMMAND_SRC_0 ((0x860 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_COMMAND_SRC_1 ((0x864 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_COMMAND_SRC_0 ((0x868 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_COMMAND_SRC_1 ((0x86C - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_DATA_READ ((0x8C0 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_DATA_READ ((0x8C4 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_ABCD_INPUT_MASK ((0x9D0 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_E_INPUT_MASK ((0x9D4 - GPIO_1_8V_REG_OFFSET) >> 2)
> -#define GPIO_1_8V_MEM_SIZE 0x9D8
> -#define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
> - GPIO_1_8V_REG_OFFSET) >> 2)
> +#define GPIO_1_8V_ABCD_DATA_VALUE (0x000 >> 2)
> +#define GPIO_1_8V_ABCD_DIRECTION (0x004 >> 2)
> +#define GPIO_1_8V_ABCD_INT_ENABLE (0x008 >> 2)
> +#define GPIO_1_8V_ABCD_INT_SENS_0 (0x00C >> 2)
> +#define GPIO_1_8V_ABCD_INT_SENS_1 (0x010 >> 2)
> +#define GPIO_1_8V_ABCD_INT_SENS_2 (0x014 >> 2)
> +#define GPIO_1_8V_ABCD_INT_STATUS (0x018 >> 2)
> +#define GPIO_1_8V_ABCD_RESET_TOLERANT (0x01C >> 2)
> +#define GPIO_1_8V_E_DATA_VALUE (0x020 >> 2)
> +#define GPIO_1_8V_E_DIRECTION (0x024 >> 2)
> +#define GPIO_1_8V_E_INT_ENABLE (0x028 >> 2)
> +#define GPIO_1_8V_E_INT_SENS_0 (0x02C >> 2)
> +#define GPIO_1_8V_E_INT_SENS_1 (0x030 >> 2)
> +#define GPIO_1_8V_E_INT_SENS_2 (0x034 >> 2)
> +#define GPIO_1_8V_E_INT_STATUS (0x038 >> 2)
> +#define GPIO_1_8V_E_RESET_TOLERANT (0x03C >> 2)
> +#define GPIO_1_8V_ABCD_DEBOUNCE_1 (0x040 >> 2)
> +#define GPIO_1_8V_ABCD_DEBOUNCE_2 (0x044 >> 2)
> +#define GPIO_1_8V_E_DEBOUNCE_1 (0x048 >> 2)
> +#define GPIO_1_8V_E_DEBOUNCE_2 (0x04C >> 2)
> +#define GPIO_1_8V_DEBOUNCE_TIME_1 (0x050 >> 2)
> +#define GPIO_1_8V_DEBOUNCE_TIME_2 (0x054 >> 2)
> +#define GPIO_1_8V_DEBOUNCE_TIME_3 (0x058 >> 2)
> +#define GPIO_1_8V_ABCD_COMMAND_SRC_0 (0x060 >> 2)
> +#define GPIO_1_8V_ABCD_COMMAND_SRC_1 (0x064 >> 2)
> +#define GPIO_1_8V_E_COMMAND_SRC_0 (0x068 >> 2)
> +#define GPIO_1_8V_E_COMMAND_SRC_1 (0x06C >> 2)
> +#define GPIO_1_8V_ABCD_DATA_READ (0x0C0 >> 2)
> +#define GPIO_1_8V_E_DATA_READ (0x0C4 >> 2)
> +#define GPIO_1_8V_ABCD_INPUT_MASK (0x1D0 >> 2)
> +#define GPIO_1_8V_E_INPUT_MASK (0x1D4 >> 2)
> +#define GPIO_1_8V_MEM_SIZE 0x1D8
> +#define GPIO_1_8V_REG_ARRAY_SIZE (GPIO_1_8V_MEM_SIZE >> 2)
>
> static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio)
> {
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [SPAM] [PATCH v2 3/3] hw: aspeed_gpio: Clarify GPIO controller name
2021-07-13 6:58 ` [PATCH v2 3/3] hw: aspeed_gpio: Clarify GPIO controller name Joel Stanley
2021-07-13 7:48 ` Rashmica Gupta
@ 2021-07-19 16:02 ` Cédric Le Goater
1 sibling, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2021-07-19 16:02 UTC (permalink / raw)
To: Joel Stanley; +Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, qemu-devel
On 7/13/21 8:58 AM, Joel Stanley wrote:
> There are two GPIO controllers in the ast2600; one is 3.3V and the other
> is 1.8V.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/gpio/aspeed_gpio.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index dc721aec5da7..dfa6d6cb40a9 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -164,12 +164,12 @@
> #define GPIO_YZAAAB_DIRECTION (0x1E4 >> 2)
> #define GPIO_AC_DATA_VALUE (0x1E8 >> 2)
> #define GPIO_AC_DIRECTION (0x1EC >> 2)
> -#define GPIO_3_6V_MEM_SIZE 0x1F0
> -#define GPIO_3_6V_REG_ARRAY_SIZE (GPIO_3_6V_MEM_SIZE >> 2)
> +#define GPIO_3_3V_MEM_SIZE 0x1F0
> +#define GPIO_3_3V_REG_ARRAY_SIZE (GPIO_3_3V_MEM_SIZE >> 2)
>
> /* AST2600 only - 1.8V gpios */
> /*
> - * The AST2600 two copies of the GPIO controller: the same 3.6V gpios as the
> + * The AST2600 two copies of the GPIO controller: the same 3.3V gpios as the
> * AST2400 (memory offsets 0x0-0x198) and a second controller with 1.8V gpios
> * (memory offsets 0x800-0x9D4).
> */
> @@ -380,7 +380,7 @@ static uint32_t update_value_control_source(GPIOSets *regs, uint32_t old_value,
> return new_value;
> }
>
> -static const AspeedGPIOReg aspeed_3_6v_gpios[GPIO_3_6V_REG_ARRAY_SIZE] = {
> +static const AspeedGPIOReg aspeed_3_3v_gpios[GPIO_3_3V_REG_ARRAY_SIZE] = {
> /* Set ABCD */
> [GPIO_ABCD_DATA_VALUE] = { 0, gpio_reg_data_value },
> [GPIO_ABCD_DIRECTION] = { 0, gpio_reg_direction },
> @@ -800,7 +800,7 @@ static const GPIOSetProperties ast2500_set_props[] = {
> [7] = {0x000000ff, 0x000000ff, {"AC"} },
> };
>
> -static GPIOSetProperties ast2600_3_6v_set_props[] = {
> +static GPIOSetProperties ast2600_3_3v_set_props[] = {
> [0] = {0xffffffff, 0xffffffff, {"A", "B", "C", "D"} },
> [1] = {0xffffffff, 0xffffffff, {"E", "F", "G", "H"} },
> [2] = {0xffffffff, 0xffffffff, {"I", "J", "K", "L"} },
> @@ -927,7 +927,7 @@ static void aspeed_gpio_ast2400_class_init(ObjectClass *klass, void *data)
> agc->nr_gpio_pins = 216;
> agc->nr_gpio_sets = 7;
> agc->gap = 196;
> - agc->reg_table = aspeed_3_6v_gpios;
> + agc->reg_table = aspeed_3_3v_gpios;
> }
>
> static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
> @@ -938,17 +938,17 @@ static void aspeed_gpio_2500_class_init(ObjectClass *klass, void *data)
> agc->nr_gpio_pins = 228;
> agc->nr_gpio_sets = 8;
> agc->gap = 220;
> - agc->reg_table = aspeed_3_6v_gpios;
> + agc->reg_table = aspeed_3_3v_gpios;
> }
>
> -static void aspeed_gpio_ast2600_3_6v_class_init(ObjectClass *klass, void *data)
> +static void aspeed_gpio_ast2600_3_3v_class_init(ObjectClass *klass, void *data)
> {
> AspeedGPIOClass *agc = ASPEED_GPIO_CLASS(klass);
>
> - agc->props = ast2600_3_6v_set_props;
> + agc->props = ast2600_3_3v_set_props;
> agc->nr_gpio_pins = 208;
> agc->nr_gpio_sets = 7;
> - agc->reg_table = aspeed_3_6v_gpios;
> + agc->reg_table = aspeed_3_3v_gpios;
> }
>
> static void aspeed_gpio_ast2600_1_8v_class_init(ObjectClass *klass, void *data)
> @@ -984,10 +984,10 @@ static const TypeInfo aspeed_gpio_ast2500_info = {
> .instance_init = aspeed_gpio_init,
> };
>
> -static const TypeInfo aspeed_gpio_ast2600_3_6v_info = {
> +static const TypeInfo aspeed_gpio_ast2600_3_3v_info = {
> .name = TYPE_ASPEED_GPIO "-ast2600",
> .parent = TYPE_ASPEED_GPIO,
> - .class_init = aspeed_gpio_ast2600_3_6v_class_init,
> + .class_init = aspeed_gpio_ast2600_3_3v_class_init,
> .instance_init = aspeed_gpio_init,
> };
>
> @@ -1003,7 +1003,7 @@ static void aspeed_gpio_register_types(void)
> type_register_static(&aspeed_gpio_info);
> type_register_static(&aspeed_gpio_ast2400_info);
> type_register_static(&aspeed_gpio_ast2500_info);
> - type_register_static(&aspeed_gpio_ast2600_3_6v_info);
> + type_register_static(&aspeed_gpio_ast2600_3_3v_info);
> type_register_static(&aspeed_gpio_ast2600_1_8v_info);
> }
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines
2021-07-13 6:58 ` [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines Joel Stanley
2021-07-13 7:46 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
@ 2021-07-19 22:05 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-19 22:05 UTC (permalink / raw)
To: Joel Stanley, Cédric Le Goater
Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, qemu-devel
On 7/13/21 8:58 AM, Joel Stanley wrote:
> There's no need to define the registers relative to the 0x800 offset
> where the controller is mapped, as the device is instantiated as it's
> own model at the correct memory address.
>
> Simplify the defines and remove the offset to save future confusion.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> hw/gpio/aspeed_gpio.c | 73 +++++++++++++++++++++----------------------
> 1 file changed, 36 insertions(+), 37 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [SPAM] [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
@ 2021-07-27 8:02 ` Joel Stanley
2021-07-27 9:58 ` Peter Maydell
0 siblings, 1 reply; 13+ messages in thread
From: Joel Stanley @ 2021-07-27 8:02 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell
Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, QEMU Developers
On Mon, 19 Jul 2021 at 16:02, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 7/13/21 8:58 AM, Joel Stanley wrote:
> > The macro used to calculate the maximum memory size of the MMIO region
> > had a mistake, causing all GPIO models to create a mapping of 0x9D8.
> > The intent was to have it be 0x9D8 - 0x800.
> >
> > This extra size doesn't matter on ast2400 and ast2500, which have a 4KB
> > region set aside for the GPIO controller.
> >
> > On the ast2600 the 3.3V and 1.8V GPIO controllers are 2KB apart, so the
> > regions would overlap. Worse was the 1.8V controller would map over the
> > top of the following perianal, which happens to be the RTC.
> >
> > The mmio region used by each device is a maximum of 2KB, so avoid the
> > calculations and hard code this as the maximum.
> >
> > Fixes: 36d737ee82b2 ("hw/gpio: Add in AST2600 specific implementation")
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
Cedric, Peter; can we please get this merged for 6.1? Without it the
RTC model is not functional on the ast2500.
The other patches in this series are cleanups that can wait for future releases.
Cheers,
Joel
>
> > ---
> > hw/gpio/aspeed_gpio.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> > index 6ae0116be70b..b3dec4448009 100644
> > --- a/hw/gpio/aspeed_gpio.c
> > +++ b/hw/gpio/aspeed_gpio.c
> > @@ -207,7 +207,6 @@
> > #define GPIO_1_8V_MEM_SIZE 0x9D8
> > #define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
> > GPIO_1_8V_REG_OFFSET) >> 2)
> > -#define GPIO_MAX_MEM_SIZE MAX(GPIO_3_6V_MEM_SIZE, GPIO_1_8V_MEM_SIZE)
> >
> > static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio)
> > {
> > @@ -849,7 +848,7 @@ static void aspeed_gpio_realize(DeviceState *dev, Error **errp)
> > }
> >
> > memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_gpio_ops, s,
> > - TYPE_ASPEED_GPIO, GPIO_MAX_MEM_SIZE);
> > + TYPE_ASPEED_GPIO, 0x800);
> >
> > sysbus_init_mmio(sbd, &s->iomem);
> > }
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [SPAM] [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size
2021-07-27 8:02 ` Joel Stanley
@ 2021-07-27 9:58 ` Peter Maydell
0 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2021-07-27 9:58 UTC (permalink / raw)
To: Joel Stanley
Cc: Andrew Jeffery, Rashmica Gupta, qemu-arm, Cédric Le Goater,
QEMU Developers
On Tue, 27 Jul 2021 at 09:02, Joel Stanley <joel@jms.id.au> wrote:
>
> On Mon, 19 Jul 2021 at 16:02, Cédric Le Goater <clg@kaod.org> wrote:
> >
> > On 7/13/21 8:58 AM, Joel Stanley wrote:
> > > The macro used to calculate the maximum memory size of the MMIO region
> > > had a mistake, causing all GPIO models to create a mapping of 0x9D8.
> > > The intent was to have it be 0x9D8 - 0x800.
> > >
> > > This extra size doesn't matter on ast2400 and ast2500, which have a 4KB
> > > region set aside for the GPIO controller.
> > >
> > > On the ast2600 the 3.3V and 1.8V GPIO controllers are 2KB apart, so the
> > > regions would overlap. Worse was the 1.8V controller would map over the
> > > top of the following perianal, which happens to be the RTC.
I'm going to assume this is an unfortunate autocorrect for
"following peripheral", and will tweak the commit message...
> > >
> > > The mmio region used by each device is a maximum of 2KB, so avoid the
> > > calculations and hard code this as the maximum.
> > >
> > > Fixes: 36d737ee82b2 ("hw/gpio: Add in AST2600 specific implementation")
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> >
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
>
> Cedric, Peter; can we please get this merged for 6.1? Without it the
> RTC model is not functional on the ast2500.
I'm doing an arm pullreq today so I'll put it into that.
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-07-27 10:00 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 6:58 [PATCH v2 0/3] hw: aspeed_gpio: MMIO region fix and cleanups Joel Stanley
2021-07-13 6:58 ` [PATCH v2 1/3] hw: aspeed_gpio: Fix memory size Joel Stanley
2021-07-13 7:41 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
2021-07-27 8:02 ` Joel Stanley
2021-07-27 9:58 ` Peter Maydell
2021-07-13 6:58 ` [PATCH v2 2/3] hw: aspeed_gpio: Simplify 1.8V defines Joel Stanley
2021-07-13 7:46 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
2021-07-19 22:05 ` Philippe Mathieu-Daudé
2021-07-13 6:58 ` [PATCH v2 3/3] hw: aspeed_gpio: Clarify GPIO controller name Joel Stanley
2021-07-13 7:48 ` Rashmica Gupta
2021-07-19 16:02 ` [SPAM] " Cédric Le Goater
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).