Re: [Qemu-devel] [PATCH v1 2/3] i.MX: Add GPIO device to i.MX31 SOC

[Jean-Christophe DUBOIS]

Le 05/09/2015 21:36, Peter Crosthwaite a écrit :
> "devices" in subject.
>
> On Sat, Sep 5, 2015 at 1:17 AM, Jean-Christophe Dubois
> <address@hidden> wrote:
> > Signed-off-by: Jean-Christophe Dubois <address@hidden>
> > ---
> >   hw/arm/fsl-imx31.c         | 30 ++++++++++++++++++++++++++++++
> >   include/hw/arm/fsl-imx31.h | 12 ++++++++++++
> >   2 files changed, 42 insertions(+)
> >
> > diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
> > index 87548c8..aa938ed 100644
> > --- a/hw/arm/fsl-imx31.c
> > +++ b/hw/arm/fsl-imx31.c
> > @@ -55,6 +55,11 @@ static void fsl_imx31_init(Object *obj)
> >           object_initialize(&s->i2c[i], sizeof(s->i2c[i]), TYPE_IMX_I2C);
> >           qdev_set_parent_bus(DEVICE(&s->i2c[i]), sysbus_get_default());
> >       }
> > +
> > +    for (i = 0; i < FSL_IMX31_NUM_GPIOS; i++) {
> > +        object_initialize(&s->gpio[i], sizeof(s->gpio[i]), TYPE_IMX_GPIO);
> > +        qdev_set_parent_bus(DEVICE(&s->gpio[i]), sysbus_get_default());
> > +    }
> >   }
> >
> >   static void fsl_imx31_realize(DeviceState *dev, Error **errp)
> > @@ -184,6 +189,31 @@ static void fsl_imx31_realize(DeviceState *dev, Error 
> > **errp)
> >                                               i2c_table[i].irq));
> >       }
> >
> > +    /* Initialize all GPIO */
> "GPIOs"
OK
> > +    for (i = 0; i < FSL_IMX31_NUM_GPIOS; i++) {
> > +        static const struct {
> > +            hwaddr addr;
> > +            unsigned int irq;
> > +        } gpio_table[FSL_IMX31_NUM_GPIOS] = {
> > +            { FSL_IMX31_GPIO1_ADDR, FSL_IMX31_GPIO1_IRQ },
> > +            { FSL_IMX31_GPIO2_ADDR, FSL_IMX31_GPIO2_IRQ },
> > +            { FSL_IMX31_GPIO3_ADDR, FSL_IMX31_GPIO3_IRQ }
> > +        };
> > +
> > +        /* Initialize the GPIO */
> Self docced, comment not needed.
OK
> > +        object_property_set_bool(OBJECT(&s->gpio[i]), true, "realized", &err);
> > +        if (err) {
> > +            error_propagate(errp, err);
> > +            return;
> > +        }
> > +        /* Map GPIO memory */
> Same.
OK
> Otherwise,
>
> Reviewed-by: Peter Crosthwaite <address@hidden>
>
> Regards,
> Peter
>
> > +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0, gpio_table[i].addr);
> > +        /* Connect GPIO IRQ to PIC */
> > +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio[i]), 0,
> > +                           qdev_get_gpio_in(DEVICE(&s->avic),
> > +                                            gpio_table[i].irq));
> > +    }
> > +
> >       /* On a real system, the first 16k is a `secure boot rom' */
> >       memory_region_init_rom_device(&s->secure_rom, NULL, NULL, NULL,
> >                                     "imx31.secure_rom",
> > diff --git a/include/hw/arm/fsl-imx31.h b/include/hw/arm/fsl-imx31.h
> > index 891166f..5e8f795 100644
> > --- a/include/hw/arm/fsl-imx31.h
> > +++ b/include/hw/arm/fsl-imx31.h
> > @@ -24,6 +24,7 @@
> >   #include "hw/timer/imx_gpt.h"
> >   #include "hw/timer/imx_epit.h"
> >   #include "hw/i2c/imx_i2c.h"
> > +#include "hw/gpio/imx_gpio.h"
> >   #include "exec/memory.h"
> >
> >   #define TYPE_FSL_IMX31 "fsl,imx31"
> > @@ -32,6 +33,7 @@
> >   #define FSL_IMX31_NUM_UARTS 2
> >   #define FSL_IMX31_NUM_EPITS 2
> >   #define FSL_IMX31_NUM_I2CS 3
> > +#define FSL_IMX31_NUM_GPIOS 3
> >
> >   typedef struct FslIMX31State {
> >       /*< private >*/
> > @@ -45,6 +47,7 @@ typedef struct FslIMX31State {
> >       IMXGPTState    gpt;
> >       IMXEPITState   epit[FSL_IMX31_NUM_EPITS];
> >       IMXI2CState    i2c[FSL_IMX31_NUM_I2CS];
> > +    IMXGPIOState   gpio[FSL_IMX31_NUM_GPIOS];
> >       MemoryRegion   secure_rom;
> >       MemoryRegion   rom;
> >       MemoryRegion   iram;
> > @@ -77,6 +80,12 @@ typedef struct FslIMX31State {
> >   #define FSL_IMX31_EPIT1_SIZE            0x4000
> >   #define FSL_IMX31_EPIT2_ADDR            0x53F98000
> >   #define FSL_IMX31_EPIT2_SIZE            0x4000
> > +#define FSL_IMX31_GPIO3_ADDR            0x53FA4000
> > +#define FSL_IMX31_GPIO3_SIZE            0x4000
> > +#define FSL_IMX31_GPIO1_ADDR            0x53FCC000
> > +#define FSL_IMX31_GPIO1_SIZE            0x4000
> > +#define FSL_IMX31_GPIO2_ADDR            0x53FD0000
> > +#define FSL_IMX31_GPIO2_SIZE            0x4000
> >   #define FSL_IMX31_AVIC_ADDR             0x68000000
> >   #define FSL_IMX31_AVIC_SIZE             0x100
> >   #define FSL_IMX31_SDRAM0_ADDR           0x80000000
> > @@ -106,5 +115,8 @@ typedef struct FslIMX31State {
> >   #define FSL_IMX31_I2C1_IRQ              10
> >   #define FSL_IMX31_I2C2_IRQ              4
> >   #define FSL_IMX31_I2C3_IRQ              3
> > +#define FSL_IMX31_GPIO1_IRQ             52
> > +#define FSL_IMX31_GPIO2_IRQ             51
> > +#define FSL_IMX31_GPIO3_IRQ             56
> >
> >   #endif /* FSL_IMX31_H */
> > --
> > 2.1.4