diff mbox series

[4/4] clk: renesas: r8a77980-cpg-mssr: add RPC clocks

Message ID 0e51165d-65cb-1522-3174-b63818180070@cogentembedded.com (mailing list archive)
State Changes Requested, archived
Headers show
Series Renesas R8A77980 CPG/MSSR RPC clock support | expand

Commit Message

Sergei Shtylyov Nov. 22, 2018, 6:45 p.m. UTC
Add the RPCSRC internal clock on R-Car V3H (R8A77980) -- it's controlled
by the RPCCKCR.DIV[4:3] on all the R-Car gen3 SoCs except V3M (R8A77970)
but the encoding of this field is different between SoCs.

Add the RPC[D2] clocks (derived from this internal clock) and the RPC-IF
module clock as well...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 drivers/clk/renesas/r8a77980-cpg-mssr.c |   40 +++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven Nov. 23, 2018, 12:59 p.m. UTC | #1
Hi Sergei,

Thanks for your patch!

On Thu, Nov 22, 2018 at 7:45 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Add the RPCSRC internal clock on R-Car V3H (R8A77980) -- it's controlled
> by the RPCCKCR.DIV[4:3] on all the R-Car gen3 SoCs except V3M (R8A77970)
> but the encoding of this field is different between SoCs.

Given the tables and encoding are the same on H3, M3-W, M3-N, and V3H,
I think it makes sense to move the common support to rcar-gen3-cpg.c.

