Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions adc/ad7779/imx6ull/dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ static void sdma_configure(void)
cfg.trig = sdma_trig__event;
cfg.event = event_channel;
cfg.priority = SDMA_CHANNEL_PRIORITY_MIN + 1;
cfg.options = sdma_chOption__auto_bd_done;
sdma_channel_configure(&sdma_common.sdma, &cfg);
}

Expand Down
17 changes: 17 additions & 0 deletions dma/imx6ull-sdma/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
SDMA driver for IMX6ULL

## (in)Frequently Asked Questions

### Why there are references to BP / DSP / StarCore in IMX6 Reference Manual?

It seems that SDMA documentation was copied from older devices using same IP core without proper care. Descriptions and
register were left with original terms that are not defined anywhere.

* BP - Baseband Processor/Platform (aka. StarCore/DSP)
* AP - Application Processor/Platform

There is limited amount of publicly available documentation but this [MXC300 fact-sheet] contains diagram that includes
SDMA with both application processor and DSP core. There are other diagrams here: [MOTOKRZR K3 – Theory of operation].

[MXC300 fact-sheet]: https://www.nxp.com/docs/en/fact-sheet/MXC300301FS.pdf
[MOTOKRZR K3 – Theory of operation]: https://firmware.center/firmware/Motorola/KRZR%20K3%20%28Sumba%29/Service%20Docs/MOTOKRZR_K3_Theory_of_Operation_v.1.0.pdf
58 changes: 24 additions & 34 deletions dma/imx6ull-sdma/imx6ull-sdma.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*/

