Re: [PATCH] dmaengine: rcar-dmac: don't use DMAC error interrupt

[Geert Uytterhoeven]

Hi Morimoto-san,

CC devicetree

Thanks for your patch!

On Tue, Jun 12, 2018 at 9:40 AM Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
>
> Current rcar-dmac is using DMAC error interrupt which will handle all
> channel's error. But in this design, error handling itself will be
> issue if user want to use virtualization, multi OS, etc.
> This patch removes current DMAC error interrupt handling, and handle it
> on each channel interrupt handler.
>
> Signed-off-by: Magnus Damm <damm+renesas@xxxxxxxxxxxxx>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>

Who wrote this patch, you or Magnus? Or is this a joint effort?

> --- a/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.txt
> +++ b/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.txt
> @@ -35,9 +35,8 @@ Required Properties:
>
>  - interrupts: interrupt specifiers for the DMAC, one for each entry in
>    interrupt-names.
> -- interrupt-names: one entry for the error interrupt, named "error", plus one
> -  entry per channel, named "ch%u", where %u is the channel number ranging from
> -  zero to the number of channels minus one.
> +- interrupt-names: one entry per channel, named "ch%u", where %u is the
> +  channel number ranging from zero to the number of channels minus one.

DT bindings describe hardware, not software policy.
So IMHO the error interrupt must not be removed from the bindings (and
from the DTS and example).

> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1522,25 +1518,37 @@ static irqreturn_t rcar_dmac_isr_channel(int irq, void *dev)

[...]

>
> +       if (reinit) {
> +               dev_err(chan->chan.device->dev, "Channel Address Error happen\n");

s/happen/happened/, or just drop the word (reducing kernel size ;-)

The rest looks good to me (but I haven't tested it), so with the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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