Heck, you could even just select a different table on D3/E3 using
soc_device_match(), if only one encoding would not select a different parent
clock :-(

> Add the RPC[D2] clocks (derived from this internal clock) and the RPC-IF
> module clock as well...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> --- renesas-drivers.orig/drivers/clk/renesas/r8a77980-cpg-mssr.c
> +++ renesas-drivers/drivers/clk/renesas/r8a77980-cpg-mssr.c

> @@ -21,6 +22,10 @@
>  #include "renesas-cpg-mssr.h"
>  #include "rcar-gen3-cpg.h"
>
> +enum r8a77980_clk_types {
> +       CLK_TYPE_R8A77980_RPCSRC = CLK_TYPE_GEN3_SOC_BASE,

Rename and move to rcar_gen3_clk_types?

> @@ -215,6 +232,27 @@ static int __init r8a77980_cpg_mssr_init
>         return rcar_gen3_cpg_init(cpg_pll_config, CLK_EXTALR, cpg_mode);
>  }
>
> +static struct clk * __init r8a77980_cpg_clk_register(struct device *dev,
> +       const struct cpg_core_clk *core, const struct cpg_mssr_info *info,
> +       struct clk **clks, void __iomem *base,
> +       struct raw_notifier_head *notifiers)
> +{
> +       if (core->type == CLK_TYPE_R8A77980_RPCSRC) {

I'd use a switch() statement here, for consistency with other drivers.

> +               const struct clk *parent = clks[core->parent];
> +
> +               if (IS_ERR(parent))
> +                       return ERR_CAST(parent);
> +
> +               return clk_register_divider_table(NULL, core->name,
> +                                                 __clk_get_name(parent), 0,
> +                                                 base + CPG_RPCCKCR, 3, 2, 0,
> +                                                 cpg_rpcsrc_div_table, NULL);

Don't you need a spinlock (last parameter, currently NULL)?
This needs to be synchronized with controlling the RPCD2 and RPC clocks,
as they operate on the same register.

However, that would deadlock, as enabling e.g. RPC-IF will enable
all parent clocks?

> +       } else  {
> +               return rcar_gen3_cpg_clk_register(dev, core, info, clks, base,
> +                                                 notifiers);
> +       }

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
Sergei Shtylyov Nov. 27, 2018, 5:45 p.m. UTC | #2
On 11/23/2018 03:59 PM, Geert Uytterhoeven wrote:

>> Add the RPCSRC internal clock on R-Car V3H (R8A77980) -- it's controlled
>> by the RPCCKCR.DIV[4:3] on all the R-Car gen3 SoCs except V3M (R8A77970)
>> but the encoding of this field is different between SoCs.
> 
> Given the tables and encoding are the same on H3, M3-W, M3-N, and V3H,
> I think it makes sense to move the common support to rcar-gen3-cpg.c.

   Done.

> Heck, you could even just select a different table on D3/E3 using
> soc_device_match(), if only one encoding would not select a different parent
> clock :-(

   Indeed...

>> Add the RPC[D2] clocks (derived from this internal clock) and the RPC-IF
>> module clock as well...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
>> --- renesas-drivers.orig/drivers/clk/renesas/r8a77980-cpg-mssr.c
>> +++ renesas-drivers/drivers/clk/renesas/r8a77980-cpg-mssr.c
[...]
>> +               const struct clk *parent = clks[core->parent];
>> +
>> +               if (IS_ERR(parent))
>> +                       return ERR_CAST(parent);
>> +
>> +               return clk_register_divider_table(NULL, core->name,
>> +                                                 __clk_get_name(parent), 0,
>> +                                                 base + CPG_RPCCKCR, 3, 2, 0,
>> +                                                 cpg_rpcsrc_div_table, NULL);
> 
> Don't you need a spinlock (last parameter, currently NULL)?
> This needs to be synchronized with controlling the RPCD2 and RPC clocks,
> as they operate on the same register.

   Indeed. How about the RMW accesses to the other register? I'd like to place
the lock/unlock right in cpg_reg_modify()...

> However, that would deadlock, as enabling e.g. RPC-IF will enable
> all parent clocks?

  Could toy please elaborate?

>> +       } else  {
>> +               return rcar_gen3_cpg_clk_register(dev, core, info, clks, base,
>> +                                                 notifiers);
>> +       }
> 
> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergei
Geert Uytterhoeven Nov. 27, 2018, 5:51 p.m. UTC | #3
Hi Sergei,

On Tue, Nov 27, 2018 at 6:45 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 11/23/2018 03:59 PM, Geert Uytterhoeven wrote:
> >> Add the RPCSRC internal clock on R-Car V3H (R8A77980) -- it's controlled
> >> by the RPCCKCR.DIV[4:3] on all the R-Car gen3 SoCs except V3M (R8A77970)
> >> but the encoding of this field is different between SoCs.

> >> --- renesas-drivers.orig/drivers/clk/renesas/r8a77980-cpg-mssr.c
> >> +++ renesas-drivers/drivers/clk/renesas/r8a77980-cpg-mssr.c
> [...]
> >> +               const struct clk *parent = clks[core->parent];
> >> +
> >> +               if (IS_ERR(parent))
> >> +                       return ERR_CAST(parent);
> >> +
> >> +               return clk_register_divider_table(NULL, core->name,
> >> +                                                 __clk_get_name(parent), 0,
> >> +                                                 base + CPG_RPCCKCR, 3, 2, 0,
> >> +                                                 cpg_rpcsrc_div_table, NULL);
> >
> > Don't you need a spinlock (last parameter, currently NULL)?
> > This needs to be synchronized with controlling the RPCD2 and RPC clocks,
> > as they operate on the same register.
>
>    Indeed. How about the RMW accesses to the other register? I'd like to place
> the lock/unlock right in cpg_reg_modify()...

Yes, RMW, too.

> > However, that would deadlock, as enabling e.g. RPC-IF will enable
> > all parent clocks?
>
>   Could toy please elaborate?

Sorry, I incorrectly assumed it would hold the lock while calling into the
parent, which would deadlock if using the same lock.
But as it only holds the lock for register access, it's fine.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

Index: renesas-drivers/drivers/clk/renesas/r8a77980-cpg-mssr.c
===================================================================
--- renesas-drivers.orig/drivers/clk/renesas/r8a77980-cpg-mssr.c
+++ renesas-drivers/drivers/clk/renesas/r8a77980-cpg-mssr.c
@@ -10,6 +10,7 @@ 
  * Copyright (C) 2015 Glider bvba
  */
 
+#include <linux/clk-provider.h>
 #include <linux/device.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -21,6 +22,10 @@ 
 #include "renesas-cpg-mssr.h"
 #include "rcar-gen3-cpg.h"
 
+enum r8a77980_clk_types {
+	CLK_TYPE_R8A77980_RPCSRC = CLK_TYPE_GEN3_SOC_BASE,
+};
+
 enum clk_ids {
 	/* Core Clock Outputs exported to DT */
 	LAST_DT_CORE_CLK = R8A77980_CLK_OSC,
@@ -41,12 +46,17 @@  enum clk_ids {
 	CLK_S2,
 	CLK_S3,
 	CLK_SDSRC,
+	CLK_RPCSRC,
 	CLK_OCO,
 
 	/* Module Clocks */
 	MOD_CLK_BASE
 };
 
+static const struct clk_div_table cpg_rpcsrc_div_table[] = {
+	{ 2, 5 }, { 3, 6 }, { 0, 0 },
+};
+
 static const struct cpg_core_clk r8a77980_core_clks[] __initconst = {
 	/* External Clock Inputs */
 	DEF_INPUT("extal",  CLK_EXTAL),
@@ -65,8 +75,14 @@  static const struct cpg_core_clk r8a7798
 	DEF_FIXED(".s2",	CLK_S2,		   CLK_PLL1_DIV2,  4, 1),
 	DEF_FIXED(".s3",	CLK_S3,		   CLK_PLL1_DIV2,  6, 1),
 	DEF_FIXED(".sdsrc",	CLK_SDSRC,	   CLK_PLL1_DIV2,  2, 1),
+	DEF_BASE(".rpcsrc",	CLK_RPCSRC, CLK_TYPE_R8A77980_RPCSRC, CLK_PLL1),
 	DEF_RATE(".oco",	CLK_OCO,           32768),
 
+	DEF_BASE("rpc",		R8A77980_CLK_RPC, CLK_TYPE_GEN3_RPC,
+		 CLK_RPCSRC),
+	DEF_BASE("rpcd2",	R8A77980_CLK_RPCD2, CLK_TYPE_GEN3_RPCD2,
+		 R8A77980_CLK_RPC),
+
 	/* Core Clock Outputs */
 	DEF_FIXED("ztr",	R8A77980_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
 	DEF_FIXED("ztrd2",	R8A77980_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1),
@@ -164,6 +180,7 @@  static const struct mssr_mod_clk r8a7798
 	DEF_MOD("gpio1",		 911,	R8A77980_CLK_CP),
 	DEF_MOD("gpio0",		 912,	R8A77980_CLK_CP),
 	DEF_MOD("can-fd",		 914,	R8A77980_CLK_S3D2),
+	DEF_MOD("rpc-if",		 917,	R8A77980_CLK_RPC),
 	DEF_MOD("i2c4",			 927,	R8A77980_CLK_S0D6),
 	DEF_MOD("i2c3",			 928,	R8A77980_CLK_S0D6),
 	DEF_MOD("i2c2",			 929,	R8A77980_CLK_S3D2),
@@ -215,6 +232,27 @@  static int __init r8a77980_cpg_mssr_init
 	return rcar_gen3_cpg_init(cpg_pll_config, CLK_EXTALR, cpg_mode);
 }
 
+static struct clk * __init r8a77980_cpg_clk_register(struct device *dev,
+	const struct cpg_core_clk *core, const struct cpg_mssr_info *info,
+	struct clk **clks, void __iomem *base,
+	struct raw_notifier_head *notifiers)
+{
+	if (core->type == CLK_TYPE_R8A77980_RPCSRC) {
+		const struct clk *parent = clks[core->parent];
+
+		if (IS_ERR(parent))
+			return ERR_CAST(parent);
+
+		return clk_register_divider_table(NULL, core->name,
+						  __clk_get_name(parent), 0,
+						  base + CPG_RPCCKCR, 3, 2, 0,
+						  cpg_rpcsrc_div_table, NULL);
+	} else	{
+		return rcar_gen3_cpg_clk_register(dev, core, info, clks, base,
+						  notifiers);
+	}
+}
+
 const struct cpg_mssr_info r8a77980_cpg_mssr_info __initconst = {
 	/* Core Clocks */
 	.core_clks = r8a77980_core_clks,
@@ -233,5 +271,5 @@  const struct cpg_mssr_info r8a77980_cpg_
 
 	/* Callbacks */
 	.init = r8a77980_cpg_mssr_init,
-	.cpg_clk_register = rcar_gen3_cpg_clk_register,
+	.cpg_clk_register = r8a77980_cpg_clk_register,
 };