[PATCH v1 1/1] gpiolib/grgpio: Add support for newer grgpio features

Jan.Sommer at dlr.de Jan.Sommer at dlr.de
Thu Sep 9 08:12:38 UTC 2021


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-bounces at rtems.org> On Behalf Of Jan.Sommer at dlr.de
Sent: Friday, August 27, 2021 2:36 PM
To: daniel at gaisler.com; devel at rtems.org
Cc: software at 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 <daniel at gaisler.com<mailto:daniel at gaisler.com>>
Sent: Thursday, August 26, 2021 3:11 PM
To: Sommer, Jan <Jan.Sommer at dlr.de<mailto:Jan.Sommer at dlr.de>>; devel at rtems.org<mailto:devel at rtems.org>
Cc: software at gaisler.com<mailto:software at 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.Sommer at dlr.de<mailto:Jan.Sommer at 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 at rtems.org<mailto:devel at rtems.org>

Cc: software at gaisler.com<mailto:software at gaisler.com>; Sommer, Jan <Jan.Sommer at dlr.de><mailto:Jan.Sommer at 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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210909/25da903a/attachment-0001.html>


More information about the devel mailing list