diff mbox

[v2,03/11] doc/devicetree: Add Aspeed clock bindings

Message ID 1461225849-28074-4-git-send-email-joel@jms.id.au (mailing list archive)
State New, archived
Headers show

Commit Message

Joel Stanley April 21, 2016, 8:04 a.m. UTC
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 .../devicetree/bindings/clock/aspeed-clock.txt     | 137 +++++++++++++++++++++
 1 file changed, 137 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/aspeed-clock.txt

Comments

Heiko Stübner April 21, 2016, 11:20 a.m. UTC | #1
[+clock-related patches should include clock-maintainers and lists]

Am Donnerstag, 21. April 2016, 17:34:01 schrieb Joel Stanley:
> +For example:
> +
> +	clk_clkin: clk_clkin {
> +		#clock-cells = <0>;
> +		compatible = "fixed-clock";
> +		clock-frequency = <48000000>;
> +	};
> +
> +	clk_hpll: clk_hpll {
> +		compatible = "aspeed,g4-hpll-clock";
> +		#clock-cells = <0>;
> +		reg = <0x1e6e2008 0x4>;
> +	};
> +
> +	clk_apb: clk_apb@1e6e2008 {
> +		#clock-cells = <0>;
> +		compatible = "aspeed,g4-apb-clock";
> +		reg = <0x1e6e2008 0x4>;
> +		clocks = <&clk_hpll>;
> +	};

You have both the hpll and apb_clk in the same register (probably even more 
clocks?) and separate clock instances where each instance will of_iomap the 
register itself (and thus multiple times in general).

From what I remember exposing the clock controller as one block (instead of 
declaring each clock individually in the dts) is still the preferred way but I 
don't think I can find Mike's mail from back then easily.

And I think most clock-controller implementations actually use that paradigm.


Heiko
Joel Stanley April 27, 2016, 8:31 a.m. UTC | #2
On Thu, Apr 21, 2016 at 8:50 PM, Heiko Stübner <heiko@sntech.de> wrote:
> [+clock-related patches should include clock-maintainers and lists]

Thanks.

> Am Donnerstag, 21. April 2016, 17:34:01 schrieb Joel Stanley:
>> +For example:
>> +
>> +     clk_clkin: clk_clkin {
>> +             #clock-cells = <0>;
>> +             compatible = "fixed-clock";
>> +             clock-frequency = <48000000>;
>> +     };
>> +
>> +     clk_hpll: clk_hpll {
>> +             compatible = "aspeed,g4-hpll-clock";
>> +             #clock-cells = <0>;
>> +             reg = <0x1e6e2008 0x4>;
>> +     };
>> +
>> +     clk_apb: clk_apb@1e6e2008 {
>> +             #clock-cells = <0>;
>> +             compatible = "aspeed,g4-apb-clock";
>> +             reg = <0x1e6e2008 0x4>;
>> +             clocks = <&clk_hpll>;
>> +     };
>
> You have both the hpll and apb_clk in the same register (probably even more
> clocks?) and separate clock instances where each instance will of_iomap the
> register itself (and thus multiple times in general).

Yep. I agree that's not ideal.

>
> From what I remember exposing the clock controller as one block (instead of
> declaring each clock individually in the dts) is still the preferred way but I
> don't think I can find Mike's mail from back then easily.

I can't picture how that would look. I took my lead from the moxart
clock driver; is there a better example that I should follow?

Cheers,

Joel
Heiko Stübner April 27, 2016, 9:12 a.m. UTC | #3
Hi Joel,

