Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753690AbbKWISn (ORCPT ); Mon, 23 Nov 2015 03:18:43 -0500 Received: from mx0b-0016f401.pphosted.com ([67.231.156.173]:53230 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753260AbbKWISl (ORCPT ); Mon, 23 Nov 2015 03:18:41 -0500 Date: Mon, 23 Nov 2015 16:14:29 +0800 From: Jisheng Zhang To: Sebastian Hesselbarth CC: , , , , , , , , , , , , , Subject: Re: [PATCH v2 6/6] arm64: dts: berlin4ct: add pll and clock nodes Message-ID: <20151123161429.2a3a9243@xhacker> In-Reply-To: <20151123152158.483aa6b5@xhacker> References: <1448008952-1787-1-git-send-email-jszhang@marvell.com> <1448008952-1787-7-git-send-email-jszhang@marvell.com> <564F8B73.7070403@gmail.com> <20151123152158.483aa6b5@xhacker> X-Mailer: Claws Mail 3.13.0 (GTK+ 2.24.28; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2015-11-23_04:,, signatures=0 X-Proofpoint-Spam-Details: rule=inbound_notspam policy=inbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1507310000 definitions=main-1511230156 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7633 Lines: 245 Dear all, On Mon, 23 Nov 2015 15:21:58 +0800 Jisheng Zhang wrote: > Dear Sebastian, > > On Fri, 20 Nov 2015 22:06:59 +0100 > Sebastian Hesselbarth wrote: > > > On 20.11.2015 09:42, Jisheng Zhang wrote: > > > Add syspll, mempll, cpupll, gateclk and berlin-clk nodes. > > > > > > Signed-off-by: Jisheng Zhang > > > --- > > > arch/arm64/boot/dts/marvell/berlin4ct.dtsi | 38 ++++++++++++++++++++++++++++++ > > > 1 file changed, 38 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > > > index a4a1876..808a997 100644 > > > --- a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > > > +++ b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi > > > @@ -42,6 +42,7 @@ > > > * OTHER DEALINGS IN THE SOFTWARE. > > > */ > > > > > > +#include > > > #include > > > > > > / { > > > @@ -135,6 +136,22 @@ > > > interrupts = ; > > > }; > > > > > > + cpupll: cpupll { > > > + compatible = "marvell,berlin-pll"; > > > + reg = <0x922000 0x14>, <0xea0710 4>; > > > + #clock-cells = <0>; > > > + clocks = <&osc>, <&clk CLK_CPUFASTREF>; > > > + bypass-shift = /bits/ 8 <2>; > > > + }; > > > + > > > + mempll: mempll { > > > + compatible = "marvell,berlin-pll"; > > > + reg = <0x940034 0x14>, <0xea0710 4>; > > > > Whenever you see overlapping/repeating reg ranges, e.g. <0xea0710 4> > > you can be sure you are not representing HW structure but driver > > structure here. > > > > Please merge clocks/gates/plls to a single clock complex node > > and deal with the internals by using "simple-mfd" and "syscon" regmaps. > > > > > + #clock-cells = <0>; > > > + clocks = <&osc>, <&clk CLK_MEMFASTREF>; > > > + bypass-shift = /bits/ 8 <1>; > > > + }; > > > + > > > apb@e80000 { > > > compatible = "simple-bus"; > > > #address-cells = <1>; > > > @@ -225,6 +242,27 @@ > > > }; > > > }; > > > > > > + syspll: syspll { > > > + compatible = "marvell,berlin-pll"; > > > + reg = <0xea0200 0x14>, <0xea0710 4>; > > > + #clock-cells = <0>; > > > + clocks = <&osc>; > > > + bypass-shift = /bits/ 8 <0>; > > > + }; > > > + > > > + gateclk: gateclk { > > > + compatible = "marvell,berlin4ct-gateclk"; > > > + reg = <0xea0700 4>; > > > + #clock-cells = <1>; > > > + }; > > > + > > > + clk: clk { > > > + compatible = "marvell,berlin4ct-clk"; > > > + reg = <0xea0720 0x144>; > > > > Looking at the reg ranges, I'd say that they are all clock related > > and pretty close to each other: > > > > gateclk: reg = <0xea0700 4>; > > bypass: reg = <0xea0710 4>; > > clk: reg = <0xea0720 0x144>; > > Although these ranges sit close, but we should represent HW structure as you > said. > > First of all, let me describe the clks/plls in BG4CT. BG4CT contains: > > two kinds of PLL: normal PLL and AVPLL. These PLLs are put with their users > together. For example: mempll pll registers <0xf7940034, 0x14> is put together > with mem controller registers. AVPLL control registers are put with AV devices. > You can also check mempll, cpupll and syspll ranges: > > cpupll: <0x922000 0x14> > > mempll: <0x940034 0x14> > > syspll: <0xea0200 0x14> > > > We have three normal PLLS: cpupll, mempll and syspll. All these three PLLs use > 25MHZ osc as clocksource. These plls can be bypassed. when syspll is bypassed > the 25MHZ osc is directly output to syspllclk. When mempll/cpupll is bypassed, > its corresponding fastrefclk is directly output to ddrphyclk/cpuclk: > > > ---25MHZ osc----------|\ > ________ | |-- syspllclk > ---| SYSPLL |---------|/ > > > > ---cpufastrefclk------|\ > ________ | |-- cpuclk > ---| CPUPLL |---------|/ > > > ---memfastrefclk------|\ > ________ | |-- ddrphyclk > ---| MEMPLL |---------|/ > > NOTE: the fastrefclk is the so called normal clk below. > > > > two kinds of clk: normal clk and gate clk. The normal clk supports changing > divider, selecting clock source, disabling/enabling etc. The gate clk only > supports disabling/enabling. normal clks use syspllclk as clocksource, while > gate clks use perifsysclk as clocksource. > I hope I have described the BG4CT HW clk/pll clearly, I really need your advice about my proposal. The clk nodes in my proposal will finally look like: cpupll: cpupll { compatible = "marvell,berlin-pll"; reg = <0x922000 0x14> #clock-cells = <0>; clocks = <&osc>; }; mempll: mempll { compatible = "marvell,berlin-pll"; reg = <0x940034 0x14> #clock-cells = <0>; clocks = <&osc>; }; syspll: syspll { compatible = "marvell,berlin-pll"; reg = <0xea0200 0x14> #clock-cells = <0>; clocks = <&osc>; }; pllclk: pllclk { compatible = "marvell,berlin4ct-pllclk"; reg = <0xea0710 4> #clock-cells = <1>; }; gateclk: gateclk { compatible = "marvell,berlin4ct-gateclk"; reg = <0xea0700 4>; #clock-cells = <1>; }; clk: clk { compatible = "marvell,berlin4ct-clk"; reg = <0xea0720 0x144>; #clock-cells = <1>; clocks = <&pllclk SYSPLLCLK>; }; Using the ccf clk-mux, there's no overlapping/repeating reg ranges any more. If you want more BG4CT clk/pll details, please let me know. Thanks, Jisheng > > So what's the representing HW structure in fact? Here is my proposal: > > 1. have mempll, cpupll and syspll node in dts > > 2. one gateclk node in dts for gateclks > > 3. one normalclk node in dts for normal clks > > 4. one ccf clock-mux for cpuclk, ddrphyclk and syspllclk. > > what do you think? > > > From another side, let's have a look at driver/clk/mvebu. As can be seen, > different clks register are close each other, for example, gateclk and coreclk > in arch/arm/boot/dts/armada-xp.dtsi. > > And drivers/clk/sunxi, arch/arm/boot/dts/sun7i-a20.dtsi, the pll4, pll12, gt_clk > and ahb*, apb* etc... > > why these SoCs don't merge clocks/gates/plls to a single clock complex node? > I think that's because they are representing real HW structure. > > Thanks, > Jisheng > > > > > > So, please just follow the OF/driver structure we already > > have for Berlin2. > > > > Sebastian > > > > > + #clock-cells = <1>; > > > + clocks = <&syspll>; > > > + }; > > > + > > > soc_pinctrl: pin-controller@ea8000 { > > > compatible = "marvell,berlin4ct-soc-pinctrl"; > > > reg = <0xea8000 0x14>; > > > > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/