DIO Library 1.1.0: C6472 - are dbells and LSU interrupts atomic?

Eddie,

I'd have to look at the code more closely to see exactly how the ISR is handling the read of the ICSR bits and the finally the clearing of the interrupts with the ICCR bits , but I can say that you won't ever lose an interrupt from the hardware perspective.  Typically, when you enter an ISR, it will read the ICSR (source interrupt bits) and determine which interrupts to service.  It will then service the interrupts appropriately and then finally clear only those bits and set the interrupt pacing register.  Any new pending interrupt that may have occurred while in the ISR, will fire a new interrupt to the DSP as soon as the pacing allows, thus re-entering the ISR.  Also remember, a specific dbell ICSR bit can not be set by the peripheral until the it has been cleared by the software ISR. Each LSU has its own ICSR bit and each LSU only sets one interrupt at a time.  So basically, the hardware will not lose and interrupt.

Regards,

Travis

Hi Travis

Below is the DIO lib func called by the DIO lib dbell ISR. I can see where you say it has to clear the ICSR bits. But what I wonder is what happens when this sequence happens:

1) we get a doorbell message with the info bits set that cause a bit in doorbell ICSR0 to get set. The DSP fires the interrupt, it starts the ISR,

2) Before the ISR calls the function below, another dbell message arrives with info bits set for ICSR2

3) The function below gets called by the ISR.

When the loop below executes, will the ICSR2 register bits be set? Or will they be set when the ISR returns and fires again due to the second dbell message?

Cheers

DIO_getPendingDbell (
  RIO_SrioHandle srioHandle,
  RIO_DbellStatus* hPendingdBell
)
{
    CSL_SrioRegs *srioRegs = (CSL_SrioRegs*)(srioHandle->hCslObj->regs);
    Uint32 i;
   
    for (i=0;i<RIO_DBELL_SUBGROUP_SIZE_MAX;i++) {
        // Retrieve and store the pending doorbells
        hPendingdBell[i] = (Uint16)(srioRegs->DOORBELL_INTR[i].DOORBELL_ICSR);
        // Clear the pending doorbells
        srioRegs->DOORBELL_INTR[i].DOORBELL_ICCR = hPendingdBell[i];
    }

}

Eddie,

The function you point out, does look at all 4 of the Doorbell ICSR registers, but whether the ICSR2 bit is set in your example before the ISR actually reads the ISCR registers is really a race condition.  Let me give you an example that will help answer your question and point out a needed ISR code change that I see:

1) 2 bits are set in ICSR0 and 2 bits are set in ICSR2 (four previous doorbells arrived)

2)  the ICRR registers routes one ICSR0 interrupt bit to core 0 and one interrupt bit to core 2, and both ICSR2 interrupt bits to core 2

3) The interrupt pacing timer expires for the INTDST associated with core 0, so an interrupt is fired to the core 0.

4) Core 0 enters the ISR and the for loop, it takes a snapshot of all the ICSR registers (it sees all set bits)

5) another doorbell comes in and sets a third bit in ICSR0 that happens to be routed to core 0

6) ISR does what ever processing it needs to do - BASED ON THE SNAPSHOT OF THE ICSR REGISTERS ABOVE

7) The final steps in the ISR are to clear the ICSR bits by writing to the ICCR, then write the pacing register.  The key here is that it should only clear the ISCR bits that a) belong to core 0, and b) that were set when the snapshot was taken.

In this sequence above, core 0 will only process/clear one ISCR bit.  But if the timing worked out such that steps 4 and 5 are reversed, then core 0 will process/clear two ISCR bits.  Hopefully that answers your question.  Now, unfortunately, it looks like the ISR code snippet writes the whole ICCR register when clearing the interrupts.  So it would actually clear all ICSR bits that are set when the snapshot is taken.  This is wrong.  This would cause the three core 2 interrupt bits to be cleared, and again depending on timing could even clear the second interrupt bit for core 0 erroneously.  This needs to be fixed to manipulate the ICCR registers bit by bit.

Regards,

Travis

Hi Travis

I want to thank you for this discussion. It really helps to flush out the DIO interrupt issues that could adversely affect out logic.

Fortunately I am only routine the dbells to one core. But hopefully others see the gotcha you have presented.

Assuming in step 7 that core 0 does not clear the third bit in ICSR0 that was intended for core 0 (i.e. the snapshot read occurs before the dbell occurs), would step 8 be;

