Hi Sergei,
On Thu, Nov 22, 2018 at 7:43 PM Sergei Shtylyov
<sergei.shtylyov@xxxxxxxxxxxxxxxxxx> wrote:
> Add the RPCD2 clock for the R-Car gen3 SoCs -- this clock is en/disabled
> via the RPCCKCR register on all the R-Car gen3 SoCs except V3M (R8A77970)
> and has a fixed divisor of 2 (applied to the RPC clock).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
Thanks for your patch!
> --- renesas-drivers.orig/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -524,6 +524,89 @@ static struct clk * __init cpg_rpc_clk_r
> return clk;
> }
>
> +static int cpg_rpcd2_clock_enable(struct clk_hw *hw)
> +{
> + struct rpc_clock *clock = to_rpc_clock(hw);
> +
> + cpg_reg_modify(clock->reg, CPG_RPC_CKSTP2, 0);
> +
> + return 0;
> +}
> +
> +static void cpg_rpcd2_clock_disable(struct clk_hw *hw)
> +{
> + struct rpc_clock *clock = to_rpc_clock(hw);
> +
> + cpg_reg_modify(clock->reg, 0, CPG_RPC_CKSTP2);
> +}
> +
> +static int cpg_rpcd2_clock_is_enabled(struct clk_hw *hw)
> +{
> + struct rpc_clock *clock = to_rpc_clock(hw);
> +
> + return !(readl(clock->reg) & CPG_RPC_CKSTP2);
> +}
As the above 3 functions are identical to their rpc_*() counterparts,
except for the bit touched, would it make sense to share them, e.g. by
storing the bit number in struct rpc_clock?
> +static long cpg_rpcd2_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + return *parent_rate / 2;
Given you set CLK_SET_RATE_PARENT, shouldn't you propagate up,
cfr. drivers/clk/clk-fixed-factor.c:clk_factor_round_rate()?
> +}
> +static struct clk * __init cpg_rpcd2_clk_register(const struct cpg_core_clk *core,
> + void __iomem *base,
> + const char *parent_name)
> +{
> + struct clk_init_data init;
> + struct rpc_clock *clock;
> + struct clk *clk;
> +
> + clock = kzalloc(sizeof(*clock), GFP_KERNEL);
> + if (!clock)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = core->name;
> + init.ops = &cpg_rpcd2_clock_ops;
> + init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
I don't think CLK_IS_BASIC is appropriate?
#define CLK_IS_BASIC BIT(5) /* Basic clk, can't do a to_clk_foo() */
Given RPCD2 is the combination of a gate and fixed-divider clock, would
it make sense to use clk_composite?
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