#include <stdint.h>
#include <stdbool.h>
#include <stdio.h>
#include <errno.h>
#include <unistd.h>
Expand Down Expand Up @@ -135,7 +136,7 @@ typedef enum {

typedef struct {
int active;
int auto_bd_done;
bool auto_bd_done;

sdma_buffer_desc_t *bd;
addr_t bd_paddr;
Expand Down Expand Up @@ -339,30 +340,22 @@ static int sdma_free_uncached(void *vaddr, size_t size)
return munmap(vaddr, n*_PAGE_SIZE);
}

static int __attribute__((unused)) sdma_program_memory_dump(uint16_t addr,
addr_t buffer,
size_t size)
static int __attribute__((unused)) sdma_program_memory_dump(uint16_t addr, addr_t buffer, size_t size)
{
return sdma_run_channel0_cmd(size, SDMA_CMD_C0_GET_PM, buffer, addr);
}

static int __attribute__((unused)) sdma_program_memory_write(uint16_t addr,
addr_t buffer,
size_t size)
static int __attribute__((unused)) sdma_program_memory_write(uint16_t addr, addr_t buffer, size_t size)
{
return sdma_run_channel0_cmd(size, SDMA_CMD_C0_SET_PM, buffer, addr);
}

static int sdma_data_memory_dump(uint16_t addr,
addr_t buffer,
size_t size)
static int sdma_data_memory_dump(uint16_t addr, addr_t buffer, size_t size)
{
return sdma_run_channel0_cmd(size, SDMA_CMD_C0_GET_DM, buffer, addr);
}

static int sdma_data_memory_write(uint16_t addr,
addr_t buffer,
size_t size)
static int sdma_data_memory_write(uint16_t addr, addr_t buffer, size_t size)
{
return sdma_run_channel0_cmd(size, SDMA_CMD_C0_SET_DM, buffer, addr);
}
Expand All @@ -379,20 +372,22 @@ static int sdma_intr(unsigned int intr, void *arg)

unsigned i;
for (i = 1; i < NUM_OF_SDMA_CHANNELS; i++) {
sdma_channel_t *channel = &cmn->channel[i];

/* Check if channel is active and it's interrupt flag is set */
if (_INTR & (1 << i) && cmn->channel[i].active) {

/* Set BD_DONE in all buffer descriptors */
sdma_buffer_desc_t *current = cmn->channel[i].bd;
do {
if (!(current->flags & SDMA_BD_DONE))
current->flags |= SDMA_BD_DONE;
} while (!((current++)->flags & SDMA_BD_WRAP));
if (_INTR & (1 << i) && channel->active) {
if (channel->auto_bd_done) {
/* Set BD_DONE in all buffer descriptors */
sdma_buffer_desc_t *current = channel->bd;
do {
if (!(current->flags & SDMA_BD_DONE))
current->flags |= SDMA_BD_DONE;
} while (!((current++)->flags & SDMA_BD_WRAP));
}

/* Increase interrupt count to notify dispatcher that interrupt for
* this channel occurred */
cmn->channel[i].intr_cnt++;
channel->intr_cnt++;
}
}
}
Expand Down Expand Up @@ -461,7 +456,6 @@ static void sdma_init_core(void)

common.regs->EVTOVR = 0;
common.regs->HOSTOVR = 0;
common.regs->DSPOVR = 0xffffffff;

/* Clear channel pending status */
common.regs->EVTPEND = common.regs->EVTPEND;
Expand Down Expand Up @@ -566,8 +560,6 @@ static int sdma_channel_configure(uint8_t channel_id, sdma_channel_config_t *cfg
return -1;
}

common.regs->DSPOVR |= 1 << channel_id;

if (cfg->trig == sdma_trig__event) {
if (cfg->event >= NUM_OF_SDMA_REQUESTS) {
log_error("event number is too high (%d)", cfg->event);
Expand All @@ -584,6 +576,7 @@ static int sdma_channel_configure(uint8_t channel_id, sdma_channel_config_t *cfg
common.regs->HOSTOVR &= ~(1 << channel_id);
}

common.channel[channel_id].auto_bd_done = (cfg->options & sdma_chOption__auto_bd_done) != 0;
sdma_set_channel_priority(channel_id, cfg->priority);

if ((res = sdma_set_bd_array(channel_id, cfg->bd_paddr, cfg->bd_cnt)) < 0) {
Expand Down Expand Up @@ -706,19 +699,19 @@ static int dev_ctl(msg_t *msg)
return EOK;

case sdma_dev_ctl__data_mem_write:
if (msg->o.size != dev_ctl.mem.len || msg->o.size > common.tmp_size) {
if ((msg->o.size != dev_ctl.mem.len) || (msg->o.size > common.tmp_size) || (msg->o.size % 4 != 0)) {
log_error("dev_ctl: invalid size");
return -EIO;
}
memcpy(common.tmp, msg->o.data, msg->o.size);
return sdma_data_memory_write(dev_ctl.mem.addr, common.tmp_paddr, dev_ctl.mem.len);
return sdma_data_memory_write(dev_ctl.mem.addr, common.tmp_paddr, dev_ctl.mem.len / 4);

case sdma_dev_ctl__data_mem_read:
if (msg->o.size != dev_ctl.mem.len || msg->o.size > common.tmp_size) {
if ((msg->o.size != dev_ctl.mem.len) || (msg->o.size > common.tmp_size) || (msg->o.size % 4 != 0)) {
log_error("dev_ctl: invalid size");
return -EIO;
}
res = sdma_data_memory_dump(dev_ctl.mem.addr, common.tmp_paddr, dev_ctl.mem.len);
res = sdma_data_memory_dump(dev_ctl.mem.addr, common.tmp_paddr, dev_ctl.mem.len / 4);
Comment thread
xvuko marked this conversation as resolved.
memcpy(msg->o.data, common.tmp, msg->o.size);
return res;

Expand Down Expand Up @@ -1082,8 +1075,8 @@ int main(int argc, char *argv[])
unsigned i, intr_cnt[NUM_OF_SDMA_CHANNELS], cnt;
memset(intr_cnt, 0, sizeof(intr_cnt));

mutexLock(common.lock);
while (1) {
mutexLock(common.lock);
res = condWait(common.intr_cond, common.lock, INTR_WAIT_TIMEOUT_US);

if (res == -ETIME) {
Expand All @@ -1099,8 +1092,6 @@ int main(int argc, char *argv[])

common.broken = 1;
}

mutexUnlock(common.lock);
continue;
}

Expand All @@ -1121,11 +1112,10 @@ int main(int argc, char *argv[])
condSignal(common.channel[i].intr_cond);
intr_cnt[i] = cnt;
}

mutexUnlock(common.lock);
}

/* Should never be reached */
mutexUnlock(common.lock);
log_error("Exiting!");
return 0;
}
4 changes: 2 additions & 2 deletions dma/imx6ull-sdma/libsdma.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@ int sdma_channel_configure(sdma_t *s, sdma_channel_config_t *cfg)
return sdma_dev_ctl(s, &dev_ctl, NULL, 0);
}

int sdma_data_mem_write(sdma_t *s, void *data, size_t size, addr_t addr)
int sdma_data_mem_write(sdma_t *s, const void *data, size_t size, addr_t addr)
{
sdma_dev_ctl_t dev_ctl;

dev_ctl.type = sdma_dev_ctl__data_mem_write;
dev_ctl.mem.addr = addr;
dev_ctl.mem.len = size;

return sdma_dev_ctl(s, &dev_ctl, data, size);
return sdma_dev_ctl(s, &dev_ctl, (void *)data, size);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Casting away const from the data pointer when calling sdma_dev_ctl is potentially unsafe. Although sdma_data_mem_write is a write operation (where the buffer is only read), the sdma_dev_ctl function takes a non-const void * and could potentially modify the buffer if the implementation changes or if the function is misused in other contexts. Ideally, the underlying sdma_dev_ctl API should be updated to be const-correct.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required as msg_t uses non const pointer.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this data is o->data (pointer to output data). This is not how it supposed to work!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment for now. Doing this properly requires more changes (ie splitting devctl type) and needs to be tested.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And now this comment is actually pushed to this issue

}

int sdma_data_mem_read(sdma_t *s, void *data, size_t size, addr_t addr)
Expand Down
6 changes: 6 additions & 0 deletions dma/imx6ull-sdma/sdma-api.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,17 @@ typedef enum {
sdma_trig__host,
} sdma_trig_t;

enum sdma_chOption {
sdma_chOption__auto_bd_done = (1 << 0),
};

typedef struct {
addr_t bd_paddr; /* Physical address of buffer descriptor array */
unsigned bd_cnt;
sdma_trig_t trig;
unsigned event;
unsigned priority;
unsigned options;
} sdma_channel_config_t;

typedef enum {
Expand All @@ -124,6 +129,7 @@ typedef enum {
sdma_dev_ctl__ocram_alloc
} sdma_dev_ctl_type_t;

/* TODO: use separate types for input/output devctl and remove msg.o being used for input data (both raw and data) */
typedef struct {
sdma_dev_ctl_type_t type;

Expand Down
16 changes: 15 additions & 1 deletion dma/imx6ull-sdma/sdma.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,21 @@ int sdma_close(sdma_t *s);

int sdma_channel_configure(sdma_t *s, sdma_channel_config_t *cfg);

int sdma_data_mem_write(sdma_t *s, void *data, size_t size, addr_t addr);
/*
* write SDMA memory using data mode (32 bit word access)
*
* @param[in] data,size source buffer, expressed in bytes, must be multiple of 4 bytes (32 bit word)
* @param[in] addr destination address in 32 bit words
*/
int sdma_data_mem_write(sdma_t *s, const void *data, size_t size, addr_t addr);


/*
* read SDMA memory using data mode (32 bit word access)
*
* @param[out] data,size destination buffer; expressed in bytes; must be multiple of 4 bytes (32 bit word)
* @param[in] addr source address in 32 bit words
*/
int sdma_data_mem_read(sdma_t *s, void *data, size_t size, addr_t addr);

int sdma_context_dump(sdma_t *s, sdma_context_t *ctx);
Expand Down
Loading