| Message ID | 1461225849-28074-4-git-send-email-joel@jms.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
[+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
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
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
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
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.
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 --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>; + }; +
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