Re: [PATCH v2 6/6] arm64: dts: berlin4ct: add pll and clock nodes
From: Jisheng Zhang
Date: Mon Nov 23 2015 - 03:18:47 EST
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 <jszhang@xxxxxxxxxxx>
> > > ---
> > > 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 <dt-bindings/clock/berlin4ct.h>
> > > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > >
> > > / {
> > > @@ -135,6 +136,22 @@
> > > interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> > > };
> > >
> > > + 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@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/