| Message ID | 1445985169-13999-1-git-send-email-jcd@tribudubois.net |
|---|---|
| State | New |
| Headers | show |
On Tue, Oct 27, 2015 at 3:32 PM, Jean-Christophe Dubois <jcd@tribudubois.net > wrote: > The i.MX6 GPIO device supports 2 interrupts instead of one. > > * 1 for the lower 16 GPIOs. > * 1 for the upper 16 GPIOs. > > i.MX31 and i.MX25 only support 1 interrupt for the 32 GPIOs. > > So an architectural question, is it the case that the IP always has two outbound interrupt lines but MX31 and 25 always OR together on the SoC level? If that is the case I think it would be cleaner to do on the board level. Regards, Peter > So we add a property to turn the behavior on when required. > > Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> > --- > hw/gpio/imx_gpio.c | 12 ++++++++++-- > include/hw/gpio/imx_gpio.h | 3 ++- > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c > index 3170585..a6d7cab 100644 > --- a/hw/gpio/imx_gpio.c > +++ b/hw/gpio/imx_gpio.c > @@ -62,7 +62,12 @@ static const char *imx_gpio_reg_name(uint32_t reg) > > static void imx_gpio_update_int(IMXGPIOState *s) > { > - qemu_set_irq(s->irq, (s->isr & s->imr) ? 1 : 0); > + if (s->has_upper_pin_irq) { > + qemu_set_irq(s->irq[0], (s->isr & s->imr & 0x0000FFFF) ? 1 : 0); > + qemu_set_irq(s->irq[1], (s->isr & s->imr & 0xFFFF0000) ? 1 : 0); > + } else { > + qemu_set_irq(s->irq[0], (s->isr & s->imr) ? 1 : 0); > + } > } > > static void imx_gpio_set_int_line(IMXGPIOState *s, int line, IMXGPIOLevel > level) > @@ -282,6 +287,8 @@ static const VMStateDescription vmstate_imx_gpio = { > > static Property imx_gpio_properties[] = { > DEFINE_PROP_BOOL("has-edge-sel", IMXGPIOState, has_edge_sel, true), > + DEFINE_PROP_BOOL("has-upper-pin-irq", IMXGPIOState, has_upper_pin_irq, > + false), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -311,7 +318,8 @@ static void imx_gpio_realize(DeviceState *dev, Error > **errp) > qdev_init_gpio_in(DEVICE(s), imx_gpio_set, IMX_GPIO_PIN_COUNT); > qdev_init_gpio_out(DEVICE(s), s->output, IMX_GPIO_PIN_COUNT); > > - sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); > + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[0]); > + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[1]); > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); > } > > diff --git a/include/hw/gpio/imx_gpio.h b/include/hw/gpio/imx_gpio.h > index 517b261..b15a09f 100644 > --- a/include/hw/gpio/imx_gpio.h > +++ b/include/hw/gpio/imx_gpio.h > @@ -54,8 +54,9 @@ typedef struct IMXGPIOState { > uint32_t isr; > bool has_edge_sel; > uint32_t edge_sel; > + bool has_upper_pin_irq; > > - qemu_irq irq; > + qemu_irq irq[2]; > qemu_irq output[IMX_GPIO_PIN_COUNT]; > } IMXGPIOState; > > -- > 2.5.0 > >
Le 27/10/2015 23:41, Peter Crosthwaite a écrit : > > > On Tue, Oct 27, 2015 at 3:32 PM, Jean-Christophe Dubois > <jcd@tribudubois.net <mailto:jcd@tribudubois.net>> wrote: > > The i.MX6 GPIO device supports 2 interrupts instead of one. > > * 1 for the lower 16 GPIOs. > * 1 for the upper 16 GPIOs. > > i.MX31 and i.MX25 only support 1 interrupt for the 32 GPIOs. > > > So an architectural question, is it the case that the IP always has > two outbound interrupt lines but MX31 and 25 always OR together on the > SoC level? Well, I am not part of Freescale so I just don't know anything about internal implementation details of this IP and its use inside the SOC. What we can say is that the dual interrupt version (i.MX6) is newer than the single one (i.MX31, i.MX25). So my guess is that this is an evolution of the IP and the goal was to make GPIO interrupt handling easier, faster and prioritisable. > > If that is the case I think it would be cleaner to do on the board level. At the SOC level I was planning to set (or not) the new property and to assign 1 (or 2) IRQ to the device. Regards JC > > Regards, > Peter > > So we add a property to turn the behavior on when required. > > Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net > <mailto:jcd@tribudubois.net>> > --- > hw/gpio/imx_gpio.c | 12 ++++++++++-- > include/hw/gpio/imx_gpio.h | 3 ++- > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c > index 3170585..a6d7cab 100644 > --- a/hw/gpio/imx_gpio.c > +++ b/hw/gpio/imx_gpio.c > @@ -62,7 +62,12 @@ static const char *imx_gpio_reg_name(uint32_t reg) > > static void imx_gpio_update_int(IMXGPIOState *s) > { > - qemu_set_irq(s->irq, (s->isr & s->imr) ? 1 : 0); > + if (s->has_upper_pin_irq) { > + qemu_set_irq(s->irq[0], (s->isr & s->imr & 0x0000FFFF) ? > 1 : 0); > + qemu_set_irq(s->irq[1], (s->isr & s->imr & 0xFFFF0000) ? > 1 : 0); > + } else { > + qemu_set_irq(s->irq[0], (s->isr & s->imr) ? 1 : 0); > + } > } > > static void imx_gpio_set_int_line(IMXGPIOState *s, int line, > IMXGPIOLevel level) > @@ -282,6 +287,8 @@ static const VMStateDescription > vmstate_imx_gpio = { > > static Property imx_gpio_properties[] = { > DEFINE_PROP_BOOL("has-edge-sel", IMXGPIOState, has_edge_sel, > true), > + DEFINE_PROP_BOOL("has-upper-pin-irq", IMXGPIOState, > has_upper_pin_irq, > + false), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -311,7 +318,8 @@ static void imx_gpio_realize(DeviceState *dev, > Error **errp) > qdev_init_gpio_in(DEVICE(s), imx_gpio_set, IMX_GPIO_PIN_COUNT); > qdev_init_gpio_out(DEVICE(s), s->output, IMX_GPIO_PIN_COUNT); > > - sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); > + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[0]); > + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[1]); > sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); > } > > diff --git a/include/hw/gpio/imx_gpio.h b/include/hw/gpio/imx_gpio.h > index 517b261..b15a09f 100644 > --- a/include/hw/gpio/imx_gpio.h > +++ b/include/hw/gpio/imx_gpio.h > @@ -54,8 +54,9 @@ typedef struct IMXGPIOState { > uint32_t isr; > bool has_edge_sel; > uint32_t edge_sel; > + bool has_upper_pin_irq; > > - qemu_irq irq; > + qemu_irq irq[2]; > qemu_irq output[IMX_GPIO_PIN_COUNT]; > } IMXGPIOState; > > -- > 2.5.0 > >
On Wed, Oct 28, 2015 at 12:10 AM, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote: > Le 27/10/2015 23:41, Peter Crosthwaite a écrit : > > > > On Tue, Oct 27, 2015 at 3:32 PM, Jean-Christophe Dubois > <jcd@tribudubois.net> wrote: >> >> The i.MX6 GPIO device supports 2 interrupts instead of one. >> >> * 1 for the lower 16 GPIOs. >> * 1 for the upper 16 GPIOs. >> >> i.MX31 and i.MX25 only support 1 interrupt for the 32 GPIOs. >> > > So an architectural question, is it the case that the IP always has two > outbound interrupt lines but MX31 and 25 always OR together on the SoC > level? > > > Well, I am not part of Freescale so I just don't know anything about > internal implementation details of this IP and its use inside the SOC. > > What we can say is that the dual interrupt version (i.MX6) is newer than the > single one (i.MX31, i.MX25). So my guess is that this is an evolution of the > IP and the goal was to make GPIO interrupt handling easier, faster and > prioritisable. > > > > If that is the case I think it would be cleaner to do on the board level. > > > At the SOC level I was planning to set (or not) the new property and to > assign 1 (or 2) IRQ to the device. > Ok, there were no architecture clues in the TRM so this approach wins. Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> One possibility is to do some lightweight subclasses for the imx25/31 and imx6 versions, to avoid SoCs having to manipulate number properties for fixed IPs. See hw/usb/hcd-ehci-sysbus.c for some examples of SoCs tweaking a peripheral to get their own version of it as a customized device type. Regards, Peter > Regards > > JC > > > > Regards, > Peter > > >> >> So we add a property to turn the behavior on when required. >> >> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> >> uint32_t isr; >> bool has_edge_sel; >> uint32_t edge_sel; >> + bool has_upper_pin_irq; >> >> - qemu_irq irq; >> + qemu_irq irq[2]; >> qemu_irq output[IMX_GPIO_PIN_COUNT]; >> } IMXGPIOState; >> >> -- >> 2.5.0 >> > >
diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c index 3170585..a6d7cab 100644 --- a/hw/gpio/imx_gpio.c +++ b/hw/gpio/imx_gpio.c @@ -62,7 +62,12 @@ static const char *imx_gpio_reg_name(uint32_t reg) static void imx_gpio_update_int(IMXGPIOState *s) { - qemu_set_irq(s->irq, (s->isr & s->imr) ? 1 : 0); + if (s->has_upper_pin_irq) { + qemu_set_irq(s->irq[0], (s->isr & s->imr & 0x0000FFFF) ? 1 : 0); + qemu_set_irq(s->irq[1], (s->isr & s->imr & 0xFFFF0000) ? 1 : 0); + } else { + qemu_set_irq(s->irq[0], (s->isr & s->imr) ? 1 : 0); + } } static void imx_gpio_set_int_line(IMXGPIOState *s, int line, IMXGPIOLevel level) @@ -282,6 +287,8 @@ static const VMStateDescription vmstate_imx_gpio = { static Property imx_gpio_properties[] = { DEFINE_PROP_BOOL("has-edge-sel", IMXGPIOState, has_edge_sel, true), + DEFINE_PROP_BOOL("has-upper-pin-irq", IMXGPIOState, has_upper_pin_irq, + false), DEFINE_PROP_END_OF_LIST(), }; @@ -311,7 +318,8 @@ static void imx_gpio_realize(DeviceState *dev, Error **errp) qdev_init_gpio_in(DEVICE(s), imx_gpio_set, IMX_GPIO_PIN_COUNT); qdev_init_gpio_out(DEVICE(s), s->output, IMX_GPIO_PIN_COUNT); - sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[0]); + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[1]); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); } diff --git a/include/hw/gpio/imx_gpio.h b/include/hw/gpio/imx_gpio.h index 517b261..b15a09f 100644 --- a/include/hw/gpio/imx_gpio.h +++ b/include/hw/gpio/imx_gpio.h @@ -54,8 +54,9 @@ typedef struct IMXGPIOState { uint32_t isr; bool has_edge_sel; uint32_t edge_sel; + bool has_upper_pin_irq; - qemu_irq irq; + qemu_irq irq[2]; qemu_irq output[IMX_GPIO_PIN_COUNT]; } IMXGPIOState;
The i.MX6 GPIO device supports 2 interrupts instead of one. * 1 for the lower 16 GPIOs. * 1 for the upper 16 GPIOs. i.MX31 and i.MX25 only support 1 interrupt for the 32 GPIOs. So we add a property to turn the behavior on when required. Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net> --- hw/gpio/imx_gpio.c | 12 ++++++++++-- include/hw/gpio/imx_gpio.h | 3 ++- 2 files changed, 12 insertions(+), 3 deletions(-)