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

Message

Wolfram Sang Oct. 1, 2024, 12:43 p.m. UTC

Changes since v1:
* added tags to patches 1 and 3 (Thanks Biju, Claudiu, and Philipp!)
* used A1H instead of A1L (Thanks, Geert!)
* reworded comments and descriptions to use a more generic "RZ DMA"
  term without mentioning all the SoCs in patches 2 and 3.

Old cover-letter:

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 ;)

Patch 1 is a generic fix. The other patches are the actual enablement.
A branch with DTS additions for MMCIF can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/genmai-upstreaming

These will be upstreamed once the driver parts are in next. Adding SDHI
is still WIP because RZ/A1L usage exposes a SDHI driver bug. So much for
the value of regression testing...

Wolfram Sang (3):
  dmaengine: sh: rz-dmac: handle configs where one address is zero
  dt-bindings: dma: rz-dmac: Document RZ/A1H SoC
  dmaengine: sh: rz-dmac: add r7s72100 support

 .../bindings/dma/renesas,rz-dmac.yaml         | 27 +++++++++++++------
 drivers/dma/sh/Kconfig                        |  8 +++---
 drivers/dma/sh/rz-dmac.c                      | 27 ++++++++++---------
 3 files changed, 38 insertions(+), 24 deletions(-)

Comments

Geert Uytterhoeven Nov. 15, 2024, 2:47 p.m. UTC | #1

Hi Wolfram,

CC linux-mmc

On Tue, Oct 1, 2024 at 2:43 PM Wolfram Sang
<[e-mail]> 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

Wolfram Sang Nov. 15, 2024, 5:08 p.m. UTC | #2

> 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.