Hi Daniel,
Did you by any chance found the time to have another look at it or test it on
the GR712RC?
Best regards,
Jan
From: devel <devel-boun...@rtems.org> On Behalf Of jan.som...@dlr.de
Sent: Friday, August 27, 2021 2:36 PM
To: dan...@gaisler.com; devel@rtems.org
Cc: softw...@gaisler.com
Subject: RE: [PATCH v1 1/1] gpiolib/grgpio: Add support for newer grgpio
features
Hi Daniel,
As far as I understand the code the bus sets the “info.irq” to the pirq value
from the PnP information of the devices.
Then the “drvmgr_interrupt_register(priv->dev, irq, "grgpio", isr, arg) )“ will
register the isr with “irq” as an offset of pirq.
So, for devices with GENIRQ == 0, this should yield the behavior you describe
with one irq per GPIO line (with pirq as the first interrupt line).
For devices with GENIRQ == 1 a shared ISR for pirq is registered.
In our test setup (with grpio with GENIRQ == 1) the grgpio devices with
interrupts have info.irq set to pirq (e.g. 0x15 in our setup) and for the
grgpio without interrupt support it is set to 0.
I don’t have a GR712RC for testing, so I don’t know if it behaves differently.
Best regards,
Jan
From: Daniel Hellstrom <dan...@gaisler.com<mailto:dan...@gaisler.com>>
Sent: Thursday, August 26, 2021 3:11 PM
To: Sommer, Jan <jan.som...@dlr.de<mailto:jan.som...@dlr.de>>;
devel@rtems.org<mailto:devel@rtems.org>
Cc: softw...@gaisler.com<mailto:softw...@gaisler.com>
Subject: Re: [PATCH v1 1/1] gpiolib/grgpio: Add support for newer grgpio
features
Hi Jan,
I haven't tested the code, but it seems to be that the ordinary case when
GPIO[N] is connected to Interrupt[N] will not work? The fixup makes GPIO[N] pin
be associated with interrupt[0] (which is not available) and GPIO[1] with
Interrupt[1] and so one which is the case with U699/UT700/GR712RC. I believe it
is the bus-driver's role to initialize the device structure to indicate the
device's all interrupts (the first interrupt in the ambapp_bus case), and the
driver's role to select which interrupt of the available. I'm worrying that
leaving "info.irq = -1" will disable all interrupts from that GPIO core?
Kind Regards,
Daniel
On 2021-08-25 10:13, jan.som...@dlr.de<mailto:jan.som...@dlr.de> wrote:
Does anyone have any objections to this?
See also https://lists.rtems.org/pipermail/devel/2021-July/068086.html for the
cover letter.
Best regards,
Jan
-----Original Message-----
From: Sommer, Jan
Sent: Thursday, July 1, 2021 5:44 PM
To: devel@rtems.org<mailto:devel@rtems.org>
Cc: softw...@gaisler.com<mailto:softw...@gaisler.com>; Sommer, Jan
<jan.som...@dlr.de><mailto:jan.som...@dlr.de>
Subject: [PATCH v1 1/1] gpiolib/grgpio: Add support for newer grgpio
features
- Use proper typedef for isr (avoid warning in user application)
- Use set input enable register together with pin direction
- Support irqgen == 1 mode if present in capabilities register
---
bsps/include/grlib/gpiolib.h | 7 +++++--
bsps/include/grlib/grlib.h | 4 +++-
bsps/shared/grlib/drvmgr/ambapp_bus.c | 5 +----
bsps/shared/grlib/gpio/gpiolib.c | 2 +-
bsps/shared/grlib/gpio/grgpio.c | 22 ++++++++++++++++------
5 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/bsps/include/grlib/gpiolib.h b/bsps/include/grlib/gpiolib.h index
f82d4fa2c2..37ac140862 100644
--- a/bsps/include/grlib/gpiolib.h
+++ b/bsps/include/grlib/gpiolib.h
@@ -28,6 +28,9 @@ struct gpiolib_config { #define GPIOLIB_IRQ_POL_LOW
0 #define GPIOLIB_IRQ_POL_HIGH 1
+/* Interrupt Service Routine (ISR) */
+typedef void (*gpiolib_isr)(void *arg);
+
/* Libarary initialize function must be called befor any other */ extern int
gpiolib_initialize(void);
@@ -54,7 +57,7 @@ extern int gpiolib_irq_disable(void *handle); extern int
gpiolib_irq_mask(void *handle); extern int gpiolib_irq_unmask(void
*handle); extern int gpiolib_irq_force(void *handle); -extern int
gpiolib_irq_register(void *handle, void *func, void *arg);
+extern int gpiolib_irq_register(void *handle, gpiolib_isr func, void
+*arg);
/*** Driver Interface ***/
@@ -66,7 +69,7 @@ struct gpiolib_drv_ops {
int (*config)(void *handle, struct gpiolib_config *cfg);
int (*get)(void *handle, int *val);
int (*irq_opts)(void *handle, unsigned int options);
- int (*irq_register)(void *handle, void *func, void *arg);
+ int (*irq_register)(void *handle, gpiolib_isr func, void
*arg);
int (*open)(void *handle);
int (*set)(void *handle, int dir, int outval);
int (*show)(void *handle);
diff --git a/bsps/include/grlib/grlib.h b/bsps/include/grlib/grlib.h index
49d9999807..4aa3e9df4a 100644
--- a/bsps/include/grlib/grlib.h
+++ b/bsps/include/grlib/grlib.h
@@ -17,6 +17,7 @@
#define __GRLIB_H__
#include <stdbool.h>
+#include <bsp/utility.h>
#ifdef __cplusplus
extern "C" {
@@ -125,6 +126,7 @@ struct grgpio_regs {
volatile unsigned int iedge; /* 0x14 Interrupt edge register */
volatile unsigned int bypass; /* 0x18 Bypass register */
volatile unsigned int cap; /* 0x1C Capability register */
+#define GRGPIO_CAP_IRQGEN(reg) BSP_FLD32GET(reg, 8, 12)
volatile unsigned int irqmap[4]; /* 0x20 - 0x2C Interrupt map registers */
volatile unsigned int res_30; /* 0x30 Reserved */
volatile unsigned int res_34; /* 0x34 Reserved */
@@ -132,7 +134,7 @@ struct grgpio_regs {
volatile unsigned int res_3C; /* 0x3C Reserved */
volatile unsigned int iavail; /* 0x40 Interrupt available register */
volatile unsigned int iflag; /* 0x44 Interrupt flag register */
- volatile unsigned int res_48; /* 0x48 Reserved */
+ volatile unsigned int input_en; /* 0x48 Input enable (if present) */
volatile unsigned int pulse; /* 0x4C Pulse register */
volatile unsigned int res_50; /* 0x50 Reserved */
volatile unsigned int output_or; /* 0x54 I/O port output register,
logical-OR
*/
diff --git a/bsps/shared/grlib/drvmgr/ambapp_bus.c
b/bsps/shared/grlib/drvmgr/ambapp_bus.c
index 3c38fc16e0..0aed29224c 100644
--- a/bsps/shared/grlib/drvmgr/ambapp_bus.c
+++ b/bsps/shared/grlib/drvmgr/ambapp_bus.c
@@ -521,11 +521,8 @@ static int ambapp_dev_fixup(struct drvmgr_dev
*dev, struct amba_dev_info *pnp)
for(core = 0; core < subcores; core++)
drvmgr_dev_register(devs_to_register[core]);
return 1;
- } else if ( (pnp->info.device == GAISLER_GPIO) &&
- (pnp->info.vendor == VENDOR_GAISLER) ) {
- /* PIO[N] is connected to IRQ[N]. */
- pnp->info.irq = 0;
}
+
return 0;
}
diff --git a/bsps/shared/grlib/gpio/gpiolib.c
b/bsps/shared/grlib/gpio/gpiolib.c
index cf0038c5bb..0cb76402cc 100644
--- a/bsps/shared/grlib/gpio/gpiolib.c
+++ b/bsps/shared/grlib/gpio/gpiolib.c
@@ -201,7 +201,7 @@ int gpiolib_get(void *handle, int *inval) }
/*** IRQ Functions ***/
-int gpiolib_irq_register(void *handle, void *func, void *arg)
+int gpiolib_irq_register(void *handle, gpiolib_isr func, void *arg)
{
struct gpiolib_port *port = handle;
diff --git a/bsps/shared/grlib/gpio/grgpio.c b/bsps/shared/grlib/gpio/grgpio.c
index 05504ef020..5bce5f530a 100644
--- a/bsps/shared/grlib/gpio/grgpio.c
+++ b/bsps/shared/grlib/gpio/grgpio.c
@@ -229,6 +229,7 @@ static int grgpio_grpiolib_irq_opts(void *handle,
unsigned int options) {
struct grgpio_priv *priv;
int portnr;
+ int irq;
drvmgr_isr isr;
void *arg;
@@ -244,33 +245,41 @@ static int grgpio_grpiolib_irq_opts(void *handle,
unsigned int options)
isr = priv->isrs[portnr].isr;
arg = priv->isrs[portnr].arg;
+ /* For irqgen == 1, irq == pirq, i.e. no offset */
+ irq = 0;
+ if (GRGPIO_CAP_IRQGEN(priv->regs->cap) == 0) {
+ /* For irqgen == 0, irq == pirq + portnr */
+ irq += portnr;
+ }
+ /* FIXME: irqgen > 1 currently not supported */
+
if ( options & GPIOLIB_IRQ_DISABLE ) {
/* Disable interrupt at interrupt controller */
- if ( drvmgr_interrupt_unregister(priv->dev, portnr, isr, arg) ) {
+ if ( drvmgr_interrupt_unregister(priv->dev, irq, isr, arg) ) {
return -1;
}
}
if ( options & GPIOLIB_IRQ_CLEAR ) {
/* Clear interrupt at interrupt controller */
- if ( drvmgr_interrupt_clear(priv->dev, portnr) ) {
+ if ( drvmgr_interrupt_clear(priv->dev, irq) ) {
return -1;
}
}
if ( options & GPIOLIB_IRQ_ENABLE ) {
/* Enable interrupt at interrupt controller */
- if ( drvmgr_interrupt_register(priv->dev, portnr, "grgpio", isr,
arg) ) {
+ if ( drvmgr_interrupt_register(priv->dev, irq, "grgpio", isr,
arg) )
+{
return -1;
}
}
if ( options & GPIOLIB_IRQ_MASK ) {
/* Mask (disable) interrupt at interrupt controller */
- if ( drvmgr_interrupt_mask(priv->dev, portnr) ) {
+ if ( drvmgr_interrupt_mask(priv->dev, irq) ) {
return -1;
}
}
if ( options & GPIOLIB_IRQ_UNMASK ) {
/* Unmask (enable) interrupt at interrupt controller */
- if ( drvmgr_interrupt_unmask(priv->dev, portnr) ) {
+ if ( drvmgr_interrupt_unmask(priv->dev, irq) ) {
return -1;
}
}
@@ -278,7 +287,7 @@ static int grgpio_grpiolib_irq_opts(void *handle,
unsigned int options)
return 0;
}
-static int grgpio_grpiolib_irq_register(void *handle, void *func, void *arg)
+static int grgpio_grpiolib_irq_register(void *handle, gpiolib_isr func,
+void *arg)
{
struct grgpio_priv *priv;
int portnr;
@@ -313,6 +322,7 @@ static int grgpio_grpiolib_set(void *handle, int dir, int
outval)
/* Set Direction and Output */
mask = 1<<portnr;
priv->regs->dir = (priv->regs->dir & ~mask) | (dir ? mask : 0);
+ priv->regs->input_en = (priv->regs->input_en & ~mask) | (dir ? 0 :
+mask);
priv->regs->output = (priv->regs->output & ~mask) | (outval ? mask :
0);
return 0;
--
2.17.1
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