Am Mittwoch, 27. April 2016, 18:01:00 schrieb Joel Stanley:
> On Thu, Apr 21, 2016 at 8:50 PM, Heiko Stübner <heiko@sntech.de> wrote:
> > [+clock-related patches should include clock-maintainers and lists]
> 
> Thanks.
> 
> > Am Donnerstag, 21. April 2016, 17:34:01 schrieb Joel Stanley:
> >> +For example:
> >> +
> >> +     clk_clkin: clk_clkin {
> >> +             #clock-cells = <0>;
> >> +             compatible = "fixed-clock";
> >> +             clock-frequency = <48000000>;
> >> +     };
> >> +
> >> +     clk_hpll: clk_hpll {
> >> +             compatible = "aspeed,g4-hpll-clock";
> >> +             #clock-cells = <0>;
> >> +             reg = <0x1e6e2008 0x4>;
> >> +     };
> >> +
> >> +     clk_apb: clk_apb@1e6e2008 {
> >> +             #clock-cells = <0>;
> >> +             compatible = "aspeed,g4-apb-clock";
> >> +             reg = <0x1e6e2008 0x4>;
> >> +             clocks = <&clk_hpll>;
> >> +     };
> > 
> > You have both the hpll and apb_clk in the same register (probably even
> > more
> > clocks?) and separate clock instances where each instance will of_iomap
> > the
> > register itself (and thus multiple times in general).
> 
> Yep. I agree that's not ideal.
> 
> > From what I remember exposing the clock controller as one block (instead
> > of
> > declaring each clock individually in the dts) is still the preferred way
> > but I don't think I can find Mike's mail from back then easily.
> 
> I can't picture how that would look. I took my lead from the moxart
> clock driver; is there a better example that I should follow?

qcom, samsung, rockchip, hisilicon, imx, ...

I guess the design would depend on the actual layout of your clock- / system-
controller - aka what else is contained there.


Heiko
Joel Stanley April 28, 2016, 6:50 a.m. UTC | #4
On Wed, Apr 27, 2016 at 6:42 PM, Heiko Stübner <heiko@sntech.de> wrote:
> Am Mittwoch, 27. April 2016, 18:01:00 schrieb Joel Stanley:
>> > From what I remember exposing the clock controller as one block (instead
>> > of
>> > declaring each clock individually in the dts) is still the preferred way
>> > but I don't think I can find Mike's mail from back then easily.
>>
>> I can't picture how that would look. I took my lead from the moxart
>> clock driver; is there a better example that I should follow?
>
> qcom, samsung, rockchip, hisilicon, imx, ...

I had a look here, and they appear to be much more complex than I
need. The qcom directory is 41000 lines of code! The moxart driver is
similar to what we do, but as you mentioned it is not arranged how you
want it.

> I guess the design would depend on the actual layout of your clock- / system-
> controller - aka what else is contained there.

In the fourth generation parts, such as the ast2400, we have this layout:

    clock                rate
 -----------------------------
  clk_clkin          48000000
     clk_hpll       384000000
        clk_apb      48000000

clkin is the oscillator that may be running at 24, 25 or 48MHz. We can
determine this from the strapping register.

The hpll divisor is controlled by strapping resistors, and indicated
in the strapping register.

The apb is controlled by a register in the SCU, the Aspeed's
bucket-of-bits for controlling various parts of the soc.

In our case we want don't need to adjust any clocks. We do want struct
clk's so attached device drivers to know how fast they are being
clocked. How do you see this laid out?

Cheers,

Joel
Heiko Stübner April 28, 2016, 7:25 a.m. UTC | #5
Am Donnerstag, 28. April 2016, 16:20:04 schrieb Joel Stanley:
> On Wed, Apr 27, 2016 at 6:42 PM, Heiko Stübner <heiko@sntech.de> wrote:
> > Am Mittwoch, 27. April 2016, 18:01:00 schrieb Joel Stanley:
> >> > From what I remember exposing the clock controller as one block
> >> > (instead
> >> > of
> >> > declaring each clock individually in the dts) is still the preferred
> >> > way
> >> > but I don't think I can find Mike's mail from back then easily.
> >> 
> >> I can't picture how that would look. I took my lead from the moxart
> >> clock driver; is there a better example that I should follow?
> > 
> > qcom, samsung, rockchip, hisilicon, imx, ...
> 
> I had a look here, and they appear to be much more complex than I
> need. The qcom directory is 41000 lines of code! The moxart driver is
> similar to what we do, but as you mentioned it is not arranged how you
> want it.

I'm by no means authoritative ;-), but from what you describe below, clk-
asm9260.c or clk-efm32gg.c might be going in that direction of very simple 
clock-controllers.

Sorry about pointing to more complex drivers for bigger socs at first :-)


