Re: [PATCH v2 0/3] dmaengine: sh: rz-dmac: add r7s72100 support

Re: [PATCH v2 0/3] dmaengine: sh: rz-dmac: add r7s72100 support

[Geert Uytterhoeven]

Hi Wolfram,

CC linux-mmc

On Tue, Oct 1, 2024 at 2:43 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> When activating good old Genmai board for regression testing, I found
> out that not much is needed to activate the DMA controller for A1H.
> Which makes sense, because the driver was initially written for this
> SoC. Let it come home ;)

[...]

> Adding SDHI
> is still WIP because RZ/A1L usage exposes a SDHI driver bug. So much for
> the value of regression testing...

I had completely forgotten about this, so I just ran into the same
issue while trying to enable more DMA support :-(

The SDHI callback does:

    static void renesas_sdhi_sys_dmac_dma_callback(void *arg)
    {
            ...

            wait_for_completion(&priv->dma_priv.dma_dataend);
            ...
    }

i.e. it assumes it is not called in atomic context.
On R-Car Gen2, that is true, as the R-Car DMAC IRQ thread does:

    static irqreturn_t rcar_dmac_isr_channel_thread(int irq, void *dev)
    {
            ...
            spin_unlock_irq(&chan->lock);
            dmaengine_desc_callback_invoke(&cb, NULL);
            spin_lock_irq(&chan->lock);
            ...
    }

On RZ/A1, the RZ DMAC driver uses virt-dma, and offloads this to the
vchan tasklet, which does:

    static void vchan_complete(struct tasklet_struct *t)
    {
            ...
            spin_unlock_irq(&vc->lock);

            dmaengine_desc_callback_invoke(&cb, &vd->tx_result);
            ...
    }

However, the tasklet runs in softirq context, causing:

    BUG: scheduling while atomic: ksoftirqd/0/8/0x00000100
    CPU: 0 UID: 0 PID: 8 Comm: ksoftirqd/0 Not tainted
6.12.0-rc7-rskrza1-08263-g3b9979a62f8e #885
    Hardware name: Generic R7S72100 (Flattened Device Tree)
    Call trace:
     unwind_backtrace from show_stack+0x10/0x14
     show_stack from dump_stack_lvl+0x34/0x54
     dump_stack_lvl from __schedule_bug+0x44/0x64
     __schedule_bug from __schedule+0x44/0x48c
     __schedule from schedule+0x28/0x44
     schedule from schedule_timeout+0x28/0xdc
     schedule_timeout from __wait_for_common+0x80/0x108
     __wait_for_common from renesas_sdhi_sys_dmac_dma_callback+0x58/0x84
     renesas_sdhi_sys_dmac_dma_callback from
dmaengine_desc_callback_invoke+0x6c/0x7c
     dmaengine_desc_callback_invoke from vchan_complete+0x118/0x13c
     vchan_complete from tasklet_action_common+0x64/0x90
     tasklet_action_common from handle_softirqs+0x164/0x1cc
     handle_softirqs from run_ksoftirqd+0x20/0x38

I am not sure if the SDHI driver or the RZ-DMAC driver (or virt-dma)
should be fixed, as the documentation[1] states:

     Note that callbacks will always be invoked from the DMA
     engines tasklet, never from interrupt context.

Thanks for your comments!

[1] https://elixir.bootlin.com/linux/v6.11.8/source/Documentation/driver-api/dmaengine/client.rst#L164

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 0/3] dmaengine: sh: rz-dmac: add r7s72100 support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]


> I am not sure if the SDHI driver or the RZ-DMAC driver (or virt-dma)
> should be fixed, as the documentation[1] states:
> 
>      Note that callbacks will always be invoked from the DMA
>      engines tasklet, never from interrupt context.

Back then, I had the impression that we can rework the SDHI SYSDMAC part
to not use a completion like the internal DMAC version does. But it has
been a while and I got completely side-tracked meanwhile.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]