| 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 |
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
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
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
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, };
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(-)