DMA: dma.c in rp2_common does not take into account changes/additions of RP2350 #1986

New issue

Closed

opened 2024-10-15 09:14:14 -05:00 by Manu3l0us · 3 comments

Manu3l0us commented

2024-10-15 09:14:14 -05:00

(Migrated from github.com)

While checking out the SDK I found that the code in rp2_common/hardware_dma/dma.c does not support additional capabilities of the RP2350 DMA, e.g. on channel cleanup irq2 and irq3 are not disabled.

I would expect that the additional interrupts would also need to be included in the cleanup.

    // disable IRQs first as abort can cause spurious IRQs
    dma_channel_set_irq0_enabled(channel, false);
    dma_channel_set_irq1_enabled(channel, false);
    dma_channel_abort(channel);
    // finally clear the IRQ status, which may have been set during abort
    dma_hw->intr = 1u << channel;
}

While checking out the SDK I found that the code in `rp2_common/hardware_dma/dma.c` does not support additional capabilities of the RP2350 DMA, e.g. on channel cleanup irq2 and irq3 are not disabled. I would expect that the additional interrupts would also need to be included in the cleanup. ``` // disable IRQs first as abort can cause spurious IRQs dma_channel_set_irq0_enabled(channel, false); dma_channel_set_irq1_enabled(channel, false); dma_channel_abort(channel); // finally clear the IRQ status, which may have been set during abort dma_hw->intr = 1u << channel; } ```

AlexMykha1lov commented

2024-11-15 07:23:00 -06:00

(Migrated from github.com)

