Re: [PATCH v2 3/8] soc: renesas: rzn1-sysc: Export function to set dmamux

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Miquel,

On Tue, Feb 22, 2022 at 11:35 AM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
> The dmamux register is located within the system controller.
>
> Without syscon, we need an extra helper in order to give write access to
> this register to a dmamux driver.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r9a06g032-clocks.c
> +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> @@ -922,6 +947,12 @@ static int __init r9a06g032_clocks_probe(struct platform_device *pdev)
>         clocks->reg = of_iomap(np, 0);
>         if (WARN_ON(!clocks->reg))
>                 return -ENOMEM;
> +
> +       if (sysctrl_priv)
> +               return -EBUSY;

Can this actually happen, or can the check be removed?

> +
> +       sysctrl_priv = clocks;

Oh yes, it can happen, if any of the clock registrations below fail
due to -EPROBE_DEFER. So I think the assignment to sysctrl_priv
should be postponed until we're sure that can no longer happen.
Then the check above can be removed, too.

> +
>         for (i = 0; i < ARRAY_SIZE(r9a06g032_clocks); ++i) {
>                 const struct r9a06g032_clkdesc *d = &r9a06g032_clocks[i];
>                 const char *parent_name = d->source ?

The rest LGTM.

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