clk: Add APM X-Gene SoC clock driver
Subsystems (2)
v4 cover clk: Add APM X-Gene SoC clock driver
This patch adds APM X-Gene SoC clock driver support. Reference, PCP PLL, SoC PCP, and Ethernet clocks are defined via the DTS.
-v4:
* Remove flags from DTS and check status instead
* Change DTS reg entry to be identified by reg-names property
-v3:
* Make DTS compatible type for SoC PLL and PCP PLL
* Make use of of_iomap instead direct ioremap call
* Make kzalloc with variable instead structure
* Remove DTS binding for fixed-clock and fixed_factor_clock
* Remove unnecessary arch_initcall
-v2:
* Make depends on ARM64
* Fix duplicate registration
-v1 - Initial post
Loc Ho (3):
clk: Add APM X-Gene SoC clock driver for reference, PLL, and device
clocks.
clk: arm64: Add DTS clock entry for APM X-Gene Storm SoC with
reference to
Documentation: Add documentation for APM X-Gene clock binding with
PLL and
Documentation/devicetree/bindings/clock/xgene.txt | 109 +++++ arch/arm64/boot/dts/apm-storm.dtsi | 75 +++ drivers/clk/Kconfig | 7 + drivers/clk/Makefile | 1 + drivers/clk/clk-xgene.c | 538 +++++++++++++++++++++ 5 files changed, 730 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/xgene.txt create mode 100644 drivers/clk/clk-xgene.c
PATCH v4 1/3 clk: Add APM X-Gene SoC clock driver
clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.
--- drivers/clk/Kconfig | 7 + drivers/clk/Makefile | 1 + drivers/clk/clk-xgene.c | 538 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 546 insertions(+), 0 deletions(-) create mode 100644 drivers/clk/clk-xgene.c
drivers/clk/Kconfig +7 -0
@@ -81,6 +81,13 @@ config COMMON_CLK_AXI_CLKGEN Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx FPGAs. It is commonly used in Analog Devices' reference designs. +config COMMON_CLK_XGENE + bool "Clock driver for APM XGene SoC" + default y + depends on ARM64 + ---help--- + Sypport for the APM X-Gene SoC reference, PLL, and device clocks. + endmenu source "drivers/clk/mvebu/Kconfig"
drivers/clk/Makefile +1 -0
@@ -39,3 +39,4 @@ obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o +obj-$(CONFIG_COMMON_CLK_XGENE) += clk-xgene.o
drivers/clk/clk-xgene.c new file +538 -0
538 lines not on this page · raw message
Signature · 1 line
1.5.5
PATCH v4 2/3 clk: arm64: Add DTS clock entry for APM X-Gene Storm SoC
clk: arm64: Add DTS clock entry for APM X-Gene Storm SoC with reference to reference, PCP PLL, SoC PLL, and Ethernet clocks.
--- arch/arm64/boot/dts/apm-storm.dtsi | 75 ++++++++++++++++++++++++++++++++++++ 1 files changed, 75 insertions(+), 0 deletions(-)
arch/arm64/boot/dts/apm-storm.dtsi +75 -0
@@ -103,6 +103,81 @@ #size-cells = <2>; ranges; + clocks { + #address-cells = <2>; + #size-cells = <2>; + ranges; + refclk: refclk { + compatible = "fixed-clock"; + #clock-cells = <1>; + clock-frequency = <100000000>; + clock-output-names = "refclk"; + }; + + pcppll: pcppll at 17000100 { + compatible = "apm,xgene-pcppll-clock"; + #clock-cells = <1>; + clocks = <&refclk 0>; + clock-names = "pcppll"; + reg = <0x0 0x17000100 0x0 0x1000>; + clock-output-names = "pcppll"; + type = <0>; + }; + + socpll: socpll at 17000120 { + compatible = "apm,xgene-socpll-clock"; + #clock-cells = <1>; + clocks = <&refclk 0>; + clock-names = "socpll"; + reg = <0x0 0x17000120 0x0 0x1000>; + clock-output-names = "socpll"; + type = <1>; + }; + + socplldiv2: socplldiv2 { + compatible = "fixed-factor-clock"; + #clock-cells = <1>; + clocks = <&socpll 0>; + clock-names = "socplldiv2"; + clock-mult = <1>; + clock-div = <2>; + clock-output-names = "socplldiv2"; + }; + + qmlclk: qmlclk { + compatible = "apm,xgene-device-clock"; + #clock-cells = <1>; + clocks = <&socplldiv2 0>; + clock-names = "qmlclk"; + reg = <0x0 0x1703C000 0x0 0x1000>; + reg-names = "csr-reg"; + clock-output-names = "qmlclk"; + }; + + ethclk: ethclk { + compatible = "apm,xgene-device-clock"; + #clock-cells = <1>; + clocks = <&socplldiv2 0>; + clock-names = "ethclk"; + reg = <0x0 0x17000000 0x0 0x1000>; + reg-names = "div-reg"; + divider-offset = <0x238>; + divider-width = <0x9>; + divider-shift = <0x0>; + clock-output-names = "ethclk"; + }; + + eth8clk: eth8clk { + compatible = "apm,xgene-device-clock"; + #clock-cells = <1>; + clocks = <ðclk 0>; + clock-names = "eth8clk"; + reg = <0x0 0x1702C000 0x0 0x1000>; + reg-names = "csr-reg"; + clock-output-names = "eth8clk"; + }; + }; + serial0: serial at 1c020000 { device_type = "serial"; compatible = "ns16550";
Signature · 1 line
1.5.5
PATCH v4 3/3 Documentation: Add documentation for APM X-Gene clock binding
Documentation: Add documentation for APM X-Gene clock binding with PLL and device clocks.
--- Documentation/devicetree/bindings/clock/xgene.txt | 109 +++++++++++++++++++++ 1 files changed, 109 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/xgene.txt
Documentation/devicetree/bindings/clock/xgene.txt new file +109 -0
@@ -0,0 +1,109 @@ +Device Tree Clock bindings for APM X-Gene + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of the following: + "apm,xgene-socpll-clock" - for a X-Gene SoC PLL clock + "apm,xgene-pcppll-clock" - for a X-Gene PCP PLL clock + "apm,xgene-device-clock" - for a X-Gene device clock + +Required properties for SoC or PCP PLL clocks: +- reg : shall be the physical PLL register address for the pll clock. +- clocks : shall be the input parent clock phandle for the clock. This should + be the reference clock. +- #clock-cells : shall be set to 1. +- clock-output-names : shall be the name of the PLL referenced by derive + clock. +Optional properties for PLL clocks: +- clock-names : shall be the name of the PLL. If missing, use the device name. + +Required properties for device clocks: +- reg : shall be the physical CSR reset address base and/or physical CSR + divider address base if one existed. +- reg-names : shall be string describing the reg resource. Supported + register names are "csr-reg" and "div-reg" which default + to "csr-reg" if not specified. +- clocks : shall be the input parent clock phandle for the clock. +- #clock-cells : shall be set to 1. +- clock-output-names : shall be the name of the device referenced. +Optional properties for device clocks: +- clock-names : shall be the name of the device clock. If missing, use the + device name. +- csr-offset : Offset to the CSR reset register from the reset address base. + Default is 0. +- csr-mask : CSR reset mask bit. Default is 0xF. +- enable-offset : Offset to the enable register from the reset address base. + Default is 0x8. +- enable-mask : CSR enable mask bit. Default is 0xF. +- divider-offset : Offset to the divider CSR register from the divider base. + Default is 0x0. +- divider-width : Width of the divider register. Default is 0. +- divider-shift : Bit shift of the divider register. Default is 0. + +For example: + + pcppll: pcppll at 17000100 { + compatible = "apm,xgene-pcppll-clock"; + #clock-cells = <1>; + clocks = <&refclk 0>; + clock-names = "pcppll"; + reg = <0x0 0x17000100 0x0 0x1000>; + clock-output-names = "pcppll"; + type = <0>; + }; + + socpll: socpll at 17000120 { + compatible = "apm,xgene-socpll-clock"; + #clock-cells = <1>; + clocks = <&refclk 0>; + clock-names = "socpll"; + reg = <0x0 0x17000120 0x0 0x1000>; + clock-output-names = "socpll"; + type = <1>; + }; + + qmlclk: qmlclk { + compatible = "apm,xgene-device-clock"; + #clock-cells = <1>; + clocks = <&socplldiv2 0>; + clock-names = "qmlclk"; + reg = <0x0 0x1703C000 0x0 0x1000>; + reg-name = "csr-reg"; + clock-output-names = "qmlclk"; + }; + + ethclk: ethclk { + compatible = "apm,xgene-device-clock"; + #clock-cells = <1>; + clocks = <&socplldiv2 0>; + clock-names = "ethclk"; + reg = <0x0 0x17000000 0x0 0x1000>; + reg-names = "div-reg"; + divider-offset = <0x238>; + divider-width = <0x9>; + divider-shift = <0x0>; + clock-output-names = "ethclk"; + }; + + apbclk: apbclk { + compatible = "apm,xgene-device-clock"; + #clock-cells = <1>; + clocks = <&ahbclk 0>; + clock-names = "apbclk"; + reg = <0x0 0x1F2AC000 0x0 0x1000 + 0x0 0x1F2AC000 0x0 0x1000>; + reg-names = "csr-reg", "div-reg"; + csr-offset = <0x0>; + csr-mask = <0x200>; + enable-offset = <0x8>; + enable-mask = <0x200>; + divider-offset = <0x10>; + divider-width = <0x2>; + divider-shift = <0x0>; + flags = <0x8>; + clock-output-names = "apbclk"; + }; +
Signature · 1 line
1.5.5
PATCH v4 3/3 Documentation: Add documentation for APM X-Gene clock binding
Hi,
I'd recommend on your next posting that you Cc devicetree-discuss (devicetree-discuss at lists.ozlabs.org), as bindings should go there for review.
Quoted 36 lines · Loc Ho
Documentation: Add documentation for APM X-Gene clock binding with PLL and device clocks.
Signed-off-by Loc Ho <[email protected]>Signed-off-by Kumar Sankaran <[email protected]>Signed-off-by Vinayak Kale <[email protected]>Signed-off-by Feng Kan <[email protected]>--- Documentation/devicetree/bindings/clock/xgene.txt | 109 +++++++++++++++++++++ 1 files changed, 109 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/xgene.txtDocumentation/devicetree/bindings/clock/xgene.txt new file +18
-0@@ -0,0 +1,109 @@ +Device Tree Clock bindings for APM X-Gene + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be one of the following: + "apm,xgene-socpll-clock" - for a X-Gene SoC PLL clock + "apm,xgene-pcppll-clock" - for a X-Gene PCP PLL clock + "apm,xgene-device-clock" - for a X-Gene device clock + +Required properties for SoC or PCP PLL clocks: +- reg : shall be the physical PLL register address for the pll clock. +- clocks : shall be the input parent clock phandle for the clock. This should + be the reference clock. +- #clock-cells : shall be set to 1. +- clock-output-names : shall be the name of the PLL referenced by derive
Documentation/devicetree/bindings/clock/xgene.txt line 28
Quoted 10 lines
Documentation/devicetree/bindings/clock/xgene.txt new file +10
-0+ clock. +Optional properties for PLL clocks: +- clock-names : shall be the name of the PLL. If missing, use the device name. + +Required properties for device clocks: +- reg : shall be the physical CSR reset address base and/or physical CSR + divider address base if one existed. +- reg-names : shall be string describing the reg resource. Supported + register names are "csr-reg" and "div-reg" which default + to "csr-reg" if not specified.
This is somewhat difficult to read. How about:
- reg : shall be a list of address and length pairs describing the CSR reset and/or the divider. Either may be omitted, but at least one must be present.
- reg-names : shall be a string list describing the reg resource. This may include "csr-reg" and/or "div-reg". If this property is not present, the reg property is assumed to describe only "csr-reg".
Otherwise, the binding looks good to me.
Thanks,
Mark.
PATCH v4 1/3 clk: Add APM X-Gene SoC clock driver
In reply to Loc HoHi,
Quoted 12 lines · Loc Ho
clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.
Signed-off-by Loc Ho <[email protected]>Signed-off-by Kumar Sankaran <[email protected]>Signed-off-by Vinayak Kale <[email protected]>Signed-off-by Feng Kan <[email protected]>--- drivers/clk/Kconfig | 7 + drivers/clk/Makefile | 1 + drivers/clk/clk-xgene.c | 538 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 546 insertions(+), 0 deletions(-) create mode 100644 drivers/clk/clk-xgene.c
[...]
Quoted 33 lines
drivers/clk/clk-xgene.c new file +33
-0+static void xgene_socpllclk_init (struct device_node *np) +{ +const char *clk_name = np->full_name; +struct clk *clk; +void *reg; + + reg =of_iomap (np,0 ); +if (reg ==NULL ) { +pr_err ("Unable to map CSR register for %s\n" , np->full_name); +return ; + } +of_property_read_string (np,"clock-output-names" , &clk_name); + clk =xgene_register_clk_pll (NULL , + clk_name,of_clk_get_parent_name (np,0 ), +CLK_IS_ROOT , reg,0 ,PLL_TYPE_SOC , &clk_lock); +if (!IS_ERR (clk)) { +of_clk_add_provider (np, of_clk_src_simple_get, clk); +clk_register_clkdev (clk, clk_name,NULL ); +pr_debug ("Add %s clock PLL\n" , clk_name); + } +} + +static void xgene_pcppllclk_init (struct device_node *np) +{ +const char *clk_name = np->full_name; +struct clk *clk; +void *reg; + + reg =of_iomap (np,0 ); +if (reg ==NULL ) { +pr_err ("Unable to map CSR register for %s\n" , np->full_name); +return ; + }
drivers/clk/clk-xgene.c line 208
Quoted 10 lines
drivers/clk/clk-xgene.c new file +10
-0+of_property_read_string (np,"clock-output-names" , &clk_name); + clk =xgene_register_clk_pll (NULL , + clk_name,of_clk_get_parent_name (np,0 ), +CLK_IS_ROOT , reg,0 ,PLL_TYPE_PCP , &clk_lock); +if (!IS_ERR (clk)) { +of_clk_add_provider (np, of_clk_src_simple_get, clk); +clk_register_clkdev (clk, clk_name,NULL ); +pr_debug ("Add %s clock PLL\n" , clk_name); + } +}
Unless I've missed something, these two functions look identical other than the PLL_TYPE_{PCP,SOC} passed to xgene_register_clkdev. You could unify most of the logic by having a helper (xgene_pllck_init?) that took the xgene_pll_type, and then xgene_{soc,pll}clk_init would look something like:
static void xgene_socpllclk_init(struct device_node *np)
{
xgene_pllclk_init(np, PLL_TYPE_SOC);
}drivers/clk/clk-xgene.c line 212
Quoted 4 lines
drivers/clk/clk-xgene.c new file +4
-0+ +/* IP Clock */ +struct xgene_dev_parameters { +u32 flags;/* Any flags to the clock framework */
As this is always zero now, it could be removed.
Quoted 10 lines
drivers/clk/clk-xgene.c new file +10
-0+void __iomem *csr_reg;/* CSR for IP clock */ +u32 reg_clk_offset;/* Offset to clock enable CSR */ +u32 reg_clk_mask;/* Mask bit for clock enable */ +u32 reg_csr_offset;/* Offset to CSR reset */ +u32 reg_csr_mask;/* Mask bit for disable CSR reset */ +void __iomem *divider_reg;/* CSR for divider */ +u32 reg_divider_offset;/* Offset to divider register */ +u32 reg_divider_shift;/* Bit shift to divider field */ +u32 reg_divider_width;/* Width of the bit to divider field */ +};
[...]
Quoted 25 lines
drivers/clk/clk-xgene.c new file +25
-0+static void __init xgene_devclk_init (struct device_node *np) +{ +const char *clk_name = np->full_name; +struct clk *clk; +struct resource res; +int rc; +struct xgene_dev_parameters parameters; +int i; + +/* Check if the entry is disabled */ +if (!of_device_is_available (np)) +return ; + +/* Parse the DTS register for resource */ + parameters.csr_reg =NULL ; + parameters.divider_reg =NULL ; +for (i =0 ; i <2 ; i++) { +void *map_res; + rc =of_address_to_resource (np, i, &res); +if (rc !=0 ) { +if (i ==0 ) { +pr_err ("no DTS register for %s\n" , + np->full_name); +return ; + }
drivers/clk/clk-xgene.c line 489
Quoted 10 lines
drivers/clk/clk-xgene.c new file +10
-0+break ; + } + map_res =of_iomap (np, i); +if (map_res ==NULL ) { +pr_err ("Unable to map resource %d for %s\n" , + i, np->full_name); +if (parameters.csr_reg) +iounmap (parameters.csr_reg); +if (parameters.divider_reg) +iounmap (parameters.divider_reg);
You could move this freeing of the register iomaps to the end of the function with some label (err_unmap?) and use a goto. You could then use a goto in the IS_ERR(clk) case below, and unify the unmapping.
Quoted 35 lines
drivers/clk/clk-xgene.c new file +35
-0+return ; + } +if (strcmp (res.name,"div-reg" ) ==0 ) + parameters.divider_reg = map_res; +else /* if (strcmp(res->name, "csr-reg") == 0) */ + parameters.csr_reg = map_res; + } + parameters.flags =0 ; +if (of_property_read_u32 (np,"csr-offset" , ¶meters.reg_csr_offset)) + parameters.reg_csr_offset =0 ; +if (of_property_read_u32 (np,"csr-mask" , ¶meters.reg_csr_mask)) + parameters.reg_csr_mask =0xF ; +if (of_property_read_u32 (np,"enable-offset" , + ¶meters.reg_clk_offset)) + parameters.reg_clk_offset =0x8 ; +if (of_property_read_u32 (np,"enable-mask" , ¶meters.reg_clk_mask)) + parameters.reg_clk_mask =0xF ; +if (of_property_read_u32 (np,"divider-offset" , + ¶meters.reg_divider_offset)) + parameters.reg_divider_offset =0 ; +if (of_property_read_u32 (np,"divider-width" , + ¶meters.reg_divider_width)) + parameters.reg_divider_width =0 ; +if (of_property_read_u32 (np,"divider-shift" , + ¶meters.reg_divider_shift)) + parameters.reg_divider_shift =0 ; +of_property_read_string (np,"clock-output-names" , &clk_name); + + clk =xgene_register_clk (NULL , clk_name, +of_clk_get_parent_name (np,0 ), ¶meters, &clk_lock); +if (IS_ERR (clk)) { +if (parameters.csr_reg) +iounmap (parameters.csr_reg); +if (parameters.divider_reg) +iounmap (parameters.divider_reg);
drivers/clk/clk-xgene.c line 534
Quoted 10 lines
drivers/clk/clk-xgene.c new file +10
-0+return ; + } +pr_debug ("Add %s clock\n" , clk_name); + rc =of_clk_add_provider (np, of_clk_src_simple_get, clk); +if (rc !=0 ) { +pr_err ("%s: could register provider clk %s\n" , __func__, + np->full_name); +return ; + } +}
The return is currently unnecessary as there's nothing beyond it in the function. You could remove it, and since that leaves one statement in the braces, you could remove the braces too.
Thanks,
Mark.