> > I guess the design would depend on the actual layout of your clock- /
> > system- controller - aka what else is contained there.
> 
> In the fourth generation parts, such as the ast2400, we have this layout:
> 
>     clock                rate
>  -----------------------------
>   clk_clkin          48000000
>      clk_hpll       384000000
>         clk_apb      48000000
> 
> clkin is the oscillator that may be running at 24, 25 or 48MHz. We can
> determine this from the strapping register.
> 
> The hpll divisor is controlled by strapping resistors, and indicated
> in the strapping register.
> 
> The apb is controlled by a register in the SCU, the Aspeed's
> bucket-of-bits for controlling various parts of the soc.

I remember that from working on Samsung s3c24xx socs, the system-controller 
area also worked as sort of catch-all :-) .

> 
> In our case we want don't need to adjust any clocks. We do want struct
> clk's so attached device drivers to know how fast they are being
> clocked. How do you see this laid out?

see drivers referenced above.
Benjamin Herrenschmidt April 28, 2016, 8:38 a.m. UTC | #6
On Wed, 2016-04-27 at 11:12 +0200, Heiko Stübner wrote:
> I guess the design would depend on the actual layout of your clock- /
> system-
> controller - aka what else is contained there.

One register controls most clocks so it makes sense to have one driver.

Cheers,
Ben.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/clock/aspeed-clock.txt b/Documentation/devicetree/bindings/clock/aspeed-clock.txt
new file mode 100644
index 000000000000..91bdf34e5473
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/aspeed-clock.txt
@@ -0,0 +1,137 @@ 
+Device Tree Clock bindings for the Aspeed SoCs
+
+Aspeed SoCs have a fixed frequency input osciallator is used to create the PLL
+and APB clocks. We can determine these frequencies by reading registers that
+are set according to strapping bits.
+
+Forth generation boards
+-----------------------
+
+eg, ast2400.
+
+CLKIN:
+ - compatible : Must be "fixed-clock"
+ - #clock-cells : Should be 0
+ - clock-frequency: 48e6, 25e6 or 24e6 depending on the input clock
+
+PLL:
+
+Required properties:
+ - compatible : Must be "aspeed,g4-hpll-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the input clock (clkin)
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+
+APB:
+
+Required properties:
+ - compatible : Must be "aspeed,g4-apb-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the h-pll
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+
+For example:
+
+	clk_clkin: clk_clkin {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <48000000>;
+	};
+
+	clk_hpll: clk_hpll {
+		compatible = "aspeed,g4-hpll-clock";
+		#clock-cells = <0>;
+		reg = <0x1e6e2008 0x4>;
+	};
+
+	clk_apb: clk_apb@1e6e2008 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g4-apb-clock";
+		reg = <0x1e6e2008 0x4>;
+		clocks = <&clk_hpll>;
+	};
+
+
+
+Fifth generation boards
+-----------------------
+
+eg, ast2500.
+
+CLKIN:
+Required properties:
+ - compatible : Must be "fixed-clock"
+ - #clock-cells : Should be 0
+ - clock-frequency: 25000000 or 24000000 depending on the input clock
+
+H-PLL:
+
+Required properties:
+ - compatible : Must be "aspeed,g5-hpll-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the input clock (clkin)
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+
+AHB:
+
+Required properties:
+ - compatible : Must be "aspeed,g5-ahb-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the the h-pll
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+APB:
+
+Required properties:
+ - compatible : Must be "aspeed,g4-apb-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the the h-pll
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+
+For example:
+	clk_clkin: clk_clkin@1e6e2070 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g5-clkin-clock";
+		reg = <0x1e6e2070 0x04>;
+	};
+
+	clk_hpll: clk_hpll@1e6e2024 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g5-hpll-clock";
+		reg = <0x1e6e2024 0x4>;
+		clocks = <&clk_clkin>;
+	};
+
+	clk_ahb: clk_ahb@1e6e2070 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g5-ahb-clock";
+		reg = <0x1e6e2070 0x4>;
+		clocks = <&clk_hpll>;
+	};
+
+	clk_apb: clk_apb@1e6e2008 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g5-apb-clock";
+		reg = <0x1e6e2008 0x4>;
+		clocks = <&clk_hpll>;
+	};
+