From patchwork Wed Aug 19 07:49:28 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Geert Uytterhoeven X-Patchwork-Id: 7036041 Return-Path: X-Original-To: patchwork-linux-sh@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 315A29F344 for ; Wed, 19 Aug 2015 07:49:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A251920873 for ; Wed, 19 Aug 2015 07:49:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5829E207EF for ; Wed, 19 Aug 2015 07:49:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753120AbbHSHta (ORCPT ); Wed, 19 Aug 2015 03:49:30 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:34420 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752813AbbHSHt3 (ORCPT ); Wed, 19 Aug 2015 03:49:29 -0400 Received: by oiey141 with SMTP id y141so8756348oie.1; Wed, 19 Aug 2015 00:49:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=xcKgY5EmjfCVolswWInEgqarJ3f7m0Bd+XABHFmd1nY=; b=d1iGS0V9rQq83P0ychpgCjaIEDBcV762JJUYoXr3Kumybiw6cnWpTs3TXoAAclkghb TODbRLOGxp1HtdeASyOgH4Zy8/H+zc+QvVW+dSLvBWQ9+7wtf5n8ln0WUTOFSs8bu2Bm W5MDFwaxF7nMg0cQrbfc5xrtz5VjGL2nWaPpKkvHEHzmsdhlDa82Urb5gyKF3X82lMhf HL1YT+IC/UuR9dSbIJ4TW8Opq1GPoSn5I+6fo+CtpEbnbCyHzu9HjSuVNOppMt+Ipu+G RPBzYtw4yYJKpbJqOZSEeFMVwxhsTuB3GBtAOwcVWD4+pvHrPgjDGSFozKBNidGqTaV4 LSZg== MIME-Version: 1.0 X-Received: by 10.202.199.69 with SMTP id x66mr9281866oif.121.1439970568619; Wed, 19 Aug 2015 00:49:28 -0700 (PDT) Received: by 10.60.168.82 with HTTP; Wed, 19 Aug 2015 00:49:28 -0700 (PDT) In-Reply-To: <20150818002018.31346.63666@quantum> References: <877fpdw3av.wl%kuninori.morimoto.gx@renesas.com> <873801w377.wl%kuninori.morimoto.gx@renesas.com> <1559394.i3FG4v33fI@avalon> <20150818002018.31346.63666@quantum> Date: Wed, 19 Aug 2015 09:49:28 +0200 X-Google-Sender-Auth: RXuW8jx-d7cpVUd1FtPvhqoYGOU Message-ID: Subject: Re: [PATCH 3/4 v3][RFC] arm64: renesas: Add initial r8a7795 SoC support From: Geert Uytterhoeven To: Michael Turquette Cc: Laurent Pinchart , Kuninori Morimoto , Simon , Magnus , Linux-sh list , linux-clk@vger.kernel.org, "devicetree@vger.kernel.org" Sender: linux-sh-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sh@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Mike, On Tue, Aug 18, 2015 at 2:20 AM, Michael Turquette wrote: > Quoting Geert Uytterhoeven (2015-08-04 05:34:06) >> On Tue, Aug 4, 2015 at 2:22 PM, Laurent Pinchart >> wrote: >> > On Monday 03 August 2015 01:53:23 Kuninori Morimoto wrote: >> >> --- /dev/null >> >> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> >> @@ -0,0 +1,93 @@ >> >> +/ { >> >> >> + clocks { >> > >> > Let's try to make it right from the start on Gen3. The CPG node should be a >> > direct child of the bus node mentioned above, and the MSTP clocks should be >> > children of the CPG node. >> >> Agreed. >> >> > I'm not sure where to put the non-memory-mapped clocks though, should they be >> > directly under the root node ? It would make sense for extal_clk, but how >> > about the fixed-factor clocks ? Should they be children of the CPG node too ? >> >> I believe the current trend is to put clocks like "extal_clk" under the root >> node. >> As the fixed-factor clocks are generated by the CPG module, and we do have >> a device node for it, I'd make them children of the CPG node, too. >> >> Any comments from the clk+dt experts? > > I don't know if anyone is an expert ;-) > > extal_clk should be under the root node. That is true for all > board-level clocks and clock controllers. OK. > Within the SoC we want to model the clock controller as a node in DT, > not necessarily all of the individual clocks. So you definitely need a > "cpg" node in DT with #clock-cells > 0. OK. > Whether or not you enumerate the individual clocks in DT is up to you. I > do not like the data-driven approach of putting the clock definition > data into DT. It makes it awkward to do things like set a flag on a > single clock later on. Simply using the clock controller phandle plus > one or more offsets is preferred over a per-clock phandle. > > Stephen and I have been discussing what a formal clock-controller > binding would look like, and one item we came up with is that any > sub-nodes of the controller would not be allowed to have a #clock-cells > property. Does that mean #clock-cells is inherited from the parent (cpg) node, or does that preclude having any "fixed-factor-clock" or "renesas,cpg-div6-clock" (both use #clock-cells = <0>), or "renesas,cpg-mstp-clocks" subnodes? > Also, while you're thinking about the perfect clock binding, please do > consider dropping clock-output-names if you can. Specifying clock-names > alongside the clocks property inside of the consumer node is a bit more > elegant in my opinion. This is also a bit easier if you think about > expressing your clock data with C inside of your provider driver. Makes sense. I don't think anything relies on the "clock-output-names" ... currently. I was going to use it for identifying the GIC clock, though: (FTR, on R-Car Gen3 the clock is called "intc-ap", not "intc-sys", doh...). This does put some reliance on (undocumented) naming in DT, though, so not having the "clock-output-names" would solve that. However, while "intc-sys"/"intc-ap" (if existent) is always MSTP408, MSTP408 is used for other modules on R-Car Gen1 and most older SoCs. so it would complicate the driver code. >> >> + #address-cells = <2>; >> >> + #size-cells = <2>; >> >> + ranges; >> >> + >> >> + extal_clk: extal_clk { >> >> + compatible = "fixed-clock"; >> >> + #clock-cells = <0>; >> >> + clock-frequency = <0>; >> >> + clock-output-names = "extal"; >> >> + }; >> >> + cpg_clocks: cpg_clocks@e6150000 { >> >> + compatible = "renesas,r8a7795-cpg-clocks", >> >> + "renesas,rcar-gen3-cpg-clocks"; >> >> + reg = <0 0xe6150000 0 0x1000>; >> >> + clocks = <&extal_clk>; >> >> + #clock-cells = <1>; >> >> + clock-output-names = "main", "pll0", "pll1","pll2", >> >> + "pll3", "pll4"; >> >> + }; >> >> + p_clk: p_clk { >> >> + compatible = "fixed-factor-clock"; >> >> + clocks = <&cpg_clocks RCAR_GEN3_CLK_PLL1>; >> >> + #clock-cells = <0>; >> >> + clock-div = <24>; >> >> + clock-mult = <1>; >> >> + clock-output-names = "p"; >> >> + }; >> >> + mstp3_clks: mstp3_clks@e615013c { >> >> + compatible = "renesas,r8a7795-mstp-clocks", >> >> + "renesas,cpg-mstp-clocks"; >> >> + reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>; >> >> + clocks = <&p_clk>; >> >> + #clock-cells = <1>; >> >> + renesas,clock-indices = ; >> >> + clock-output-names = "irda"; >> >> + }; >> >> + }; >> >> +}; 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/drivers/clk/shmobile/clk-mstp.c +++ b/drivers/clk/shmobile/clk-mstp.c @@ -144,6 +144,9 @@ cpg_mstp_clock_register(const char *name, const char *paren init.name = name; init.ops = &cpg_mstp_clock_ops; init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT; + /* INTC-SYS is the module clock of the GIC, and must not be disabled */ + if (!strcmp(name, "intc-sys")) + init.flags |= CLK_ENABLE_HAND_OFF; init.parent_names = &parent_name; init.num_parents = 1;