| Message ID | 29eba2fe-942a-6b71-afc1-e30ac603f162@cogentembedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Renesas R8A77980 CPG/MSSR RPC clock support | expand |
Hi Sergei, On Tue, Dec 4, 2018 at 8:51 PM Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > The RPCSRC internal clock is 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; it makes sense to support the most common case > of this encoding in the R-Car gen3 CPG driver... > > After adding the RPCSRC clock, we can add the RPC[D2] clocks derived from > it and controlled by the RPCCKCR register on all the R-Car gen3 SoCs except > V3M (R8A77970); the composite clock driver seems handy for this task, using > the spinlock added in the previous patch... > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > --- > Changes in version 2: > - merged in the RPCD2 clock support from the next patch; > - moved in the RPCSRC clock support from the R8A77980 CPG/MSSR driver patch; > - switched the RPC and RPCSD2 clock support to the composite clock driver; > - changed the 1st parameter of cpg_rpc[d2]_clk_register(); > - rewrote the patch description, renamed the patch. Thanks for the update! > --- renesas-drivers.orig/drivers/clk/renesas/rcar-gen3-cpg.c > +++ renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.c > @@ -415,6 +415,90 @@ free_clock: > return clk; > } > > +struct rpc_clock { > + struct clk_divider div; > + struct clk_gate gate; > + struct cpg_simple_notifier csn; > +}; > + > +static const struct clk_div_table cpg_rpcsrc_div_table[] = { > + { 2, 5 }, { 3, 6 }, { 0, 0 }, > +}; > + > +static const struct clk_div_table cpg_rpc_div_table[] = { > + { 1, 2 }, { 3, 4 }, { 5, 6 }, { 7, 8 }, { 0, 0 }, > +}; > + > +static struct clk * __init cpg_rpc_clk_register(const char *name, > + void __iomem *base, const char *parent_name, > + struct raw_notifier_head *notifiers) > +{ > + struct rpc_clock *rpc; > + struct clk *clk; > + > + rpc = kzalloc(sizeof(*rpc), GFP_KERNEL); > + if (!rpc) > + return ERR_PTR(-ENOMEM); > + > + rpc->div.reg = base + CPG_RPCCKCR; > + rpc->div.width = 3; > + rpc->div.table = cpg_rpc_div_table; > + rpc->div.lock = &cpg_lock; > + > + rpc->gate.reg = base + CPG_RPCCKCR; > + rpc->gate.bit_idx = 8; > + rpc->gate.flags = CLK_GATE_SET_TO_DISABLE; > + rpc->gate.lock = &cpg_lock; > + > + rpc->csn.reg = base + CPG_RPCCKCR; > + > + clk = clk_register_composite(NULL, name, &parent_name, 1, NULL, NULL, > + &rpc->div.hw, &clk_divider_ops, > + &rpc->gate.hw, &clk_gate_ops, 0); > + if (IS_ERR(clk)) > + kfree(rpc); > + > + cpg_simple_notifier_register(notifiers, &rpc->csn); > + return clk; > +} > + > +static struct clk * __init cpg_rpcd2_clk_register(const char *name, > + void __iomem *base, > + const char *parent_name) > +{ > + struct clk_fixed_factor *fixed; > + struct clk_gate *gate; > + struct clk *clk; > + > + fixed = kzalloc(sizeof(*fixed), GFP_KERNEL); > + if (!fixed) > + return ERR_PTR(-ENOMEM); > + > + fixed->mult = 1; > + fixed->div = 2; > + > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > + if (!gate) { > + kfree(fixed); > + return ERR_PTR(-ENOMEM); > + } Why allocate two separate structures here, instead of grouping them in a single struct rpcd2_clock structure, like for the RPC clock? > + > + gate->reg = base + CPG_RPCCKCR; > + gate->bit_idx = 9; > + gate->flags = CLK_GATE_SET_TO_DISABLE; > + gate->lock = &cpg_lock; > + > + clk = clk_register_composite(NULL, name, &parent_name, 1, NULL, NULL, > + &fixed->hw, &clk_fixed_factor_ops, > + &gate->hw, &clk_gate_ops, 0); > + if (IS_ERR(clk)) { > + kfree(fixed); > + kfree(gate); > + } I first wondered why there's no notifier to save/restore the clock during system PSCI suspend/resume, until I realized the RPC and RPCD2 clocks share the same hardware register, so saving/restoring the register once is sufficient. A comment (in struct rpc_clock?) explaining that would be appreciated. The rest looks good to me. Using the composite clock seems to have reduced LoC by ca. 60%, nice! Gr{oetje,eeting}s, Geert
Index: renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.c =================================================================== --- renesas-drivers.orig/drivers/clk/renesas/rcar-gen3-cpg.c +++ renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.c @@ -415,6 +415,90 @@ free_clock: return clk; } +struct rpc_clock { + struct clk_divider div; + struct clk_gate gate; + struct cpg_simple_notifier csn; +}; + +static const struct clk_div_table cpg_rpcsrc_div_table[] = { + { 2, 5 }, { 3, 6 }, { 0, 0 }, +}; + +static const struct clk_div_table cpg_rpc_div_table[] = { + { 1, 2 }, { 3, 4 }, { 5, 6 }, { 7, 8 }, { 0, 0 }, +}; + +static struct clk * __init cpg_rpc_clk_register(const char *name, + void __iomem *base, const char *parent_name, + struct raw_notifier_head *notifiers) +{ + struct rpc_clock *rpc; + struct clk *clk; + + rpc = kzalloc(sizeof(*rpc), GFP_KERNEL); + if (!rpc) + return ERR_PTR(-ENOMEM); + + rpc->div.reg = base + CPG_RPCCKCR; + rpc->div.width = 3; + rpc->div.table = cpg_rpc_div_table; + rpc->div.lock = &cpg_lock; + + rpc->gate.reg = base + CPG_RPCCKCR; + rpc->gate.bit_idx = 8; + rpc->gate.flags = CLK_GATE_SET_TO_DISABLE; + rpc->gate.lock = &cpg_lock; + + rpc->csn.reg = base + CPG_RPCCKCR; + + clk = clk_register_composite(NULL, name, &parent_name, 1, NULL, NULL, + &rpc->div.hw, &clk_divider_ops, + &rpc->gate.hw, &clk_gate_ops, 0); + if (IS_ERR(clk)) + kfree(rpc); + + cpg_simple_notifier_register(notifiers, &rpc->csn); + return clk; +} + +static struct clk * __init cpg_rpcd2_clk_register(const char *name, + void __iomem *base, + const char *parent_name) +{ + struct clk_fixed_factor *fixed; + struct clk_gate *gate; + struct clk *clk; + + fixed = kzalloc(sizeof(*fixed), GFP_KERNEL); + if (!fixed) + return ERR_PTR(-ENOMEM); + + fixed->mult = 1; + fixed->div = 2; + + gate = kzalloc(sizeof(*gate), GFP_KERNEL); + if (!gate) { + kfree(fixed); + return ERR_PTR(-ENOMEM); + } + + gate->reg = base + CPG_RPCCKCR; + gate->bit_idx = 9; + gate->flags = CLK_GATE_SET_TO_DISABLE; + gate->lock = &cpg_lock; + + clk = clk_register_composite(NULL, name, &parent_name, 1, NULL, NULL, + &fixed->hw, &clk_fixed_factor_ops, + &gate->hw, &clk_gate_ops, 0); + if (IS_ERR(clk)) { + kfree(fixed); + kfree(gate); + } + + return clk; +} + static const struct rcar_gen3_cpg_pll_config *cpg_pll_config __initdata; static unsigned int cpg_clk_extalr __initdata; @@ -589,6 +673,21 @@ struct clk * __init rcar_gen3_cpg_clk_re } break; + case CLK_TYPE_GEN3_RPCSRC: + return clk_register_divider_table(NULL, core->name, + __clk_get_name(parent), 0, + base + CPG_RPCCKCR, 3, 2, 0, + cpg_rpcsrc_div_table, + &cpg_lock); + + case CLK_TYPE_GEN3_RPC: + return cpg_rpc_clk_register(core->name, base, + __clk_get_name(parent), notifiers); + + case CLK_TYPE_GEN3_RPCD2: + return cpg_rpcd2_clk_register(core->name, base, + __clk_get_name(parent)); + default: return ERR_PTR(-EINVAL); } Index: renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.h =================================================================== --- renesas-drivers.orig/drivers/clk/renesas/rcar-gen3-cpg.h +++ renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.h @@ -23,6 +23,9 @@ enum rcar_gen3_clk_types { CLK_TYPE_GEN3_Z2, CLK_TYPE_GEN3_OSC, /* OSC EXTAL predivider and fixed divider */ CLK_TYPE_GEN3_RCKSEL, /* Select parent/divider using RCKCR.CKSEL */ + CLK_TYPE_GEN3_RPCSRC, + CLK_TYPE_GEN3_RPC, + CLK_TYPE_GEN3_RPCD2, /* SoC specific definitions start here */ CLK_TYPE_GEN3_SOC_BASE, @@ -57,6 +60,7 @@ struct rcar_gen3_cpg_pll_config { u8 osc_prediv; }; +#define CPG_RPCCKCR 0x238 #define CPG_RCKCR 0x240 struct clk *rcar_gen3_cpg_clk_register(struct device *dev,
The RPCSRC internal clock is 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; it makes sense to support the most common case of this encoding in the R-Car gen3 CPG driver... After adding the RPCSRC clock, we can add the RPC[D2] clocks derived from it and controlled by the RPCCKCR register on all the R-Car gen3 SoCs except V3M (R8A77970); the composite clock driver seems handy for this task, using the spinlock added in the previous patch... Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 2: - merged in the RPCD2 clock support from the next patch; - moved in the RPCSRC clock support from the R8A77980 CPG/MSSR driver patch; - switched the RPC and RPCSD2 clock support to the composite clock driver; - changed the 1st parameter of cpg_rpc[d2]_clk_register(); - rewrote the patch description, renamed the patch. drivers/clk/renesas/rcar-gen3-cpg.c | 99 ++++++++++++++++++++++++++++++++++++ drivers/clk/renesas/rcar-gen3-cpg.h | 4 + 2 files changed, 103 insertions(+)