Re: [Qemu-devel] [PATCH v4 3/3] hw/gpio: Add in AST2600 specific implementation
On Wed, 2019-08-14 at 14:37 +0200, Cédric Le Goater wrote:
> On 14/08/2019 09:14, Rashmica Gupta wrote:
...
> > +static void aspeed_2600_gpio_realize(DeviceState *dev, Error
> > **errp)
> > +{
> > +AspeedGPIOState *s = ASPEED_GPIO(dev);
> > +AspeedGPIOState *s_1_8, *s_3_6;
> > +SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > +AspeedGPIOClass *agc, *agc2;
> > +int pin;
> > +void *obj;
> > +
> > +/* Create and setup the 1.8V gpio state/class */
> > +obj = object_new(TYPE_ASPEED_GPIO "-ast2600");
> > +s_1_8 = ASPEED_GPIO(obj);
> > +object_property_add_child(OBJECT(dev), TYPE_ASPEED_GPIO "-
> > ast2600-1.8v",
> > + obj, errp);
> > +if (error_abort) {
> > +error_propagate(errp, error_abort);
> > +}
>
> This looks wrong.
>
> Shouldn't we instantiate 2 GPIOs devices under the AST2600 SoC with
> different
> types ?
>
We were trying to keep one GPIO device with two states, one state for
3.6V gpios and one for the 1.8V gpios. As you can see I couldn't find
a nice way to do that. I'll have a go at just making two seperate
devices.
> C.
>
>
> > +agc = ASPEED_GPIO_GET_CLASS(s_1_8);
> > +agc->ctrl = (void *)&aspeed_gpio_ast2600_1_8v_controller;
> > +aspeed_gpio_init(obj);
> > +
> > +/* Create and setup the 3.6V gpio state/class */
> > +obj = object_new(TYPE_ASPEED_GPIO "-ast2600");
> > +s_3_6 = ASPEED_GPIO(obj);
> > +object_property_add_child(OBJECT(dev), TYPE_ASPEED_GPIO "-
> > ast2600-3.6v",
> > + obj, errp);
> > +if (error_abort) {
> > +error_propagate(errp, error_abort);
> > +}
> > +agc2 = ASPEED_GPIO_GET_CLASS(s_3_6);
> > +agc2->ctrl = (void *)&aspeed_gpio_ast2600_3_6v_controller;
> > +aspeed_gpio_init(obj);
> > +
> > +for (pin = 0; pin < agc->ctrl->nr_gpio_pins; pin++) {
> > +sysbus_init_irq(sbd, &s->gpios[pin]);
> > +}
> > +
> > +memory_region_init_io(&s_3_6->iomem, OBJECT(s_3_6),
> > &aspeed_gpio_ops, s_3_6,
> > +TYPE_ASPEED_GPIO, GPIO_3_6V_MEM_SIZE +
> > GPIO_1_8V_MEM_SIZE);
> > +s_3_6->lookup = aspeed_3_6v_gpios;
> > +
> > +memory_region_init_io(&s_1_8->iomem, OBJECT(s_1_8),
> > &aspeed_gpio_ops, s_1_8,
> > +TYPE_ASPEED_GPIO, GPIO_1_8V_MEM_SIZE);
> > +memory_region_add_subregion(&s_3_6->iomem,
> > GPIO_1_8V_REG_OFFSET,
> > +&s_1_8->iomem);
> > +s_1_8->lookup = aspeed_1_8v_gpios;
> > +
> > +sysbus_init_mmio(sbd, &s_3_6->iomem);
> > +sysbus_init_mmio(sbd, &s_1_8->iomem);
> > +}
> > +
> > static const VMStateDescription vmstate_gpio_regs = {
> > .name = TYPE_ASPEED_GPIO"/regs",
> > .version_id = 1,
> > @@ -830,6 +991,16 @@ static const VMStateDescription
> > vmstate_aspeed_gpio = {
> > }
> > };
> >
> > +static void aspeed_gpio_ast2600_class_init(ObjectClass *klass,
> > void *data)
> > +{
> > +DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +dc->realize = aspeed_2600_gpio_realize;
> > +dc->reset = aspeed_gpio_reset;
> > +dc->desc = "Aspeed GPIO Controller";
> > +dc->vmsd = &vmstate_aspeed_gpio;
> > +}
> > +
> > static void aspeed_gpio_class_init(ObjectClass *klass, void *data)
> > {
> > DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -866,11 +1037,18 @@ static const TypeInfo
> > aspeed_gpio_ast2500_info = {
> > .class_data = (void *)&aspeed_gpio_ast2500_controller,
> > };
> >
> > +static const TypeInfo aspeed_gpio_ast2600_info = {
> > +.name = TYPE_ASPEED_GPIO "-ast2600",
> > +.parent = TYPE_ASPEED_GPIO,
> > +.class_init = aspeed_gpio_ast2600_class_init,
> > +};
> > +
> > 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_info);
> > }
> >
> > type_init(aspeed_gpio_register_types);
> > diff --git a/slirp b/slirp
> > index f0da672620..126c04acba 16
> > --- a/slirp
> > +++ b/slirp
> > @@ -1 +1 @@
> > -Subproject commit f0da6726207b740f6101028b2992f918477a4b08
> > +Subproject commit 126c04acbabd7ad32c2b018fe10dfac2a3bc1210
> >
Re: [Qemu-devel] [PATCH v4 3/3] hw/gpio: Add in AST2600 specific implementation
On 14/08/2019 09:14, Rashmica Gupta wrote:
> The AST2600 has the same sets of 3.6v gpios as the AST2400 plus an
> addtional two sets of 1.8V gpios.
>
> Signed-off-by: Rashmica Gupta
> ---
> hw/gpio/aspeed_gpio.c | 188 --
> slirp | 2 +-
> 2 files changed, 184 insertions(+), 6 deletions(-)
>
> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
> index bd5af81336..f781fbb6dc 100644
> --- a/hw/gpio/aspeed_gpio.c
> +++ b/hw/gpio/aspeed_gpio.c
> @@ -167,6 +167,49 @@
> #define GPIO_3_6V_MEM_SIZE 0x1F0
> #define GPIO_3_6V_REG_ARRAY_SIZE (GPIO_3_6V_MEM_SIZE >> 2)
>
> +/* AST2600 only - 1.8V gpios */
> +/*
> + * The AST2600 has same 3.6V gpios as the AST2400 (memory offsets 0x0-0x198)
> + * and addtional 1.8V gpios (memory offsets 0x800-0x9D4). We create one
> + * GPIOState for the 3.6V gpios mapped at 0x0 and another GPIOState for the
> + * 1.8V gpios mapped at 0x800.
> + */
> +#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_SIZE0x9D8
> +#define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
> + GPIO_1_8V_REG_OFFSET) >> 2)
> +
> static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio)
> {
> uint32_t falling_edge = 0, rising_edge = 0;
> @@ -465,6 +508,39 @@ static const AspeedGPIOReg
> aspeed_3_6v_gpios[GPIO_3_6V_REG_ARRAY_SIZE] = {
> [GPIO_AC_INPUT_MASK] = { 7, gpio_reg_input_mask },
> };
>
> +static const AspeedGPIOReg aspeed_1_8v_gpios[GPIO_1_8V_REG_ARRAY_SIZE] = {
> +/* 1.8V Set ABCD */
> +[GPIO_1_8V_ABCD_DATA_VALUE] = {0, gpio_reg_data_value},
> +[GPIO_1_8V_ABCD_DIRECTION] = {0, gpio_reg_direction},
> +[GPIO_1_8V_ABCD_INT_ENABLE] = {0, gpio_reg_int_enable},
> +[GPIO_1_8V_ABCD_INT_SENS_0] = {0, gpio_reg_int_sens_0},
> +[GPIO_1_8V_ABCD_INT_SENS_1] = {0, gpio_reg_int_sens_1},
> +[GPIO_1_8V_ABCD_INT_SENS_2] = {0, gpio_reg_int_sens_2},
> +[GPIO_1_8V_ABCD_INT_STATUS] = {0, gpio_reg_int_status},
> +[GPIO_1_8V_ABCD_RESET_TOLERANT] = {0, gpio_reg_reset_tolerant},
> +[GPIO_1_8V_ABCD_DEBOUNCE_1] = {0, gpio_reg_debounce_1},
> +[GPIO_1_8V_ABCD_DEBOUNCE_2] = {0, gpio_reg_debounce_2},
> +[GPIO_1_8V_ABCD_COMMAND_SRC_0] = {0, gpio_reg_cmd_source_0},
> +[GPIO_1_8V_ABCD_COMMAND_SRC_1] = {0, gpio_reg_cmd_source_1},
> +[GPIO_1_8V_ABCD_DATA_READ] = {0, gpio_reg_data_read},
> +[GPIO_1_8V_ABCD_INPUT_MASK] = {0, gpio_reg_inp
[Qemu-devel] [PATCH v4 3/3] hw/gpio: Add in AST2600 specific implementation
The AST2600 has the same sets of 3.6v gpios as the AST2400 plus an
addtional two sets of 1.8V gpios.
Signed-off-by: Rashmica Gupta
---
hw/gpio/aspeed_gpio.c | 188 --
slirp | 2 +-
2 files changed, 184 insertions(+), 6 deletions(-)
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index bd5af81336..f781fbb6dc 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -167,6 +167,49 @@
#define GPIO_3_6V_MEM_SIZE 0x1F0
#define GPIO_3_6V_REG_ARRAY_SIZE (GPIO_3_6V_MEM_SIZE >> 2)
+/* AST2600 only - 1.8V gpios */
+/*
+ * The AST2600 has same 3.6V gpios as the AST2400 (memory offsets 0x0-0x198)
+ * and addtional 1.8V gpios (memory offsets 0x800-0x9D4). We create one
+ * GPIOState for the 3.6V gpios mapped at 0x0 and another GPIOState for the
+ * 1.8V gpios mapped at 0x800.
+ */
+#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_SIZE0x9D8
+#define GPIO_1_8V_REG_ARRAY_SIZE ((GPIO_1_8V_MEM_SIZE - \
+ GPIO_1_8V_REG_OFFSET) >> 2)
+
static int aspeed_evaluate_irq(GPIOSets *regs, int gpio_prev_high, int gpio)
{
uint32_t falling_edge = 0, rising_edge = 0;
@@ -465,6 +508,39 @@ static const AspeedGPIOReg
aspeed_3_6v_gpios[GPIO_3_6V_REG_ARRAY_SIZE] = {
[GPIO_AC_INPUT_MASK] = { 7, gpio_reg_input_mask },
};
+static const AspeedGPIOReg aspeed_1_8v_gpios[GPIO_1_8V_REG_ARRAY_SIZE] = {
+/* 1.8V Set ABCD */
+[GPIO_1_8V_ABCD_DATA_VALUE] = {0, gpio_reg_data_value},
+[GPIO_1_8V_ABCD_DIRECTION] = {0, gpio_reg_direction},
+[GPIO_1_8V_ABCD_INT_ENABLE] = {0, gpio_reg_int_enable},
+[GPIO_1_8V_ABCD_INT_SENS_0] = {0, gpio_reg_int_sens_0},
+[GPIO_1_8V_ABCD_INT_SENS_1] = {0, gpio_reg_int_sens_1},
+[GPIO_1_8V_ABCD_INT_SENS_2] = {0, gpio_reg_int_sens_2},
+[GPIO_1_8V_ABCD_INT_STATUS] = {0, gpio_reg_int_status},
+[GPIO_1_8V_ABCD_RESET_TOLERANT] = {0, gpio_reg_reset_tolerant},
+[GPIO_1_8V_ABCD_DEBOUNCE_1] = {0, gpio_reg_debounce_1},
+[GPIO_1_8V_ABCD_DEBOUNCE_2] = {0, gpio_reg_debounce_2},
+[GPIO_1_8V_ABCD_COMMAND_SRC_0] = {0, gpio_reg_cmd_source_0},
+[GPIO_1_8V_ABCD_COMMAND_SRC_1] = {0, gpio_reg_cmd_source_1},
+[GPIO_1_8V_ABCD_DATA_READ] = {0, gpio_reg_data_read},
+[GPIO_1_8V_ABCD_INPUT_MASK] = {0, gpio_reg_input_mask},
+/* 1.8V Set E */
+[GPIO_1_8V_E_DATA_VALUE] = {1, gpio_reg_data_value},
+[GPIO_1_8V_E_DIRECTION] = {1, gpio_reg_direction},
+[GPIO_1_8V_E_INT_ENABLE] = {1, gpio_reg_int_ena
