adafruit / seesaw

I2C friend to expand capabilities of other chips.
Other
76 stars 34 forks source link

NVIC_SetPriority and IRQ #35

Open wallarug opened 4 years ago

wallarug commented 4 years ago

Hi there,

I was not sure where to turn to for assistance on this one. I am trying to make the SAMD51 work with SeeSaw. I have noticed some significant differences with how the SERCOM IRQ works. I cannot find any documentation about this in the datasheet or on the internet.

The best I have been able to do is look at the samd arduino core to see how they do it, however, it is completely different to how seesaw is set up.

The core problem is that the SAMD51 has 4 different IRQ lines for each SERCOM interface. So instead of passing a single IRQ 'object' to the function NVIC_SetPriority I am at a loss for how to proceed.

The file I am looking at from SeeSaw is source/bsp.cpp lines 166 onwards.

Is anyone able to point me towards somewhere I can learn how these IRQ lines work or explain how I should proceed for SAMD51 support?

Thanks in advance.

Fork is here: https://github.com/wallarug/seesaw on samd51support branch.

tannewt commented 4 years ago

Do you know which SERCOM interrupts are used by Seesaw? They get split over the four IRQs so that they can have different priorities.

wallarug commented 4 years ago

Hi @tannewt ,

I do not quite understand your question. Could you elaborate on how the priorities work? I understand where the number four comes from - there is 0 ... 3 for the IRQ lines. I just do not know which ones are allocated to what functions or how it relates to the single IRQ system in SAMD21.

This is the section I have been focusing on. File

void QF::onStartup(void) {

    // assigning all priority bits for preemption-prio. and none to sub-prio.
    //NVIC_SetPriorityGrouping(0U);

    NVIC_SetPriority(PendSV_IRQn, 0xFF);
    SysTick_Config(SystemCoreClock / BSP_TICKS_PER_SEC);
    NVIC_SetPriority(SysTick_IRQn, SYSTICK_PRIO);
#if CONFIG_I2C_SLAVE
    NVIC_SetPriority(CONFIG_I2C_SLAVE_IRQn, I2C_SLAVE_ISR_PRIO);
#endif

#if CONFIG_SPI_SLAVE
    NVIC_SetPriority(CONFIG_SPI_SLAVE_IRQn, SPI_SLAVE_ISR_PRIO);
#endif

#if defined(SAMD21)
    NVIC_SetPriority(USB_IRQn, USB_ISR_PRIO);
#endif
    //NVIC_SetPriority(NVMCTRL_IRQn, NVMCTRL_ISR_PRIO);

#if defined(SERCOM0)
    NVIC_SetPriority(SERCOM0_IRQn, SERCOM_ISR_PRIO);
#endif

#if defined(SERCOM1)
    NVIC_SetPriority(SERCOM1_IRQn, SERCOM_ISR_PRIO);
#endif

#if defined(SERCOM2)
    NVIC_SetPriority(SERCOM2_IRQn, SERCOM_ISR_PRIO);
#endif

#if defined(SERCOM5)
    NVIC_SetPriority(SERCOM5_IRQn, SERCOM_ISR_PRIO);
#endif
tannewt commented 4 years ago

On the SAMD21 all of the peripheral interrupts defined in INTFLAG get ORed to produce the single IRQn in the NVIC. On the SAMD they are split into four groups. Section 10.2.2 of the data sheet documents what each is. Unfortunately, it's not that helpful for SERCOMs: Screen Shot 2020-03-13 at 10 50 28 AM

The (1) designation says that the number there is the bit position in INTFLAG. It can't be more specific because the SERCOM can be multiple things. For I2C it is this from 36.10.7: Screen Shot 2020-03-13 at 10 54 17 AM

I'm not sure which interrupts are actually used by Seesaw. (Look for use of the INTENSET.) You could just set the priority for all four to the same level.

wallarug commented 4 years ago

Hi @tannewt thank you for your reply.

Based on what you are saying it would have to look something like this...

#if defined(SERCOM0)
    NVIC_SetPriority(SERCOM0_0_IRQn, SERCOM_ISR_PRIO);
        NVIC_SetPriority(SERCOM0_1_IRQn, SERCOM_ISR_PRIO);
        NVIC_SetPriority(SERCOM0_2_IRQn, SERCOM_ISR_PRIO);
        NVIC_SetPriority(SERCOM0_3_IRQn, SERCOM_ISR_PRIO);
#endif

...for each time the SERCOM IRQ is referenced in the SAMD21 equivalent?

My main concern when I first raised this is that setting them all up the same way will cause some issues. I guess I just need to know if the NVIC_SetPriority function works to be able to get past this or if this has any dependencies further on.

I went to have a read of section 10.2.2 of the D21 and D51 datasheet to try and understand this a bit better. They look similar based on the description given, however, I do not understand how the NVIC lines all work between multiple sercom instances? Why does each SERCOM have 0 - 7 associated with them - what is source? I am guessing this could refer to each bit in the INTFLAG register based on your comments above. However, this means in most cases the flags are all in unique 'groups' that you refer to. Most values sit in the 3:0 range.

I also went through all the SERCOM register tables to try and make sense of how the interrupts work between the D21 and D51. I picked USART to compare (as this was the only interface I could find that referenced INTENSET in SeeSaw). Both register tables for INTFLAG are the same. This is the same for I2C as well. My understanding of how the NVIC was not helped by this process 😕 .

Of all the things I have changed so far in SeeSaw for the SAMD51 - there has been a direct equivalent (sometimes in a different register). This does not equate 😞 .

This just has not clicked with me yet.

tannewt commented 4 years ago

That looks right to me.

I'm not sure what you mean by source.

Each sercom has four interrupt lines connected to the NVIC. INTENSET determines whether the interrupt signals propagate out of the SERCOM. The state is available by reading INTFLAG and cleared by writing. The NVIC lives next to the CPU and it's job is to interrupt the CPU. It keeps track of the currently running interrupt handler and only interrupts the CPU if the incoming IRQ is higher priority. If the priority levels are equivalent it'll be run after the current interrupt handler is finished. The same is true for lower priority interrupts.

In the SAMD21 all of the bits in INTFLAG are ORed together so that only one is connected to the NVIC. In the SAMD51 they are split over four lines.

Hope that helps.

wallarug commented 4 years ago

Hey @tannewt thanks again for an informative response.

I think I understand the role of the NVIC now and how it functions. The Priorities are set with that function above NVIC_SetPriority and that's that.

The bit I am now confused/clarifying about is the 0 to 3 numbers for the IRQ objects and how they relate to the Handlers. I can set them all the same, but then does that mean I need to duplicate the Handlers as well??

Source is the middle column... image

in this case it refers to:

The integer number specified in the source refers to the respective bit position in the INTFLAG register of respective peripheral.

I'm still slightly confused which INTFLAG register it is referring to (which part's INTFLAG?). Maybe a bit of an explanation around this would help my understanding a bit better. This is the SERCOM - UART register.