It was also noted that there is no possibility to configure the Count Modes field of the DMA`s CHx_TRANS_COUNT register for RP2350 - its processing is missing in the SDK API version 2.0.0.

From RP2350 datasheet:

File "~\.pico-sdk\sdk\2.0.0\src\rp2_common\hardware_dma\include\hardware\dma.h"
Actual code:

static inline void dma_channel_set_trans_count(uint channel, uint32_t trans_count, bool trigger) {
    if (!trigger) {
        dma_channel_hw_addr(channel)->transfer_count = trans_count;
    } else {
        dma_channel_hw_addr(channel)->al1_transfer_count_trig = trans_count;
    }
}

Expected code example:

static inline void dma_channel_set_trans_count(uint channel, uint32_t mode, uint32_t trans_count, bool trigger) {
    assert(mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_NORMAL || mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_TRIGGER_SELF || mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_ENDLESS);
    assert(trans_count < (1 << DMA_CH0_TRANS_COUNT_MODE_LSB));
    if (!trigger) {
        dma_channel_hw_addr(channel)->transfer_count = trans_count;
    } else {
        dma_channel_hw_addr(channel)->al1_transfer_count_trig = trans_count;
    }
    dma_channel_hw_addr(channel)->transfer_count |= mode << DMA_CH0_TRANS_COUNT_MODE_LSB;
}

It was also noted that there is no possibility to configure the Count Modes field of the DMA`s CHx_TRANS_COUNT register for RP2350 - its processing is missing in the SDK API version 2.0.0. From RP2350 datasheet: ![image](https://github.com/user-attachments/assets/12b1c7e6-088b-4984-bb79-58e11f72f1da) ![image](https://github.com/user-attachments/assets/fdf64a21-78f6-4bc6-84ca-87122b9f76b9) File "~\\.pico-sdk\sdk\2.0.0\src\rp2_common\hardware_dma\include\hardware\dma.h" Actual code: ``` static inline void dma_channel_set_trans_count(uint channel, uint32_t trans_count, bool trigger) { if (!trigger) { dma_channel_hw_addr(channel)->transfer_count = trans_count; } else { dma_channel_hw_addr(channel)->al1_transfer_count_trig = trans_count; } } ``` Expected code example: ``` static inline void dma_channel_set_trans_count(uint channel, uint32_t mode, uint32_t trans_count, bool trigger) { assert(mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_NORMAL || mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_TRIGGER_SELF || mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_ENDLESS); assert(trans_count < (1 << DMA_CH0_TRANS_COUNT_MODE_LSB)); if (!trigger) { dma_channel_hw_addr(channel)->transfer_count = trans_count; } else { dma_channel_hw_addr(channel)->al1_transfer_count_trig = trans_count; } dma_channel_hw_addr(channel)->transfer_count |= mode << DMA_CH0_TRANS_COUNT_MODE_LSB; } ```

kilograham commented

2024-11-19 15:30:19 -06:00

(Migrated from github.com)

It was also noted that there is no possibility to configure the Count Modes field of the DMA`s CHx_TRANS_COUNT register for RP2350 - its processing is missing in the SDK API version 2.0.0.

From RP2350 datasheet:

File "~.pico-sdk\sdk\2.0.0\src\rp2_common\hardware_dma\include\hardware\dma.h" Actual code:

static inline void dma_channel_set_trans_count(uint channel, uint32_t trans_count, bool trigger) {
    if (!trigger) {
        dma_channel_hw_addr(channel)->transfer_count = trans_count;
    } else {
        dma_channel_hw_addr(channel)->al1_transfer_count_trig = trans_count;
    }
}

Expected code example:

static inline void dma_channel_set_trans_count(uint channel, uint32_t mode, uint32_t trans_count, bool trigger) {
    assert(mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_NORMAL || mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_TRIGGER_SELF || mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_ENDLESS);
    assert(trans_count < (1 << DMA_CH0_TRANS_COUNT_MODE_LSB));
    if (!trigger) {
        dma_channel_hw_addr(channel)->transfer_count = trans_count;
    } else {
        dma_channel_hw_addr(channel)->al1_transfer_count_trig = trans_count;
    }
    dma_channel_hw_addr(channel)->transfer_count |= mode << DMA_CH0_TRANS_COUNT_MODE_LSB;
}

This has been moved to #2059 since it is a separate issue

> It was also noted that there is no possibility to configure the Count Modes field of the DMA`s CHx_TRANS_COUNT register for RP2350 - its processing is missing in the SDK API version 2.0.0. > > From RP2350 datasheet: ![image](https://private-user-images.githubusercontent.com/90247220/386611141-12b1c7e6-088b-4984-bb79-58e11f72f1da.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzIwNTE2MTcsIm5iZiI6MTczMjA1MTMxNywicGF0aCI6Ii85MDI0NzIyMC8zODY2MTExNDEtMTJiMWM3ZTYtMDg4Yi00OTg0LWJiNzktNThlMTFmNzJmMWRhLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMTklMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTE5VDIxMjE1N1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTcyMjFlZjFhMTM5YTk4MjMxYTA2ZTU4OWQ2MTIwMmVjNDc1Nzk5ZmEzYTQ0OWYyZjYzMGRkNjM2MmM1OTk3MWMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.wMYm2TzWIcRp5D4sN81dbmZWkp0ObvFwo_YhddZAsLM) ![image](https://private-user-images.githubusercontent.com/90247220/386616474-fdf64a21-78f6-4bc6-84ca-87122b9f76b9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzIwNTE2MTcsIm5iZiI6MTczMjA1MTMxNywicGF0aCI6Ii85MDI0NzIyMC8zODY2MTY0NzQtZmRmNjRhMjEtNzhmNi00YmM2LTg0Y2EtODcxMjJiOWY3NmI5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDExMTklMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMTE5VDIxMjE1N1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWE3MzM5MGEzZTY4NzEyNzE1OWQzNTc2ZWEwZWIzNGUyODViZjZmM2FlYzdkMjM4MmJiNmFhMWM2N2I3MDViYjMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.ZiBspMLofBhyYOjdm6IkiEuwMyWA--qOkULilJqrT44) > > File "~\.pico-sdk\sdk\2.0.0\src\rp2_common\hardware_dma\include\hardware\dma.h" Actual code: > > ``` > static inline void dma_channel_set_trans_count(uint channel, uint32_t trans_count, bool trigger) { > if (!trigger) { > dma_channel_hw_addr(channel)->transfer_count = trans_count; > } else { > dma_channel_hw_addr(channel)->al1_transfer_count_trig = trans_count; > } > } > ``` > > Expected code example: > > ``` > static inline void dma_channel_set_trans_count(uint channel, uint32_t mode, uint32_t trans_count, bool trigger) { > assert(mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_NORMAL || mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_TRIGGER_SELF || mode == DMA_CH0_TRANS_COUNT_MODE_VALUE_ENDLESS); > assert(trans_count < (1 << DMA_CH0_TRANS_COUNT_MODE_LSB)); > if (!trigger) { > dma_channel_hw_addr(channel)->transfer_count = trans_count; > } else { > dma_channel_hw_addr(channel)->al1_transfer_count_trig = trans_count; > } > dma_channel_hw_addr(channel)->transfer_count |= mode << DMA_CH0_TRANS_COUNT_MODE_LSB; > } > ``` This has been moved to #2059 since it is a separate issue

kilograham commented

2024-11-21 16:52:13 -06:00

(Migrated from github.com)