| Message ID | 1448357536-26613-5-git-send-email-wens@csie.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
+ Sebastian On Tue, 24 Nov 2015 17:32:15 +0800 Chen-Yu Tsai wrote: > This adds the supported PRCM clocks and reset controls to the A80 dtsi. > The DAUDIO module clocks are not supported yet. > > Also update clock and reset phandles for r_uart. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > arch/arm/boot/dts/sun9i-a80.dtsi | 79 +++++++++++++++++++++++++++++++++++++++- > 1 file changed, 78 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi > index 1118bf5cc4fb..a4ce348c0831 100644 > --- a/arch/arm/boot/dts/sun9i-a80.dtsi > +++ b/arch/arm/boot/dts/sun9i-a80.dtsi > @@ -164,6 +164,14 @@ > "usb_phy2", "usb_hsic_12M"; > }; > > + pll3: clk@06000008 { > + /* placeholder until implemented */ > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-rate = <0>; > + clock-output-names = "pll3"; > + }; > + > pll4: clk@0600000c { > #clock-cells = <0>; > compatible = "allwinner,sun9i-a80-pll4-clk"; > @@ -350,6 +358,68 @@ > "apb1_uart2", "apb1_uart3", > "apb1_uart4", "apb1_uart5"; > }; > + > + cpus_clk: clk@08001410 { > + compatible = "allwinner,sun9i-a80-cpus-clk"; > + reg = <0x08001410 0x4>; > + #clock-cells = <0>; > + clocks = <&osc32k>, <&osc24M>, <&pll4>, <&pll3>; > + clock-output-names = "cpus"; > + }; > + > + ahbs: ahbs_clk { > + compatible = "fixed-factor-clock"; > + #clock-cells = <0>; > + clock-div = <1>; > + clock-mult = <1>; > + clocks = <&cpus_clk>; > + clock-output-names = "ahbs"; > + }; Dear Sebastian and all, I just want to take the sunxi clk support in mainline for example. I'm not sure I understand correctly, it seems to me that some maintainers draw a line: "having a node for every clock" is a no, no[1]. But here we saw one node for cpus_clk and apbs below. And <0x08001410 0x4>; <0x0800141c 0x4>; shows they are close each other, so should we merge them into a single clock complex node then use mfd, regmap in clk driver? But IMHO, sunxi dts nodes really represent real HW, so I still can't understand why we could not have each node for cpus_clk and apbs. Can you please kindly teach me? Another question: is "Not having a node for every clock" a rule, Would you please confirm? if yes, I'll strictly follow this rule. [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/387335.html Thank you very much, Jisheng > + > + apbs: clk@0800141c { > + compatible = "allwinner,sun8i-a23-apb0-clk"; > + reg = <0x0800141c 0x4>; > + #clock-cells = <0>; > + clocks = <&ahbs>; > + clock-output-names = "apbs"; > + }; > + > + apbs_gates: clk@08001428 { > + compatible = "allwinner,sun9i-a80-apbs-gates-clk"; > + reg = <0x08001428 0x4>; > + #clock-cells = <1>; > + clocks = <&apbs>; > + clock-indices = <0>, <1>, > + <2>, <3>, > + <4>, <5>, > + <6>, <7>, > + <12>, <13>, > + <16>, <17>, > + <18>, <20>; > + clock-output-names = "apbs_pio", "apbs_ir", > + "apbs_timer", "apbs_rsb", > + "apbs_uart", "apbs_1wire", > + "apbs_i2c0", "apbs_i2c1", > + "apbs_ps2_0", "apbs_ps2_1", > + "apbs_dma", "apbs_i2s0", > + "apbs_i2s1", "apbs_twd"; > + }; This is for gate clocks, so we have a node for gateclks. gateclks are merged into one node. > + > + r_1wire_clk: clk@08001450 { > + reg = <0x08001450 0x4>; > + #clock-cells = <0>; > + compatible = "allwinner,sun4i-a10-mod0-clk"; > + clocks = <&osc32k>, <&osc24M>; > + clock-output-names = "r_1wire"; > + }; > + > + r_ir_clk: clk@08001454 { > + reg = <0x08001454 0x4>; > + #clock-cells = <0>; > + compatible = "allwinner,sun4i-a10-mod0-clk"; > + clocks = <&osc32k>, <&osc24M>; > + clock-output-names = "r_ir"; > + }; > }; > > soc { > @@ -764,13 +834,20 @@ > interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; > }; > > + apbs_rst: reset@080014b0 { > + reg = <0x080014b0 0x4>; > + compatible = "allwinner,sun6i-a31-clock-reset"; > + #reset-cells = <1>; > + }; > + > r_uart: serial@08002800 { > compatible = "snps,dw-apb-uart"; > reg = <0x08002800 0x400>; > interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>; > reg-shift = <2>; > reg-io-width = <4>; > - clocks = <&osc24M>; > + clocks = <&apbs_gates 4>; > + resets = <&apbs_rst 4>; > status = "disabled"; > }; > };
On Tue, Nov 24, 2015 at 06:27:09PM +0800, Jisheng Zhang wrote: > + Sebastian > > On Tue, 24 Nov 2015 17:32:15 +0800 > Chen-Yu Tsai wrote: > > > This adds the supported PRCM clocks and reset controls to the A80 dtsi. > > The DAUDIO module clocks are not supported yet. > > > > Also update clock and reset phandles for r_uart. > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > --- > > arch/arm/boot/dts/sun9i-a80.dtsi | 79 +++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 78 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi > > index 1118bf5cc4fb..a4ce348c0831 100644 > > --- a/arch/arm/boot/dts/sun9i-a80.dtsi > > +++ b/arch/arm/boot/dts/sun9i-a80.dtsi > > @@ -164,6 +164,14 @@ > > "usb_phy2", "usb_hsic_12M"; > > }; > > > > + pll3: clk@06000008 { > > + /* placeholder until implemented */ > > + #clock-cells = <0>; > > + compatible = "fixed-clock"; > > + clock-rate = <0>; > > + clock-output-names = "pll3"; > > + }; > > + > > pll4: clk@0600000c { > > #clock-cells = <0>; > > compatible = "allwinner,sun9i-a80-pll4-clk"; > > @@ -350,6 +358,68 @@ > > "apb1_uart2", "apb1_uart3", > > "apb1_uart4", "apb1_uart5"; > > }; > > + > > + cpus_clk: clk@08001410 { > > + compatible = "allwinner,sun9i-a80-cpus-clk"; > > + reg = <0x08001410 0x4>; > > + #clock-cells = <0>; > > + clocks = <&osc32k>, <&osc24M>, <&pll4>, <&pll3>; > > + clock-output-names = "cpus"; > > + }; > > + > > + ahbs: ahbs_clk { > > + compatible = "fixed-factor-clock"; > > + #clock-cells = <0>; > > + clock-div = <1>; > > + clock-mult = <1>; > > + clocks = <&cpus_clk>; > > + clock-output-names = "ahbs"; > > + }; > > Dear Sebastian and all, > > I just want to take the sunxi clk support in mainline for example. > > I'm not sure I understand correctly, it seems to me that some maintainers draw a > line: "having a node for every clock" is a no, no[1]. But here we saw one node for > cpus_clk and apbs below. And <0x08001410 0x4>; <0x0800141c 0x4>; shows they > are close each other, so should we merge them into a single clock complex node > then use mfd, regmap in clk driver? > > But IMHO, sunxi dts nodes really represent real HW, so I still can't understand > why we could not have each node for cpus_clk and apbs. Can you please kindly > teach me? I'm totally lacking any context, but I'll reply. My view on this is that they both represent the hardware, simply with a different model. This preference of the clk maintainers and active clk developers regarding the model to choose has evolved over time. When we started the sunxi support, having one node per clock was the preferred way to do things. Berlin came much later, and the preference at that time was to have the entire clock controller represented as single opaque block to the consumers. Both have pros and cons. The approach we took allows for an easier mix and match, especially if the clocks you have in one SoC are reused in others, without modifying the source code (or barely). AFAIK, this is also the approach took by mvebu, except that their clock tree is much much much simpler. The approach Berlin took allows to have an easier maintainance and more flexibility, for example to deal with clock registration ordering, or clocks that share registers. Our approach works just fine in our case, and I feel no incentive to move to the new model (quite the opposite actually), but I guess it also depends on how your clock controller is built, how many SoCs you have to support, and how much clocks they are sharing. I hope it clears things up. Maxime
Dear Maxime, On Thu, 26 Nov 2015 21:09:42 +0100 Maxime Ripard wrote: > On Tue, Nov 24, 2015 at 06:27:09PM +0800, Jisheng Zhang wrote: > > + Sebastian > > > > On Tue, 24 Nov 2015 17:32:15 +0800 > > Chen-Yu Tsai wrote: > > > > > This adds the supported PRCM clocks and reset controls to the A80 dtsi. > > > The DAUDIO module clocks are not supported yet. > > > > > > Also update clock and reset phandles for r_uart. > > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > > > --- > > > arch/arm/boot/dts/sun9i-a80.dtsi | 79 +++++++++++++++++++++++++++++++++++++++- > > > 1 file changed, 78 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi > > > index 1118bf5cc4fb..a4ce348c0831 100644 > > > --- a/arch/arm/boot/dts/sun9i-a80.dtsi > > > +++ b/arch/arm/boot/dts/sun9i-a80.dtsi > > > @@ -164,6 +164,14 @@ > > > "usb_phy2", "usb_hsic_12M"; > > > }; > > > > > > + pll3: clk@06000008 { > > > + /* placeholder until implemented */ > > > + #clock-cells = <0>; > > > + compatible = "fixed-clock"; > > > + clock-rate = <0>; > > > + clock-output-names = "pll3"; > > > + }; > > > + > > > pll4: clk@0600000c { > > > #clock-cells = <0>; > > > compatible = "allwinner,sun9i-a80-pll4-clk"; > > > @@ -350,6 +358,68 @@ > > > "apb1_uart2", "apb1_uart3", > > > "apb1_uart4", "apb1_uart5"; > > > }; > > > + > > > + cpus_clk: clk@08001410 { > > > + compatible = "allwinner,sun9i-a80-cpus-clk"; > > > + reg = <0x08001410 0x4>; > > > + #clock-cells = <0>; > > > + clocks = <&osc32k>, <&osc24M>, <&pll4>, <&pll3>; > > > + clock-output-names = "cpus"; > > > + }; > > > + > > > + ahbs: ahbs_clk { > > > + compatible = "fixed-factor-clock"; > > > + #clock-cells = <0>; > > > + clock-div = <1>; > > > + clock-mult = <1>; > > > + clocks = <&cpus_clk>; > > > + clock-output-names = "ahbs"; > > > + }; > > > > Dear Sebastian and all, > > > > I just want to take the sunxi clk support in mainline for example. > > > > I'm not sure I understand correctly, it seems to me that some maintainers draw a > > line: "having a node for every clock" is a no, no[1]. But here we saw one node for > > cpus_clk and apbs below. And <0x08001410 0x4>; <0x0800141c 0x4>; shows they > > are close each other, so should we merge them into a single clock complex node > > then use mfd, regmap in clk driver? > > > > But IMHO, sunxi dts nodes really represent real HW, so I still can't understand > > why we could not have each node for cpus_clk and apbs. Can you please kindly > > teach me? > > I'm totally lacking any context, but I'll reply. My view on this is > that they both represent the hardware, simply with a different model. > > This preference of the clk maintainers and active clk developers > regarding the model to choose has evolved over time. When we started > the sunxi support, having one node per clock was the preferred way to > do things. > > Berlin came much later, and the preference at that time was to have > the entire clock controller represented as single opaque block to the > consumers. > > Both have pros and cons. The approach we took allows for an easier mix > and match, especially if the clocks you have in one SoC are reused in > others, without modifying the source code (or barely). AFAIK, this is > also the approach took by mvebu, except that their clock tree is much > much much simpler. > > The approach Berlin took allows to have an easier maintainance and > more flexibility, for example to deal with clock registration > ordering, or clocks that share registers. > > Our approach works just fine in our case, and I feel no incentive to > move to the new model (quite the opposite actually), but I guess it > also depends on how your clock controller is built, how many SoCs you > have to support, and how much clocks they are sharing. Great! Thank you a lot for the detailed explanation. Now I can understand why the clk drivers follow different models, I think I get Sebastian and Stephen's view points. Although the BG4CT clock/pll IP are shared in all newer than BG2Q SoCs (the only difference is how many clocks, how many plls, and their register address), I'll follow Sebastian's suggestions -- one entire clock controller node for all clocks/plls block > > I hope it clears things up. Yes, it's clear now. I knew the history and the reason why there are different models in clk drivers. Will cook a newer BG4CT clk series for review. Thank you, Jisheng
diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi index 1118bf5cc4fb..a4ce348c0831 100644 --- a/arch/arm/boot/dts/sun9i-a80.dtsi +++ b/arch/arm/boot/dts/sun9i-a80.dtsi @@ -164,6 +164,14 @@ "usb_phy2", "usb_hsic_12M"; }; + pll3: clk@06000008 { + /* placeholder until implemented */ + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-rate = <0>; + clock-output-names = "pll3"; + }; + pll4: clk@0600000c { #clock-cells = <0>; compatible = "allwinner,sun9i-a80-pll4-clk"; @@ -350,6 +358,68 @@ "apb1_uart2", "apb1_uart3", "apb1_uart4", "apb1_uart5"; }; + + cpus_clk: clk@08001410 { + compatible = "allwinner,sun9i-a80-cpus-clk"; + reg = <0x08001410 0x4>; + #clock-cells = <0>; + clocks = <&osc32k>, <&osc24M>, <&pll4>, <&pll3>; + clock-output-names = "cpus"; + }; + + ahbs: ahbs_clk { + compatible = "fixed-factor-clock"; + #clock-cells = <0>; + clock-div = <1>; + clock-mult = <1>; + clocks = <&cpus_clk>; + clock-output-names = "ahbs"; + }; + + apbs: clk@0800141c { + compatible = "allwinner,sun8i-a23-apb0-clk"; + reg = <0x0800141c 0x4>; + #clock-cells = <0>; + clocks = <&ahbs>; + clock-output-names = "apbs"; + }; + + apbs_gates: clk@08001428 { + compatible = "allwinner,sun9i-a80-apbs-gates-clk"; + reg = <0x08001428 0x4>; + #clock-cells = <1>; + clocks = <&apbs>; + clock-indices = <0>, <1>, + <2>, <3>, + <4>, <5>, + <6>, <7>, + <12>, <13>, + <16>, <17>, + <18>, <20>; + clock-output-names = "apbs_pio", "apbs_ir", + "apbs_timer", "apbs_rsb", + "apbs_uart", "apbs_1wire", + "apbs_i2c0", "apbs_i2c1", + "apbs_ps2_0", "apbs_ps2_1", + "apbs_dma", "apbs_i2s0", + "apbs_i2s1", "apbs_twd"; + }; + + r_1wire_clk: clk@08001450 { + reg = <0x08001450 0x4>; + #clock-cells = <0>; + compatible = "allwinner,sun4i-a10-mod0-clk"; + clocks = <&osc32k>, <&osc24M>; + clock-output-names = "r_1wire"; + }; + + r_ir_clk: clk@08001454 { + reg = <0x08001454 0x4>; + #clock-cells = <0>; + compatible = "allwinner,sun4i-a10-mod0-clk"; + clocks = <&osc32k>, <&osc24M>; + clock-output-names = "r_ir"; + }; }; soc { @@ -764,13 +834,20 @@ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; }; + apbs_rst: reset@080014b0 { + reg = <0x080014b0 0x4>; + compatible = "allwinner,sun6i-a31-clock-reset"; + #reset-cells = <1>; + }; + r_uart: serial@08002800 { compatible = "snps,dw-apb-uart"; reg = <0x08002800 0x400>; interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>; reg-shift = <2>; reg-io-width = <4>; - clocks = <&osc24M>; + clocks = <&apbs_gates 4>; + resets = <&apbs_rst 4>; status = "disabled"; }; };
This adds the supported PRCM clocks and reset controls to the A80 dtsi. The DAUDIO module clocks are not supported yet. Also update clock and reset phandles for r_uart. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- arch/arm/boot/dts/sun9i-a80.dtsi | 79 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-)