image

I want to understand so when I get around to the next bit (doing the Handlers) so it marries up correctly.

My current understanding is the below (combined) table:

SAMD51 - USART SERCOM Peripheral Source Bit Pos. INTFLAG INTFLAG name NVIC Line Software Ref
SERCOM0 0 0 DRE 46 SERCOM0_0_IRQ
SERCOM0 1 1 TXC 47 SERCOM0_1_IRQ
SERCOM0 2 2 RXC 48 SERCOM0_2_IRQ
SERCOM0 3 3 RXS 49 SERCOM0_3_IRQ
SERCOM0 4 4 CTSIC 49 SERCOM0_3_IRQ
SERCOM0 5 5 RXBRK 49 SERCOM0_3_IRQ
SERCOM0 6 6 49 SERCOM0_3_IRQ
SERCOM0 7 7 ERROR 49 SERCOM0_3_IRQ

Where the unique NVIC lines need different Handlers based on the number 0 to 3. 46 is 0, 47 is 1, 48 is 2 and 49 is 3 (with five bits combined into one Handler). I can see the correlation between the 0 to 3 to 46 to 49 in the software.

lib\samd51\samd51a\include\samd51g19a.h

SERCOM0_0_IRQn           = 46, /**< 46 SAMD51G19A Serial Communication Interface 0 (SERCOM0) IRQ 0 */
  SERCOM0_1_IRQn           = 47, /**< 47 SAMD51G19A Serial Communication Interface 0 (SERCOM0) IRQ 1 */
  SERCOM0_2_IRQn           = 48, /**< 48 SAMD51G19A Serial Communication Interface 0 (SERCOM0) IRQ 2 */
  SERCOM0_3_IRQn           = 49, /**< 49 SAMD51G19A Serial Communication Interface 0 (SERCOM0) IRQ 3 */

Now I am just guessing that if I want to use all the functions of the SERCOM interface (Data Register Empty, Transmit Complete, Receive Complete, etc) then I need respective Handlers setup.

lib\samd51\samd51a\include\samd51g19a.h

void SERCOM0_0_Handler           ( void );
void SERCOM0_1_Handler           ( void );
void SERCOM0_2_Handler           ( void );
void SERCOM0_3_Handler           ( void );

I don't know if have correctly understood this yet. Please let me know if I have missed anything.

Thanks again!

wallarug commented 4 years ago

This is the IRQ Handler for SERCOM UART in Arduino/Adafruit for all SAMD processors:

arduino...\cores\Uart.cpp

