PRU to ARM interrupts are fired twice

[cmicali]

Note: This is from http://e2e.ti.com/support/arm/sitara_arm/f/791/t/239735.aspx

Hi,

Maybe someone has seen this before, but I could not find it in any post. As far as I can see there is a bug in prussdrv.c in the git repository: https://github.com/beagleboard/am335x_pru_package.git

The problem is that when an interrupt is sent from a PRU to the ARM processor (using a beaglebone -> AM3359), the interrupt is first disabled in kernel-space, in uio_pruss.c/pruss_handler(). After the interrupts is delivered as an event to the user-space lib (prussdrv.c) the interrupt is enabled again, before it is cleared. I have in the attached patch moved the re-enabling of the interrupt from prussdrv_pru_wait_event() to prussdrv_pru_clear_event() right after the interrupt is cleared. This prevents duplicate interrupts. The implemetation in the patch is such that there is a linear search in the struct used to configure interrupts, to find the host number from the system event number. Maybe this is not good from a performance perspective ? Comments on the patch are appreciated from someone more familiar with the pruss driver/lib.

Attached is also a demo application that reproduceses the fault. When running strace on the PRU_demo application, one can clearly see that there are two returns from the read() call in prussdrv_pru_wait_event() for each interrupt sent from the PRU, if the patch is not applied.

As said before, any comments appreciated.

br Håkan

patch: http://e2e.ti.com/cfs-file.ashx/__key/telligent-evolution-components-attachments/00-791-00-00-00-23-97-35/attachments.tar.gz

[lafras-h]

Hi,

I have a patch that is much simpler, that fixes the root cause. Hope this helps. Sorry this is inline :(

diff --git pru_sw/app_loader/include/prussdrv.h pru_sw/app_loader/include/prussdrv.h
index 90c489b..7528b60 100755
--- pru_sw/app_loader/include/prussdrv.h
+++ pru_sw/app_loader/include/prussdrv.h
@@ -140,7 +140,7 @@ extern "C" {

     int prussdrv_pru_send_event(unsigned int eventnum);

-    int prussdrv_pru_clear_event(unsigned int eventnum);
+    int prussdrv_pru_clear_event(unsigned int eventnum, unsigned int pru_evtout_num);

     int prussdrv_pru_send_wait_clear_event(unsigned int send_eventnum,
                                            unsigned int pru_evtout_num,
diff --git pru_sw/app_loader/interface/prussdrv.c pru_sw/app_loader/interface/prussdrv.c
index 21e4ba4..8812a20 100755
--- pru_sw/app_loader/interface/prussdrv.c
+++ pru_sw/app_loader/interface/prussdrv.c
@@ -421,20 +421,20 @@ int prussdrv_pru_send_event(unsigned int eventnum)
 int prussdrv_pru_wait_event(unsigned int pru_evtout_num)
 {
     int event_count;
-    unsigned int *pruintc_io = (unsigned int *) prussdrv.intc_base;
     read(prussdrv.fd[pru_evtout_num], &event_count, sizeof(int));
-    pruintc_io[PRU_INTC_HIEISR_REG >> 2] = pru_evtout_num+2; 
     return 0;

 }

-int prussdrv_pru_clear_event(unsigned int eventnum)
+int prussdrv_pru_clear_event(unsigned int eventnum, unsigned int pru_evtout_num)
 {
     unsigned int *pruintc_io = (unsigned int *) prussdrv.intc_base;
     if (eventnum < 32)
         pruintc_io[PRU_INTC_SECR1_REG >> 2] = 1 << eventnum;
     else
         pruintc_io[PRU_INTC_SECR2_REG >> 2] = 1 << (eventnum - 32);
+
+    pruintc_io[PRU_INTC_HIEISR_REG >> 2] = pru_evtout_num+2;   
     return 0;
 }

@@ -444,7 +444,7 @@ int prussdrv_pru_send_wait_clear_event(unsigned int send_eventnum,
 {
     prussdrv_pru_send_event(send_eventnum);
     prussdrv_pru_wait_event(pru_evtout_num);
-    prussdrv_pru_clear_event(ack_eventnum);
+    prussdrv_pru_clear_event(ack_eventnum, pru_evtout_num);
     return 0;

 }

[olsonse]

Seems like several of us have fixed this! I'll also submit a pull request for my changes. (I also have some additions that add a python interface the to prussdrv library.

[olsonse]

Wondering if we need to create a new coordination point for our prussdrv contributions since jadonk (Kridner) isn't responding.

[jadonk]

If this issue is still outstanding, can you please provide a pull request to fix?