Hi Geert,

geert@linux-m68k.org wrote on Mon, 21 Feb 2022 10:16:24 +0100:

Hi Miquel,

On Fri, Feb 18, 2022 at 7:12 PM Miquel Raynal [off-list ref] wrote:

quoted

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!

quoted

--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c  

Missing #include <linux/soc/renesas/r9a06g032-syscon.h>

quoted

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

Ok.

quoted

+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().

Yes.

quoted

+
+       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,  

quoted

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

I've moved it to the top of the file. There definitions are a bit mixed
with the code, I don't like this, so I kept it at the top.

quoted

--- /dev/null
+++ b/include/linux/soc/renesas/r9a06g032-syscon.h  

r9a06g032-sysctrl.h etc.

Done.

quoted

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

Thanks,
Miquèl