From: Simon Horman <horms@verge.net.au>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 04/19] ARM: shmobile: r8a7779: Add clocks
Date: Fri, 14 Mar 2014 08:08:04 +0000 [thread overview]
Message-ID: <20140314080803.GB31900@verge.net.au> (raw)
In-Reply-To: <2144303.VvnA5cuI5A@avalon>
Hi Laurent,
On Thu, Mar 13, 2014 at 10:54:03AM +0100, Laurent Pinchart wrote:
> Hi Simon,
>
> Thank you for the patch.
Thanks for your review.
I will fix things up as you suggest.
>
> On Thursday 13 March 2014 17:59:47 Simon Horman wrote:
> > Declare all core and MSTP clocks currently used by r8a7779-based boards.
> >
> > Based on work by Laurent Pinchart for the r8a7790 and r8a7791 SoCs.
> >
> > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >
> > ---
> > v4
> > * As suggested by Geert Uytterhoeven
> > - Wrap lines at < 80columns
> > - Beef-up short lines towards 80 columns
> > * As suggested by Laurent Pinchart
> > - Add HSCIF clocks
> > - Correct many clock sources
> > - Correct reg of cpg_clocks
> > * Add CPG clock "b" which is now part of the binding
> >
> > v3
> > * As suggested by Laurent Pinchart
> > - Add and use extal_clk
> > - Fix bogus status register use for MSTP clocks
> > - Fix bogus mstp3_cls to use its own entries rather than
> > that of mstp1_clks
> >
> > * Update to use "main" in cpg_clocks as per updated
> > binding in previous patch
> > * Update for new, consolidated and renamed index macros
> > - R8A7779_CLK_ETHER
> > - R8A7779_CLK_HSCIF
> > - R8A7779_CLK_HSPI
> > - R8A7779_CLK_MMC0,1
> > - R8A7779_CLK_PCIE
> > - R8A7779_CLK_USB01,2
> > ---
> > arch/arm/boot/dts/r8a7779.dtsi | 151 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 151 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi
> > index d0561d4..c0802aa 100644
> > --- a/arch/arm/boot/dts/r8a7779.dtsi
> > +++ b/arch/arm/boot/dts/r8a7779.dtsi
> > @@ -11,6 +11,7 @@
> >
> > /include/ "skeleton.dtsi"
> >
> > +#include <dt-bindings/clock/r8a7779-clock.h>
> > #include <dt-bindings/interrupt-controller/irq.h>
> >
> > / {
> > @@ -278,4 +279,154 @@
> > interrupts = <0 75 IRQ_TYPE_LEVEL_HIGH>;
> > status = "disabled";
> > };
> > +
> > + clocks {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + /* External root clock */
> > + extal_clk: extal_clk {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + /* This value must be overriden by the board. */
> > + clock-frequency = <0>;
> > + clock-output-names = "extal";
> > + };
> > +
> > + /* Special CPG clocks */
> > + cpg_clocks: cpg_clocks@0xe6150000 {
> > + compatible = "renesas,r8a7779-cpg-clocks";
> > + reg = <0 0xffc80000 0 0x30>;
> > + clocks = <&extal_clk>;
> > + #clock-cells = <1>;
> > + clock-output-names = "plla", "z", "zs", "s",
> > + "s1", "p", "b", "out";
> > + };
> > +
> > + /* Fixed factor clocks */
> > + i_clk: i_clk {
> > + compatible = "fixed-factor-clock";
> > + clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
> > + #clock-cells = <0>;
> > + clock-div = <2>;
> > + clock-mult = <1>;
> > + clock-output-names = "i";
> > + };
> > + s3_clk: s3_clk {
> > + compatible = "fixed-factor-clock";
> > + clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
> > + #clock-cells = <0>;
> > + clock-div = <8>;
> > + clock-mult = <1>;
> > + clock-output-names = "s3";
> > + };
> > + s4_clk: s4_clk {
> > + compatible = "fixed-factor-clock";
> > + clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
> > + #clock-cells = <0>;
> > + clock-div = <16>;
> > + clock-mult = <1>;
> > + clock-output-names = "s4";
> > + };
> > + g_clk: g_clk {
> > + compatible = "fixed-factor-clock";
> > + clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
> > + #clock-cells = <0>;
> > + clock-div = <24>;
> > + clock-mult = <1>;
> > + clock-output-names = "g";
> > + };
> > +
> > + /* Gate clocks */
> > + mstp0_clks: mstp0_clks {
> > + compatible = "renesas,r8a7779-mstp-clocks",
> > + "renesas,cpg-mstp-clocks";
> > + reg = <0 0xffc80030 0 4>;
> > + clocks = <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_S1>,
> > + <&cpg_clocks R8A7779_CLK_S1>,
> > + <&cpg_clocks R8A7779_CLK_S1>,
> > + <&cpg_clocks R8A7779_CLK_S1>,
> > + <&cpg_clocks R8A7779_CLK_S1>,
> > + <&cpg_clocks R8A7779_CLK_S1>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>;
> > + #clock-cells = <1>;
> > + renesas,clock-indices = <
> > + R8A7779_CLK_HSPI R8A7779_CLK_HSPI
> > + R8A7779_CLK_HSPI
>
> I would define a single HSPI clock and reference it from the three HSPI
> instances. Having three identical copies of a single clock will not work, the
> first clock to be disabled will disable the other two as they would all share
> the same register bit. Don't forget to remove two of the three CLK_S
> references above.
>
> > R8A7779_CLK_TMU0
> > + R8A7779_CLK_TMU0 R8A7779_CLK_TMU0
>
> Bad copy and paste ? This should be TMU2, TMU1 and TMU0.
>
> > + R8A7779_CLK_HSCIF0 R8A7779_CLK_HSCIF0
>
> HSCIF1 and HSCIF0 ?
>
> > + R8A7779_CLK_SCIF5 R8A7779_CLK_SCIF4
> > + R8A7779_CLK_SCIF3 R8A7779_CLK_SCIF2
> > + R8A7779_CLK_SCIF1 R8A7779_CLK_SCIF0
> > + R8A7779_CLK_I2C3 R8A7779_CLK_I2C2
> > + R8A7779_CLK_I2C1 R8A7779_CLK_I2C0
> > + >;
> > + clock-output-names > > + "hspi0", "hspi1", "hspi2", "tmu0", "tmu1",
> > + "tmu2",
>
> Following the comments above, this should become
>
> "hspi", "tmu2", "tmu1", "tmu0".
>
> > "hscif1", "hscif0", "scif5", "scif4",
> > + "scif3", "scif2", "scif1", "scif0", "i2c3",
> > + "i2c2", "i2c1", "i2c0";
> > + };
> > + mstp1_clks: mstp1_clks {
> > + compatible = "renesas,r8a7779-mstp-clocks",
> > + "renesas,cpg-mstp-clocks";
> > + reg = <0 0xffc80034 0 4>, <0 0xffc80044 0 4>;
> > + clocks = <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_S>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_P>,
> > + <&cpg_clocks R8A7779_CLK_S>;
> > + #clock-cells = <1>;
> > + renesas,clock-indices = <
> > + R8A7779_CLK_USB01 R8A7779_CLK_USB01
>
> Same as above, two instances of the same hardware clock won't fly.
>
> > + R8A7779_CLK_USB2 R8A7779_CLK_USB2
>
> Same here.
>
> > + R8A7779_CLK_DU R8A7779_CLK_VIN2
> > + R8A7779_CLK_VIN1 R8A7779_CLK_VIN0
> > + R8A7779_CLK_ETHER R8A7779_CLK_SATA
> > + R8A7779_CLK_PCIE R8A7779_CLK_VIN3
> > + >;
> > + clock-output-names > > + "ehci0", "ohci0",
> > + "ehci1", "ohci1",
>
> Following the comments above, this would become "usb01", "usb2".
>
> > + "du", "vin2",
> > + "vin1", "vin0",
> > + "ether", "sata",
> > + "pcie", "vin3";
> > + };
> > + mstp3_clks: mstp3_clks {
> > + compatible = "renesas,r8a7779-mstp-clocks",
> > + "renesas,cpg-mstp-clocks";
> > + reg = <0 0xffc8003c 0 4>;
> > + clocks = <&s4_clk>, <&s4_clk>, <&s4_clk>, <&s4_clk>,
> > + <&s4_clk>, <&s4_clk>;
> > + #clock-cells = <1>;
> > + renesas,clock-indices = <
> > + R8A7779_CLK_SDHI3 R8A7779_CLK_SDHI2
> > + R8A7779_CLK_SDHI1 R8A7779_CLK_SDHI0
> > + R8A7779_CLK_MMC1 R8A7779_CLK_MMC0
> > + >;
> > + clock-output-names > > + "sdhi3", "sdhi2", "sdhi1", "sdhi0",
> > + "mmc1", "mmc0";
> > + };
> > + };
> > };
>
> --
> Regards,
>
> Laurent Pinchart
>
next prev parent reply other threads:[~2014-03-14 8:08 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-13 8:59 [PATCH v4 00/19] ARM: shmobile: r8a7779, marzen: CCF and multiplatform Simon Horman
2014-03-13 8:59 ` [PATCH v4 01/19] clk: shmobile: r8a7779: Add clocks support Simon Horman
2014-03-13 9:34 ` Laurent Pinchart
2014-03-14 7:59 ` Simon Horman
2014-03-13 8:59 ` [PATCH v4 02/19] clk: shmobile: r8a7779: Add MSTP clock support Simon Horman
2014-03-13 8:59 ` [PATCH v4 03/19] ARM: shmobile: r8a7779: Add clock index macros for DT sources Simon Horman
2014-03-13 8:59 ` [PATCH v4 04/19] ARM: shmobile: r8a7779: Add clocks Simon Horman
2014-03-13 9:54 ` Laurent Pinchart
2014-03-14 8:08 ` Simon Horman [this message]
2014-03-13 8:59 ` [PATCH v4 05/19] ARM: shmobile: Sync Marzen DTS with Marzen reference DTS Simon Horman
2014-03-13 8:59 ` [PATCH v4 06/19] ARM: shmobile: marzen: Specify external clock frequency in DT Simon Horman
2014-03-13 8:59 ` [PATCH v4 07/19] ARM: shmobile: r8a7779: Reference clocks Simon Horman
2014-03-13 8:59 ` [PATCH v4 08/19] ARM: shmobile: r8a7779: Add helper to read mode pins Simon Horman
2014-03-13 8:59 ` [PATCH v4 09/19] ARM: shmobile: r8a7779: Move r8a7779_earlytimer_init to clock-r8a7779.c Simon Horman
2014-03-13 8:59 ` [PATCH v4 10/19] ARM: shmobile: marzen-reference: Move clock and OF device initialisation into board Simon Horman
2014-03-13 8:59 ` [PATCH v4 11/19] ARM: shmobile: r8a7779: Do not include sh_clk.h in r8a7779.h Simon Horman
2014-03-13 8:59 ` [PATCH v4 12/19] ARM: shmobile: r8a7779: Initial multiplatform support Simon Horman
2014-03-13 8:59 ` [PATCH v4 13/19] ARM: shmobile: marzen-reference: Initialize CPG device Simon Horman
2014-03-13 8:59 ` [PATCH v4 14/19] ARM: shmobile: marzen-reference: Instantiate clkdevs for SCIF and TMU Simon Horman
2014-03-13 8:59 ` [PATCH v4 15/19] ARM: shmobile: marzen: Add to shmobile defconfig Simon Horman
2014-03-13 8:59 ` [PATCH v4 16/19] ARM: shmobile: Remove non-multiplatform Marzen reference support Simon Horman
2014-03-13 9:00 ` [PATCH v4 17/19] ARM: shmobile: Let Marzen multiplatform boot with Marzen DTB Simon Horman
2014-03-13 9:00 ` [PATCH v4 18/19] ARM: shmobile: Remove Marzen reference DTS Simon Horman
[not found] ` <CANqRtoQc=+w3VjW4+=Yd+oRm=g_A_RCDbc3TF1RrR3u3HiMrjw@mail.gmail.com>
2014-03-14 8:08 ` Simon Horman
2014-03-13 9:00 ` [PATCH v4 19/19] ARM: shmobile: marzen-reference: Remove legacy clock support Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140314080803.GB31900@verge.net.au \
--to=horms@verge.net.au \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox