Hi Miquel, On Fri, Feb 18, 2022 at 7:12 PM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> 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@xxxxxxxxxxx> Thanks for your patch! > --- a/drivers/clk/renesas/r9a06g032-clocks.c > +++ b/drivers/clk/renesas/r9a06g032-clocks.c Missing #include <linux/soc/renesas/r9a06g032-syscon.h> > @@ -315,6 +315,27 @@ struct r9a06g032_priv { > void __iomem *reg; > }; > > +/* Exported helper to access the DMAMUX register */ > +static struct r9a06g032_priv *syscon_priv; I'd call this sysctrl_priv, as that matches the bindings and binding header file name. > +int r9a06g032_syscon_set_dmamux(u32 mask, u32 val) > +{ > + u32 dmamux; > + > + if (!syscon_priv) > + return -EPROBE_DEFER; > + > + spin_lock(&syscon_priv->lock); This needs propection against interrupts => spin_lock_irqsave(). > + > + dmamux = readl(syscon_priv->reg + R9A06G032_SYSCON_DMAMUX); > + dmamux &= ~mask; > + dmamux |= val & mask; > + writel(dmamux, syscon_priv->reg + R9A06G032_SYSCON_DMAMUX); > + > + spin_unlock(&syscon_priv->lock); > + > + return 0; > +} > + > /* register/bit pairs are encoded as an uint16_t */ > static void > clk_rdesc_set(struct r9a06g032_priv *clocks, > --- a/include/dt-bindings/clock/r9a06g032-sysctrl.h > +++ b/include/dt-bindings/clock/r9a06g032-sysctrl.h > @@ -145,4 +145,6 @@ > #define R9A06G032_CLK_UART6 152 > #define R9A06G032_CLK_UART7 153 > > +#define R9A06G032_SYSCON_DMAMUX 0xA0 I don't think this needs to be part of the bindings, so please move it to the driver source file. > --- /dev/null > +++ b/include/linux/soc/renesas/r9a06g032-syscon.h r9a06g032-sysctrl.h etc. > @@ -0,0 +1,11 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __LINUX_SOC_RENESAS_R9A06G032_SYSCON_H__ > +#define __LINUX_SOC_RENESAS_R9A06G032_SYSCON_H__ > + > +#ifdef CONFIG_CLK_R9A06G032 > +int r9a06g032_syscon_set_dmamux(u32 mask, u32 val); > +#else > +static inline int r9a06g032_syscon_set_dmamux(u32 mask, u32 val) { return -ENODEV; } > +#endif > + > +#endif /* __LINUX_SOC_RENESAS_R9A06G032_SYSCON_H__ */ > -- > 2.27.0 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