void Uart::IrqHandler()
{
  if (sercom->isFrameErrorUART()) {
    // frame error, next byte is invalid so read and discard it
    sercom->readDataUART();

    sercom->clearFrameErrorUART();
  }

  if (sercom->availableDataUART()) {
    rxBuffer.store_char(sercom->readDataUART());

    if (uc_pinRTS != NO_RTS_PIN) {
      // RX buffer space is below the threshold, de-assert RTS
      if (rxBuffer.availableForStore() < RTS_RX_THRESHOLD) {
        *pul_outsetRTS = ul_pinMaskRTS;
      }
    }
  }

  if (sercom->isDataRegisterEmptyUART()) {
    if (txBuffer.available()) {
      uint8_t data = txBuffer.read_char();

      sercom->writeDataUART(data);
    } else {
      sercom->disableDataRegisterEmptyInterruptUART();
    }
  }

  if (sercom->isUARTError()) {
    sercom->acknowledgeUARTError();
    // TODO: if (sercom->isBufferOverflowErrorUART()) ....
    // TODO: if (sercom->isParityErrorUART()) ....
    sercom->clearStatusUART();
  }
}

arduino...\feather_m4\variant.cpp

Uart Serial1( &sercom5, PIN_SERIAL1_RX, PIN_SERIAL1_TX, PAD_SERIAL1_RX, PAD_SERIAL1_TX ) ;

void SERCOM5_0_Handler()
{
  Serial1.IrqHandler();
}
void SERCOM5_1_Handler()
{
  Serial1.IrqHandler();
}
void SERCOM5_2_Handler()
{
  Serial1.IrqHandler();
}
void SERCOM5_3_Handler()
{
  Serial1.IrqHandler();
}

I presume that if I do something similar for SeeSaw it should be ok. It's going to be a bit of a hack up because of the way it is currently written. It would be a lot of duplication in files generated for SAMD51 if I go ahead at put in 6 SERCOM ports with 4 Handlers each (total 24) following the same way it is currently written.

source\AOSERCOM.cpp (existing)

extern "C" {

void sercom_handler( Sercom * sercom )
{
    if( isEnabledUART( sercom ) ){
        if (availableDataUART( sercom )) {
            uint8_t c = readDataUART( sercom );
#ifndef USB_UART_DIRECT
            m_rxFifo->Write(&c, 1);
            AOSERCOM::RxCallback();
#else       
            USBDevice.send(CDC_ENDPOINT_IN, &c, 1);
#endif
        }

        if (isUARTError( sercom )) {
            acknowledgeUARTError( sercom );
            // TODO: if (sercom->isBufferOverflowErrorUART()) ....
            // TODO: if (sercom->isFrameErrorUART()) ....
            // TODO: if (sercom->isParityErrorUART()) ....
            clearStatusUART( sercom );
        }
    }
    //TODO: else if ( isEnabledSPI( sercom ) ) { }
}
#if defined(SERCOM0) && CONFIG_SERCOM0
void SERCOM0_Handler(void){
    QXK_ISR_ENTRY();
    sercom_handler(SERCOM0);
    QXK_ISR_EXIT();
}
#endif
#if defined(SERCOM1) && CONFIG_SERCOM1
void SERCOM1_Handler(void){
    QXK_ISR_ENTRY();
    sercom_handler(SERCOM1);
    QXK_ISR_EXIT();
}
#endif
#if defined(SERCOM2) && CONFIG_SERCOM2
void SERCOM2_Handler(void){
    QXK_ISR_ENTRY();
    sercom_handler(SERCOM2);
    QXK_ISR_EXIT();
}
#endif
#if defined(SERCOM5) && CONFIG_SERCOM5
    void SERCOM5_Handler(void){
        QXK_ISR_ENTRY();
        sercom_handler(SERCOM5);
        QXK_ISR_EXIT();
    }
#endif
};
tannewt commented 4 years ago

I presume that if I do something similar for SeeSaw it should be ok. It's going to be a bit of a hack up because of the way it is currently written. It would be a lot of duplication in files generated for SAMD51 if I go ahead at put in 6 SERCOM ports with 4 Handlers each (total 24) following the same way it is currently written.

Duplication is ok. I tend to make all of the functions and then call a common handler. In SeeSaw I imagine you'll just call the SAMD21 variant from the four SAMD51 functions. We could change the interrupt table but it's code we get from the HAL that is easiest to leave as is.

The SERCOM is actually one peripheral with a number of modes. So even though the datasheet has UART, SPI and I2C the INTFLAG register is the same for all three. (Literally the same address.) What the bits mean depends on the SERCOM mode (in CTRLA iirc.) That is why the mux table can't give more detail.

Overall, I think you are on the right track. Keep up the good work!

wallarug commented 4 years ago

Thanks @tannewt! I'll let you know how I go. I'll have a go at it after work.