hw/arm/bcm2835_peripherals.c | 2 + hw/gpio/bcm2835_gpio.c | 105 +++++++++++++++++++++++++++++++++ hw/intc/bcm2835_ic.c | 2 - include/hw/gpio/bcm2835_gpio.h | 12 ++++ 4 files changed, 119 insertions(+), 2 deletions(-)
The bcm2835 GPIOs now generate interrupts.
This modification enables QTEST clients to trigger interrupt-based
interfaces.
Signed-off-by: Davide Berardi <berardi.dav@gmail.com>
---
hw/arm/bcm2835_peripherals.c | 2 +
hw/gpio/bcm2835_gpio.c | 105 +++++++++++++++++++++++++++++++++
hw/intc/bcm2835_ic.c | 2 -
include/hw/gpio/bcm2835_gpio.h | 12 ++++
4 files changed, 119 insertions(+), 2 deletions(-)
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index dcff13433e..101c3e6fba 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -114,6 +114,8 @@ static void bcm2835_peripherals_init(Object *obj)
OBJECT(&s->sdhci.sdbus));
object_property_add_const_link(OBJECT(&s->gpio), "sdbus-sdhost",
OBJECT(&s->sdhost.sdbus));
+ object_property_add_const_link(OBJECT(&s->gpio), "ic",
+ OBJECT(&s->ic));
/* Mphi */
object_initialize_child(obj, "mphi", &s->mphi, TYPE_BCM2835_MPHI);
diff --git a/hw/gpio/bcm2835_gpio.c b/hw/gpio/bcm2835_gpio.c
index abdddbc67c..2952ebe1da 100644
--- a/hw/gpio/bcm2835_gpio.c
+++ b/hw/gpio/bcm2835_gpio.c
@@ -7,6 +7,9 @@
* Clement Deschamps <clement.deschamps@antfield.fr>
* Luc Michel <luc.michel@antfield.fr>
*
+ * GPIO Interrupt support
+ * Davide Berardi <berardi.dav@gmail.com>
+ *
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
*/
@@ -20,6 +23,7 @@
#include "migration/vmstate.h"
#include "hw/sd/sd.h"
#include "hw/gpio/bcm2835_gpio.h"
+#include "hw/intc/bcm2835_ic.h"
#include "hw/irq.h"
#define GPFSEL0 0x00
@@ -110,6 +114,29 @@ static int gpfsel_is_out(BCM2835GpioState *s, int index)
return 0;
}
+static inline int get_bit_2_u32(const uint32_t idx,
+ const uint32_t v1, const uint32_t v2)
+{
+ uint64_t v = v1 | ((uint64_t)v2) << 32;
+ return !!(v & (1 << idx));
+}
+
+static int ren_detect(BCM2835GpioState *s, int index)
+{
+ if (index >= 0 && index < 54) {
+ return get_bit_2_u32(index, s->ren0, s->ren1);
+ }
+ return 0;
+}
+
+static int fen_detect(BCM2835GpioState *s, int index)
+{
+ if (index >= 0 && index < 54) {
+ return get_bit_2_u32(index, s->fen0, s->fen1);
+ }
+ return 0;
+}
+
static void gpset(BCM2835GpioState *s,
uint32_t val, uint8_t start, uint8_t count, uint32_t *lev)
{
@@ -120,6 +147,20 @@ static void gpset(BCM2835GpioState *s,
for (i = 0; i < count; i++) {
if ((changes & cur) && (gpfsel_is_out(s, start + i))) {
qemu_set_irq(s->out[start + i], 1);
+ } else if ((changes & cur) && ren_detect(s, start + i)) {
+ /* If this is an input and must check rising edge */
+ int irqline = 0;
+ if (i > 27)
+ irqline = 1;
+ if (i > 45)
+ irqline = 2;
+
+ /* Set the bit in the events */
+ if (i < 32)
+ s->eds0 |= cur;
+ else
+ s->eds1 |= cur;
+ qemu_set_irq(s->irq[irqline], 1);
}
cur <<= 1;
}
@@ -137,6 +178,15 @@ static void gpclr(BCM2835GpioState *s,
for (i = 0; i < count; i++) {
if ((changes & cur) && (gpfsel_is_out(s, start + i))) {
qemu_set_irq(s->out[start + i], 0);
+ } else if ((changes & cur) && fen_detect(s, start + i)) {
+ /* If this is an input we must check falling edge */
+ int irqline = 0;
+ if (i > 27)
+ irqline = 1;
+ if (i > 45)
+ irqline = 2;
+
+ qemu_set_irq(s->irq[irqline], 1);
}
cur <<= 1;
}
@@ -144,6 +194,34 @@ static void gpclr(BCM2835GpioState *s,
*lev &= ~val;
}
+static int gpio_from_value(uint64_t value, int bank)
+{
+ int i;
+ for (i = 0 ; i < 32; ++i)
+ if (value & (1 << i))
+ return i + (32 * bank);
+ return 0;
+}
+
+static void eds_clear(BCM2835GpioState *s, uint64_t value, int bank)
+{
+ int gpio = 0;
+ int irqline = 0;
+ if (bank) {
+ s->eds0 &= ~value;
+ } else {
+ s->eds1 &= (~value & 0x3f);
+ }
+ gpio = gpio_from_value(value, bank);
+
+ if (gpio > 27)
+ irqline = 1;
+ if (gpio > 45)
+ irqline = 2;
+
+ qemu_set_irq(s->irq[irqline], 0);
+}
+
static uint64_t bcm2835_gpio_read(void *opaque, hwaddr offset,
unsigned size)
{
@@ -170,11 +248,17 @@ static uint64_t bcm2835_gpio_read(void *opaque, hwaddr offset,
case GPLEV1:
return s->lev1;
case GPEDS0:
+ return s->eds0;
case GPEDS1:
+ return s->eds1;
case GPREN0:
+ return s->ren0;
case GPREN1:
+ return s->ren1;
case GPFEN0:
+ return s->fen0;
case GPFEN1:
+ return s->fen1;
case GPHEN0:
case GPHEN1:
case GPLEN0:
@@ -228,11 +312,23 @@ static void bcm2835_gpio_write(void *opaque, hwaddr offset,
/* Read Only */
break;
case GPEDS0:
+ eds_clear(s, value, 0);
+ break;
case GPEDS1:
+ eds_clear(s, value, 1);
+ break;
case GPREN0:
+ s->ren0 = value;
+ break;
case GPREN1:
+ s->ren1 = value;
+ break;
case GPFEN0:
+ s->fen0 = value;
+ break;
case GPFEN1:
+ s->fen1 = value;
+ break;
case GPHEN0:
case GPHEN1:
case GPLEN0:
@@ -310,6 +406,7 @@ static void bcm2835_gpio_init(Object *obj)
static void bcm2835_gpio_realize(DeviceState *dev, Error **errp)
{
+ int i;
BCM2835GpioState *s = BCM2835_GPIO(dev);
Object *obj;
@@ -318,6 +415,14 @@ static void bcm2835_gpio_realize(DeviceState *dev, Error **errp)
obj = object_property_get_link(OBJECT(dev), "sdbus-sdhost", &error_abort);
s->sdbus_sdhost = SD_BUS(obj);
+
+ obj = object_property_get_link(OBJECT(dev), "ic", &error_abort);
+ s->ic = BCM2835_IC(obj);
+
+ for (i = 0 ; i < 3; ++i) {
+ s->irq[i] = qdev_get_gpio_in_named(DEVICE(obj),
+ BCM2835_IC_GPU_IRQ, i + 49);
+ }
}
static void bcm2835_gpio_class_init(ObjectClass *klass, void *data)
diff --git a/hw/intc/bcm2835_ic.c b/hw/intc/bcm2835_ic.c
index 9000d995e8..b381dc6603 100644
--- a/hw/intc/bcm2835_ic.c
+++ b/hw/intc/bcm2835_ic.c
@@ -57,7 +57,6 @@ static void bcm2835_ic_update(BCM2835ICState *s)
static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level)
{
BCM2835ICState *s = opaque;
-
assert(irq >= 0 && irq < 64);
trace_bcm2835_ic_set_gpu_irq(irq, level);
s->gpu_irq_level = deposit64(s->gpu_irq_level, irq, 1, level != 0);
@@ -67,7 +66,6 @@ static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level)
static void bcm2835_ic_set_arm_irq(void *opaque, int irq, int level)
{
BCM2835ICState *s = opaque;
-
assert(irq >= 0 && irq < 8);
trace_bcm2835_ic_set_cpu_irq(irq, level);
s->arm_irq_level = deposit32(s->arm_irq_level, irq, 1, level != 0);
diff --git a/include/hw/gpio/bcm2835_gpio.h b/include/hw/gpio/bcm2835_gpio.h
index 1c53a05090..cad3e987d3 100644
--- a/include/hw/gpio/bcm2835_gpio.h
+++ b/include/hw/gpio/bcm2835_gpio.h
@@ -7,6 +7,9 @@
* Clement Deschamps <clement.deschamps@antfield.fr>
* Luc Michel <luc.michel@antfield.fr>
*
+ * GPIO External support
+ * Davide Berardi <berardi.dav@gmail.com>
+ *
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
*/
@@ -17,6 +20,7 @@
#include "hw/sd/sd.h"
#include "hw/sysbus.h"
#include "qom/object.h"
+#include "hw/intc/bcm2835_ic.h"
struct BCM2835GpioState {
SysBusDevice parent_obj;
@@ -27,11 +31,19 @@ struct BCM2835GpioState {
SDBus sdbus;
SDBus *sdbus_sdhci;
SDBus *sdbus_sdhost;
+ BCM2835ICState *ic;
uint8_t fsel[54];
uint32_t lev0, lev1;
+ /* Event detection */
+ uint32_t eds0, eds1;
+ /* Edge selector */
+ uint32_t ren0, ren1;
+ uint32_t fen0, fen1;
+
uint8_t sd_fsel;
qemu_irq out[54];
+ qemu_irq irq[3];
};
#define TYPE_BCM2835_GPIO "bcm2835_gpio"
--
2.30.0
Hi Davide, On 2/9/21 12:02 PM, Davide Berardi wrote: > The bcm2835 GPIOs now generate interrupts. > This modification enables QTEST clients to trigger interrupt-based > interfaces. Thanks for your patch! Can you provide QTEST client example? Even better would be a qtest! > > Signed-off-by: Davide Berardi <berardi.dav@gmail.com> > --- > hw/arm/bcm2835_peripherals.c | 2 + > hw/gpio/bcm2835_gpio.c | 105 +++++++++++++++++++++++++++++++++ > hw/intc/bcm2835_ic.c | 2 - > include/hw/gpio/bcm2835_gpio.h | 12 ++++ > 4 files changed, 119 insertions(+), 2 deletions(-) ... I wonder how you generated your patch, it doesn't apply: Applying: Raspberry PI GPIO interrupt support error: patch failed: hw/arm/bcm2835_peripherals.c:114 error: hw/arm/bcm2835_peripherals.c: patch does not apply error: patch failed: hw/gpio/bcm2835_gpio.c:7 error: hw/gpio/bcm2835_gpio.c: patch does not apply error: patch failed: hw/intc/bcm2835_ic.c:57 error: hw/intc/bcm2835_ic.c: patch does not apply error: patch failed: include/hw/gpio/bcm2835_gpio.h:7 error: include/hw/gpio/bcm2835_gpio.h: patch does not apply Patch failed at 0001 Raspberry PI GPIO interrupt support You can find the guidelines here: https://wiki.qemu.org/Contribute/SubmitAPatch#Submitting_your_Patches > +static inline int get_bit_2_u32(const uint32_t idx, > + const uint32_t v1, const uint32_t v2) > +{ > + uint64_t v = v1 | ((uint64_t)v2) << 32; > + return !!(v & (1 << idx)); > +} > + > +static int ren_detect(BCM2835GpioState *s, int index) > +{ > + if (index >= 0 && index < 54) { > + return get_bit_2_u32(index, s->ren0, s->ren1); > + } > + return 0; > +} > + > +static int fen_detect(BCM2835GpioState *s, int index) > +{ > + if (index >= 0 && index < 54) { > + return get_bit_2_u32(index, s->fen0, s->fen1); > + } > + return 0; > +} I suppose you can simplify by using 'uint64_t fen' and ren, and the extract64() method from "qemu/bitops.h". > @@ -137,6 +178,15 @@ static void gpclr(BCM2835GpioState *s, > for (i = 0; i < count; i++) { > if ((changes & cur) && (gpfsel_is_out(s, start + i))) { > qemu_set_irq(s->out[start + i], 0); > + } else if ((changes & cur) && fen_detect(s, start + i)) { > + /* If this is an input we must check falling edge */ > + int irqline = 0; > + if (i > 27) > + irqline = 1; > + if (i > 45) > + irqline = 2; Please respect QEMU's CODING_STYLE.rst. Matter of taste, this seems easier to follow: int irqline; if (i > 45) { irqline = 2; } else if > 27 irqline = 1; } else { irqline = 0; } > + > + qemu_set_irq(s->irq[irqline], 1); > } > cur <<= 1; > } > @@ -144,6 +194,34 @@ static void gpclr(BCM2835GpioState *s, > *lev &= ~val; > } > +static int gpio_from_value(uint64_t value, int bank) > +{ > + int i; > + for (i = 0 ; i < 32; ++i) > + if (value & (1 << i)) Please use extract32(). > + return i + (32 * bank); > + return 0; > +} > + > +static void eds_clear(BCM2835GpioState *s, uint64_t value, int bank) > +{ > + int gpio = 0; > + int irqline = 0; > + if (bank) { > + s->eds0 &= ~value; > + } else { > + s->eds1 &= (~value & 0x3f); > + } Similarly, using 'uint64_t eds' should simplify this. > + gpio = gpio_from_value(value, bank); > + > + if (gpio > 27) > + irqline = 1; > + if (gpio > 45) > + irqline = 2; > + > + qemu_set_irq(s->irq[irqline], 0); > +} > + > static uint64_t bcm2835_gpio_read(void *opaque, hwaddr offset, > unsigned size) > { > @@ -170,11 +248,17 @@ static uint64_t bcm2835_gpio_read(void *opaque, > hwaddr offset, > case GPLEV1: > return s->lev1; > case GPEDS0: > + return s->eds0; > case GPEDS1: > + return s->eds1; Using 'uint64_t eds' this becomes: case GPEDS0: case GPEDS1: return extract64(s->eds, offset == GPEDS1 ? 0 : 32, 32); > @@ -318,6 +415,14 @@ static void bcm2835_gpio_realize(DeviceState > *dev, Error **errp) > obj = object_property_get_link(OBJECT(dev), "sdbus-sdhost", > &error_abort); > s->sdbus_sdhost = SD_BUS(obj); > + > + obj = object_property_get_link(OBJECT(dev), "ic", &error_abort); > + s->ic = BCM2835_IC(obj); > + > + for (i = 0 ; i < 3; ++i) { Please replace this magic 3 by a definition, maybe BCM2835_EXTERNAL_IRQ_COUNT? > + s->irq[i] = qdev_get_gpio_in_named(DEVICE(obj), > + BCM2835_IC_GPU_IRQ, i + 49); > + } > } > static void bcm2835_gpio_class_init(ObjectClass *klass, void *data) > diff --git a/hw/intc/bcm2835_ic.c b/hw/intc/bcm2835_ic.c > index 9000d995e8..b381dc6603 100644 > --- a/hw/intc/bcm2835_ic.c > +++ b/hw/intc/bcm2835_ic.c > @@ -57,7 +57,6 @@ static void bcm2835_ic_update(BCM2835ICState *s) > static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level) > { > BCM2835ICState *s = opaque; > - Spurious change. > assert(irq >= 0 && irq < 64); > trace_bcm2835_ic_set_gpu_irq(irq, level); > s->gpu_irq_level = deposit64(s->gpu_irq_level, irq, 1, level != 0); > @@ -67,7 +66,6 @@ static void bcm2835_ic_set_gpu_irq(void *opaque, int > irq, int level) > static void bcm2835_ic_set_arm_irq(void *opaque, int irq, int level) > { > BCM2835ICState *s = opaque; > - Ditto. > assert(irq >= 0 && irq < 8); > trace_bcm2835_ic_set_cpu_irq(irq, level); > s->arm_irq_level = deposit32(s->arm_irq_level, irq, 1, level != 0); > diff --git a/include/hw/gpio/bcm2835_gpio.h > b/include/hw/gpio/bcm2835_gpio.h > index 1c53a05090..cad3e987d3 100644 > --- a/include/hw/gpio/bcm2835_gpio.h > +++ b/include/hw/gpio/bcm2835_gpio.h > @@ -7,6 +7,9 @@ > * Clement Deschamps <clement.deschamps@antfield.fr> > * Luc Michel <luc.michel@antfield.fr> > * > + * GPIO External support > + * Davide Berardi <berardi.dav@gmail.com> > + * > * This work is licensed under the terms of the GNU GPL, version 2 or > later. > * See the COPYING file in the top-level directory. > */ > @@ -17,6 +20,7 @@ > #include "hw/sd/sd.h" > #include "hw/sysbus.h" > #include "qom/object.h" > +#include "hw/intc/bcm2835_ic.h" Here would go: #define BCM2835_EXTERNAL_IRQ_COUNT 3 > struct BCM2835GpioState { > SysBusDevice parent_obj; > @@ -27,11 +31,19 @@ struct BCM2835GpioState { > SDBus sdbus; > SDBus *sdbus_sdhci; > SDBus *sdbus_sdhost; > + BCM2835ICState *ic; > uint8_t fsel[54]; > uint32_t lev0, lev1; > + /* Event detection */ > + uint32_t eds0, eds1; > + /* Edge selector */ > + uint32_t ren0, ren1; > + uint32_t fen0, fen1; > + > uint8_t sd_fsel; > qemu_irq out[54]; > + qemu_irq irq[3]; and here 3 -> BCM2835_EXTERNAL_IRQ_COUNT. > }; > #define TYPE_BCM2835_GPIO "bcm2835_gpio" Regards, Phil.
Dear Philippe, thank you for your kind response. I will send a new version for the patch with the modifications you've highlighted, but first I've some question on the best way to implement them. On Tue, Feb 09, 2021 at 12:30:32PM +0100, Philippe Mathieu-Daudé wrote: >Hi Davide, >On 2/9/21 12:02 PM, Davide Berardi wrote: >> The bcm2835 GPIOs now generate interrupts. >> This modification enables QTEST clients to trigger interrupt-based >> interfaces. > >Thanks for your patch! > >Can you provide QTEST client example? Even better would be a qtest! > The minimal client I've developed can be found at https://github.com/berdav/qemu-rpi-gpio As I have introduced, I'm building a qtest but I've found a problem: In my test, I want to check if the IRQs of the named GPIOs /machine/soc/peripherals/ic/gpu-irq are correctly handled by the controller. I was thinking to use irq_intercept_in to get the status of the IRQ lines using something similar to the following: qtest_irq_intercept_in(s, "/machine/soc/peripherals/ic"); And then check it using qtest_get_irq(s, 49 + irqline); But it isn't triggering. Looking in the source of qtest I've found this check in the qtest_irq_intercept_in function: QLIST_FOREACH(ngl, &dev->gpios, node) { /* We don't support intercept of named GPIOs yet */ if (ngl->name) { continue; } I've removed the if and the qtests seems to work just fine. Is there particular caveats or details to implement this part of the framework? I can send separated patches if you have some details on the desiderata of the support, or maybe start a new thread with this question. Otherwise, I can remove the part of my qtest to not check the IRQs, but I don't know if it would be the best choice. > >I wonder how you generated your patch, it doesn't apply: > >Applying: Raspberry PI GPIO interrupt support >error: patch failed: hw/arm/bcm2835_peripherals.c:114 >error: hw/arm/bcm2835_peripherals.c: patch does not apply >error: patch failed: hw/gpio/bcm2835_gpio.c:7 >error: hw/gpio/bcm2835_gpio.c: patch does not apply >error: patch failed: hw/intc/bcm2835_ic.c:57 >error: hw/intc/bcm2835_ic.c: patch does not apply >error: patch failed: include/hw/gpio/bcm2835_gpio.h:7 >error: include/hw/gpio/bcm2835_gpio.h: patch does not apply >Patch failed at 0001 Raspberry PI GPIO interrupt support > >You can find the guidelines here: >https://wiki.qemu.org/Contribute/SubmitAPatch#Submitting_your_Patches > I have just created the patch using git format patch -s , it does not give me any error applying it over the master branch on https://github.com/qemu/QEMU . I will take more care and apply it on the master branch of https://git.qemu.org/git/qemu.git/ , thank you. > [... cut ...] > >Regards, > >Phil. I'm adapting the patch using the Coding style script now. I've also introduced a new costant for the number of GPIOs. Thank you for your time, D.
© 2016 - 2024 Red Hat, Inc.