Hi Sergei,
On Wed, Nov 2, 2016 at 12:08 PM, Geert Uytterhoeven
<[email protected]> wrote:
> On Thu, Oct 27, 2016 at 9:53 PM, Sergei Shtylyov
> <[email protected]> wrote:
>> Add the common R-Car Gen2 (and RZ/G) Clock Pulse Generator / Module Standby
>> and Software Reset support code, using the CPG/MSSR driver core.
>>
>> Based on the proof-of-concept R8A7791 CPG/MSSR patch by Geert Uytterhoeven
>> <[email protected]>.
>>
>> Signed-off-by: Sergei Shtylyov <[email protected]>
>> Reviewed-by: Geert Uytterhoeven <[email protected]>
>>
>> ---
>> This patch is against the 'clk-next' branch of CLK group's 'linux.git' repo.
>>
>> Changes in version 2:
>> - added support for non-existing PLL0CR;
>> - removed the function reading the mode pins;
>> - added/used the #define's for PLL0CR.STC;
>> - used CPG_FRQCRC_ZFC_SHIFT to #define CPG_FRQCRC_ZFC_MASK;
>> - removed rcar_gen2_read_modemr();
>> - added Geert's tag.
>>
>> drivers/clk/renesas/rcar-gen2-cpg.c | 369
>> ++++++++++++++++++++++++++++++++++++
>> drivers/clk/renesas/rcar-gen2-cpg.h | 42 ++++
>> 2 files changed, 411 insertions(+)
>>
>> Index: linux/drivers/clk/renesas/rcar-gen2-cpg.c
>> ===================================================================
>> --- /dev/null
>> +++ linux/drivers/clk/renesas/rcar-gen2-cpg.c
>> @@ -0,0 +1,369 @@
>
>> +struct clk * __init rcar_gen2_cpg_clk_register(struct device *dev,
>> + const struct cpg_core_clk
>> *core,
>> + const struct cpg_mssr_info
>> *info,
>> + struct clk **clks,
>> + void __iomem *base)
>> +{
>
>> + case CLK_TYPE_GEN2_PLL0:
>> + /*
>> + * PLL0 is a configurable multiplier clock except on R-Car
>> E2.
>
> ... and V2H.
>
>> + * Register the PLL0 clock as a fixed factor clock for now as
>> + * there's no generic multiplier clock implementation and we
>> + * currently have no need to change the multiplier value.
>> + */
>> + mult = cpg_pll_config->pll0_mult;
>> + if (mult) {
>> + /* PLL0 is VCO/3 on R-Car E2 */
>
> ... and V2H.
>
>> + div = 3;
After seeing the r8a7745 driver, I think it would be better to use a divider
value of 1 here (dropping the need for this branch), and handle the /3 in the
SoC-specific driver.
This would make it more similar to the other SoCs here (div = 1 in the else
branch below), and to similar clocks in the SoC-specific driver
(e.g. DEF_FIXED("zg", ..., 3, 1) in r8a7743-cpg-mssr.c).
>> + } else {
>> + u32 pll0cr = readl(base + CPG_PLL0CR);
>> +
>> + mult = ((pll0cr & CPG_PLL0CR_STC_MASK) >>
>> + CPG_PLL0CR_STC_SHIFT) + 1;
>> + }
>
> Looks OK to me. I assume you've verified the Z2 clock frequency in
> /sys/kernel/debug/clk/clk_summary?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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