8) ISR returns and fires the ISR for core0

Also, this statement confuses me.

tscheck said:

and again depending on timing could even clear the second interrupt bit for core 0 erroneously.

If the bit is not set when the ISCR snapshot is read by the ISR, how could the ISR clear the core0 bit 3?

Cheers

Assuming in step 7 that core 0 does not clear the third bit in ICSR0 that was intended for core 0 (i.e. the snapshot read occurs before the dbell occurs), would step 8 be;

8) ISR returns and fires the ISR for core0

12/14 - Yes, after step 7, as soon as the pacing register would expire again, a new interrupt would be fired to the cpu, which would then re-enter the ISR again.

Also, this statement confuses me.

tscheck said:

and again depending on timing could even clear the second interrupt bit for core 0 erroneously.

If the bit is not set when the ISCR snapshot is read by the ISR, how could the ISR clear the core0 bit 3?

12/14 - What I was trying to highlight here is that if the ICCR clearing mechanism writes the whole register, not just the interrupt bits that were serviced in the ISR during the snapshot, then you run the risk of clearing interrupts that were not serviced.  So not only do you potentially clear the ICSR bits for a different core, but you could also clear an interrupt for the same core that came in after the snapshot.

Cheers

Hi Travis

Sorry about being so slow to grasp the issue you present.....

tscheck said:

So not only do you potentially clear the ICSR bits for a different core, but you could also clear an interrupt for the same core that came in after the snapshot.

According to sprue13 C6472 users guide, the ICCR register will only clear the interrupts for the bits set high when I write to the register.

So based on the users guide, here is what I understand happens.....

1) the dbell isr fires and in the ISR I read a snapshot of ICSR. Its value is say 0x2 for ICSR (for example)

2) and then while still in the ISR, another dbell occurs that causes b0 to be set,

3) I write 0x2 back to ICCR0

4) the HW clears b1 of ICSR0

5) the HW leaves b0 of ICSR0 set because it was not set in the snapshot was read, and hence not written to ICCR0

6) The ISR returns and fires again (assuming the interrupt cnt is zero)

7) The ISR reads the ICSR0 reg. Its value is 0x1.

Are you saying that the writing of ICCR clears all ICSR bits, or is the issue that the snapshot reads will be clears before lower priority tasks or swi's can read the snapshot and act accordingly.

Cheers

Your understanding is correct.  I agree with all your steps, but my interpretation from the below snippet is that the i index was referring to the register number, not the actual bits in the registers, so all the ICCR bits are written too.  So I interpret that in step 3, the write is actually 0xFF back to ICCR0.  If this is not the case, then maybe there isn't a problem, but I don't see any reference to only writing back the set bits from the initial snapshot.

// Clear the pending doorbells
        srioRegs->DOORBELL_INTR[i].DOORBELL_ICCR = hPendingdBell[i];

Let me know what you see when running it.

Regards,

Travis

Hi Travis

I'm not understanding something here. Specifically where the 0xFF is coming from.

Let say I take the code

        hPendingdBell[i] = (Uint16)(srioRegs->DOORBELL_INTR[i].DOORBELL_ICSR);
        // Clear the pending doorbells
        srioRegs->DOORBELL_INTR[i].DOORBELL_ICCR = hPendingdBell[i];

and changed it to be (removed hPendingBell[])

        srioRegs->DOORBELL_INTR[i].DOORBELL_ICCR = (Uint16)(srioRegs->DOORBELL_INTR[i].DOORBELL_ICSR);
       

Would this equate to only the bits set by the received dbell info bits being written back to the ICCR register? Hence using the intermediate variable hPendingdBell in the original code snippet would do the same wouldn't it?

Sorry for the confusion and hence request for clarification.....

Cheers

Eddie,

You are correct, I mis-interpreted the code.  The ISR is not writing 0xFF to the ICCR, it is only clearing the bits that were high when the snapshot of the ICSR happened.  Therefore, there is no concern about a single core clearing an interrupt that wasn't serviced. 

So the only concern is the one I mentioned above where multiple cores may be servicing different ICSR bits from the same register.  In this case, the user must mask out the ICSR bits that it doesn't service (belonging to the other core), so that it doesn't clear those unintentionally.  This is not comprehended in the DIOLib example ISR.

Sorry for the confusion, but I think we got you straightened out.

Regards,

Travis