DMA2 support on F091 missing? Topic is solved

[Giovanni]

The driver does some special handling of shared interrupts, it has a mask of all channels sharing an ISR. I would look that handling in the alloc/release functions.

Giovanni

[steved]

Think I've got it! (Whether I can explain it is another matter). And to do the fix properly definitely requires the touch of the maestro.

Look at dmaStreamAllocate():

Code: Select all

bool dmaStreamAllocate(const stm32_dma_stream_t *dmastp,
                       uint32_t priority,
                       stm32_dmaisr_t func,
                       void *param) {

  osalDbgCheck(dmastp != NULL);

  /* Checks if the stream is already taken.*/
  if ((dma_streams_mask & (1U << dmastp->selfindex)) != 0U)
    return true;
--- BBB ---
  /* Installs the DMA handler.*/
  _stm32_dma_isr_redir[dmastp->selfindex].dma_func  = func;
  _stm32_dma_isr_redir[dmastp->selfindex].dma_param = param;

--- CCC ---
  /* Enabling DMA clocks required by the current streams set.*/
  if ((dma_streams_mask & STM32_DMA1_STREAMS_MASK) == 0U) {
    rccEnableDMA1(false);
  }
#if STM32_DMA2_NUM_CHANNELS > 0
  if ((dma_streams_mask & STM32_DMA2_STREAMS_MASK) == 0U) {
    rccEnableDMA2(false);
  }
#endif

  /* Putting the stream in a safe state.*/
  dmaStreamDisable(dmastp);
  dmastp->channel->CCR = STM32_DMA_CCR_RESET_VALUE;

  /* Enables the associated IRQ vector if not alread enabled and if a
     callback is defined.*/
     --- AAA ---
  if (((dma_streams_mask & dmastp->cmask) == 0U) &&
      (func != NULL)) {
    nvicEnableVector(dmastp->vector, priority);
  }

  /* Marks the stream as allocated.*/
  dma_streams_mask |= (1U << dmastp->selfindex);      --- BBB ---

  return false;
}

--- AAA --- The real problem is related to the use of dma_streams_mask. I can see that updating dma_streams_mask at the end of the routine was probably a conscious decision, to allow picking up of changes from disabled to enabled on the various clocks and vectors. The problem with this is that in consequence all the tests exclude the stream just being added. Consider where a decision is made as to whether to enable the vector. And for simplicity we're calling dmaStreamAlloc() just twice, with an interrupt function not defined in the first call, and the DMA channels share an interrupt vector. On the first call, dma_streams_mask is zero, so the test on dma_streams_mask is zero, but because func is NULL the interrupt isn't enabled. On the second call, the test on dma_streams_mask is non-zero, so there is no question of enabling the interrupt.

My tentative thought is that a temporary copy of dma_streams_mask should be made, before the channel flag is set, and that setting the channel flag should be done as early as possible. Then both values are available for tests as needed.

--- BBB --- In some ways a side issue, but I suggest that unless dmaStreamAlloc() is made into a critical section, the mask should be updated immediately following the test. It has the potential to be called from multiple threads.

--- CCC --- The way the code's written, both DMA clocks are always enabled, regardless of whether they are needed.


The test project (runs on F091 Nucleo) should demonstrate this fairly well; just set a breakpoint on dmaStreamAlloc() and make dma_streams_mask global so that you can see the value. There are a total of four calls - two from the I2C startup, followed by two from the SPI startup.

[steved]

Actually, fix might be simpler than I thought. The problem occurs when the first demand for a DMA channel on a shared interrupt doesn't have an interrupt handler. So I just added another mask variable to track whether the DMA channel needed interrupts.

Superficially, the attached works.

[Giovanni]

Hi,

I merged the changes with just few style adjustment and a small optimization. Could you give it a try?

Fixed as bug #891.

Giovanni

[steved]

Seems to work OK; will try and give it a better test next week.

Should there be some state and reentrancy management in dmaStreamAllocate()? Superfically, it looks as if a context change part way through could cause confusion.

Presumably none of the devices using the DMA2 controller have shared interrupts? (There's no code to manage shared interrupts in the driver)

[Giovanni]

Hi,

Those functions are always called within a critical zone, probably I should add the "I" suffix to make that clear.

Giovanni