RIOT-OS has great design, But there are too many bugs.

[skyarm]

I found several major bugs within a few weeks, in version 2021.01: 1) In function spi_acquire at stm32/periph/spi.c uint16_t cr1_settings = ((br << BR_SHIFT) | mode | SPI_CR1_MSTR); cr1_settings become slave mode when mode = SPI_MODE_1. but SPI_MODE_1 was defined here. /< CPOL=0, CPHA=1 */** The mode variable need to left shift 1 bit: uint16_t cr1_settings = ((br << BR_SHIFT) | (mode << 1) | SPI_CR1_MSTR);

2) In function spi_acquire at stm32/periph/spi.c dma_acquire(spi_config[bus].tx_dma); dma_setup(spi_config[bus].tx_dma, spi_config[bus].tx_dma_chan, (uint32_t)&(dev(bus)->DR), DMA_MEM_TO_PERIPH, 0, DMA_DATA_WIDTH_BYTE); But the prototype is: void dma_setup(dma_t dma, int chan, void periph_addr, dma_mode_t mode, uint8_t width, bool inc_periph); Last two parameters position is wrong, But can get correct result, because DMA_DATA_WIDTH_BYTE equals zero, equals false too!

3)In function dma_stream at stm32/periph/dma.c

elif CPU_FAM_STM32F0 || CPU_FAM_STM32L0 || CPU_FAM_STM32G0

if (stream == 0) {
    return (DMA1_Channel1_IRQn);
}
else if (stream < 3 || (stream >= 8 && stream < 11)) {
    return (DMA1_Channel2_3_IRQn);
}
else if (stream < 7 || stream >= 11) {
    return (DMA1_Channel4_5_6_7_IRQn);
}

else

if (stream < 7) {
    return ((IRQn_Type)((int)DMA1_Channel1_IRQn + stream));
}

if defined(DMA2_BASE)

if defined(CPU_FAM_STM32F1)

else if (stream < 11) {

else

else if (stream < 13 ) {

endif

    _**return ((IRQn_Type)((int)DMA2_Channel1_IRQn + stream));**_
}

if !defined(CPU_FAM_STM32L1)

else {

if defined(CPU_FAM_STM32F1)

    return (DMA2_Channel4_5_IRQn);

else

    **return ((IRQn_Type)((int)DMA2_Channel6_IRQn + stream));**

endif

Get wrong result when the stream >= 7 on board stm32l4xx

These functions are frequently called, but bugs still be found.

[skyarm]

What can I do when a foundation code has too many bugs? So I had to check all the foundation code when I got unexpected result.

[skyarm]

I can found major bugs every week. These codes are often called, why are there bugs still?

[fjmolinas]

Hi @skyarm, Thanks a lot for opening an issue on these bugs! These bugs probably haven't been caught in the review process as the DMA peripherals of most microcontrollers are tricky and hard to get right. Could you please submit a PR fixing these bugs? If not you can leave a comment and we'll fix it as soon as possible.

See the CONTRIBUTING.md file for more information if you want to open a PR and the CODING_CONVENTIONS.md for how the code should be formatted.

How did you come up across these bugs? They are bluntly evident when you point them out but if we could expose them through a test then it would improve the coverage of those features.

Other than that we strive for the best code quality we can and try to test as much as possible on as many platforms as we can, but bugs do slip in, like in any software project. We support over 200 BOARD but a lot of those BOARDs are contributed by the community, even CPUs, etc... so at the end of the day the community or "users" (you in this case) also play a big role in testing and finding bugs.

Let me know if you have time to PR those identified fixes, and if you have a way of reproducing your experienced bug that would be helpful as well. Cheers.

[fjmolinas]

I'm taking a look at the pointed issues:

I found several major bugs within a few weeks, in version 2021.01: 1) In function spi_acquire at stm32/periph/spi.c

    uint16_t cr1_settings = ((br << BR_SHIFT) | mode | SPI_CR1_MSTR);
    cr1_settings become slave mode when mode = SPI_MODE_1.
    **but SPI_MODE_1 was defined here.  /**< CPOL=0, CPHA=1 */**

The mode variable need to left shift 1 bit:

    uint16_t cr1_settings = ((br << BR_SHIFT) | (mode << 1) | SPI_CR1_MSTR);

I don't thnk what you mention is correct, CR1 should not be shifted since SPI_MODE_1 is an enum with value 1, and if you look at the register:

So no shift needed, can you clarify this if I'm wrong?

[fjmolinas]

Regarding:

3)In function dma_stream at stm32/periph/dma.c Get wrong result when the stream >= 7 on board stm32l4xx

I looked at some datasheets and I can only find 7 channels/streams for this family can you point me to a family with more than 7?

[skyarm]

Yes, I”m wrong, Your code is correct, I am sorry for it.

发自我的iPhone

在 2021年4月30日，15:16，Francisco @.***> 写道：



Regarding:

3)In function dma_stream at stm32/periph/dma.c Get wrong result when the stream >= 7 on board stm32l4xx

I looked at some datasheets and I can only find 7 channels/streams for this family can you point me to a family with more than 7?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/RIOT-OS/RIOT/issues/16308#issuecomment-829894271, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMLPDFMEFQZSRXM3GAESXQTTLJKMRANCNFSM42XFB7HA.

[fjmolinas]

@skyarm with #16418 merged can we close this issue?

[skyarm]

Yes, Just I made a mistake.

发自我的iPhone

在 2021年4月30日，21:24，Francisco @.***> 写道：



@skyarmhttps://github.com/skyarm with #16418https://github.com/RIOT-OS/RIOT/pull/16418 merged can we close this issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/RIOT-OS/RIOT/issues/16308#issuecomment-830092448, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AMLPDFIQUWAX4OZIAV6GNNTTLKVSNANCNFSM42XFB7HA.