dmac core cyclic not cleared

[dshekter]

Hi,

The axi_dmac driver doesn't properly set/clear the cyclic bit in axi_dmac_transfer_start. In my testing,

    // Set flags as needed
    if (dmac->transfer.cyclic != CYCLIC) {
        axi_dmac_read(dmac, AXI_DMAC_REG_FLAGS, &reg_val);
        reg_val = reg_val & ~DMA_CYCLIC;
        axi_dmac_write(dmac, AXI_DMAC_REG_FLAGS, reg_val);
    }

added before enabling seems to have worked.

Thanks

[danmois]

Hello @dshekter ,

Hardware cyclic transfers are possible only for MEM to DEV transfers. The DMA_CYCLIC flag will be set only if the HDL allows cyclic transfers and the transfer size is smaller than the maximum transfer size. Otherwise, the software has to implement the cyclic transfer.

We can continue the conversation on Engineer Zone. If clarifications are needed, please open a thread here: https://ez.analog.com/microcontroller-no-os-drivers/

Regards, George

[dshekter]

Hello @danmois

You are correct. However, if you look at the code, you'll not that the negative case is not handled correctly. Perhaps what you want is instead of:

    /* Cyclic transfers set to HW or SW depending on size for MEM to DEV. */
    if ((dmac->direction == DMA_MEM_TO_DEV) && (dmac->transfer.cyclic == CYCLIC)) {
        if ((dmac->remaining_size - 1) <= dmac->max_length) {
            if (dmac->hw_cyclic) {
                axi_dmac_read(dmac, AXI_DMAC_REG_FLAGS, &reg_val);
                reg_val = reg_val | DMA_CYCLIC;
                axi_dmac_write(dmac, AXI_DMAC_REG_FLAGS, reg_val);
            }
        } else {
            axi_dmac_read(dmac, AXI_DMAC_REG_FLAGS, &reg_val);
            reg_val = reg_val & ~DMA_CYCLIC;
            axi_dmac_write(dmac, AXI_DMAC_REG_FLAGS, reg_val);
        }
    }

to have

    /* Cyclic transfers set to HW or SW depending on size for MEM to DEV. */
    if ((dmac->direction == DMA_MEM_TO_DEV) && (dmac->transfer.cyclic == CYCLIC)) &&
        ((dmac->remaining_size - 1) <= dmac->max_length) &&
             (dmac->hw_cyclic)) {
                axi_dmac_read(dmac, AXI_DMAC_REG_FLAGS, &reg_val);
                reg_val = reg_val | DMA_CYCLIC;
                axi_dmac_write(dmac, AXI_DMAC_REG_FLAGS, reg_val);
            } else {
                axi_dmac_read(dmac, AXI_DMAC_REG_FLAGS, &reg_val);
                reg_val = reg_val & ~DMA_CYCLIC;
                axi_dmac_write(dmac, AXI_DMAC_REG_FLAGS, reg_val);
    }

(kinda writing the code in this comment, so please ignore any syntactical / formatting issues). What you're saying is

The DMA_CYCLIC flag will be set only if ...

Which is true, but my issue is you're not unsetting it when the transfer isn't supposed to be cyclic. This should be 1 if statement where either the bit is set or cleared accordingly.

I had a transfer that was set to not be cyclic but the bit in the control register wasn't cleared. Please implement a fix to clear the bit when cyclic transfers are not requested.

Thank you

[danmois]

Hi @dshekter,

Maybe adding another if statement after the one checking cyclic transfers would do the trick?

`

if (dmac->transfer.cyclic == NO) {

    axi_dmac_read(dmac, AXI_DMAC_REG_FLAGS, &reg_val);

    reg_val = reg_val & ~DMA_CYCLIC;

    axi_dmac_write(dmac, AXI_DMAC_REG_FLAGS, reg_val);

}

`

We will implement the changes in the following days after testing.

Regards, George

[dshekter]

Hi @danmois

That was initially how I solved it but I would advise against code duplication and instead fix the if statement to operate as you've described its intent. Alternatively, always clear the cyclic bit and then go through the nested if to clear it without the else block. Despite the draw of DMA for systems with relatively capacious memory, I'm trying to keep the code footprint down in my design.

Thank you. I look forward to the implemented changes, however they are done.

[danmois]

Hi @dshekter ,

Thank you for the suggestion. Please feel free to comment in the corresponding PR.

Regards, George

[rbolboac]

@dshekter , a fix was merged so I am closing this issue.