From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f52.google.com ([209.85.215.52]:34663 "EHLO mail-lf0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756484AbcKBSLZ (ORCPT ); Wed, 2 Nov 2016 14:11:25 -0400 Received: by mail-lf0-f52.google.com with SMTP id b81so19759868lfe.1 for ; Wed, 02 Nov 2016 11:11:24 -0700 (PDT) Subject: Re: [PATCH v2] clk: renesas: cpg-mssr: add common R-Car Gen2 support To: Geert Uytterhoeven References: <19700058.YBgRO3SXUI@wasted.cogentembedded.com> <3797605.lKJkRScdp8@wasted.cogentembedded.com> Cc: Michael Turquette , linux-clk , Stephen Boyd , Linux-Renesas From: Sergei Shtylyov Message-ID: Date: Wed, 2 Nov 2016 21:11:20 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: On 11/02/2016 02:38 PM, Geert Uytterhoeven 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 >>> . >>> >>> Signed-off-by: Sergei Shtylyov >>> Reviewed-by: Geert Uytterhoeven >>> >>> --- >>> 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). We'll then have to lie about the PLL0 output freq? I don't like it... MBR, Sergei