* [PATCH 0/2] Move imx6q clock lookup over to device tree
@ 2012-08-16 8:08 Shawn Guo
2012-08-16 8:08 ` [PATCH 1/2] clk: add of_clk_src_onecell_get() support Shawn Guo
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Shawn Guo @ 2012-08-16 8:08 UTC (permalink / raw)
To: linux-arm-kernel
The series adds a generic of_clk_src_onecell_get() function into clock
framework, and then moves imx6q clock lookup over to device tree, so
that any new clock lookup to be added at later time does not need to
patch kerenel.
Shawn Guo (2):
clk: add of_clk_src_onecell_get() support
ARM: imx6q: replace clk_register_clkdev with clock DT lookup
.../devicetree/bindings/clock/imx6q-clock.txt | 223 +++++++++++++++++
arch/arm/boot/dts/imx6q-sabrelite.dts | 1 +
arch/arm/boot/dts/imx6q.dtsi | 261 +++++++++++++++++++-
arch/arm/mach-imx/clk-imx6q.c | 41 +---
arch/arm/mach-imx/mach-imx6q.c | 1 -
drivers/clk/clk.c | 15 ++
include/linux/clk-provider.h | 1 +
7 files changed, 493 insertions(+), 50 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/imx6q-clock.txt
--
1.7.5.4
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/2] clk: add of_clk_src_onecell_get() support
2012-08-16 8:08 [PATCH 0/2] Move imx6q clock lookup over to device tree Shawn Guo
@ 2012-08-16 8:08 ` Shawn Guo
2012-08-20 14:30 ` Shawn Guo
2012-08-16 8:08 ` [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup Shawn Guo
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Shawn Guo @ 2012-08-16 8:08 UTC (permalink / raw)
To: linux-arm-kernel
For those SoCs that have hundreds of clock outputs, their clock
DT bindings could reasonably define #clock-cells as 1 and require
the client device specify the index of the clock it consumes in the
cell of its "clocks" phandle.
Add a generic of_clk_src_onecell_get() function for this purpose.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/clk/clk.c | 15 +++++++++++++++
include/linux/clk-provider.h | 1 +
2 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efdfd00..06bc0b5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1582,6 +1582,21 @@ struct clk *of_clk_src_simple_get(struct of_phandle_args *clkspec,
}
EXPORT_SYMBOL_GPL(of_clk_src_simple_get);
+struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data)
+{
+ const char *clk_name;
+ int idx = clkspec->args[0];
+ int ret;
+
+ ret = of_property_read_string_index(clkspec->np, "clock-output-names",
+ idx, &clk_name);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ return __clk_lookup(clk_name);
+}
+EXPORT_SYMBOL_GPL(of_clk_src_onecell_get);
+
/**
* of_clk_add_provider() - Register a clock provider for a node
* @np: Device node pointer associated with clock provider
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 77335fa..2afcadf 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -360,6 +360,7 @@ int of_clk_add_provider(struct device_node *np,
void of_clk_del_provider(struct device_node *np);
struct clk *of_clk_src_simple_get(struct of_phandle_args *clkspec,
void *data);
+struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data);
const char *of_clk_get_parent_name(struct device_node *np, int index);
void of_clk_init(const struct of_device_id *matches);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-16 8:08 [PATCH 0/2] Move imx6q clock lookup over to device tree Shawn Guo
2012-08-16 8:08 ` [PATCH 1/2] clk: add of_clk_src_onecell_get() support Shawn Guo
@ 2012-08-16 8:08 ` Shawn Guo
2012-08-20 12:51 ` Rob Herring
2012-08-19 14:05 ` [PATCH 3/4] clk: mxs: replace imx28 " Shawn Guo
2012-08-19 14:05 ` [PATCH 4/4] clk: mxs: replace imx23 " Shawn Guo
3 siblings, 1 reply; 19+ messages in thread
From: Shawn Guo @ 2012-08-16 8:08 UTC (permalink / raw)
To: linux-arm-kernel
It really becomes an issue that every time a device needs to look
up (clk_get) a clock we have to patch kernel clock file to call
clk_register_clkdev for that clock.
Since clock DT support which is meant to resolve clock lookup in device
tree is in place, the patch moves imx6q client devices' clock lookup
over to device tree, so that any new lookup to be added at later time
can just get done in DT instead of kernel.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
.../devicetree/bindings/clock/imx6q-clock.txt | 223 +++++++++++++++++
arch/arm/boot/dts/imx6q-sabrelite.dts | 1 +
arch/arm/boot/dts/imx6q.dtsi | 261 +++++++++++++++++++-
arch/arm/mach-imx/clk-imx6q.c | 41 +---
arch/arm/mach-imx/mach-imx6q.c | 1 -
5 files changed, 477 insertions(+), 50 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/imx6q-clock.txt
diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
new file mode 100644
index 0000000..19d8126
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
@@ -0,0 +1,223 @@
+* Clock bindings for Freescale i.MX6 Quad
+
+Required properties:
+- compatible: Should be "fsl,imx6q-ccm"
+- reg: Address and length of the register set
+- interrupts: Should contain CCM interrupt
+- #clock-cells: Should be <1>
+- clock-output-names: A list of clock output names that CCM provides.
+ The full list of all valid clock names and IDs is below.
+
+ Name ID
+ ---------------------------
+ dummy 0
+ ckil 1
+ ckih 2
+ osc 3
+ pll2_pfd0_352m 4
+ pll2_pfd1_594m 5
+ pll2_pfd2_396m 6
+ pll3_pfd0_720m 7
+ pll3_pfd1_540m 8
+ pll3_pfd2_508m 9
+ pll3_pfd3_454m 10
+ pll2_198m 11
+ pll3_120m 12
+ pll3_80m 13
+ pll3_60m 14
+ twd 15
+ step 16
+ pll1_sw 17
+ periph_pre 18
+ periph2_pre 19
+ periph_clk2_sel 20
+ periph2_clk2_sel 21
+ axi_sel 22
+ esai_sel 23
+ asrc_sel 24
+ spdif_sel 25
+ gpu2d_axi 26
+ gpu3d_axi 27
+ gpu2d_core_sel 28
+ gpu3d_core_sel 29
+ gpu3d_shader_sel 30
+ ipu1_sel 31
+ ipu2_sel 32
+ ldb_di0_sel 33
+ ldb_di1_sel 34
+ ipu1_di0_pre_sel 35
+ ipu1_di1_pre_sel 36
+ ipu2_di0_pre_sel 37
+ ipu2_di1_pre_sel 38
+ ipu1_di0_sel 39
+ ipu1_di1_sel 40
+ ipu2_di0_sel 41
+ ipu2_di1_sel 42
+ hsi_tx_sel 43
+ pcie_axi_sel 44
+ ssi1_sel 45
+ ssi2_sel 46
+ ssi3_sel 47
+ usdhc1_sel 48
+ usdhc2_sel 49
+ usdhc3_sel 50
+ usdhc4_sel 51
+ enfc_sel 52
+ emi_sel 53
+ emi_slow_sel 54
+ vdo_axi_sel 55
+ vpu_axi_sel 56
+ cko1_sel 57
+ periph 58
+ periph2 59
+ periph_clk2 60
+ periph2_clk2 61
+ ipg 62
+ ipg_per 63
+ esai_pred 64
+ esai_podf 65
+ asrc_pred 66
+ asrc_podf 67
+ spdif_pred 68
+ spdif_podf 69
+ can_root 70
+ ecspi_root 71
+ gpu2d_core_podf 72
+ gpu3d_core_podf 73
+ gpu3d_shader 74
+ ipu1_podf 75
+ ipu2_podf 76
+ ldb_di0_podf 77
+ ldb_di1_podf 78
+ ipu1_di0_pre 79
+ ipu1_di1_pre 80
+ ipu2_di0_pre 81
+ ipu2_di1_pre 82
+ hsi_tx_podf 83
+ ssi1_pred 84
+ ssi1_podf 85
+ ssi2_pred 86
+ ssi2_podf 87
+ ssi3_pred 88
+ ssi3_podf 89
+ uart_serial_podf 90
+ usdhc1_podf 91
+ usdhc2_podf 92
+ usdhc3_podf 93
+ usdhc4_podf 94
+ enfc_pred 95
+ enfc_podf 96
+ emi_podf 97
+ emi_slow_podf 98
+ vpu_axi_podf 99
+ cko1_podf 100
+ axi 101
+ mmdc_ch0_axi_podf 102
+ mmdc_ch1_axi_podf 103
+ arm 104
+ ahb 105
+ apbh_dma 106
+ asrc 107
+ can1_ipg 108
+ can1_serial 109
+ can2_ipg 110
+ can2_serial 111
+ ecspi1 112
+ ecspi2 113
+ ecspi3 114
+ ecspi4 115
+ ecspi5 116
+ enet 117
+ esai 118
+ gpt_ipg 119
+ gpt_ipg_per 120
+ gpu2d_core 121
+ gpu3d_core 122
+ hdmi_iahb 123
+ hdmi_isfr 124
+ i2c1 125
+ i2c2 126
+ i2c3 127
+ iim 128
+ enfc 129
+ ipu1 130
+ ipu1_di0 131
+ ipu1_di1 132
+ ipu2 133
+ ipu2_di0 134
+ ldb_di0 135
+ ldb_di1 136
+ ipu2_di1 137
+ hsi_tx 138
+ mlb 139
+ mmdc_ch0_axi 140
+ mmdc_ch1_axi 141
+ ocram 142
+ openvg_axi 143
+ pcie_axi 144
+ pwm1 145
+ pwm2 146
+ pwm3 147
+ pwm4 148
+ per1_bch 149
+ gpmi_bch_apb 150
+ gpmi_bch 151
+ gpmi_io 152
+ gpmi_apb 153
+ sata 154
+ sdma 155
+ spba 156
+ ssi1 157
+ ssi2 158
+ ssi3 159
+ uart_ipg 160
+ uart_serial 161
+ usboh3 162
+ usdhc1 163
+ usdhc2 164
+ usdhc3 165
+ usdhc4 166
+ vdo_axi 167
+ vpu_axi 168
+ cko1 169
+ pll1_sys 170
+ pll2_bus 171
+ pll3_usb_otg 172
+ pll4_audio 173
+ pll5_video 174
+ pll6_mlb 175
+ pll7_usb_host 176
+ pll8_enet 177
+ ssi1_ipg 178
+ ssi2_ipg 179
+ ssi3_ipg 180
+ rom 181
+ usbphy1 182
+ usbphy2 183
+ ldb_di0_div_3_5 184
+ ldb_di1_div_3_5 185
+
+ The ID will be used by clock consumer in the first cell of "clocks"
+ phandle to specify the desired clock.
+
+Examples:
+
+clks: ccm at 020c4000 {
+ compatible = "fsl,imx6q-ccm";
+ reg = <0x020c4000 0x4000>;
+ interrupts = <0 87 0x04 0 88 0x04>;
+ #clock-cells = <1>;
+ clock-output-names = ...
+ "uart_ipg",
+ "uart_serial",
+ ...;
+};
+
+uart1: serial at 02020000 {
+ compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
+ reg = <0x02020000 0x4000>;
+ interrupts = <0 26 0x04>;
+ clocks = <&clks 160>, <&clks 161>;
+ clock-names = "ipg", "per";
+ status = "disabled";
+};
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index 72f30f3..cfdbe53 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -111,6 +111,7 @@
codec: sgtl5000 at 0a {
compatible = "fsl,sgtl5000";
reg = <0x0a>;
+ clocks = <&clks 169>;
VDDA-supply = <®_2p5v>;
VDDIO-supply = <®_3p3v>;
};
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 0052fe7..acff2dc 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -93,18 +93,23 @@
dma-apbh at 00110000 {
compatible = "fsl,imx6q-dma-apbh", "fsl,imx28-dma-apbh";
reg = <0x00110000 0x2000>;
+ clocks = <&clks 106>;
};
gpmi-nand at 00112000 {
- compatible = "fsl,imx6q-gpmi-nand";
- #address-cells = <1>;
- #size-cells = <1>;
- reg = <0x00112000 0x2000>, <0x00114000 0x2000>;
- reg-names = "gpmi-nand", "bch";
- interrupts = <0 13 0x04>, <0 15 0x04>;
- interrupt-names = "gpmi-dma", "bch";
- fsl,gpmi-dma-channel = <0>;
- status = "disabled";
+ compatible = "fsl,imx6q-gpmi-nand";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x00112000 0x2000>, <0x00114000 0x2000>;
+ reg-names = "gpmi-nand", "bch";
+ interrupts = <0 13 0x04>, <0 15 0x04>;
+ interrupt-names = "gpmi-dma", "bch";
+ clocks = <&clks 152>, <&clks 153>, <&clks 151>,
+ <&clks 150>, <&clks 149>;
+ clock-names = "gpmi_io", "gpmi_apb", "gpmi_bch",
+ "gpmi_bch_apb", "per1_bch";
+ fsl,gpmi-dma-channel = <0>;
+ status = "disabled";
};
timer at 00a00600 {
@@ -146,6 +151,8 @@
compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
reg = <0x02008000 0x4000>;
interrupts = <0 31 0x04>;
+ clocks = <&clks 112>, <&clks 112>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -155,6 +162,8 @@
compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
reg = <0x0200c000 0x4000>;
interrupts = <0 32 0x04>;
+ clocks = <&clks 113>, <&clks 113>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -164,6 +173,8 @@
compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
reg = <0x02010000 0x4000>;
interrupts = <0 33 0x04>;
+ clocks = <&clks 114>, <&clks 114>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -173,6 +184,8 @@
compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
reg = <0x02014000 0x4000>;
interrupts = <0 34 0x04>;
+ clocks = <&clks 115>, <&clks 115>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -182,6 +195,8 @@
compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
reg = <0x02018000 0x4000>;
interrupts = <0 35 0x04>;
+ clocks = <&clks 116>, <&clks 116>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -189,6 +204,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x02020000 0x4000>;
interrupts = <0 26 0x04>;
+ clocks = <&clks 160>, <&clks 161>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -201,6 +218,7 @@
compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
reg = <0x02028000 0x4000>;
interrupts = <0 46 0x04>;
+ clocks = <&clks 178>;
fsl,fifo-depth = <15>;
fsl,ssi-dma-events = <38 37>;
status = "disabled";
@@ -210,6 +228,7 @@
compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
reg = <0x0202c000 0x4000>;
interrupts = <0 47 0x04>;
+ clocks = <&clks 179>;
fsl,fifo-depth = <15>;
fsl,ssi-dma-events = <42 41>;
status = "disabled";
@@ -219,6 +238,7 @@
compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
reg = <0x02030000 0x4000>;
interrupts = <0 48 0x04>;
+ clocks = <&clks 180>;
fsl,fifo-depth = <15>;
fsl,ssi-dma-events = <46 45>;
status = "disabled";
@@ -358,6 +378,7 @@
compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
reg = <0x020bc000 0x4000>;
interrupts = <0 80 0x04>;
+ clocks = <&clks 0>;
status = "disabled";
};
@@ -365,13 +386,203 @@
compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
reg = <0x020c0000 0x4000>;
interrupts = <0 81 0x04>;
+ clocks = <&clks 0>;
status = "disabled";
};
- ccm at 020c4000 {
+ clks: ccm at 020c4000 {
compatible = "fsl,imx6q-ccm";
reg = <0x020c4000 0x4000>;
interrupts = <0 87 0x04 0 88 0x04>;
+ #clock-cells = <1>;
+ clock-output-names =
+ "dummy", /* 0 */
+ "ckil", /* 1 */
+ "ckih", /* 2 */
+ "osc", /* 3 */
+ "pll2_pfd0_352m", /* 4 */
+ "pll2_pfd1_594m", /* 5 */
+ "pll2_pfd2_396m", /* 6 */
+ "pll3_pfd0_720m", /* 7 */
+ "pll3_pfd1_540m", /* 8 */
+ "pll3_pfd2_508m", /* 9 */
+ "pll3_pfd3_454m", /* 10 */
+ "pll2_198m", /* 11 */
+ "pll3_120m", /* 12 */
+ "pll3_80m", /* 13 */
+ "pll3_60m", /* 14 */
+ "twd", /* 15 */
+ "step", /* 16 */
+ "pll1_sw", /* 17 */
+ "periph_pre", /* 18 */
+ "periph2_pre", /* 19 */
+ "periph_clk2_sel", /* 20 */
+ "periph2_clk2_sel", /* 21 */
+ "axi_sel", /* 22 */
+ "esai_sel", /* 23 */
+ "asrc_sel", /* 24 */
+ "spdif_sel", /* 25 */
+ "gpu2d_axi", /* 26 */
+ "gpu3d_axi", /* 27 */
+ "gpu2d_core_sel", /* 28 */
+ "gpu3d_core_sel", /* 29 */
+ "gpu3d_shader_sel", /* 30 */
+ "ipu1_sel", /* 31 */
+ "ipu2_sel", /* 32 */
+ "ldb_di0_sel", /* 33 */
+ "ldb_di1_sel", /* 34 */
+ "ipu1_di0_pre_sel", /* 35 */
+ "ipu1_di1_pre_sel", /* 36 */
+ "ipu2_di0_pre_sel", /* 37 */
+ "ipu2_di1_pre_sel", /* 38 */
+ "ipu1_di0_sel", /* 39 */
+ "ipu1_di1_sel", /* 40 */
+ "ipu2_di0_sel", /* 41 */
+ "ipu2_di1_sel", /* 42 */
+ "hsi_tx_sel", /* 43 */
+ "pcie_axi_sel", /* 44 */
+ "ssi1_sel", /* 45 */
+ "ssi2_sel", /* 46 */
+ "ssi3_sel", /* 47 */
+ "usdhc1_sel", /* 48 */
+ "usdhc2_sel", /* 49 */
+ "usdhc3_sel", /* 50 */
+ "usdhc4_sel", /* 51 */
+ "enfc_sel", /* 52 */
+ "emi_sel", /* 53 */
+ "emi_slow_sel", /* 54 */
+ "vdo_axi_sel", /* 55 */
+ "vpu_axi_sel", /* 56 */
+ "cko1_sel", /* 57 */
+ "periph", /* 58 */
+ "periph2", /* 59 */
+ "periph_clk2", /* 60 */
+ "periph2_clk2", /* 61 */
+ "ipg", /* 62 */
+ "ipg_per", /* 63 */
+ "esai_pred", /* 64 */
+ "esai_podf", /* 65 */
+ "asrc_pred", /* 66 */
+ "asrc_podf", /* 67 */
+ "spdif_pred", /* 68 */
+ "spdif_podf", /* 69 */
+ "can_root", /* 70 */
+ "ecspi_root", /* 71 */
+ "gpu2d_core_podf", /* 72 */
+ "gpu3d_core_podf", /* 73 */
+ "gpu3d_shader", /* 74 */
+ "ipu1_podf", /* 75 */
+ "ipu2_podf", /* 76 */
+ "ldb_di0_podf", /* 77 */
+ "ldb_di1_podf", /* 78 */
+ "ipu1_di0_pre", /* 79 */
+ "ipu1_di1_pre", /* 80 */
+ "ipu2_di0_pre", /* 81 */
+ "ipu2_di1_pre", /* 82 */
+ "hsi_tx_podf", /* 83 */
+ "ssi1_pred", /* 84 */
+ "ssi1_podf", /* 85 */
+ "ssi2_pred", /* 86 */
+ "ssi2_podf", /* 87 */
+ "ssi3_pred", /* 88 */
+ "ssi3_podf", /* 89 */
+ "uart_serial_podf", /* 90 */
+ "usdhc1_podf", /* 91 */
+ "usdhc2_podf", /* 92 */
+ "usdhc3_podf", /* 93 */
+ "usdhc4_podf", /* 94 */
+ "enfc_pred", /* 95 */
+ "enfc_podf", /* 96 */
+ "emi_podf", /* 97 */
+ "emi_slow_podf", /* 98 */
+ "vpu_axi_podf", /* 99 */
+ "cko1_podf", /* 100 */
+ "axi", /* 101 */
+ "mmdc_ch0_axi_podf", /* 102 */
+ "mmdc_ch1_axi_podf", /* 103 */
+ "arm", /* 104 */
+ "ahb", /* 105 */
+ "apbh_dma", /* 106 */
+ "asrc", /* 107 */
+ "can1_ipg", /* 108 */
+ "can1_serial", /* 109 */
+ "can2_ipg", /* 110 */
+ "can2_serial", /* 111 */
+ "ecspi1", /* 112 */
+ "ecspi2", /* 113 */
+ "ecspi3", /* 114 */
+ "ecspi4", /* 115 */
+ "ecspi5", /* 116 */
+ "enet", /* 117 */
+ "esai", /* 118 */
+ "gpt_ipg", /* 119 */
+ "gpt_ipg_per", /* 120 */
+ "gpu2d_core", /* 121 */
+ "gpu3d_core", /* 122 */
+ "hdmi_iahb", /* 123 */
+ "hdmi_isfr", /* 124 */
+ "i2c1", /* 125 */
+ "i2c2", /* 126 */
+ "i2c3", /* 127 */
+ "iim", /* 128 */
+ "enfc", /* 129 */
+ "ipu1", /* 130 */
+ "ipu1_di0", /* 131 */
+ "ipu1_di1", /* 132 */
+ "ipu2", /* 133 */
+ "ipu2_di0", /* 134 */
+ "ldb_di0", /* 135 */
+ "ldb_di1", /* 136 */
+ "ipu2_di1", /* 137 */
+ "hsi_tx", /* 138 */
+ "mlb", /* 139 */
+ "mmdc_ch0_axi", /* 140 */
+ "mmdc_ch1_axi", /* 141 */
+ "ocram", /* 142 */
+ "openvg_axi", /* 143 */
+ "pcie_axi", /* 144 */
+ "pwm1", /* 145 */
+ "pwm2", /* 146 */
+ "pwm3", /* 147 */
+ "pwm4", /* 148 */
+ "per1_bch", /* 149 */
+ "gpmi_bch_apb", /* 150 */
+ "gpmi_bch", /* 151 */
+ "gpmi_io", /* 152 */
+ "gpmi_apb", /* 153 */
+ "sata", /* 154 */
+ "sdma", /* 155 */
+ "spba", /* 156 */
+ "ssi1", /* 157 */
+ "ssi2", /* 158 */
+ "ssi3", /* 159 */
+ "uart_ipg", /* 160 */
+ "uart_serial", /* 161 */
+ "usboh3", /* 162 */
+ "usdhc1", /* 163 */
+ "usdhc2", /* 164 */
+ "usdhc3", /* 165 */
+ "usdhc4", /* 166 */
+ "vdo_axi", /* 167 */
+ "vpu_axi", /* 168 */
+ "cko1", /* 169 */
+ "pll1_sys", /* 170 */
+ "pll2_bus", /* 171 */
+ "pll3_usb_otg", /* 172 */
+ "pll4_audio", /* 173 */
+ "pll5_video", /* 174 */
+ "pll6_mlb", /* 175 */
+ "pll7_usb_host", /* 176 */
+ "pll8_enet", /* 177 */
+ "ssi1_ipg", /* 178 */
+ "ssi2_ipg", /* 179 */
+ "ssi3_ipg", /* 180 */
+ "rom", /* 181 */
+ "usbphy1", /* 182 */
+ "usbphy2", /* 183 */
+ "ldb_di0_div_3_5", /* 184 */
+ "ldb_di1_div_3_5", /* 185 */
+ "end_of_list";
};
anatop at 020c8000 {
@@ -468,12 +679,14 @@
compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
reg = <0x020c9000 0x1000>;
interrupts = <0 44 0x04>;
+ clocks = <&clks 182>;
};
usbphy2: usbphy at 020ca000 {
compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
reg = <0x020ca000 0x1000>;
interrupts = <0 45 0x04>;
+ clocks = <&clks 183>;
};
snvs at 020cc000 {
@@ -608,6 +821,9 @@
compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
reg = <0x020ec000 0x4000>;
interrupts = <0 2 0x04>;
+ clocks = <&clks 155>, <&clks 155>;
+ clock-names = "ipg", "ahb";
+ fsl,sdma-ram-script-name = "imx/sdma/sdma-imx6q-to1.bin";
};
};
@@ -631,6 +847,7 @@
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x02184000 0x200>;
interrupts = <0 43 0x04>;
+ clocks = <&clks 162>;
fsl,usbphy = <&usbphy1>;
status = "disabled";
};
@@ -639,6 +856,7 @@
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x02184200 0x200>;
interrupts = <0 40 0x04>;
+ clocks = <&clks 162>;
fsl,usbphy = <&usbphy2>;
status = "disabled";
};
@@ -647,6 +865,7 @@
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x02184400 0x200>;
interrupts = <0 41 0x04>;
+ clocks = <&clks 162>;
status = "disabled";
};
@@ -654,6 +873,7 @@
compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
reg = <0x02184600 0x200>;
interrupts = <0 42 0x04>;
+ clocks = <&clks 162>;
status = "disabled";
};
@@ -661,6 +881,8 @@
compatible = "fsl,imx6q-fec";
reg = <0x02188000 0x4000>;
interrupts = <0 118 0x04 0 119 0x04>;
+ clocks = <&clks 117>, <&clks 117>;
+ clock-names = "ipg", "ahb";
status = "disabled";
};
@@ -673,6 +895,8 @@
compatible = "fsl,imx6q-usdhc";
reg = <0x02190000 0x4000>;
interrupts = <0 22 0x04>;
+ clocks = <&clks 163>, <&clks 163>, <&clks 163>;
+ clock-names = "ipg", "ahb", "per";
status = "disabled";
};
@@ -680,6 +904,8 @@
compatible = "fsl,imx6q-usdhc";
reg = <0x02194000 0x4000>;
interrupts = <0 23 0x04>;
+ clocks = <&clks 164>, <&clks 164>, <&clks 164>;
+ clock-names = "ipg", "ahb", "per";
status = "disabled";
};
@@ -687,6 +913,8 @@
compatible = "fsl,imx6q-usdhc";
reg = <0x02198000 0x4000>;
interrupts = <0 24 0x04>;
+ clocks = <&clks 165>, <&clks 165>, <&clks 165>;
+ clock-names = "ipg", "ahb", "per";
status = "disabled";
};
@@ -694,6 +922,8 @@
compatible = "fsl,imx6q-usdhc";
reg = <0x0219c000 0x4000>;
interrupts = <0 25 0x04>;
+ clocks = <&clks 166>, <&clks 166>, <&clks 166>;
+ clock-names = "ipg", "ahb", "per";
status = "disabled";
};
@@ -703,6 +933,7 @@
compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
reg = <0x021a0000 0x4000>;
interrupts = <0 36 0x04>;
+ clocks = <&clks 125>;
status = "disabled";
};
@@ -712,6 +943,7 @@
compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
reg = <0x021a4000 0x4000>;
interrupts = <0 37 0x04>;
+ clocks = <&clks 126>;
status = "disabled";
};
@@ -721,6 +953,7 @@
compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
reg = <0x021a8000 0x4000>;
interrupts = <0 38 0x04>;
+ clocks = <&clks 127>;
status = "disabled";
};
@@ -784,6 +1017,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x021e8000 0x4000>;
interrupts = <0 27 0x04>;
+ clocks = <&clks 160>, <&clks 161>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -791,6 +1026,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x021ec000 0x4000>;
interrupts = <0 28 0x04>;
+ clocks = <&clks 160>, <&clks 161>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -798,6 +1035,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x021f0000 0x4000>;
interrupts = <0 29 0x04>;
+ clocks = <&clks 160>, <&clks 161>;
+ clock-names = "ipg", "per";
status = "disabled";
};
@@ -805,6 +1044,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x021f4000 0x4000>;
interrupts = <0 30 0x04>;
+ clocks = <&clks 160>, <&clks 161>;
+ clock-names = "ipg", "per";
status = "disabled";
};
};
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 8e46407..433c683 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -387,48 +387,11 @@ int __init mx6q_clocks_init(void)
pr_err("i.MX6q clk %d: register failed with %ld\n",
i, PTR_ERR(clk[i]));
+ of_clk_add_provider(np, of_clk_src_onecell_get, NULL);
+
clk_register_clkdev(clk[gpt_ipg], "ipg", "imx-gpt.0");
clk_register_clkdev(clk[gpt_ipg_per], "per", "imx-gpt.0");
clk_register_clkdev(clk[twd], NULL, "smp_twd");
- clk_register_clkdev(clk[apbh_dma], NULL, "110000.dma-apbh");
- clk_register_clkdev(clk[per1_bch], "per1_bch", "112000.gpmi-nand");
- clk_register_clkdev(clk[gpmi_bch_apb], "gpmi_bch_apb", "112000.gpmi-nand");
- clk_register_clkdev(clk[gpmi_bch], "gpmi_bch", "112000.gpmi-nand");
- clk_register_clkdev(clk[gpmi_apb], "gpmi_apb", "112000.gpmi-nand");
- clk_register_clkdev(clk[gpmi_io], "gpmi_io", "112000.gpmi-nand");
- clk_register_clkdev(clk[usboh3], NULL, "2184000.usb");
- clk_register_clkdev(clk[usboh3], NULL, "2184200.usb");
- clk_register_clkdev(clk[usboh3], NULL, "2184400.usb");
- clk_register_clkdev(clk[usboh3], NULL, "2184600.usb");
- clk_register_clkdev(clk[usbphy1], NULL, "20c9000.usbphy");
- clk_register_clkdev(clk[usbphy2], NULL, "20ca000.usbphy");
- clk_register_clkdev(clk[uart_serial], "per", "2020000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "2020000.serial");
- clk_register_clkdev(clk[uart_serial], "per", "21e8000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "21e8000.serial");
- clk_register_clkdev(clk[uart_serial], "per", "21ec000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "21ec000.serial");
- clk_register_clkdev(clk[uart_serial], "per", "21f0000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "21f0000.serial");
- clk_register_clkdev(clk[uart_serial], "per", "21f4000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "21f4000.serial");
- clk_register_clkdev(clk[enet], NULL, "2188000.ethernet");
- clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc");
- clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc");
- clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc");
- clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");
- clk_register_clkdev(clk[i2c1], NULL, "21a0000.i2c");
- clk_register_clkdev(clk[i2c2], NULL, "21a4000.i2c");
- clk_register_clkdev(clk[i2c3], NULL, "21a8000.i2c");
- clk_register_clkdev(clk[ecspi1], NULL, "2008000.ecspi");
- clk_register_clkdev(clk[ecspi2], NULL, "200c000.ecspi");
- clk_register_clkdev(clk[ecspi3], NULL, "2010000.ecspi");
- clk_register_clkdev(clk[ecspi4], NULL, "2014000.ecspi");
- clk_register_clkdev(clk[ecspi5], NULL, "2018000.ecspi");
- clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma");
- clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
- clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
- clk_register_clkdev(clk[ssi1_ipg], NULL, "2028000.ssi");
clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL);
clk_register_clkdev(clk[ahb], "ahb", NULL);
clk_register_clkdev(clk[cko1], "cko1", NULL);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 6f9c23b..957f5fe 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -94,7 +94,6 @@ static void __init imx6q_sabrelite_cko1_setup(void)
clk_set_parent(cko1_sel, ahb);
rate = clk_round_rate(cko1, 16000000);
clk_set_rate(cko1, rate);
- clk_register_clkdev(cko1, NULL, "0-000a");
put_clk:
if (!IS_ERR(cko1_sel))
clk_put(cko1_sel);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/4] clk: mxs: replace imx28 clk_register_clkdev with clock DT lookup
2012-08-16 8:08 [PATCH 0/2] Move imx6q clock lookup over to device tree Shawn Guo
2012-08-16 8:08 ` [PATCH 1/2] clk: add of_clk_src_onecell_get() support Shawn Guo
2012-08-16 8:08 ` [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup Shawn Guo
@ 2012-08-19 14:05 ` Shawn Guo
2012-08-19 14:05 ` [PATCH 4/4] clk: mxs: replace imx23 " Shawn Guo
3 siblings, 0 replies; 19+ messages in thread
From: Shawn Guo @ 2012-08-19 14:05 UTC (permalink / raw)
To: linux-arm-kernel
It really becomes a maintenance issue that every time a device needs
to look up (clk_get) a clock we have to patch kernel clock file to call
clk_register_clkdev for that clock.
Since clock DT support which is meant to resolve clock lookup in device
tree is in place, the patch moves imx28 client devices' clock lookup
over to device tree, so that any new lookup to be added at later time
can just get done in DT instead of kernel.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
.../devicetree/bindings/clock/imx28-clock.txt | 100 ++++++++++++++++++
arch/arm/boot/dts/imx28.dtsi | 100 ++++++++++++++++++-
drivers/clk/mxs/clk-imx28.c | 109 +------------------
3 files changed, 204 insertions(+), 105 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/imx28-clock.txt
diff --git a/Documentation/devicetree/bindings/clock/imx28-clock.txt b/Documentation/devicetree/bindings/clock/imx28-clock.txt
new file mode 100644
index 0000000..85b72e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx28-clock.txt
@@ -0,0 +1,100 @@
+* Clock bindings for Freescale i.MX28
+
+Required properties:
+- compatible: Should be "fsl,imx28-clkctrl"
+- reg: Address and length of the register set
+- #clock-cells: Should be <1>
+- clock-output-names: A list of clock output names that i.MX28 CLKCTRL
+ provides. The full list of all valid clock names and IDs is below.
+
+ Name ID
+ ------------------
+ ref_xtal 0
+ pll0 1
+ pll1 2
+ pll2 3
+ ref_cpu 4
+ ref_emi 5
+ ref_io0 6
+ ref_io1 7
+ ref_pix 8
+ ref_hsadc 9
+ ref_gpmi 10
+ saif0_sel 11
+ saif1_sel 12
+ gpmi_sel 13
+ ssp0_sel 14
+ ssp1_sel 15
+ ssp2_sel 16
+ ssp3_sel 17
+ emi_sel 18
+ etm_sel 19
+ lcdif_sel 20
+ cpu 21
+ ptp_sel 22
+ cpu_pll 23
+ cpu_xtal 24
+ hbus 25
+ xbus 26
+ ssp0_div 27
+ ssp1_div 28
+ ssp2_div 29
+ ssp3_div 30
+ gpmi_div 31
+ emi_pll 32
+ emi_xtal 33
+ lcdif_div 34
+ etm_div 35
+ ptp 36
+ saif0_div 37
+ saif1_div 38
+ clk32k_div 39
+ rtc 40
+ lradc 41
+ spdif_div 42
+ clk32k 43
+ pwm 44
+ uart 45
+ ssp0 46
+ ssp1 47
+ ssp2 48
+ ssp3 49
+ gpmi 50
+ spdif 51
+ emi 52
+ saif0 53
+ saif1 54
+ lcdif 55
+ etm 56
+ fec 57
+ can0 58
+ can1 59
+ usb0 60
+ usb1 61
+ usb0_pwr 62
+ usb1_pwr 63
+ enet_out 64
+
+ The ID will be used by clock consumer in the first cell of "clocks"
+ phandle to specify the desired clock.
+
+Examples:
+
+clks: clkctrl at 80040000 {
+ compatible = "fsl,imx28-clkctrl";
+ reg = <0x80040000 0x2000>;
+ #clock-cells = <1>;
+ clock-output-names =
+ ...
+ "uart", /* 45 */
+ ...
+ "end_of_list";
+};
+
+auart0: serial at 8006a000 {
+ compatible = "fsl,imx28-auart", "fsl,imx23-auart";
+ reg = <0x8006a000 0x2000>;
+ interrupts = <112 70 71>;
+ clocks = <&clks 45>;
+ status = "disabled";
+};
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index e2240da..2e3ec2e 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -65,6 +65,7 @@
dma-apbh at 80004000 {
compatible = "fsl,imx28-dma-apbh";
reg = <0x80004000 0x2000>;
+ clocks = <&clks 25>;
};
perfmon at 80006000 {
@@ -81,6 +82,7 @@
reg-names = "gpmi-nand", "bch";
interrupts = <88>, <41>;
interrupt-names = "gpmi-dma", "bch";
+ clocks = <&clks 50>;
fsl,gpmi-dma-channel = <4>;
status = "disabled";
};
@@ -88,6 +90,7 @@
ssp0: ssp at 80010000 {
reg = <0x80010000 0x2000>;
interrupts = <96 82>;
+ clocks = <&clks 46>;
fsl,ssp-dma-channel = <0>;
status = "disabled";
};
@@ -95,6 +98,7 @@
ssp1: ssp at 80012000 {
reg = <0x80012000 0x2000>;
interrupts = <97 83>;
+ clocks = <&clks 47>;
fsl,ssp-dma-channel = <1>;
status = "disabled";
};
@@ -102,6 +106,7 @@
ssp2: ssp at 80014000 {
reg = <0x80014000 0x2000>;
interrupts = <98 84>;
+ clocks = <&clks 48>;
fsl,ssp-dma-channel = <2>;
status = "disabled";
};
@@ -109,6 +114,7 @@
ssp3: ssp at 80016000 {
reg = <0x80016000 0x2000>;
interrupts = <99 85>;
+ clocks = <&clks 49>;
fsl,ssp-dma-channel = <3>;
status = "disabled";
};
@@ -523,6 +529,7 @@
dma-apbx at 80024000 {
compatible = "fsl,imx28-dma-apbx";
reg = <0x80024000 0x2000>;
+ clocks = <&clks 26>;
};
dcp at 80028000 {
@@ -551,6 +558,7 @@
compatible = "fsl,imx28-lcdif";
reg = <0x80030000 0x2000>;
interrupts = <38 86>;
+ clocks = <&clks 55>;
status = "disabled";
};
@@ -558,6 +566,7 @@
compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
reg = <0x80032000 0x2000>;
interrupts = <8>;
+ clocks = <&clks 58>;
status = "disabled";
};
@@ -565,6 +574,7 @@
compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
reg = <0x80034000 0x2000>;
interrupts = <9>;
+ clocks = <&clks 59>;
status = "disabled";
};
@@ -611,15 +621,84 @@
reg = <0x80040000 0x40000>;
ranges;
- clkctl at 80040000 {
+ clks: clkctrl at 80040000 {
+ compatible = "fsl,imx28-clkctrl";
reg = <0x80040000 0x2000>;
- status = "disabled";
+ #clock-cells = <1>;
+ clock-output-names =
+ "ref_xtal", /* 0 */
+ "pll0", /* 1 */
+ "pll1", /* 2 */
+ "pll2", /* 3 */
+ "ref_cpu", /* 4 */
+ "ref_emi", /* 5 */
+ "ref_io0", /* 6 */
+ "ref_io1", /* 7 */
+ "ref_pix", /* 8 */
+ "ref_hsadc", /* 9 */
+ "ref_gpmi", /* 10 */
+ "saif0_sel", /* 11 */
+ "saif1_sel", /* 12 */
+ "gpmi_sel", /* 13 */
+ "ssp0_sel", /* 14 */
+ "ssp1_sel", /* 15 */
+ "ssp2_sel", /* 16 */
+ "ssp3_sel", /* 17 */
+ "emi_sel", /* 18 */
+ "etm_sel", /* 19 */
+ "lcdif_sel", /* 20 */
+ "cpu", /* 21 */
+ "ptp_sel", /* 22 */
+ "cpu_pll", /* 23 */
+ "cpu_xtal", /* 24 */
+ "hbus", /* 25 */
+ "xbus", /* 26 */
+ "ssp0_div", /* 27 */
+ "ssp1_div", /* 28 */
+ "ssp2_div", /* 29 */
+ "ssp3_div", /* 30 */
+ "gpmi_div", /* 31 */
+ "emi_pll", /* 32 */
+ "emi_xtal", /* 33 */
+ "lcdif_div", /* 34 */
+ "etm_div", /* 35 */
+ "ptp", /* 36 */
+ "saif0_div", /* 37 */
+ "saif1_div", /* 38 */
+ "clk32k_div", /* 39 */
+ "rtc", /* 40 */
+ "lradc", /* 41 */
+ "spdif_div", /* 42 */
+ "clk32k", /* 43 */
+ "pwm", /* 44 */
+ "uart", /* 45 */
+ "ssp0", /* 46 */
+ "ssp1", /* 47 */
+ "ssp2", /* 48 */
+ "ssp3", /* 49 */
+ "gpmi", /* 50 */
+ "spdif", /* 51 */
+ "emi", /* 52 */
+ "saif0", /* 53 */
+ "saif1", /* 54 */
+ "lcdif", /* 55 */
+ "etm", /* 56 */
+ "fec", /* 57 */
+ "can0", /* 58 */
+ "can1", /* 59 */
+ "usb0", /* 60 */
+ "usb1", /* 61 */
+ "usb0_pwr", /* 62 */
+ "usb1_pwr", /* 63 */
+ "enet_out", /* 64 */
+ "end_of_list";
};
saif0: saif at 80042000 {
compatible = "fsl,imx28-saif";
reg = <0x80042000 0x2000>;
interrupts = <59 80>;
+ clocks = <&clks 53>;
fsl,saif-dma-channel = <4>;
status = "disabled";
};
@@ -633,6 +712,7 @@
compatible = "fsl,imx28-saif";
reg = <0x80046000 0x2000>;
interrupts = <58 81>;
+ clocks = <&clks 54>;
fsl,saif-dma-channel = <5>;
status = "disabled";
};
@@ -680,6 +760,7 @@
pwm: pwm at 80064000 {
compatible = "fsl,imx28-pwm", "fsl,imx23-pwm";
reg = <0x80064000 0x2000>;
+ clocks = <&clks 44>;
#pwm-cells = <2>;
fsl,pwm-number = <8>;
status = "disabled";
@@ -694,6 +775,7 @@
compatible = "fsl,imx28-auart", "fsl,imx23-auart";
reg = <0x8006a000 0x2000>;
interrupts = <112 70 71>;
+ clocks = <&clks 45>;
status = "disabled";
};
@@ -701,6 +783,7 @@
compatible = "fsl,imx28-auart", "fsl,imx23-auart";
reg = <0x8006c000 0x2000>;
interrupts = <113 72 73>;
+ clocks = <&clks 45>;
status = "disabled";
};
@@ -708,6 +791,7 @@
compatible = "fsl,imx28-auart", "fsl,imx23-auart";
reg = <0x8006e000 0x2000>;
interrupts = <114 74 75>;
+ clocks = <&clks 45>;
status = "disabled";
};
@@ -715,6 +799,7 @@
compatible = "fsl,imx28-auart", "fsl,imx23-auart";
reg = <0x80070000 0x2000>;
interrupts = <115 76 77>;
+ clocks = <&clks 45>;
status = "disabled";
};
@@ -722,6 +807,7 @@
compatible = "fsl,imx28-auart", "fsl,imx23-auart";
reg = <0x80072000 0x2000>;
interrupts = <116 78 79>;
+ clocks = <&clks 45>;
status = "disabled";
};
@@ -729,18 +815,22 @@
compatible = "arm,pl011", "arm,primecell";
reg = <0x80074000 0x1000>;
interrupts = <47>;
+ clocks = <&clks 45>, <&clks 26>;
+ clock-names = "uart", "apb_pclk";
status = "disabled";
};
usbphy0: usbphy at 8007c000 {
compatible = "fsl,imx28-usbphy", "fsl,imx23-usbphy";
reg = <0x8007c000 0x2000>;
+ clocks = <&clks 62>;
status = "disabled";
};
usbphy1: usbphy at 8007e000 {
compatible = "fsl,imx28-usbphy", "fsl,imx23-usbphy";
reg = <0x8007e000 0x2000>;
+ clocks = <&clks 63>;
status = "disabled";
};
};
@@ -757,6 +847,7 @@
compatible = "fsl,imx28-usb", "fsl,imx27-usb";
reg = <0x80080000 0x10000>;
interrupts = <93>;
+ clocks = <&clks 60>;
fsl,usbphy = <&usbphy0>;
status = "disabled";
};
@@ -765,6 +856,7 @@
compatible = "fsl,imx28-usb", "fsl,imx27-usb";
reg = <0x80090000 0x10000>;
interrupts = <92>;
+ clocks = <&clks 61>;
fsl,usbphy = <&usbphy1>;
status = "disabled";
};
@@ -778,6 +870,8 @@
compatible = "fsl,imx28-fec";
reg = <0x800f0000 0x4000>;
interrupts = <101>;
+ clocks = <&clks 57>, <&clks 57>;
+ clock-names = "ipg", "ahb";
status = "disabled";
};
@@ -785,6 +879,8 @@
compatible = "fsl,imx28-fec";
reg = <0x800f4000 0x4000>;
interrupts = <102>;
+ clocks = <&clks 57>, <&clks 57>;
+ clock-names = "ipg", "ahb";
status = "disabled";
};
diff --git a/drivers/clk/mxs/clk-imx28.c b/drivers/clk/mxs/clk-imx28.c
index e3aab67..3f62e23 100644
--- a/drivers/clk/mxs/clk-imx28.c
+++ b/drivers/clk/mxs/clk-imx28.c
@@ -14,6 +14,7 @@
#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/of.h>
#include <mach/common.h>
#include <mach/mx28.h>
#include "clk.h"
@@ -120,90 +121,6 @@ static void __init clk_misc_init(void)
writel_relaxed(val, FRAC0);
}
-static struct clk_lookup uart_lookups[] = {
- { .dev_id = "duart", },
- { .dev_id = "mxs-auart.0", },
- { .dev_id = "mxs-auart.1", },
- { .dev_id = "mxs-auart.2", },
- { .dev_id = "mxs-auart.3", },
- { .dev_id = "mxs-auart.4", },
- { .dev_id = "8006a000.serial", },
- { .dev_id = "8006c000.serial", },
- { .dev_id = "8006e000.serial", },
- { .dev_id = "80070000.serial", },
- { .dev_id = "80072000.serial", },
- { .dev_id = "80074000.serial", },
-};
-
-static struct clk_lookup hbus_lookups[] = {
- { .dev_id = "imx28-dma-apbh", },
- { .dev_id = "80004000.dma-apbh", },
-};
-
-static struct clk_lookup xbus_lookups[] = {
- { .dev_id = "duart", .con_id = "apb_pclk"},
- { .dev_id = "80074000.serial", .con_id = "apb_pclk"},
- { .dev_id = "imx28-dma-apbx", },
- { .dev_id = "80024000.dma-apbx", },
-};
-
-static struct clk_lookup ssp0_lookups[] = {
- { .dev_id = "imx28-mmc.0", },
- { .dev_id = "80010000.ssp", },
-};
-
-static struct clk_lookup ssp1_lookups[] = {
- { .dev_id = "imx28-mmc.1", },
- { .dev_id = "80012000.ssp", },
-};
-
-static struct clk_lookup ssp2_lookups[] = {
- { .dev_id = "imx28-mmc.2", },
- { .dev_id = "80014000.ssp", },
-};
-
-static struct clk_lookup ssp3_lookups[] = {
- { .dev_id = "imx28-mmc.3", },
- { .dev_id = "80016000.ssp", },
-};
-
-static struct clk_lookup lcdif_lookups[] = {
- { .dev_id = "imx28-fb", },
- { .dev_id = "80030000.lcdif", },
-};
-
-static struct clk_lookup gpmi_lookups[] = {
- { .dev_id = "imx28-gpmi-nand", },
- { .dev_id = "8000c000.gpmi-nand", },
-};
-
-static struct clk_lookup fec_lookups[] = {
- { .dev_id = "imx28-fec.0", },
- { .dev_id = "imx28-fec.1", },
- { .dev_id = "800f0000.ethernet", },
- { .dev_id = "800f4000.ethernet", },
-};
-
-static struct clk_lookup can0_lookups[] = {
- { .dev_id = "flexcan.0", },
- { .dev_id = "80032000.can", },
-};
-
-static struct clk_lookup can1_lookups[] = {
- { .dev_id = "flexcan.1", },
- { .dev_id = "80034000.can", },
-};
-
-static struct clk_lookup saif0_lookups[] = {
- { .dev_id = "mxs-saif.0", },
- { .dev_id = "80042000.saif", },
-};
-
-static struct clk_lookup saif1_lookups[] = {
- { .dev_id = "mxs-saif.1", },
- { .dev_id = "80046000.saif", },
-};
-
static const char *sel_cpu[] __initconst = { "ref_cpu", "ref_xtal", };
static const char *sel_io0[] __initconst = { "ref_io0", "ref_xtal", };
static const char *sel_io1[] __initconst = { "ref_io1", "ref_xtal", };
@@ -235,6 +152,7 @@ static enum imx28_clk clks_init_on[] __initdata = {
int __init mx28_clocks_init(void)
{
+ struct device_node *np;
int i;
clk_misc_init();
@@ -312,27 +230,12 @@ int __init mx28_clocks_init(void)
return PTR_ERR(clks[i]);
}
+ np = of_find_compatible_node(NULL, NULL, "fsl,imx28-clkctrl");
+ if (np)
+ of_clk_add_provider(np, of_clk_src_onecell_get, NULL);
+
clk_register_clkdev(clks[clk32k], NULL, "timrot");
clk_register_clkdev(clks[enet_out], NULL, "enet_out");
- clk_register_clkdev(clks[pwm], NULL, "80064000.pwm");
- clk_register_clkdevs(clks[hbus], hbus_lookups, ARRAY_SIZE(hbus_lookups));
- clk_register_clkdevs(clks[xbus], xbus_lookups, ARRAY_SIZE(xbus_lookups));
- clk_register_clkdevs(clks[uart], uart_lookups, ARRAY_SIZE(uart_lookups));
- clk_register_clkdevs(clks[ssp0], ssp0_lookups, ARRAY_SIZE(ssp0_lookups));
- clk_register_clkdevs(clks[ssp1], ssp1_lookups, ARRAY_SIZE(ssp1_lookups));
- clk_register_clkdevs(clks[ssp2], ssp2_lookups, ARRAY_SIZE(ssp2_lookups));
- clk_register_clkdevs(clks[ssp3], ssp3_lookups, ARRAY_SIZE(ssp3_lookups));
- clk_register_clkdevs(clks[gpmi], gpmi_lookups, ARRAY_SIZE(gpmi_lookups));
- clk_register_clkdevs(clks[saif0], saif0_lookups, ARRAY_SIZE(saif0_lookups));
- clk_register_clkdevs(clks[saif1], saif1_lookups, ARRAY_SIZE(saif1_lookups));
- clk_register_clkdevs(clks[lcdif], lcdif_lookups, ARRAY_SIZE(lcdif_lookups));
- clk_register_clkdevs(clks[fec], fec_lookups, ARRAY_SIZE(fec_lookups));
- clk_register_clkdevs(clks[can0], can0_lookups, ARRAY_SIZE(can0_lookups));
- clk_register_clkdevs(clks[can1], can1_lookups, ARRAY_SIZE(can1_lookups));
- clk_register_clkdev(clks[usb0_pwr], NULL, "8007c000.usbphy");
- clk_register_clkdev(clks[usb1_pwr], NULL, "8007e000.usbphy");
- clk_register_clkdev(clks[usb0], NULL, "80080000.usb");
- clk_register_clkdev(clks[usb1], NULL, "80090000.usb");
for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
clk_prepare_enable(clks[clks_init_on[i]]);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/4] clk: mxs: replace imx23 clk_register_clkdev with clock DT lookup
2012-08-16 8:08 [PATCH 0/2] Move imx6q clock lookup over to device tree Shawn Guo
` (2 preceding siblings ...)
2012-08-19 14:05 ` [PATCH 3/4] clk: mxs: replace imx28 " Shawn Guo
@ 2012-08-19 14:05 ` Shawn Guo
3 siblings, 0 replies; 19+ messages in thread
From: Shawn Guo @ 2012-08-19 14:05 UTC (permalink / raw)
To: linux-arm-kernel
It really becomes a maintenance issue that every time a device needs
to look up (clk_get) a clock we have to patch kernel clock file to call
clk_register_clkdev for that clock.
Since clock DT support which is meant to resolve clock lookup in device
tree is in place, the patch moves imx23 client devices' clock lookup
over to device tree, so that any new lookup to be added at later time
can just get done in DT instead of kernel.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
.../devicetree/bindings/clock/imx23-clock.txt | 77 ++++++++++++++++++++
arch/arm/boot/dts/imx23.dtsi | 60 +++++++++++++++-
drivers/clk/mxs/clk-imx23.c | 51 ++------------
3 files changed, 141 insertions(+), 47 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/imx23-clock.txt
diff --git a/Documentation/devicetree/bindings/clock/imx23-clock.txt b/Documentation/devicetree/bindings/clock/imx23-clock.txt
new file mode 100644
index 0000000..d8e89b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx23-clock.txt
@@ -0,0 +1,77 @@
+* Clock bindings for Freescale i.MX23
+
+Required properties:
+- compatible: Should be "fsl,imx23-clkctrl"
+- reg: Address and length of the register set
+- #clock-cells: Should be <1>
+- clock-output-names: A list of clock output names that i.MX23 CLKCTRL
+ provides. The full list of all valid clock names and IDs is below.
+
+ Name ID
+ ------------------
+ ref_xtal 0
+ pll 1
+ ref_cpu 2
+ ref_emi 3
+ ref_pix 4
+ ref_io 5
+ saif_sel 6
+ lcdif_sel 7
+ gpmi_sel 8
+ ssp_sel 9
+ emi_sel 10
+ cpu 11
+ etm_sel 12
+ cpu_pll 13
+ cpu_xtal 14
+ hbus 15
+ xbus 16
+ lcdif_div 17
+ ssp_div 18
+ gpmi_div 19
+ emi_pll 20
+ emi_xtal 21
+ etm_div 22
+ saif_div 23
+ clk32k_div 24
+ rtc 25
+ adc 26
+ spdif_div 27
+ clk32k 28
+ dri 29
+ pwm 30
+ filt 31
+ uart 32
+ ssp 33
+ gpmi 34
+ spdif 35
+ emi 36
+ saif 37
+ lcdif 38
+ etm 39
+ usb 40
+ usb_pwr 41
+
+ The ID will be used by clock consumer in the first cell of "clocks"
+ phandle to specify the desired clock.
+
+Examples:
+
+clks: clkctrl at 80040000 {
+ compatible = "fsl,imx23-clkctrl";
+ reg = <0x80040000 0x2000>;
+ #clock-cells = <1>;
+ clock-output-names =
+ ...
+ "uart", /* 32 */
+ ...
+ "end_of_list";
+};
+
+auart0: serial at 8006c000 {
+ compatible = "fsl,imx23-auart";
+ reg = <0x8006c000 0x2000>;
+ interrupts = <24 25 23>;
+ clocks = <&clks 32>;
+ status = "disabled";
+};
diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index 9700872..b7b2963 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -52,6 +52,7 @@
dma-apbh at 80004000 {
compatible = "fsl,imx23-dma-apbh";
reg = <0x80004000 0x2000>;
+ clocks = <&clks 15>;
};
ecc at 80008000 {
@@ -67,6 +68,7 @@
reg-names = "gpmi-nand", "bch";
interrupts = <13>, <56>;
interrupt-names = "gpmi-dma", "bch";
+ clocks = <&clks 34>;
fsl,gpmi-dma-channel = <4>;
status = "disabled";
};
@@ -74,6 +76,7 @@
ssp0: ssp at 80010000 {
reg = <0x80010000 0x2000>;
interrupts = <15 14>;
+ clocks = <&clks 33>;
fsl,ssp-dma-channel = <1>;
status = "disabled";
};
@@ -291,6 +294,7 @@
dma-apbx at 80024000 {
compatible = "fsl,imx23-dma-apbx";
reg = <0x80024000 0x2000>;
+ clocks = <&clks 16>;
};
dcp at 80028000 {
@@ -317,12 +321,14 @@
compatible = "fsl,imx23-lcdif";
reg = <0x80030000 2000>;
interrupts = <46 45>;
+ clocks = <&clks 38>;
status = "disabled";
};
ssp1: ssp at 80034000 {
reg = <0x80034000 0x2000>;
interrupts = <2 20>;
+ clocks = <&clks 33>;
fsl,ssp-dma-channel = <2>;
status = "disabled";
};
@@ -340,9 +346,54 @@
reg = <0x80040000 0x40000>;
ranges;
- clkctl at 80040000 {
+ clks: clkctrl at 80040000 {
+ compatible = "fsl,imx23-clkctrl";
reg = <0x80040000 0x2000>;
- status = "disabled";
+ #clock-cells = <1>;
+ clock-output-names =
+ "ref_xtal", /* 0 */
+ "pll", /* 1 */
+ "ref_cpu", /* 2 */
+ "ref_emi", /* 3 */
+ "ref_pix", /* 4 */
+ "ref_io", /* 5 */
+ "saif_sel", /* 6 */
+ "lcdif_sel", /* 7 */
+ "gpmi_sel", /* 8 */
+ "ssp_sel", /* 9 */
+ "emi_sel", /* 10 */
+ "cpu", /* 11 */
+ "etm_sel", /* 12 */
+ "cpu_pll", /* 13 */
+ "cpu_xtal", /* 14 */
+ "hbus", /* 15 */
+ "xbus", /* 16 */
+ "lcdif_div", /* 17 */
+ "ssp_div", /* 18 */
+ "gpmi_div", /* 19 */
+ "emi_pll", /* 20 */
+ "emi_xtal", /* 21 */
+ "etm_div", /* 22 */
+ "saif_div", /* 23 */
+ "clk32k_div", /* 24 */
+ "rtc", /* 25 */
+ "adc", /* 26 */
+ "spdif_div", /* 27 */
+ "clk32k", /* 28 */
+ "dri", /* 29 */
+ "pwm", /* 30 */
+ "filt", /* 31 */
+ "uart", /* 32 */
+ "ssp", /* 33 */
+ "gpmi", /* 34 */
+ "spdif", /* 35 */
+ "emi", /* 36 */
+ "saif", /* 37 */
+ "lcdif", /* 38 */
+ "etm", /* 39 */
+ "usb", /* 40 */
+ "usb_pwr", /* 41 */
+ "end_of_list";
};
saif0: saif at 80042000 {
@@ -394,6 +445,7 @@
pwm: pwm at 80064000 {
compatible = "fsl,imx23-pwm";
reg = <0x80064000 0x2000>;
+ clocks = <&clks 30>;
#pwm-cells = <2>;
fsl,pwm-number = <5>;
status = "disabled";
@@ -408,6 +460,7 @@
compatible = "fsl,imx23-auart";
reg = <0x8006c000 0x2000>;
interrupts = <24 25 23>;
+ clocks = <&clks 32>;
status = "disabled";
};
@@ -415,6 +468,7 @@
compatible = "fsl,imx23-auart";
reg = <0x8006e000 0x2000>;
interrupts = <59 60 58>;
+ clocks = <&clks 32>;
status = "disabled";
};
@@ -422,6 +476,8 @@
compatible = "arm,pl011", "arm,primecell";
reg = <0x80070000 0x2000>;
interrupts = <0>;
+ clocks = <&clks 32>, <&clks 16>;
+ clock-names = "uart", "apb_pclk";
status = "disabled";
};
diff --git a/drivers/clk/mxs/clk-imx23.c b/drivers/clk/mxs/clk-imx23.c
index 844043ad0..d1c7cdb 100644
--- a/drivers/clk/mxs/clk-imx23.c
+++ b/drivers/clk/mxs/clk-imx23.c
@@ -14,6 +14,7 @@
#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/of.h>
#include <mach/common.h>
#include <mach/mx23.h>
#include "clk.h"
@@ -71,44 +72,6 @@ static void __init clk_misc_init(void)
__mxs_setl(30 << BP_FRAC_IOFRAC, FRAC);
}
-static struct clk_lookup uart_lookups[] = {
- { .dev_id = "duart", },
- { .dev_id = "mxs-auart.0", },
- { .dev_id = "mxs-auart.1", },
- { .dev_id = "8006c000.serial", },
- { .dev_id = "8006e000.serial", },
- { .dev_id = "80070000.serial", },
-};
-
-static struct clk_lookup hbus_lookups[] = {
- { .dev_id = "imx23-dma-apbh", },
- { .dev_id = "80004000.dma-apbh", },
-};
-
-static struct clk_lookup xbus_lookups[] = {
- { .dev_id = "duart", .con_id = "apb_pclk"},
- { .dev_id = "80070000.serial", .con_id = "apb_pclk"},
- { .dev_id = "imx23-dma-apbx", },
- { .dev_id = "80024000.dma-apbx", },
-};
-
-static struct clk_lookup ssp_lookups[] = {
- { .dev_id = "imx23-mmc.0", },
- { .dev_id = "imx23-mmc.1", },
- { .dev_id = "80010000.ssp", },
- { .dev_id = "80034000.ssp", },
-};
-
-static struct clk_lookup lcdif_lookups[] = {
- { .dev_id = "imx23-fb", },
- { .dev_id = "80030000.lcdif", },
-};
-
-static struct clk_lookup gpmi_lookups[] = {
- { .dev_id = "imx23-gpmi-nand", },
- { .dev_id = "8000c000.gpmi-nand", },
-};
-
static const char *sel_pll[] __initconst = { "pll", "ref_xtal", };
static const char *sel_cpu[] __initconst = { "ref_cpu", "ref_xtal", };
static const char *sel_pix[] __initconst = { "ref_pix", "ref_xtal", };
@@ -134,6 +97,7 @@ static enum imx23_clk clks_init_on[] __initdata = {
int __init mx23_clocks_init(void)
{
+ struct device_node *np;
int i;
clk_misc_init();
@@ -188,14 +152,11 @@ int __init mx23_clocks_init(void)
return PTR_ERR(clks[i]);
}
+ np = of_find_compatible_node(NULL, NULL, "fsl,imx23-clkctrl");
+ if (np)
+ of_clk_add_provider(np, of_clk_src_onecell_get, NULL);
+
clk_register_clkdev(clks[clk32k], NULL, "timrot");
- clk_register_clkdev(clks[pwm], NULL, "80064000.pwm");
- clk_register_clkdevs(clks[hbus], hbus_lookups, ARRAY_SIZE(hbus_lookups));
- clk_register_clkdevs(clks[xbus], xbus_lookups, ARRAY_SIZE(xbus_lookups));
- clk_register_clkdevs(clks[uart], uart_lookups, ARRAY_SIZE(uart_lookups));
- clk_register_clkdevs(clks[ssp], ssp_lookups, ARRAY_SIZE(ssp_lookups));
- clk_register_clkdevs(clks[gpmi], gpmi_lookups, ARRAY_SIZE(gpmi_lookups));
- clk_register_clkdevs(clks[lcdif], lcdif_lookups, ARRAY_SIZE(lcdif_lookups));
for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
clk_prepare_enable(clks[clks_init_on[i]]);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-16 8:08 ` [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup Shawn Guo
@ 2012-08-20 12:51 ` Rob Herring
2012-08-20 14:54 ` Matt Sealey
0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2012-08-20 12:51 UTC (permalink / raw)
To: linux-arm-kernel
On 08/16/2012 03:08 AM, Shawn Guo wrote:
> It really becomes an issue that every time a device needs to look
> up (clk_get) a clock we have to patch kernel clock file to call
> clk_register_clkdev for that clock.
>
> Since clock DT support which is meant to resolve clock lookup in device
> tree is in place, the patch moves imx6q client devices' clock lookup
> over to device tree, so that any new lookup to be added at later time
> can just get done in DT instead of kernel.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Looks good. For both patches:
Reviewed-by: Rob Herring <rob.herring@calxeda.com>
> ---
> .../devicetree/bindings/clock/imx6q-clock.txt | 223 +++++++++++++++++
> arch/arm/boot/dts/imx6q-sabrelite.dts | 1 +
> arch/arm/boot/dts/imx6q.dtsi | 261 +++++++++++++++++++-
> arch/arm/mach-imx/clk-imx6q.c | 41 +---
> arch/arm/mach-imx/mach-imx6q.c | 1 -
> 5 files changed, 477 insertions(+), 50 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/imx6q-clock.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> new file mode 100644
> index 0000000..19d8126
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> @@ -0,0 +1,223 @@
> +* Clock bindings for Freescale i.MX6 Quad
> +
> +Required properties:
> +- compatible: Should be "fsl,imx6q-ccm"
> +- reg: Address and length of the register set
> +- interrupts: Should contain CCM interrupt
> +- #clock-cells: Should be <1>
> +- clock-output-names: A list of clock output names that CCM provides.
> + The full list of all valid clock names and IDs is below.
> +
> + Name ID
> + ---------------------------
> + dummy 0
> + ckil 1
> + ckih 2
> + osc 3
> + pll2_pfd0_352m 4
> + pll2_pfd1_594m 5
> + pll2_pfd2_396m 6
> + pll3_pfd0_720m 7
> + pll3_pfd1_540m 8
> + pll3_pfd2_508m 9
> + pll3_pfd3_454m 10
> + pll2_198m 11
> + pll3_120m 12
> + pll3_80m 13
> + pll3_60m 14
> + twd 15
> + step 16
> + pll1_sw 17
> + periph_pre 18
> + periph2_pre 19
> + periph_clk2_sel 20
> + periph2_clk2_sel 21
> + axi_sel 22
> + esai_sel 23
> + asrc_sel 24
> + spdif_sel 25
> + gpu2d_axi 26
> + gpu3d_axi 27
> + gpu2d_core_sel 28
> + gpu3d_core_sel 29
> + gpu3d_shader_sel 30
> + ipu1_sel 31
> + ipu2_sel 32
> + ldb_di0_sel 33
> + ldb_di1_sel 34
> + ipu1_di0_pre_sel 35
> + ipu1_di1_pre_sel 36
> + ipu2_di0_pre_sel 37
> + ipu2_di1_pre_sel 38
> + ipu1_di0_sel 39
> + ipu1_di1_sel 40
> + ipu2_di0_sel 41
> + ipu2_di1_sel 42
> + hsi_tx_sel 43
> + pcie_axi_sel 44
> + ssi1_sel 45
> + ssi2_sel 46
> + ssi3_sel 47
> + usdhc1_sel 48
> + usdhc2_sel 49
> + usdhc3_sel 50
> + usdhc4_sel 51
> + enfc_sel 52
> + emi_sel 53
> + emi_slow_sel 54
> + vdo_axi_sel 55
> + vpu_axi_sel 56
> + cko1_sel 57
> + periph 58
> + periph2 59
> + periph_clk2 60
> + periph2_clk2 61
> + ipg 62
> + ipg_per 63
> + esai_pred 64
> + esai_podf 65
> + asrc_pred 66
> + asrc_podf 67
> + spdif_pred 68
> + spdif_podf 69
> + can_root 70
> + ecspi_root 71
> + gpu2d_core_podf 72
> + gpu3d_core_podf 73
> + gpu3d_shader 74
> + ipu1_podf 75
> + ipu2_podf 76
> + ldb_di0_podf 77
> + ldb_di1_podf 78
> + ipu1_di0_pre 79
> + ipu1_di1_pre 80
> + ipu2_di0_pre 81
> + ipu2_di1_pre 82
> + hsi_tx_podf 83
> + ssi1_pred 84
> + ssi1_podf 85
> + ssi2_pred 86
> + ssi2_podf 87
> + ssi3_pred 88
> + ssi3_podf 89
> + uart_serial_podf 90
> + usdhc1_podf 91
> + usdhc2_podf 92
> + usdhc3_podf 93
> + usdhc4_podf 94
> + enfc_pred 95
> + enfc_podf 96
> + emi_podf 97
> + emi_slow_podf 98
> + vpu_axi_podf 99
> + cko1_podf 100
> + axi 101
> + mmdc_ch0_axi_podf 102
> + mmdc_ch1_axi_podf 103
> + arm 104
> + ahb 105
> + apbh_dma 106
> + asrc 107
> + can1_ipg 108
> + can1_serial 109
> + can2_ipg 110
> + can2_serial 111
> + ecspi1 112
> + ecspi2 113
> + ecspi3 114
> + ecspi4 115
> + ecspi5 116
> + enet 117
> + esai 118
> + gpt_ipg 119
> + gpt_ipg_per 120
> + gpu2d_core 121
> + gpu3d_core 122
> + hdmi_iahb 123
> + hdmi_isfr 124
> + i2c1 125
> + i2c2 126
> + i2c3 127
> + iim 128
> + enfc 129
> + ipu1 130
> + ipu1_di0 131
> + ipu1_di1 132
> + ipu2 133
> + ipu2_di0 134
> + ldb_di0 135
> + ldb_di1 136
> + ipu2_di1 137
> + hsi_tx 138
> + mlb 139
> + mmdc_ch0_axi 140
> + mmdc_ch1_axi 141
> + ocram 142
> + openvg_axi 143
> + pcie_axi 144
> + pwm1 145
> + pwm2 146
> + pwm3 147
> + pwm4 148
> + per1_bch 149
> + gpmi_bch_apb 150
> + gpmi_bch 151
> + gpmi_io 152
> + gpmi_apb 153
> + sata 154
> + sdma 155
> + spba 156
> + ssi1 157
> + ssi2 158
> + ssi3 159
> + uart_ipg 160
> + uart_serial 161
> + usboh3 162
> + usdhc1 163
> + usdhc2 164
> + usdhc3 165
> + usdhc4 166
> + vdo_axi 167
> + vpu_axi 168
> + cko1 169
> + pll1_sys 170
> + pll2_bus 171
> + pll3_usb_otg 172
> + pll4_audio 173
> + pll5_video 174
> + pll6_mlb 175
> + pll7_usb_host 176
> + pll8_enet 177
> + ssi1_ipg 178
> + ssi2_ipg 179
> + ssi3_ipg 180
> + rom 181
> + usbphy1 182
> + usbphy2 183
> + ldb_di0_div_3_5 184
> + ldb_di1_div_3_5 185
> +
> + The ID will be used by clock consumer in the first cell of "clocks"
> + phandle to specify the desired clock.
> +
> +Examples:
> +
> +clks: ccm at 020c4000 {
> + compatible = "fsl,imx6q-ccm";
> + reg = <0x020c4000 0x4000>;
> + interrupts = <0 87 0x04 0 88 0x04>;
> + #clock-cells = <1>;
> + clock-output-names = ...
> + "uart_ipg",
> + "uart_serial",
> + ...;
> +};
> +
> +uart1: serial at 02020000 {
> + compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> + reg = <0x02020000 0x4000>;
> + interrupts = <0 26 0x04>;
> + clocks = <&clks 160>, <&clks 161>;
> + clock-names = "ipg", "per";
> + status = "disabled";
> +};
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index 72f30f3..cfdbe53 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -111,6 +111,7 @@
> codec: sgtl5000 at 0a {
> compatible = "fsl,sgtl5000";
> reg = <0x0a>;
> + clocks = <&clks 169>;
> VDDA-supply = <®_2p5v>;
> VDDIO-supply = <®_3p3v>;
> };
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 0052fe7..acff2dc 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -93,18 +93,23 @@
> dma-apbh at 00110000 {
> compatible = "fsl,imx6q-dma-apbh", "fsl,imx28-dma-apbh";
> reg = <0x00110000 0x2000>;
> + clocks = <&clks 106>;
> };
>
> gpmi-nand at 00112000 {
> - compatible = "fsl,imx6q-gpmi-nand";
> - #address-cells = <1>;
> - #size-cells = <1>;
> - reg = <0x00112000 0x2000>, <0x00114000 0x2000>;
> - reg-names = "gpmi-nand", "bch";
> - interrupts = <0 13 0x04>, <0 15 0x04>;
> - interrupt-names = "gpmi-dma", "bch";
> - fsl,gpmi-dma-channel = <0>;
> - status = "disabled";
> + compatible = "fsl,imx6q-gpmi-nand";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0x00112000 0x2000>, <0x00114000 0x2000>;
> + reg-names = "gpmi-nand", "bch";
> + interrupts = <0 13 0x04>, <0 15 0x04>;
> + interrupt-names = "gpmi-dma", "bch";
> + clocks = <&clks 152>, <&clks 153>, <&clks 151>,
> + <&clks 150>, <&clks 149>;
> + clock-names = "gpmi_io", "gpmi_apb", "gpmi_bch",
> + "gpmi_bch_apb", "per1_bch";
> + fsl,gpmi-dma-channel = <0>;
> + status = "disabled";
> };
>
> timer at 00a00600 {
> @@ -146,6 +151,8 @@
> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
> reg = <0x02008000 0x4000>;
> interrupts = <0 31 0x04>;
> + clocks = <&clks 112>, <&clks 112>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -155,6 +162,8 @@
> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
> reg = <0x0200c000 0x4000>;
> interrupts = <0 32 0x04>;
> + clocks = <&clks 113>, <&clks 113>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -164,6 +173,8 @@
> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
> reg = <0x02010000 0x4000>;
> interrupts = <0 33 0x04>;
> + clocks = <&clks 114>, <&clks 114>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -173,6 +184,8 @@
> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
> reg = <0x02014000 0x4000>;
> interrupts = <0 34 0x04>;
> + clocks = <&clks 115>, <&clks 115>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -182,6 +195,8 @@
> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
> reg = <0x02018000 0x4000>;
> interrupts = <0 35 0x04>;
> + clocks = <&clks 116>, <&clks 116>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -189,6 +204,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x02020000 0x4000>;
> interrupts = <0 26 0x04>;
> + clocks = <&clks 160>, <&clks 161>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -201,6 +218,7 @@
> compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
> reg = <0x02028000 0x4000>;
> interrupts = <0 46 0x04>;
> + clocks = <&clks 178>;
> fsl,fifo-depth = <15>;
> fsl,ssi-dma-events = <38 37>;
> status = "disabled";
> @@ -210,6 +228,7 @@
> compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
> reg = <0x0202c000 0x4000>;
> interrupts = <0 47 0x04>;
> + clocks = <&clks 179>;
> fsl,fifo-depth = <15>;
> fsl,ssi-dma-events = <42 41>;
> status = "disabled";
> @@ -219,6 +238,7 @@
> compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
> reg = <0x02030000 0x4000>;
> interrupts = <0 48 0x04>;
> + clocks = <&clks 180>;
> fsl,fifo-depth = <15>;
> fsl,ssi-dma-events = <46 45>;
> status = "disabled";
> @@ -358,6 +378,7 @@
> compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
> reg = <0x020bc000 0x4000>;
> interrupts = <0 80 0x04>;
> + clocks = <&clks 0>;
> status = "disabled";
> };
>
> @@ -365,13 +386,203 @@
> compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
> reg = <0x020c0000 0x4000>;
> interrupts = <0 81 0x04>;
> + clocks = <&clks 0>;
> status = "disabled";
> };
>
> - ccm at 020c4000 {
> + clks: ccm at 020c4000 {
> compatible = "fsl,imx6q-ccm";
> reg = <0x020c4000 0x4000>;
> interrupts = <0 87 0x04 0 88 0x04>;
> + #clock-cells = <1>;
> + clock-output-names =
> + "dummy", /* 0 */
> + "ckil", /* 1 */
> + "ckih", /* 2 */
> + "osc", /* 3 */
> + "pll2_pfd0_352m", /* 4 */
> + "pll2_pfd1_594m", /* 5 */
> + "pll2_pfd2_396m", /* 6 */
> + "pll3_pfd0_720m", /* 7 */
> + "pll3_pfd1_540m", /* 8 */
> + "pll3_pfd2_508m", /* 9 */
> + "pll3_pfd3_454m", /* 10 */
> + "pll2_198m", /* 11 */
> + "pll3_120m", /* 12 */
> + "pll3_80m", /* 13 */
> + "pll3_60m", /* 14 */
> + "twd", /* 15 */
> + "step", /* 16 */
> + "pll1_sw", /* 17 */
> + "periph_pre", /* 18 */
> + "periph2_pre", /* 19 */
> + "periph_clk2_sel", /* 20 */
> + "periph2_clk2_sel", /* 21 */
> + "axi_sel", /* 22 */
> + "esai_sel", /* 23 */
> + "asrc_sel", /* 24 */
> + "spdif_sel", /* 25 */
> + "gpu2d_axi", /* 26 */
> + "gpu3d_axi", /* 27 */
> + "gpu2d_core_sel", /* 28 */
> + "gpu3d_core_sel", /* 29 */
> + "gpu3d_shader_sel", /* 30 */
> + "ipu1_sel", /* 31 */
> + "ipu2_sel", /* 32 */
> + "ldb_di0_sel", /* 33 */
> + "ldb_di1_sel", /* 34 */
> + "ipu1_di0_pre_sel", /* 35 */
> + "ipu1_di1_pre_sel", /* 36 */
> + "ipu2_di0_pre_sel", /* 37 */
> + "ipu2_di1_pre_sel", /* 38 */
> + "ipu1_di0_sel", /* 39 */
> + "ipu1_di1_sel", /* 40 */
> + "ipu2_di0_sel", /* 41 */
> + "ipu2_di1_sel", /* 42 */
> + "hsi_tx_sel", /* 43 */
> + "pcie_axi_sel", /* 44 */
> + "ssi1_sel", /* 45 */
> + "ssi2_sel", /* 46 */
> + "ssi3_sel", /* 47 */
> + "usdhc1_sel", /* 48 */
> + "usdhc2_sel", /* 49 */
> + "usdhc3_sel", /* 50 */
> + "usdhc4_sel", /* 51 */
> + "enfc_sel", /* 52 */
> + "emi_sel", /* 53 */
> + "emi_slow_sel", /* 54 */
> + "vdo_axi_sel", /* 55 */
> + "vpu_axi_sel", /* 56 */
> + "cko1_sel", /* 57 */
> + "periph", /* 58 */
> + "periph2", /* 59 */
> + "periph_clk2", /* 60 */
> + "periph2_clk2", /* 61 */
> + "ipg", /* 62 */
> + "ipg_per", /* 63 */
> + "esai_pred", /* 64 */
> + "esai_podf", /* 65 */
> + "asrc_pred", /* 66 */
> + "asrc_podf", /* 67 */
> + "spdif_pred", /* 68 */
> + "spdif_podf", /* 69 */
> + "can_root", /* 70 */
> + "ecspi_root", /* 71 */
> + "gpu2d_core_podf", /* 72 */
> + "gpu3d_core_podf", /* 73 */
> + "gpu3d_shader", /* 74 */
> + "ipu1_podf", /* 75 */
> + "ipu2_podf", /* 76 */
> + "ldb_di0_podf", /* 77 */
> + "ldb_di1_podf", /* 78 */
> + "ipu1_di0_pre", /* 79 */
> + "ipu1_di1_pre", /* 80 */
> + "ipu2_di0_pre", /* 81 */
> + "ipu2_di1_pre", /* 82 */
> + "hsi_tx_podf", /* 83 */
> + "ssi1_pred", /* 84 */
> + "ssi1_podf", /* 85 */
> + "ssi2_pred", /* 86 */
> + "ssi2_podf", /* 87 */
> + "ssi3_pred", /* 88 */
> + "ssi3_podf", /* 89 */
> + "uart_serial_podf", /* 90 */
> + "usdhc1_podf", /* 91 */
> + "usdhc2_podf", /* 92 */
> + "usdhc3_podf", /* 93 */
> + "usdhc4_podf", /* 94 */
> + "enfc_pred", /* 95 */
> + "enfc_podf", /* 96 */
> + "emi_podf", /* 97 */
> + "emi_slow_podf", /* 98 */
> + "vpu_axi_podf", /* 99 */
> + "cko1_podf", /* 100 */
> + "axi", /* 101 */
> + "mmdc_ch0_axi_podf", /* 102 */
> + "mmdc_ch1_axi_podf", /* 103 */
> + "arm", /* 104 */
> + "ahb", /* 105 */
> + "apbh_dma", /* 106 */
> + "asrc", /* 107 */
> + "can1_ipg", /* 108 */
> + "can1_serial", /* 109 */
> + "can2_ipg", /* 110 */
> + "can2_serial", /* 111 */
> + "ecspi1", /* 112 */
> + "ecspi2", /* 113 */
> + "ecspi3", /* 114 */
> + "ecspi4", /* 115 */
> + "ecspi5", /* 116 */
> + "enet", /* 117 */
> + "esai", /* 118 */
> + "gpt_ipg", /* 119 */
> + "gpt_ipg_per", /* 120 */
> + "gpu2d_core", /* 121 */
> + "gpu3d_core", /* 122 */
> + "hdmi_iahb", /* 123 */
> + "hdmi_isfr", /* 124 */
> + "i2c1", /* 125 */
> + "i2c2", /* 126 */
> + "i2c3", /* 127 */
> + "iim", /* 128 */
> + "enfc", /* 129 */
> + "ipu1", /* 130 */
> + "ipu1_di0", /* 131 */
> + "ipu1_di1", /* 132 */
> + "ipu2", /* 133 */
> + "ipu2_di0", /* 134 */
> + "ldb_di0", /* 135 */
> + "ldb_di1", /* 136 */
> + "ipu2_di1", /* 137 */
> + "hsi_tx", /* 138 */
> + "mlb", /* 139 */
> + "mmdc_ch0_axi", /* 140 */
> + "mmdc_ch1_axi", /* 141 */
> + "ocram", /* 142 */
> + "openvg_axi", /* 143 */
> + "pcie_axi", /* 144 */
> + "pwm1", /* 145 */
> + "pwm2", /* 146 */
> + "pwm3", /* 147 */
> + "pwm4", /* 148 */
> + "per1_bch", /* 149 */
> + "gpmi_bch_apb", /* 150 */
> + "gpmi_bch", /* 151 */
> + "gpmi_io", /* 152 */
> + "gpmi_apb", /* 153 */
> + "sata", /* 154 */
> + "sdma", /* 155 */
> + "spba", /* 156 */
> + "ssi1", /* 157 */
> + "ssi2", /* 158 */
> + "ssi3", /* 159 */
> + "uart_ipg", /* 160 */
> + "uart_serial", /* 161 */
> + "usboh3", /* 162 */
> + "usdhc1", /* 163 */
> + "usdhc2", /* 164 */
> + "usdhc3", /* 165 */
> + "usdhc4", /* 166 */
> + "vdo_axi", /* 167 */
> + "vpu_axi", /* 168 */
> + "cko1", /* 169 */
> + "pll1_sys", /* 170 */
> + "pll2_bus", /* 171 */
> + "pll3_usb_otg", /* 172 */
> + "pll4_audio", /* 173 */
> + "pll5_video", /* 174 */
> + "pll6_mlb", /* 175 */
> + "pll7_usb_host", /* 176 */
> + "pll8_enet", /* 177 */
> + "ssi1_ipg", /* 178 */
> + "ssi2_ipg", /* 179 */
> + "ssi3_ipg", /* 180 */
> + "rom", /* 181 */
> + "usbphy1", /* 182 */
> + "usbphy2", /* 183 */
> + "ldb_di0_div_3_5", /* 184 */
> + "ldb_di1_div_3_5", /* 185 */
> + "end_of_list";
> };
>
> anatop at 020c8000 {
> @@ -468,12 +679,14 @@
> compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> reg = <0x020c9000 0x1000>;
> interrupts = <0 44 0x04>;
> + clocks = <&clks 182>;
> };
>
> usbphy2: usbphy at 020ca000 {
> compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
> reg = <0x020ca000 0x1000>;
> interrupts = <0 45 0x04>;
> + clocks = <&clks 183>;
> };
>
> snvs at 020cc000 {
> @@ -608,6 +821,9 @@
> compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
> reg = <0x020ec000 0x4000>;
> interrupts = <0 2 0x04>;
> + clocks = <&clks 155>, <&clks 155>;
> + clock-names = "ipg", "ahb";
> + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx6q-to1.bin";
> };
> };
>
> @@ -631,6 +847,7 @@
> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> reg = <0x02184000 0x200>;
> interrupts = <0 43 0x04>;
> + clocks = <&clks 162>;
> fsl,usbphy = <&usbphy1>;
> status = "disabled";
> };
> @@ -639,6 +856,7 @@
> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> reg = <0x02184200 0x200>;
> interrupts = <0 40 0x04>;
> + clocks = <&clks 162>;
> fsl,usbphy = <&usbphy2>;
> status = "disabled";
> };
> @@ -647,6 +865,7 @@
> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> reg = <0x02184400 0x200>;
> interrupts = <0 41 0x04>;
> + clocks = <&clks 162>;
> status = "disabled";
> };
>
> @@ -654,6 +873,7 @@
> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
> reg = <0x02184600 0x200>;
> interrupts = <0 42 0x04>;
> + clocks = <&clks 162>;
> status = "disabled";
> };
>
> @@ -661,6 +881,8 @@
> compatible = "fsl,imx6q-fec";
> reg = <0x02188000 0x4000>;
> interrupts = <0 118 0x04 0 119 0x04>;
> + clocks = <&clks 117>, <&clks 117>;
> + clock-names = "ipg", "ahb";
> status = "disabled";
> };
>
> @@ -673,6 +895,8 @@
> compatible = "fsl,imx6q-usdhc";
> reg = <0x02190000 0x4000>;
> interrupts = <0 22 0x04>;
> + clocks = <&clks 163>, <&clks 163>, <&clks 163>;
> + clock-names = "ipg", "ahb", "per";
> status = "disabled";
> };
>
> @@ -680,6 +904,8 @@
> compatible = "fsl,imx6q-usdhc";
> reg = <0x02194000 0x4000>;
> interrupts = <0 23 0x04>;
> + clocks = <&clks 164>, <&clks 164>, <&clks 164>;
> + clock-names = "ipg", "ahb", "per";
> status = "disabled";
> };
>
> @@ -687,6 +913,8 @@
> compatible = "fsl,imx6q-usdhc";
> reg = <0x02198000 0x4000>;
> interrupts = <0 24 0x04>;
> + clocks = <&clks 165>, <&clks 165>, <&clks 165>;
> + clock-names = "ipg", "ahb", "per";
> status = "disabled";
> };
>
> @@ -694,6 +922,8 @@
> compatible = "fsl,imx6q-usdhc";
> reg = <0x0219c000 0x4000>;
> interrupts = <0 25 0x04>;
> + clocks = <&clks 166>, <&clks 166>, <&clks 166>;
> + clock-names = "ipg", "ahb", "per";
> status = "disabled";
> };
>
> @@ -703,6 +933,7 @@
> compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
> reg = <0x021a0000 0x4000>;
> interrupts = <0 36 0x04>;
> + clocks = <&clks 125>;
> status = "disabled";
> };
>
> @@ -712,6 +943,7 @@
> compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
> reg = <0x021a4000 0x4000>;
> interrupts = <0 37 0x04>;
> + clocks = <&clks 126>;
> status = "disabled";
> };
>
> @@ -721,6 +953,7 @@
> compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
> reg = <0x021a8000 0x4000>;
> interrupts = <0 38 0x04>;
> + clocks = <&clks 127>;
> status = "disabled";
> };
>
> @@ -784,6 +1017,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x021e8000 0x4000>;
> interrupts = <0 27 0x04>;
> + clocks = <&clks 160>, <&clks 161>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -791,6 +1026,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x021ec000 0x4000>;
> interrupts = <0 28 0x04>;
> + clocks = <&clks 160>, <&clks 161>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -798,6 +1035,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x021f0000 0x4000>;
> interrupts = <0 29 0x04>;
> + clocks = <&clks 160>, <&clks 161>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -805,6 +1044,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x021f4000 0x4000>;
> interrupts = <0 30 0x04>;
> + clocks = <&clks 160>, <&clks 161>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
> };
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index 8e46407..433c683 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -387,48 +387,11 @@ int __init mx6q_clocks_init(void)
> pr_err("i.MX6q clk %d: register failed with %ld\n",
> i, PTR_ERR(clk[i]));
>
> + of_clk_add_provider(np, of_clk_src_onecell_get, NULL);
> +
> clk_register_clkdev(clk[gpt_ipg], "ipg", "imx-gpt.0");
> clk_register_clkdev(clk[gpt_ipg_per], "per", "imx-gpt.0");
> clk_register_clkdev(clk[twd], NULL, "smp_twd");
> - clk_register_clkdev(clk[apbh_dma], NULL, "110000.dma-apbh");
> - clk_register_clkdev(clk[per1_bch], "per1_bch", "112000.gpmi-nand");
> - clk_register_clkdev(clk[gpmi_bch_apb], "gpmi_bch_apb", "112000.gpmi-nand");
> - clk_register_clkdev(clk[gpmi_bch], "gpmi_bch", "112000.gpmi-nand");
> - clk_register_clkdev(clk[gpmi_apb], "gpmi_apb", "112000.gpmi-nand");
> - clk_register_clkdev(clk[gpmi_io], "gpmi_io", "112000.gpmi-nand");
> - clk_register_clkdev(clk[usboh3], NULL, "2184000.usb");
> - clk_register_clkdev(clk[usboh3], NULL, "2184200.usb");
> - clk_register_clkdev(clk[usboh3], NULL, "2184400.usb");
> - clk_register_clkdev(clk[usboh3], NULL, "2184600.usb");
> - clk_register_clkdev(clk[usbphy1], NULL, "20c9000.usbphy");
> - clk_register_clkdev(clk[usbphy2], NULL, "20ca000.usbphy");
> - clk_register_clkdev(clk[uart_serial], "per", "2020000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "2020000.serial");
> - clk_register_clkdev(clk[uart_serial], "per", "21e8000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "21e8000.serial");
> - clk_register_clkdev(clk[uart_serial], "per", "21ec000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "21ec000.serial");
> - clk_register_clkdev(clk[uart_serial], "per", "21f0000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "21f0000.serial");
> - clk_register_clkdev(clk[uart_serial], "per", "21f4000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "21f4000.serial");
> - clk_register_clkdev(clk[enet], NULL, "2188000.ethernet");
> - clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc");
> - clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc");
> - clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc");
> - clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");
> - clk_register_clkdev(clk[i2c1], NULL, "21a0000.i2c");
> - clk_register_clkdev(clk[i2c2], NULL, "21a4000.i2c");
> - clk_register_clkdev(clk[i2c3], NULL, "21a8000.i2c");
> - clk_register_clkdev(clk[ecspi1], NULL, "2008000.ecspi");
> - clk_register_clkdev(clk[ecspi2], NULL, "200c000.ecspi");
> - clk_register_clkdev(clk[ecspi3], NULL, "2010000.ecspi");
> - clk_register_clkdev(clk[ecspi4], NULL, "2014000.ecspi");
> - clk_register_clkdev(clk[ecspi5], NULL, "2018000.ecspi");
> - clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma");
> - clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
> - clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
> - clk_register_clkdev(clk[ssi1_ipg], NULL, "2028000.ssi");
> clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL);
> clk_register_clkdev(clk[ahb], "ahb", NULL);
> clk_register_clkdev(clk[cko1], "cko1", NULL);
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index 6f9c23b..957f5fe 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -94,7 +94,6 @@ static void __init imx6q_sabrelite_cko1_setup(void)
> clk_set_parent(cko1_sel, ahb);
> rate = clk_round_rate(cko1, 16000000);
> clk_set_rate(cko1, rate);
> - clk_register_clkdev(cko1, NULL, "0-000a");
> put_clk:
> if (!IS_ERR(cko1_sel))
> clk_put(cko1_sel);
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/2] clk: add of_clk_src_onecell_get() support
2012-08-16 8:08 ` [PATCH 1/2] clk: add of_clk_src_onecell_get() support Shawn Guo
@ 2012-08-20 14:30 ` Shawn Guo
0 siblings, 0 replies; 19+ messages in thread
From: Shawn Guo @ 2012-08-20 14:30 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Aug 16, 2012 at 04:08:45PM +0800, Shawn Guo wrote:
> For those SoCs that have hundreds of clock outputs, their clock
> DT bindings could reasonably define #clock-cells as 1 and require
> the client device specify the index of the clock it consumes in the
> cell of its "clocks" phandle.
>
> Add a generic of_clk_src_onecell_get() function for this purpose.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> drivers/clk/clk.c | 15 +++++++++++++++
> include/linux/clk-provider.h | 1 +
> 2 files changed, 16 insertions(+), 0 deletions(-)
>
Mike,
Can you please apply this patch on clk tree if it looks good to you?
I plan to ask arm-soc people pull in clk tree as dependency and then
have the other 3 patches in the series to go via arm-soc tree.
Regards,
Shawn
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-20 12:51 ` Rob Herring
@ 2012-08-20 14:54 ` Matt Sealey
2012-08-20 15:16 ` Shawn Guo
0 siblings, 1 reply; 19+ messages in thread
From: Matt Sealey @ 2012-08-20 14:54 UTC (permalink / raw)
To: linux-arm-kernel
Yet another arbitrary array...
I'm not sure why you would move the registration lookup into the DT
and use an arbitrarily ordered array again. Sure, you're looking it up
entirely within the device tree here, but a better solution would be
to not name clocks twice, and structure your clock definition
properly.
What's wrong with;
ccm at 020c4000 {
...
my_clock: clock at 0 {
name = "my_clock_name";
}
}
uart at nnnnnnnn {
...
clocks = <&my_clock>;
}
?
--
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.
On Mon, Aug 20, 2012 at 7:51 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 08/16/2012 03:08 AM, Shawn Guo wrote:
>> It really becomes an issue that every time a device needs to look
>> up (clk_get) a clock we have to patch kernel clock file to call
>> clk_register_clkdev for that clock.
>>
>> Since clock DT support which is meant to resolve clock lookup in device
>> tree is in place, the patch moves imx6q client devices' clock lookup
>> over to device tree, so that any new lookup to be added at later time
>> can just get done in DT instead of kernel.
>>
>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>
> Looks good. For both patches:
>
> Reviewed-by: Rob Herring <rob.herring@calxeda.com>
>
>> ---
>> .../devicetree/bindings/clock/imx6q-clock.txt | 223 +++++++++++++++++
>> arch/arm/boot/dts/imx6q-sabrelite.dts | 1 +
>> arch/arm/boot/dts/imx6q.dtsi | 261 +++++++++++++++++++-
>> arch/arm/mach-imx/clk-imx6q.c | 41 +---
>> arch/arm/mach-imx/mach-imx6q.c | 1 -
>> 5 files changed, 477 insertions(+), 50 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/clock/imx6q-clock.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
>> new file mode 100644
>> index 0000000..19d8126
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
>> @@ -0,0 +1,223 @@
>> +* Clock bindings for Freescale i.MX6 Quad
>> +
>> +Required properties:
>> +- compatible: Should be "fsl,imx6q-ccm"
>> +- reg: Address and length of the register set
>> +- interrupts: Should contain CCM interrupt
>> +- #clock-cells: Should be <1>
>> +- clock-output-names: A list of clock output names that CCM provides.
>> + The full list of all valid clock names and IDs is below.
>> +
>> + Name ID
>> + ---------------------------
>> + dummy 0
>> + ckil 1
>> + ckih 2
>> + osc 3
>> + pll2_pfd0_352m 4
>> + pll2_pfd1_594m 5
>> + pll2_pfd2_396m 6
>> + pll3_pfd0_720m 7
>> + pll3_pfd1_540m 8
>> + pll3_pfd2_508m 9
>> + pll3_pfd3_454m 10
>> + pll2_198m 11
>> + pll3_120m 12
>> + pll3_80m 13
>> + pll3_60m 14
>> + twd 15
>> + step 16
>> + pll1_sw 17
>> + periph_pre 18
>> + periph2_pre 19
>> + periph_clk2_sel 20
>> + periph2_clk2_sel 21
>> + axi_sel 22
>> + esai_sel 23
>> + asrc_sel 24
>> + spdif_sel 25
>> + gpu2d_axi 26
>> + gpu3d_axi 27
>> + gpu2d_core_sel 28
>> + gpu3d_core_sel 29
>> + gpu3d_shader_sel 30
>> + ipu1_sel 31
>> + ipu2_sel 32
>> + ldb_di0_sel 33
>> + ldb_di1_sel 34
>> + ipu1_di0_pre_sel 35
>> + ipu1_di1_pre_sel 36
>> + ipu2_di0_pre_sel 37
>> + ipu2_di1_pre_sel 38
>> + ipu1_di0_sel 39
>> + ipu1_di1_sel 40
>> + ipu2_di0_sel 41
>> + ipu2_di1_sel 42
>> + hsi_tx_sel 43
>> + pcie_axi_sel 44
>> + ssi1_sel 45
>> + ssi2_sel 46
>> + ssi3_sel 47
>> + usdhc1_sel 48
>> + usdhc2_sel 49
>> + usdhc3_sel 50
>> + usdhc4_sel 51
>> + enfc_sel 52
>> + emi_sel 53
>> + emi_slow_sel 54
>> + vdo_axi_sel 55
>> + vpu_axi_sel 56
>> + cko1_sel 57
>> + periph 58
>> + periph2 59
>> + periph_clk2 60
>> + periph2_clk2 61
>> + ipg 62
>> + ipg_per 63
>> + esai_pred 64
>> + esai_podf 65
>> + asrc_pred 66
>> + asrc_podf 67
>> + spdif_pred 68
>> + spdif_podf 69
>> + can_root 70
>> + ecspi_root 71
>> + gpu2d_core_podf 72
>> + gpu3d_core_podf 73
>> + gpu3d_shader 74
>> + ipu1_podf 75
>> + ipu2_podf 76
>> + ldb_di0_podf 77
>> + ldb_di1_podf 78
>> + ipu1_di0_pre 79
>> + ipu1_di1_pre 80
>> + ipu2_di0_pre 81
>> + ipu2_di1_pre 82
>> + hsi_tx_podf 83
>> + ssi1_pred 84
>> + ssi1_podf 85
>> + ssi2_pred 86
>> + ssi2_podf 87
>> + ssi3_pred 88
>> + ssi3_podf 89
>> + uart_serial_podf 90
>> + usdhc1_podf 91
>> + usdhc2_podf 92
>> + usdhc3_podf 93
>> + usdhc4_podf 94
>> + enfc_pred 95
>> + enfc_podf 96
>> + emi_podf 97
>> + emi_slow_podf 98
>> + vpu_axi_podf 99
>> + cko1_podf 100
>> + axi 101
>> + mmdc_ch0_axi_podf 102
>> + mmdc_ch1_axi_podf 103
>> + arm 104
>> + ahb 105
>> + apbh_dma 106
>> + asrc 107
>> + can1_ipg 108
>> + can1_serial 109
>> + can2_ipg 110
>> + can2_serial 111
>> + ecspi1 112
>> + ecspi2 113
>> + ecspi3 114
>> + ecspi4 115
>> + ecspi5 116
>> + enet 117
>> + esai 118
>> + gpt_ipg 119
>> + gpt_ipg_per 120
>> + gpu2d_core 121
>> + gpu3d_core 122
>> + hdmi_iahb 123
>> + hdmi_isfr 124
>> + i2c1 125
>> + i2c2 126
>> + i2c3 127
>> + iim 128
>> + enfc 129
>> + ipu1 130
>> + ipu1_di0 131
>> + ipu1_di1 132
>> + ipu2 133
>> + ipu2_di0 134
>> + ldb_di0 135
>> + ldb_di1 136
>> + ipu2_di1 137
>> + hsi_tx 138
>> + mlb 139
>> + mmdc_ch0_axi 140
>> + mmdc_ch1_axi 141
>> + ocram 142
>> + openvg_axi 143
>> + pcie_axi 144
>> + pwm1 145
>> + pwm2 146
>> + pwm3 147
>> + pwm4 148
>> + per1_bch 149
>> + gpmi_bch_apb 150
>> + gpmi_bch 151
>> + gpmi_io 152
>> + gpmi_apb 153
>> + sata 154
>> + sdma 155
>> + spba 156
>> + ssi1 157
>> + ssi2 158
>> + ssi3 159
>> + uart_ipg 160
>> + uart_serial 161
>> + usboh3 162
>> + usdhc1 163
>> + usdhc2 164
>> + usdhc3 165
>> + usdhc4 166
>> + vdo_axi 167
>> + vpu_axi 168
>> + cko1 169
>> + pll1_sys 170
>> + pll2_bus 171
>> + pll3_usb_otg 172
>> + pll4_audio 173
>> + pll5_video 174
>> + pll6_mlb 175
>> + pll7_usb_host 176
>> + pll8_enet 177
>> + ssi1_ipg 178
>> + ssi2_ipg 179
>> + ssi3_ipg 180
>> + rom 181
>> + usbphy1 182
>> + usbphy2 183
>> + ldb_di0_div_3_5 184
>> + ldb_di1_div_3_5 185
>> +
>> + The ID will be used by clock consumer in the first cell of "clocks"
>> + phandle to specify the desired clock.
>> +
>> +Examples:
>> +
>> +clks: ccm at 020c4000 {
>> + compatible = "fsl,imx6q-ccm";
>> + reg = <0x020c4000 0x4000>;
>> + interrupts = <0 87 0x04 0 88 0x04>;
>> + #clock-cells = <1>;
>> + clock-output-names = ...
>> + "uart_ipg",
>> + "uart_serial",
>> + ...;
>> +};
>> +
>> +uart1: serial at 02020000 {
>> + compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
>> + reg = <0x02020000 0x4000>;
>> + interrupts = <0 26 0x04>;
>> + clocks = <&clks 160>, <&clks 161>;
>> + clock-names = "ipg", "per";
>> + status = "disabled";
>> +};
>> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
>> index 72f30f3..cfdbe53 100644
>> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
>> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
>> @@ -111,6 +111,7 @@
>> codec: sgtl5000 at 0a {
>> compatible = "fsl,sgtl5000";
>> reg = <0x0a>;
>> + clocks = <&clks 169>;
>> VDDA-supply = <®_2p5v>;
>> VDDIO-supply = <®_3p3v>;
>> };
>> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
>> index 0052fe7..acff2dc 100644
>> --- a/arch/arm/boot/dts/imx6q.dtsi
>> +++ b/arch/arm/boot/dts/imx6q.dtsi
>> @@ -93,18 +93,23 @@
>> dma-apbh at 00110000 {
>> compatible = "fsl,imx6q-dma-apbh", "fsl,imx28-dma-apbh";
>> reg = <0x00110000 0x2000>;
>> + clocks = <&clks 106>;
>> };
>>
>> gpmi-nand at 00112000 {
>> - compatible = "fsl,imx6q-gpmi-nand";
>> - #address-cells = <1>;
>> - #size-cells = <1>;
>> - reg = <0x00112000 0x2000>, <0x00114000 0x2000>;
>> - reg-names = "gpmi-nand", "bch";
>> - interrupts = <0 13 0x04>, <0 15 0x04>;
>> - interrupt-names = "gpmi-dma", "bch";
>> - fsl,gpmi-dma-channel = <0>;
>> - status = "disabled";
>> + compatible = "fsl,imx6q-gpmi-nand";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + reg = <0x00112000 0x2000>, <0x00114000 0x2000>;
>> + reg-names = "gpmi-nand", "bch";
>> + interrupts = <0 13 0x04>, <0 15 0x04>;
>> + interrupt-names = "gpmi-dma", "bch";
>> + clocks = <&clks 152>, <&clks 153>, <&clks 151>,
>> + <&clks 150>, <&clks 149>;
>> + clock-names = "gpmi_io", "gpmi_apb", "gpmi_bch",
>> + "gpmi_bch_apb", "per1_bch";
>> + fsl,gpmi-dma-channel = <0>;
>> + status = "disabled";
>> };
>>
>> timer at 00a00600 {
>> @@ -146,6 +151,8 @@
>> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
>> reg = <0x02008000 0x4000>;
>> interrupts = <0 31 0x04>;
>> + clocks = <&clks 112>, <&clks 112>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -155,6 +162,8 @@
>> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
>> reg = <0x0200c000 0x4000>;
>> interrupts = <0 32 0x04>;
>> + clocks = <&clks 113>, <&clks 113>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -164,6 +173,8 @@
>> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
>> reg = <0x02010000 0x4000>;
>> interrupts = <0 33 0x04>;
>> + clocks = <&clks 114>, <&clks 114>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -173,6 +184,8 @@
>> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
>> reg = <0x02014000 0x4000>;
>> interrupts = <0 34 0x04>;
>> + clocks = <&clks 115>, <&clks 115>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -182,6 +195,8 @@
>> compatible = "fsl,imx6q-ecspi", "fsl,imx51-ecspi";
>> reg = <0x02018000 0x4000>;
>> interrupts = <0 35 0x04>;
>> + clocks = <&clks 116>, <&clks 116>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -189,6 +204,8 @@
>> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
>> reg = <0x02020000 0x4000>;
>> interrupts = <0 26 0x04>;
>> + clocks = <&clks 160>, <&clks 161>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -201,6 +218,7 @@
>> compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
>> reg = <0x02028000 0x4000>;
>> interrupts = <0 46 0x04>;
>> + clocks = <&clks 178>;
>> fsl,fifo-depth = <15>;
>> fsl,ssi-dma-events = <38 37>;
>> status = "disabled";
>> @@ -210,6 +228,7 @@
>> compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
>> reg = <0x0202c000 0x4000>;
>> interrupts = <0 47 0x04>;
>> + clocks = <&clks 179>;
>> fsl,fifo-depth = <15>;
>> fsl,ssi-dma-events = <42 41>;
>> status = "disabled";
>> @@ -219,6 +238,7 @@
>> compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
>> reg = <0x02030000 0x4000>;
>> interrupts = <0 48 0x04>;
>> + clocks = <&clks 180>;
>> fsl,fifo-depth = <15>;
>> fsl,ssi-dma-events = <46 45>;
>> status = "disabled";
>> @@ -358,6 +378,7 @@
>> compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
>> reg = <0x020bc000 0x4000>;
>> interrupts = <0 80 0x04>;
>> + clocks = <&clks 0>;
>> status = "disabled";
>> };
>>
>> @@ -365,13 +386,203 @@
>> compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
>> reg = <0x020c0000 0x4000>;
>> interrupts = <0 81 0x04>;
>> + clocks = <&clks 0>;
>> status = "disabled";
>> };
>>
>> - ccm at 020c4000 {
>> + clks: ccm at 020c4000 {
>> compatible = "fsl,imx6q-ccm";
>> reg = <0x020c4000 0x4000>;
>> interrupts = <0 87 0x04 0 88 0x04>;
>> + #clock-cells = <1>;
>> + clock-output-names =
>> + "dummy", /* 0 */
>> + "ckil", /* 1 */
>> + "ckih", /* 2 */
>> + "osc", /* 3 */
>> + "pll2_pfd0_352m", /* 4 */
>> + "pll2_pfd1_594m", /* 5 */
>> + "pll2_pfd2_396m", /* 6 */
>> + "pll3_pfd0_720m", /* 7 */
>> + "pll3_pfd1_540m", /* 8 */
>> + "pll3_pfd2_508m", /* 9 */
>> + "pll3_pfd3_454m", /* 10 */
>> + "pll2_198m", /* 11 */
>> + "pll3_120m", /* 12 */
>> + "pll3_80m", /* 13 */
>> + "pll3_60m", /* 14 */
>> + "twd", /* 15 */
>> + "step", /* 16 */
>> + "pll1_sw", /* 17 */
>> + "periph_pre", /* 18 */
>> + "periph2_pre", /* 19 */
>> + "periph_clk2_sel", /* 20 */
>> + "periph2_clk2_sel", /* 21 */
>> + "axi_sel", /* 22 */
>> + "esai_sel", /* 23 */
>> + "asrc_sel", /* 24 */
>> + "spdif_sel", /* 25 */
>> + "gpu2d_axi", /* 26 */
>> + "gpu3d_axi", /* 27 */
>> + "gpu2d_core_sel", /* 28 */
>> + "gpu3d_core_sel", /* 29 */
>> + "gpu3d_shader_sel", /* 30 */
>> + "ipu1_sel", /* 31 */
>> + "ipu2_sel", /* 32 */
>> + "ldb_di0_sel", /* 33 */
>> + "ldb_di1_sel", /* 34 */
>> + "ipu1_di0_pre_sel", /* 35 */
>> + "ipu1_di1_pre_sel", /* 36 */
>> + "ipu2_di0_pre_sel", /* 37 */
>> + "ipu2_di1_pre_sel", /* 38 */
>> + "ipu1_di0_sel", /* 39 */
>> + "ipu1_di1_sel", /* 40 */
>> + "ipu2_di0_sel", /* 41 */
>> + "ipu2_di1_sel", /* 42 */
>> + "hsi_tx_sel", /* 43 */
>> + "pcie_axi_sel", /* 44 */
>> + "ssi1_sel", /* 45 */
>> + "ssi2_sel", /* 46 */
>> + "ssi3_sel", /* 47 */
>> + "usdhc1_sel", /* 48 */
>> + "usdhc2_sel", /* 49 */
>> + "usdhc3_sel", /* 50 */
>> + "usdhc4_sel", /* 51 */
>> + "enfc_sel", /* 52 */
>> + "emi_sel", /* 53 */
>> + "emi_slow_sel", /* 54 */
>> + "vdo_axi_sel", /* 55 */
>> + "vpu_axi_sel", /* 56 */
>> + "cko1_sel", /* 57 */
>> + "periph", /* 58 */
>> + "periph2", /* 59 */
>> + "periph_clk2", /* 60 */
>> + "periph2_clk2", /* 61 */
>> + "ipg", /* 62 */
>> + "ipg_per", /* 63 */
>> + "esai_pred", /* 64 */
>> + "esai_podf", /* 65 */
>> + "asrc_pred", /* 66 */
>> + "asrc_podf", /* 67 */
>> + "spdif_pred", /* 68 */
>> + "spdif_podf", /* 69 */
>> + "can_root", /* 70 */
>> + "ecspi_root", /* 71 */
>> + "gpu2d_core_podf", /* 72 */
>> + "gpu3d_core_podf", /* 73 */
>> + "gpu3d_shader", /* 74 */
>> + "ipu1_podf", /* 75 */
>> + "ipu2_podf", /* 76 */
>> + "ldb_di0_podf", /* 77 */
>> + "ldb_di1_podf", /* 78 */
>> + "ipu1_di0_pre", /* 79 */
>> + "ipu1_di1_pre", /* 80 */
>> + "ipu2_di0_pre", /* 81 */
>> + "ipu2_di1_pre", /* 82 */
>> + "hsi_tx_podf", /* 83 */
>> + "ssi1_pred", /* 84 */
>> + "ssi1_podf", /* 85 */
>> + "ssi2_pred", /* 86 */
>> + "ssi2_podf", /* 87 */
>> + "ssi3_pred", /* 88 */
>> + "ssi3_podf", /* 89 */
>> + "uart_serial_podf", /* 90 */
>> + "usdhc1_podf", /* 91 */
>> + "usdhc2_podf", /* 92 */
>> + "usdhc3_podf", /* 93 */
>> + "usdhc4_podf", /* 94 */
>> + "enfc_pred", /* 95 */
>> + "enfc_podf", /* 96 */
>> + "emi_podf", /* 97 */
>> + "emi_slow_podf", /* 98 */
>> + "vpu_axi_podf", /* 99 */
>> + "cko1_podf", /* 100 */
>> + "axi", /* 101 */
>> + "mmdc_ch0_axi_podf", /* 102 */
>> + "mmdc_ch1_axi_podf", /* 103 */
>> + "arm", /* 104 */
>> + "ahb", /* 105 */
>> + "apbh_dma", /* 106 */
>> + "asrc", /* 107 */
>> + "can1_ipg", /* 108 */
>> + "can1_serial", /* 109 */
>> + "can2_ipg", /* 110 */
>> + "can2_serial", /* 111 */
>> + "ecspi1", /* 112 */
>> + "ecspi2", /* 113 */
>> + "ecspi3", /* 114 */
>> + "ecspi4", /* 115 */
>> + "ecspi5", /* 116 */
>> + "enet", /* 117 */
>> + "esai", /* 118 */
>> + "gpt_ipg", /* 119 */
>> + "gpt_ipg_per", /* 120 */
>> + "gpu2d_core", /* 121 */
>> + "gpu3d_core", /* 122 */
>> + "hdmi_iahb", /* 123 */
>> + "hdmi_isfr", /* 124 */
>> + "i2c1", /* 125 */
>> + "i2c2", /* 126 */
>> + "i2c3", /* 127 */
>> + "iim", /* 128 */
>> + "enfc", /* 129 */
>> + "ipu1", /* 130 */
>> + "ipu1_di0", /* 131 */
>> + "ipu1_di1", /* 132 */
>> + "ipu2", /* 133 */
>> + "ipu2_di0", /* 134 */
>> + "ldb_di0", /* 135 */
>> + "ldb_di1", /* 136 */
>> + "ipu2_di1", /* 137 */
>> + "hsi_tx", /* 138 */
>> + "mlb", /* 139 */
>> + "mmdc_ch0_axi", /* 140 */
>> + "mmdc_ch1_axi", /* 141 */
>> + "ocram", /* 142 */
>> + "openvg_axi", /* 143 */
>> + "pcie_axi", /* 144 */
>> + "pwm1", /* 145 */
>> + "pwm2", /* 146 */
>> + "pwm3", /* 147 */
>> + "pwm4", /* 148 */
>> + "per1_bch", /* 149 */
>> + "gpmi_bch_apb", /* 150 */
>> + "gpmi_bch", /* 151 */
>> + "gpmi_io", /* 152 */
>> + "gpmi_apb", /* 153 */
>> + "sata", /* 154 */
>> + "sdma", /* 155 */
>> + "spba", /* 156 */
>> + "ssi1", /* 157 */
>> + "ssi2", /* 158 */
>> + "ssi3", /* 159 */
>> + "uart_ipg", /* 160 */
>> + "uart_serial", /* 161 */
>> + "usboh3", /* 162 */
>> + "usdhc1", /* 163 */
>> + "usdhc2", /* 164 */
>> + "usdhc3", /* 165 */
>> + "usdhc4", /* 166 */
>> + "vdo_axi", /* 167 */
>> + "vpu_axi", /* 168 */
>> + "cko1", /* 169 */
>> + "pll1_sys", /* 170 */
>> + "pll2_bus", /* 171 */
>> + "pll3_usb_otg", /* 172 */
>> + "pll4_audio", /* 173 */
>> + "pll5_video", /* 174 */
>> + "pll6_mlb", /* 175 */
>> + "pll7_usb_host", /* 176 */
>> + "pll8_enet", /* 177 */
>> + "ssi1_ipg", /* 178 */
>> + "ssi2_ipg", /* 179 */
>> + "ssi3_ipg", /* 180 */
>> + "rom", /* 181 */
>> + "usbphy1", /* 182 */
>> + "usbphy2", /* 183 */
>> + "ldb_di0_div_3_5", /* 184 */
>> + "ldb_di1_div_3_5", /* 185 */
>> + "end_of_list";
>> };
>>
>> anatop at 020c8000 {
>> @@ -468,12 +679,14 @@
>> compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
>> reg = <0x020c9000 0x1000>;
>> interrupts = <0 44 0x04>;
>> + clocks = <&clks 182>;
>> };
>>
>> usbphy2: usbphy at 020ca000 {
>> compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
>> reg = <0x020ca000 0x1000>;
>> interrupts = <0 45 0x04>;
>> + clocks = <&clks 183>;
>> };
>>
>> snvs at 020cc000 {
>> @@ -608,6 +821,9 @@
>> compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
>> reg = <0x020ec000 0x4000>;
>> interrupts = <0 2 0x04>;
>> + clocks = <&clks 155>, <&clks 155>;
>> + clock-names = "ipg", "ahb";
>> + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx6q-to1.bin";
>> };
>> };
>>
>> @@ -631,6 +847,7 @@
>> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
>> reg = <0x02184000 0x200>;
>> interrupts = <0 43 0x04>;
>> + clocks = <&clks 162>;
>> fsl,usbphy = <&usbphy1>;
>> status = "disabled";
>> };
>> @@ -639,6 +856,7 @@
>> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
>> reg = <0x02184200 0x200>;
>> interrupts = <0 40 0x04>;
>> + clocks = <&clks 162>;
>> fsl,usbphy = <&usbphy2>;
>> status = "disabled";
>> };
>> @@ -647,6 +865,7 @@
>> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
>> reg = <0x02184400 0x200>;
>> interrupts = <0 41 0x04>;
>> + clocks = <&clks 162>;
>> status = "disabled";
>> };
>>
>> @@ -654,6 +873,7 @@
>> compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
>> reg = <0x02184600 0x200>;
>> interrupts = <0 42 0x04>;
>> + clocks = <&clks 162>;
>> status = "disabled";
>> };
>>
>> @@ -661,6 +881,8 @@
>> compatible = "fsl,imx6q-fec";
>> reg = <0x02188000 0x4000>;
>> interrupts = <0 118 0x04 0 119 0x04>;
>> + clocks = <&clks 117>, <&clks 117>;
>> + clock-names = "ipg", "ahb";
>> status = "disabled";
>> };
>>
>> @@ -673,6 +895,8 @@
>> compatible = "fsl,imx6q-usdhc";
>> reg = <0x02190000 0x4000>;
>> interrupts = <0 22 0x04>;
>> + clocks = <&clks 163>, <&clks 163>, <&clks 163>;
>> + clock-names = "ipg", "ahb", "per";
>> status = "disabled";
>> };
>>
>> @@ -680,6 +904,8 @@
>> compatible = "fsl,imx6q-usdhc";
>> reg = <0x02194000 0x4000>;
>> interrupts = <0 23 0x04>;
>> + clocks = <&clks 164>, <&clks 164>, <&clks 164>;
>> + clock-names = "ipg", "ahb", "per";
>> status = "disabled";
>> };
>>
>> @@ -687,6 +913,8 @@
>> compatible = "fsl,imx6q-usdhc";
>> reg = <0x02198000 0x4000>;
>> interrupts = <0 24 0x04>;
>> + clocks = <&clks 165>, <&clks 165>, <&clks 165>;
>> + clock-names = "ipg", "ahb", "per";
>> status = "disabled";
>> };
>>
>> @@ -694,6 +922,8 @@
>> compatible = "fsl,imx6q-usdhc";
>> reg = <0x0219c000 0x4000>;
>> interrupts = <0 25 0x04>;
>> + clocks = <&clks 166>, <&clks 166>, <&clks 166>;
>> + clock-names = "ipg", "ahb", "per";
>> status = "disabled";
>> };
>>
>> @@ -703,6 +933,7 @@
>> compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
>> reg = <0x021a0000 0x4000>;
>> interrupts = <0 36 0x04>;
>> + clocks = <&clks 125>;
>> status = "disabled";
>> };
>>
>> @@ -712,6 +943,7 @@
>> compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
>> reg = <0x021a4000 0x4000>;
>> interrupts = <0 37 0x04>;
>> + clocks = <&clks 126>;
>> status = "disabled";
>> };
>>
>> @@ -721,6 +953,7 @@
>> compatible = "fsl,imx6q-i2c", "fsl,imx1-i2c";
>> reg = <0x021a8000 0x4000>;
>> interrupts = <0 38 0x04>;
>> + clocks = <&clks 127>;
>> status = "disabled";
>> };
>>
>> @@ -784,6 +1017,8 @@
>> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
>> reg = <0x021e8000 0x4000>;
>> interrupts = <0 27 0x04>;
>> + clocks = <&clks 160>, <&clks 161>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -791,6 +1026,8 @@
>> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
>> reg = <0x021ec000 0x4000>;
>> interrupts = <0 28 0x04>;
>> + clocks = <&clks 160>, <&clks 161>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -798,6 +1035,8 @@
>> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
>> reg = <0x021f0000 0x4000>;
>> interrupts = <0 29 0x04>;
>> + clocks = <&clks 160>, <&clks 161>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>>
>> @@ -805,6 +1044,8 @@
>> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
>> reg = <0x021f4000 0x4000>;
>> interrupts = <0 30 0x04>;
>> + clocks = <&clks 160>, <&clks 161>;
>> + clock-names = "ipg", "per";
>> status = "disabled";
>> };
>> };
>> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
>> index 8e46407..433c683 100644
>> --- a/arch/arm/mach-imx/clk-imx6q.c
>> +++ b/arch/arm/mach-imx/clk-imx6q.c
>> @@ -387,48 +387,11 @@ int __init mx6q_clocks_init(void)
>> pr_err("i.MX6q clk %d: register failed with %ld\n",
>> i, PTR_ERR(clk[i]));
>>
>> + of_clk_add_provider(np, of_clk_src_onecell_get, NULL);
>> +
>> clk_register_clkdev(clk[gpt_ipg], "ipg", "imx-gpt.0");
>> clk_register_clkdev(clk[gpt_ipg_per], "per", "imx-gpt.0");
>> clk_register_clkdev(clk[twd], NULL, "smp_twd");
>> - clk_register_clkdev(clk[apbh_dma], NULL, "110000.dma-apbh");
>> - clk_register_clkdev(clk[per1_bch], "per1_bch", "112000.gpmi-nand");
>> - clk_register_clkdev(clk[gpmi_bch_apb], "gpmi_bch_apb", "112000.gpmi-nand");
>> - clk_register_clkdev(clk[gpmi_bch], "gpmi_bch", "112000.gpmi-nand");
>> - clk_register_clkdev(clk[gpmi_apb], "gpmi_apb", "112000.gpmi-nand");
>> - clk_register_clkdev(clk[gpmi_io], "gpmi_io", "112000.gpmi-nand");
>> - clk_register_clkdev(clk[usboh3], NULL, "2184000.usb");
>> - clk_register_clkdev(clk[usboh3], NULL, "2184200.usb");
>> - clk_register_clkdev(clk[usboh3], NULL, "2184400.usb");
>> - clk_register_clkdev(clk[usboh3], NULL, "2184600.usb");
>> - clk_register_clkdev(clk[usbphy1], NULL, "20c9000.usbphy");
>> - clk_register_clkdev(clk[usbphy2], NULL, "20ca000.usbphy");
>> - clk_register_clkdev(clk[uart_serial], "per", "2020000.serial");
>> - clk_register_clkdev(clk[uart_ipg], "ipg", "2020000.serial");
>> - clk_register_clkdev(clk[uart_serial], "per", "21e8000.serial");
>> - clk_register_clkdev(clk[uart_ipg], "ipg", "21e8000.serial");
>> - clk_register_clkdev(clk[uart_serial], "per", "21ec000.serial");
>> - clk_register_clkdev(clk[uart_ipg], "ipg", "21ec000.serial");
>> - clk_register_clkdev(clk[uart_serial], "per", "21f0000.serial");
>> - clk_register_clkdev(clk[uart_ipg], "ipg", "21f0000.serial");
>> - clk_register_clkdev(clk[uart_serial], "per", "21f4000.serial");
>> - clk_register_clkdev(clk[uart_ipg], "ipg", "21f4000.serial");
>> - clk_register_clkdev(clk[enet], NULL, "2188000.ethernet");
>> - clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc");
>> - clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc");
>> - clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc");
>> - clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");
>> - clk_register_clkdev(clk[i2c1], NULL, "21a0000.i2c");
>> - clk_register_clkdev(clk[i2c2], NULL, "21a4000.i2c");
>> - clk_register_clkdev(clk[i2c3], NULL, "21a8000.i2c");
>> - clk_register_clkdev(clk[ecspi1], NULL, "2008000.ecspi");
>> - clk_register_clkdev(clk[ecspi2], NULL, "200c000.ecspi");
>> - clk_register_clkdev(clk[ecspi3], NULL, "2010000.ecspi");
>> - clk_register_clkdev(clk[ecspi4], NULL, "2014000.ecspi");
>> - clk_register_clkdev(clk[ecspi5], NULL, "2018000.ecspi");
>> - clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma");
>> - clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
>> - clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
>> - clk_register_clkdev(clk[ssi1_ipg], NULL, "2028000.ssi");
>> clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL);
>> clk_register_clkdev(clk[ahb], "ahb", NULL);
>> clk_register_clkdev(clk[cko1], "cko1", NULL);
>> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
>> index 6f9c23b..957f5fe 100644
>> --- a/arch/arm/mach-imx/mach-imx6q.c
>> +++ b/arch/arm/mach-imx/mach-imx6q.c
>> @@ -94,7 +94,6 @@ static void __init imx6q_sabrelite_cko1_setup(void)
>> clk_set_parent(cko1_sel, ahb);
>> rate = clk_round_rate(cko1, 16000000);
>> clk_set_rate(cko1, rate);
>> - clk_register_clkdev(cko1, NULL, "0-000a");
>> put_clk:
>> if (!IS_ERR(cko1_sel))
>> clk_put(cko1_sel);
>>
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-20 14:54 ` Matt Sealey
@ 2012-08-20 15:16 ` Shawn Guo
2012-08-20 17:22 ` Matt Sealey
0 siblings, 1 reply; 19+ messages in thread
From: Shawn Guo @ 2012-08-20 15:16 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 20, 2012 at 09:54:48AM -0500, Matt Sealey wrote:
> Yet another arbitrary array...
>
> I'm not sure why you would move the registration lookup into the DT
Because I do not want to patch kernel every time I need a new lookup.
Furthermore, looking at function imx53_qsb_init in imx53-dt.c, you
will find that lookup for sgtl5000 is really board specific and should
go to device tree.
> and use an arbitrarily ordered array again. Sure, you're looking it up
> entirely within the device tree here, but a better solution would be
> to not name clocks twice, and structure your clock definition
> properly.
>
> What's wrong with;
>
> ccm at 020c4000 {
> ...
> my_clock: clock at 0 {
> name = "my_clock_name";
> }
> }
>
> uart at nnnnnnnn {
> ...
> clocks = <&my_clock>;
> }
>
> ?
>
It turns a property into 185 nodes, which will just bloat the device
tree. This issue has been discussed for a plenty of times.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-20 15:16 ` Shawn Guo
@ 2012-08-20 17:22 ` Matt Sealey
2012-08-21 12:27 ` Russell King - ARM Linux
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Matt Sealey @ 2012-08-20 17:22 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 20, 2012 at 10:16 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Mon, Aug 20, 2012 at 09:54:48AM -0500, Matt Sealey wrote:
>> Yet another arbitrary array...
>>
>> I'm not sure why you would move the registration lookup into the DT
>
> Because I do not want to patch kernel every time I need a new lookup.
> Furthermore, looking at function imx53_qsb_init in imx53-dt.c, you
> will find that lookup for sgtl5000 is really board specific and should
> go to device tree.
>
>> and use an arbitrarily ordered array again. Sure, you're looking it up
>> entirely within the device tree here, but a better solution would be
>> to not name clocks twice, and structure your clock definition
>> properly.
>>
>> What's wrong with;
>>
>> ccm at 020c4000 {
>> ...
>> my_clock: clock at 0 {
>> name = "my_clock_name";
>> }
>> }
>>
>> uart at nnnnnnnn {
>> ...
>> clocks = <&my_clock>;
>> }
>>
>> ?
>>
> It turns a property into 185 nodes, which will just bloat the device
> tree. This issue has been discussed for a plenty of times.
It's not bloat just because it is by its very definition "a big list", is it?
How do you explain duplicating the clock names in the array AND in the
device node as NOT being bloated?
You're going to have to define these clocks as a tree with parents and
leaf nodes anyway in the clock subsystem. Why not define these in the
device tree in total and reference them by handle when you build the
entire clock tree from the ground up? Or will it just be all the
clocks defined in Linux, but the lookups (which is what I see here)
moved into the DT? Why not form the lookups as part of the definition
of the clock tree?
--
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-20 17:22 ` Matt Sealey
@ 2012-08-21 12:27 ` Russell King - ARM Linux
2012-08-21 13:11 ` Rob Herring
2012-08-21 12:53 ` Rob Herring
2012-08-22 2:47 ` Shawn Guo
2 siblings, 1 reply; 19+ messages in thread
From: Russell King - ARM Linux @ 2012-08-21 12:27 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 20, 2012 at 12:22:56PM -0500, Matt Sealey wrote:
> You're going to have to define these clocks as a tree with parents and
> leaf nodes anyway in the clock subsystem. Why not define these in the
> device tree in total and reference them by handle when you build the
> entire clock tree from the ground up? Or will it just be all the
> clocks defined in Linux, but the lookups (which is what I see here)
> moved into the DT? Why not form the lookups as part of the definition
> of the clock tree?
Well, IMHO the DT conversion of the clk lookup stuff has been done
completely wrong.
What should have been done is rather than invent a totally new bloody
lookup interface that drivers have to use instead of clk_get(), is to
embed the OF lookup _inside_ clk_get().
What you do is this:
1. Have property names in the device node like:
clock_<connection-id> = <&provider-node output>
In the case of a NULL connection id:
clock = <&provider-node output>
Remember that the connection ID is _supposed_ to be something that
is described by the hardware (like - for the AACI primecell, the
clock which runs the functional side is called "AACICLK" by the TRM,
and for the MMCI primecell, it's "MMCICLK" - even though these two
clocks may be fed by the same source in an implementation.)
2. clkdev's lookup is then modified to look at the struct device, and
check for a DT node. If there is a DT node, it formats a property
string:
if (dev->of_node) {
char *propname, *clk_prop = NULL;
struct property *p;
if (conn_id) {
clk_prop = kasprintf("clock_%s", conn_id);
propname = clk_prop;
} else {
propname = "clock";
}
p = of_find_property(dev->of_node, propname, NULL);
if (clk_prop)
kfree(clk_prop);
if (p) {
clk = clk_get_from_of_property(p);
if (clk)
return clk;
}
/* Fallthrough to clkdev table lookup */
}
So now, you're not dealing with inventing a whole load of names for clocks
on a platform, instead what you're doing is describing _where_ the clock
comes from in the system for a particular device by device node and index
into it - just like we do for interrupts.
This means there's no need for huge tables and such like of clock names.
I did mention this idea long ago but got ignored.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-20 17:22 ` Matt Sealey
2012-08-21 12:27 ` Russell King - ARM Linux
@ 2012-08-21 12:53 ` Rob Herring
2012-08-22 22:50 ` Matt Sealey
2012-08-22 2:47 ` Shawn Guo
2 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2012-08-21 12:53 UTC (permalink / raw)
To: linux-arm-kernel
On 08/20/2012 12:22 PM, Matt Sealey wrote:
> On Mon, Aug 20, 2012 at 10:16 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>> On Mon, Aug 20, 2012 at 09:54:48AM -0500, Matt Sealey wrote:
>>> Yet another arbitrary array...
>>>
>>> I'm not sure why you would move the registration lookup into the DT
>>
>> Because I do not want to patch kernel every time I need a new lookup.
>> Furthermore, looking at function imx53_qsb_init in imx53-dt.c, you
>> will find that lookup for sgtl5000 is really board specific and should
>> go to device tree.
>>
>>> and use an arbitrarily ordered array again. Sure, you're looking it up
>>> entirely within the device tree here, but a better solution would be
>>> to not name clocks twice, and structure your clock definition
>>> properly.
>>>
>>> What's wrong with;
>>>
>>> ccm at 020c4000 {
>>> ...
>>> my_clock: clock at 0 {
>>> name = "my_clock_name";
>>> }
>>> }
>>>
>>> uart at nnnnnnnn {
>>> ...
>>> clocks = <&my_clock>;
>>> }
>>>
>>> ?
>>>
>> It turns a property into 185 nodes, which will just bloat the device
>> tree. This issue has been discussed for a plenty of times.
>
> It's not bloat just because it is by its very definition "a big list", is it?
>
> How do you explain duplicating the clock names in the array AND in the
> device node as NOT being bloated?
Read the clock binding doc. Names are optional and the 2 names are not
the same. One is the name of the output on the CCM and one is the name
on input to the module.
While generally optional, Shawn has chosen to require at least the
output names. In the case of defining a signal clock controller node
with lots of outputs, that is the right choice.
Rob
>
> You're going to have to define these clocks as a tree with parents and
> leaf nodes anyway in the clock subsystem. Why not define these in the
> device tree in total and reference them by handle when you build the
> entire clock tree from the ground up? Or will it just be all the
> clocks defined in Linux, but the lookups (which is what I see here)
> moved into the DT? Why not form the lookups as part of the definition
> of the clock tree?
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-21 12:27 ` Russell King - ARM Linux
@ 2012-08-21 13:11 ` Rob Herring
2012-08-22 8:32 ` Russell King - ARM Linux
0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2012-08-21 13:11 UTC (permalink / raw)
To: linux-arm-kernel
On 08/21/2012 07:27 AM, Russell King - ARM Linux wrote:
> On Mon, Aug 20, 2012 at 12:22:56PM -0500, Matt Sealey wrote:
>> You're going to have to define these clocks as a tree with parents and
>> leaf nodes anyway in the clock subsystem. Why not define these in the
>> device tree in total and reference them by handle when you build the
>> entire clock tree from the ground up? Or will it just be all the
>> clocks defined in Linux, but the lookups (which is what I see here)
>> moved into the DT? Why not form the lookups as part of the definition
>> of the clock tree?
>
> Well, IMHO the DT conversion of the clk lookup stuff has been done
> completely wrong.
>
> What should have been done is rather than invent a totally new bloody
> lookup interface that drivers have to use instead of clk_get(), is to
> embed the OF lookup _inside_ clk_get().
That is exactly what was done. Drivers only use clk_get. Only if you
don't have a struct device, then you can use of_clk_get.
Internally, you still need a conversion of clk provider node and cell to
a struct clk. It is up to each clk provider how to do this. The lookup
done here by Shawn is using the struct clk name and using the existing
clk framework lookup.
> What you do is this:
>
> 1. Have property names in the device node like:
>
> clock_<connection-id> = <&provider-node output>
The connection id is defined by the position in the list and
supplemented with "clock-names" property.
>
> In the case of a NULL connection id:
>
> clock = <&provider-node output>
>
> Remember that the connection ID is _supposed_ to be something that
> is described by the hardware (like - for the AACI primecell, the
> clock which runs the functional side is called "AACICLK" by the TRM,
> and for the MMCI primecell, it's "MMCICLK" - even though these two
> clocks may be fed by the same source in an implementation.)
>
> 2. clkdev's lookup is then modified to look at the struct device, and
> check for a DT node. If there is a DT node, it formats a property
> string:
>
> if (dev->of_node) {
> char *propname, *clk_prop = NULL;
> struct property *p;
>
> if (conn_id) {
> clk_prop = kasprintf("clock_%s", conn_id);
> propname = clk_prop;
> } else {
> propname = "clock";
> }
>
> p = of_find_property(dev->of_node, propname, NULL);
> if (clk_prop)
> kfree(clk_prop);
>
> if (p) {
> clk = clk_get_from_of_property(p);
> if (clk)
> return clk;
> }
>
> /* Fallthrough to clkdev table lookup */
> }
>
> So now, you're not dealing with inventing a whole load of names for clocks
> on a platform, instead what you're doing is describing _where_ the clock
> comes from in the system for a particular device by device node and index
> into it - just like we do for interrupts.
That is what we're doing. The names are optional for DT, but happen to
be required for struct clk now. If we don't put something in DT, then
the clock names will have to be something generic like ccm-1..ccm-185.
Rob
>
> This means there's no need for huge tables and such like of clock names.
>
> I did mention this idea long ago but got ignored.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-20 17:22 ` Matt Sealey
2012-08-21 12:27 ` Russell King - ARM Linux
2012-08-21 12:53 ` Rob Herring
@ 2012-08-22 2:47 ` Shawn Guo
2 siblings, 0 replies; 19+ messages in thread
From: Shawn Guo @ 2012-08-22 2:47 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 20, 2012 at 12:22:56PM -0500, Matt Sealey wrote:
> It's not bloat just because it is by its very definition "a big list", is it?
>
Grep for_each_node_by_*() and for_each_*_node() (include/linux/of.h)
in the tree to see how often these global device tree searching is
used, you may know how important not having so many nodes is.
> How do you explain duplicating the clock names in the array AND in the
> device node as NOT being bloated?
>
> You're going to have to define these clocks as a tree with parents and
> leaf nodes anyway in the clock subsystem. Why not define these in the
> device tree in total and reference them by handle when you build the
> entire clock tree from the ground up? Or will it just be all the
> clocks defined in Linux, but the lookups (which is what I see here)
> moved into the DT? Why not form the lookups as part of the definition
> of the clock tree?
>
This is something I had tried long time before, but it did not get
accepted because:
* It's unnecessary to encode the entire clock tree which is SoC
specific in device tree. Clock driver is a good place for that.
* Again, doing so will bloat device tree with hundreds of nodes.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-21 13:11 ` Rob Herring
@ 2012-08-22 8:32 ` Russell King - ARM Linux
2012-08-22 9:27 ` Shawn Guo
0 siblings, 1 reply; 19+ messages in thread
From: Russell King - ARM Linux @ 2012-08-22 8:32 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 21, 2012 at 08:11:57AM -0500, Rob Herring wrote:
> On 08/21/2012 07:27 AM, Russell King - ARM Linux wrote:
> > So now, you're not dealing with inventing a whole load of names for clocks
> > on a platform, instead what you're doing is describing _where_ the clock
> > comes from in the system for a particular device by device node and index
> > into it - just like we do for interrupts.
>
> That is what we're doing. The names are optional for DT, but happen to
> be required for struct clk now. If we don't put something in DT, then
> the clock names will have to be something generic like ccm-1..ccm-185.
And that's what's wrong. Clocks themselves should _not_ be required to
be named.
If you use purely "producer node + index" then you don't need to name a
whole bunch of clocks, and you don't need to have an array of clock names
in the DT file. This also gets rid of the time consuming strcmp against
every clock, which has already been raised as a problem with the existing
clk_get().
And, like it or not, the way they're being describing them in the DT file
at the top of this sub-thread, the matching _is_ done only by producer
name, which is TOTALLY the wrong way to go about this (that's how folk
tried to use the connection ID in the clk API and IT DOESN'T WORK for
reusable drivers.)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-22 8:32 ` Russell King - ARM Linux
@ 2012-08-22 9:27 ` Shawn Guo
0 siblings, 0 replies; 19+ messages in thread
From: Shawn Guo @ 2012-08-22 9:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Aug 22, 2012 at 09:32:23AM +0100, Russell King - ARM Linux wrote:
> On Tue, Aug 21, 2012 at 08:11:57AM -0500, Rob Herring wrote:
> > On 08/21/2012 07:27 AM, Russell King - ARM Linux wrote:
> > > So now, you're not dealing with inventing a whole load of names for clocks
> > > on a platform, instead what you're doing is describing _where_ the clock
> > > comes from in the system for a particular device by device node and index
> > > into it - just like we do for interrupts.
> >
> > That is what we're doing. The names are optional for DT, but happen to
> > be required for struct clk now. If we don't put something in DT, then
> > the clock names will have to be something generic like ccm-1..ccm-185.
>
> And that's what's wrong. Clocks themselves should _not_ be required to
> be named.
>
> If you use purely "producer node + index" then you don't need to name a
> whole bunch of clocks, and you don't need to have an array of clock names
> in the DT file. This also gets rid of the time consuming strcmp against
> every clock, which has already been raised as a problem with the existing
> clk_get().
>
Ok, if I got your point correctly, it's about amending the following
changes, and then we can get rid of that big clock-output-names list
from dts.
Regards,
Shawn
--8<---
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 433c683..d52f3f4 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -387,7 +387,7 @@ int __init mx6q_clocks_init(void)
pr_err("i.MX6q clk %d: register failed with %ld\n",
i, PTR_ERR(clk[i]));
- of_clk_add_provider(np, of_clk_src_onecell_get, NULL);
+ of_clk_add_provider(np, of_clk_src_onecell_get, clk);
clk_register_clkdev(clk[gpt_ipg], "ipg", "imx-gpt.0");
clk_register_clkdev(clk[gpt_ipg_per], "per", "imx-gpt.0");
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 06bc0b5..a010ed6 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1584,16 +1584,10 @@ EXPORT_SYMBOL_GPL(of_clk_src_simple_get);
struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data)
{
- const char *clk_name;
+ struct clk **clks = data;
int idx = clkspec->args[0];
- int ret;
-
- ret = of_property_read_string_index(clkspec->np, "clock-output-names",
- idx, &clk_name);
- if (ret < 0)
- return ERR_PTR(ret);
- return __clk_lookup(clk_name);
+ return clks[idx];
}
EXPORT_SYMBOL_GPL(of_clk_src_onecell_get);
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-21 12:53 ` Rob Herring
@ 2012-08-22 22:50 ` Matt Sealey
2012-08-23 0:08 ` Matt Sealey
0 siblings, 1 reply; 19+ messages in thread
From: Matt Sealey @ 2012-08-22 22:50 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 21, 2012 at 7:53 AM, Rob Herring <rob.herring@calxeda.com> wrote:
>
>> How do you explain duplicating the clock names in the array AND in the
>> device node as NOT being bloated?
>
> Read the clock binding doc. Names are optional and the 2 names are not
> the same. One is the name of the output on the CCM and one is the name
> on input to the module.
My understanding of the i.MX clock tree naming in the docs, though, is
that the names in the DT don't match the ones in the docs, regardless.
I also don't understand how lots of nodes in a tree *is* lots of
bloat, by Shawn's definition, but lots of entirely Linux- and
common-clock-framework-specific names in a table *isn't* bloat even if
most of these clocks and names are not used for devices in the tree
anyway.
Device trees shouldn't encode data that is entirely specific to a
Linux driver. Even if the only user is Linux, you are not describing
the Linux driver, you are describing the hardware. The names match the
hardware specification in the CCM chapters of the manual, but just
barely. All Shawn has done here is make the lookup in the DT, which is
at the very least internally consistent (it's not referencing the
order of an array elsewhere than the device tree), but an index into
an array of strings is not the way we generally encode things in
device trees since the dawn of time, and certainly shouldn't be
acceptable behavior now.
I might agree somewhat with Shawn that encoding every clock for the
chip in the tree (some 190+ entries) and it's bits and entries is a
ridiculous amount, but most boards don't need all the clocks or
selects defined if they're not using those peripherals. There are ways
to make sure boards do not over-define their clock tree (and any
clocks not defined will not get enabled anyway).
That the clock tree is SoC-specific doesn't mean it should not be
encoded in the tree; the fact that Sascha could write a fairly
comprehensive common clock subsystem shows that for certain families
of SoC, certain groupings of clock mux, select and associations
between unit clock (for instance to write to registers) and peripheral
output clock (for instance to clock data to an MMC card or SPI bus or
whatever) are fairly "common" as it stands. What is not common is the
naming and the meaning of that naming, which in the end is only useful
to drivers as debugging information anyway.
What I don't get is why you'd implement a clock lookup as <&clk 160>
<&clk 161> and then futher give them names like "ipg" "per", even if
these were seperate clocks, it makes no sense to NAME them. If clock
160 is "uart_ipg" and 161 is "uart_serial", what are you gaining from
the clock-names property inside the actual device definition? You're
encoding the "ipg" root clock name twice, and "per" doesn't make any
sense here.
What's the difference with moving ALL the definitions of clock values
from the driver (such as arch/arm/mach-imx/clk-mx51-mx53.c) into the
DT and parsing it all out of the DT? Once it's registered then you
have to look it up by name, but if each DT driver node (uart@ for
example) references the clock node by phandle, it will be able to look
up that exact clock by that phandle, correct? What I'm confused about
right now is what the difference is between naming them "ipg" and
"per" vs. "donkey" and "shrek"? This is just a driver-specific naming
such that it can find these things and turn on and off the right
clocks at the right time, but it doesn't make any sense to refer to
the CCM node, then an index to a an array of names, then two more
names defining the clock definition. uart_serial cannot be used for
ANYTHING besides the uart "per" clock, so just encode this as a clock
type in the individual clock node. The clkdev subsystem can resolve
"per" to some magic value or the string in the tree...
uart_ipg_clk: clock at foo {
clock-name = "uart";
fsl,clock-type = "ipg";
...
};
uart at bar {
clocks = <&uart_ipg_clk, &uart_per_clk>
};
I counted here and we're not "bloating" anything in terms of bytes
used to define the clocks - what we save in not doing an array lookup,
we lose in defining a node. Tthis is not going to end the world. What
you "waste" in the device tree, you get to automate in the Linux
driver anyway and remove a bunch of tables, which is what Shawn's
trying to do anyway. If the UART driver needs to do;
sport->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
Then it has all the information it needs here; one of the phandle
references points to a clock which has "ipg" as it's fsl,clock-type
property. Or something else.
It is going to be an SoC-specific binding, that's for sure, but since
there is a definition for a fixed clock, why can't there be a
definition for a gated clock (enable/disable), mux select defined as a
group and possible selections, and the right clocks being defined with
the right potential parents? The entire clock tree AND it's lookups
can be built from device tree, per documentation, per actual SoC
layout and the driver has to live with getting the information from
somewhere other than the heady mix of a table in DT, and a table in
Linux which don't necessarily depend on each other?
You only have to do these lookups once when the clock subsystem starts
on Linux, it's not like every time you want to go find a clock you
have to actually enter a firmware interface, parse the entire device
tree, come back out with a small snippet of data and then go off and
do the stuff; it's been put in a readily-accessible structure, and
gets done at device probe. Register them at boot, find them in the
clock subsystem, you're ready to go.
I would rather see every clock defined in the tree, as much "bloat" as
you would seem to think it is, it makes no sense to have a huge table
of clock definitions who's names and numbers are defined in the device
tree, yet register offsets and bit widths and values and defaults and
parents are defined in the driver itself along with lots of other
lists and tables. The table in the DT is making a huge assumption
about the subsystem needs to do the clock lookups and the way the
*Linux drivers* themself actually works; this is wrong, wrong, wrong
on so many levels.
> While generally optional, Shawn has chosen to require at least the
> output names. In the case of defining a signal clock controller node
> with lots of outputs, that is the right choice.
You only need to define the inputs and outputs you use, and in the end
I think referencing an array of strings by phandle and index into a
property contained in that handle, then looking up another string in
it's own device node to make sure which clock is which, is more
complex and takes longer than referencing a clock by phandle and
looking at two specific properties defining that clock. If we're going
this far, we should DEFINITELY be encoding the clock subsystems in the
SoC entirely in the device tree and automating clock setup. If the
common clock framework lives up to it's name, all you'd need is a
single arch/arm/mach-imx/clk.c to map the specific operation for every
i.MX SoC (i.e. bindings for i.MX) and not one for *every* i.MX SoC,
and all this data can be adequately encoded in the DT.
--
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-22 22:50 ` Matt Sealey
@ 2012-08-23 0:08 ` Matt Sealey
2012-08-23 1:32 ` Rob Herring
0 siblings, 1 reply; 19+ messages in thread
From: Matt Sealey @ 2012-08-23 0:08 UTC (permalink / raw)
To: linux-arm-kernel
Ugh. Okay my blood sugar was super low. Can I just condense that into
I agree with Russell and the binding makes no sense once it gets past
the simple definition described in the binding?
If we take the pinctrl mess as an example, all the DT serves to do is
define an SoC-specific or family-specific binding into data the
pinctrl subsystem can use, via an SoC-specific abstraction in a driver
to make a generic subsystem in Linux work the way it should.
The common clock subsystem is no different - divider, fixed factor,
fixed rate, mux, gate, highbank (??) are defined in a way that
abstracts most of the actual SoC-specific stuff out to register
offsets, values, bit definitions and masks.
Right now for i.MX we have several abstraction interfaces
(imx_clk_XXX) which move values from tables around and call the
appropriate common clock function to register the clock. These
abstractions are identical for all the possible i.MX SoCs
(imx_clk_gate2, imx_clk_mux etc.) and move the values around so they
can be passed to clk_register_##same.
So if we can define a fixed-clock, why can't we define an
fsl,gated-clock or fsl,muxed-clock which defines an i.MX clock of that
type, and a much smaller subset of code that stuffs these values in
through a small abstraction and remove all these seperate
clk-imx51-imx53.c, clk-imx6q.c, clk-imx31.c files which are sharing
common functionality into one mega-abstraction which works across the
whole family?
The way I understand the binding right now (and I'm looking at this;
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob_plain;f=Documentation/devicetree/bindings/clock/clock-bindings.txt;hb=766e6a4ec602d0c107553b91b3434fe9c03474f4),
this is entirely what the clock binding is saying we should do. But I
don't understand why we're naming inputs and outputs, but not defining
how these signals are routed (possibly through gate bits in registers)
and leaving that down to some driver somewhere. Or why we have to name
clocks twice, once for the canonical name of the output in reference
to a higher level or root clock "uart_serial", vs. a driver-level name
for that input or output in the driver ("per"), since most clocks at
the driver level can't be re-used for other things anyway. If they
could, the whole prepare/unprepare and some explicit parenting/muxing
of clocks would handle that anyway so that the clock was enabled while
there was at least one user.
I do understand that in Shawn's patch and using UART as an example
again, "ipg" and "per" are definitely needed by the driver, and these
names are defined by the driver and implicit in the documentation to
enable the use of this unit, but at the end of the day the definition
at the *device* level (uart@) makes no sense except to perform a
lookup on a string, and in the end this string lookup only gets done
at the driver probe time anyway. With a standard definition of these
"ipg" and "per" strings per family of SoC or even in a generic fashion
across all possible SoCs (in this case, both are defined using
imx_clk_gate2()) then a node defining that clock and it's magical
driver-specific name would work just as well by phandle reference, and
it's parents are referenced by phandle, and all this stays within
SoC-specific abstraction of the common clocks and turns into normal
common clock structure stuff anyway (so you still get to do a string
lookup, but it's being stuffed by an i.MX driver and registered from
data it pulled from the DT).
Wouldn't we rather have a single device tree parser and clock
registration for i.MX than the current 7 which would get split into 7
clock registration drivers with some helpers that parsed half of it
via device tree? I don't really see what benefit you get out of going
for the halfway-defined model.
--
Matt Sealey <matt@genesi-usa.com>
Product Development Analyst, Genesi USA, Inc.
On Wed, Aug 22, 2012 at 5:50 PM, Matt Sealey <matt@genesi-usa.com> wrote:
> On Tue, Aug 21, 2012 at 7:53 AM, Rob Herring <rob.herring@calxeda.com> wrote:
>>
>>> How do you explain duplicating the clock names in the array AND in the
>>> device node as NOT being bloated?
>>
>> Read the clock binding doc. Names are optional and the 2 names are not
>> the same. One is the name of the output on the CCM and one is the name
>> on input to the module.
>
> My understanding of the i.MX clock tree naming in the docs, though, is
> that the names in the DT don't match the ones in the docs, regardless.
>
> I also don't understand how lots of nodes in a tree *is* lots of
> bloat, by Shawn's definition, but lots of entirely Linux- and
> common-clock-framework-specific names in a table *isn't* bloat even if
> most of these clocks and names are not used for devices in the tree
> anyway.
>
> Device trees shouldn't encode data that is entirely specific to a
> Linux driver. Even if the only user is Linux, you are not describing
> the Linux driver, you are describing the hardware. The names match the
> hardware specification in the CCM chapters of the manual, but just
> barely. All Shawn has done here is make the lookup in the DT, which is
> at the very least internally consistent (it's not referencing the
> order of an array elsewhere than the device tree), but an index into
> an array of strings is not the way we generally encode things in
> device trees since the dawn of time, and certainly shouldn't be
> acceptable behavior now.
>
> I might agree somewhat with Shawn that encoding every clock for the
> chip in the tree (some 190+ entries) and it's bits and entries is a
> ridiculous amount, but most boards don't need all the clocks or
> selects defined if they're not using those peripherals. There are ways
> to make sure boards do not over-define their clock tree (and any
> clocks not defined will not get enabled anyway).
>
> That the clock tree is SoC-specific doesn't mean it should not be
> encoded in the tree; the fact that Sascha could write a fairly
> comprehensive common clock subsystem shows that for certain families
> of SoC, certain groupings of clock mux, select and associations
> between unit clock (for instance to write to registers) and peripheral
> output clock (for instance to clock data to an MMC card or SPI bus or
> whatever) are fairly "common" as it stands. What is not common is the
> naming and the meaning of that naming, which in the end is only useful
> to drivers as debugging information anyway.
>
> What I don't get is why you'd implement a clock lookup as <&clk 160>
> <&clk 161> and then futher give them names like "ipg" "per", even if
> these were seperate clocks, it makes no sense to NAME them. If clock
> 160 is "uart_ipg" and 161 is "uart_serial", what are you gaining from
> the clock-names property inside the actual device definition? You're
> encoding the "ipg" root clock name twice, and "per" doesn't make any
> sense here.
>
> What's the difference with moving ALL the definitions of clock values
> from the driver (such as arch/arm/mach-imx/clk-mx51-mx53.c) into the
> DT and parsing it all out of the DT? Once it's registered then you
> have to look it up by name, but if each DT driver node (uart@ for
> example) references the clock node by phandle, it will be able to look
> up that exact clock by that phandle, correct? What I'm confused about
> right now is what the difference is between naming them "ipg" and
> "per" vs. "donkey" and "shrek"? This is just a driver-specific naming
> such that it can find these things and turn on and off the right
> clocks at the right time, but it doesn't make any sense to refer to
> the CCM node, then an index to a an array of names, then two more
> names defining the clock definition. uart_serial cannot be used for
> ANYTHING besides the uart "per" clock, so just encode this as a clock
> type in the individual clock node. The clkdev subsystem can resolve
> "per" to some magic value or the string in the tree...
>
> uart_ipg_clk: clock at foo {
> clock-name = "uart";
> fsl,clock-type = "ipg";
> ...
> };
>
> uart at bar {
> clocks = <&uart_ipg_clk, &uart_per_clk>
> };
>
> I counted here and we're not "bloating" anything in terms of bytes
> used to define the clocks - what we save in not doing an array lookup,
> we lose in defining a node. Tthis is not going to end the world. What
> you "waste" in the device tree, you get to automate in the Linux
> driver anyway and remove a bunch of tables, which is what Shawn's
> trying to do anyway. If the UART driver needs to do;
>
> sport->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
>
> Then it has all the information it needs here; one of the phandle
> references points to a clock which has "ipg" as it's fsl,clock-type
> property. Or something else.
>
> It is going to be an SoC-specific binding, that's for sure, but since
> there is a definition for a fixed clock, why can't there be a
> definition for a gated clock (enable/disable), mux select defined as a
> group and possible selections, and the right clocks being defined with
> the right potential parents? The entire clock tree AND it's lookups
> can be built from device tree, per documentation, per actual SoC
> layout and the driver has to live with getting the information from
> somewhere other than the heady mix of a table in DT, and a table in
> Linux which don't necessarily depend on each other?
>
> You only have to do these lookups once when the clock subsystem starts
> on Linux, it's not like every time you want to go find a clock you
> have to actually enter a firmware interface, parse the entire device
> tree, come back out with a small snippet of data and then go off and
> do the stuff; it's been put in a readily-accessible structure, and
> gets done at device probe. Register them at boot, find them in the
> clock subsystem, you're ready to go.
>
> I would rather see every clock defined in the tree, as much "bloat" as
> you would seem to think it is, it makes no sense to have a huge table
> of clock definitions who's names and numbers are defined in the device
> tree, yet register offsets and bit widths and values and defaults and
> parents are defined in the driver itself along with lots of other
> lists and tables. The table in the DT is making a huge assumption
> about the subsystem needs to do the clock lookups and the way the
> *Linux drivers* themself actually works; this is wrong, wrong, wrong
> on so many levels.
>
>> While generally optional, Shawn has chosen to require at least the
>> output names. In the case of defining a signal clock controller node
>> with lots of outputs, that is the right choice.
>
> You only need to define the inputs and outputs you use, and in the end
> I think referencing an array of strings by phandle and index into a
> property contained in that handle, then looking up another string in
> it's own device node to make sure which clock is which, is more
> complex and takes longer than referencing a clock by phandle and
> looking at two specific properties defining that clock. If we're going
> this far, we should DEFINITELY be encoding the clock subsystems in the
> SoC entirely in the device tree and automating clock setup. If the
> common clock framework lives up to it's name, all you'd need is a
> single arch/arm/mach-imx/clk.c to map the specific operation for every
> i.MX SoC (i.e. bindings for i.MX) and not one for *every* i.MX SoC,
> and all this data can be adequately encoded in the DT.
>
> --
> Matt Sealey <matt@genesi-usa.com>
> Product Development Analyst, Genesi USA, Inc.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup
2012-08-23 0:08 ` Matt Sealey
@ 2012-08-23 1:32 ` Rob Herring
0 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2012-08-23 1:32 UTC (permalink / raw)
To: linux-arm-kernel
On 08/22/2012 07:08 PM, Matt Sealey wrote:
> Ugh. Okay my blood sugar was super low. Can I just condense that into
> I agree with Russell and the binding makes no sense once it gets past
> the simple definition described in the binding?
I can't speak for Russell, but I think his issue is addressed in v2.
> If we take the pinctrl mess as an example, all the DT serves to do is
> define an SoC-specific or family-specific binding into data the
> pinctrl subsystem can use, via an SoC-specific abstraction in a driver
> to make a generic subsystem in Linux work the way it should.
pinctrl changes much more board to board, so it's more valuable to have
in device tree. The changes from board to board in the clock tree are
much less.
> The common clock subsystem is no different - divider, fixed factor,
> fixed rate, mux, gate, highbank (??) are defined in a way that
> abstracts most of the actual SoC-specific stuff out to register
> offsets, values, bit definitions and masks.
>
> Right now for i.MX we have several abstraction interfaces
> (imx_clk_XXX) which move values from tables around and call the
> appropriate common clock function to register the clock. These
> abstractions are identical for all the possible i.MX SoCs
> (imx_clk_gate2, imx_clk_mux etc.) and move the values around so they
> can be passed to clk_register_##same.
>
> So if we can define a fixed-clock, why can't we define an
> fsl,gated-clock or fsl,muxed-clock which defines an i.MX clock of that
> type, and a much smaller subset of code that stuffs these values in
> through a small abstraction and remove all these seperate
> clk-imx51-imx53.c, clk-imx6q.c, clk-imx31.c files which are sharing
> common functionality into one mega-abstraction which works across the
> whole family?
>
> The way I understand the binding right now (and I'm looking at this;
> http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob_plain;f=Documentation/devicetree/bindings/clock/clock-bindings.txt;hb=766e6a4ec602d0c107553b91b3434fe9c03474f4),
> this is entirely what the clock binding is saying we should do. But I
> don't understand why we're naming inputs and outputs, but not defining
> how these signals are routed (possibly through gate bits in registers)
> and leaving that down to some driver somewhere. Or why we have to name
> clocks twice, once for the canonical name of the output in reference
> to a higher level or root clock "uart_serial", vs. a driver-level name
> for that input or output in the driver ("per"), since most clocks at
> the driver level can't be re-used for other things anyway. If they
> could, the whole prepare/unprepare and some explicit parenting/muxing
> of clocks would handle that anyway so that the clock was enabled while
> there was at least one user.
The binding defines connections on clocks between nodes. Whether this is
a single clock controller node with many outputs or a node per mux,
divider, gate, etc. is up to the implementer. I did the latter on
highbank because I've got about 8 clocks and half are the same (pll
outputs). Having worked on many generations of iMX and knowing all the
pitfalls of their clock controllers, I would do exactly what Shawn has
done for any part with complex clock trees. I don't think trying to
abstract things to completely generic code will be worth the effort or
be able to describe all the interactions between clocks.
The value in device tree is handling board differences as there are 10's
of boards per SOC.
> I do understand that in Shawn's patch and using UART as an example
> again, "ipg" and "per" are definitely needed by the driver, and these
> names are defined by the driver and implicit in the documentation to
> enable the use of this unit, but at the end of the day the definition
> at the *device* level (uart@) makes no sense except to perform a
> lookup on a string, and in the end this string lookup only gets done
> at the driver probe time anyway. With a standard definition of these
> "ipg" and "per" strings per family of SoC or even in a generic fashion
> across all possible SoCs (in this case, both are defined using
> imx_clk_gate2()) then a node defining that clock and it's magical
> driver-specific name would work just as well by phandle reference, and
> it's parents are referenced by phandle, and all this stays within
> SoC-specific abstraction of the common clocks and turns into normal
> common clock structure stuff anyway (so you still get to do a string
> lookup, but it's being stuffed by an i.MX driver and registered from
> data it pulled from the DT).
Why don't you read v2 which removes the strings.
> Wouldn't we rather have a single device tree parser and clock
> registration for i.MX than the current 7 which would get split into 7
> clock registration drivers with some helpers that parsed half of it
> via device tree? I don't really see what benefit you get out of going
> for the halfway-defined model.
All this has been discussed already over the 2 years of iterations of
common struct clock and clock bindings.
Rob
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-08-23 1:32 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-16 8:08 [PATCH 0/2] Move imx6q clock lookup over to device tree Shawn Guo
2012-08-16 8:08 ` [PATCH 1/2] clk: add of_clk_src_onecell_get() support Shawn Guo
2012-08-20 14:30 ` Shawn Guo
2012-08-16 8:08 ` [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup Shawn Guo
2012-08-20 12:51 ` Rob Herring
2012-08-20 14:54 ` Matt Sealey
2012-08-20 15:16 ` Shawn Guo
2012-08-20 17:22 ` Matt Sealey
2012-08-21 12:27 ` Russell King - ARM Linux
2012-08-21 13:11 ` Rob Herring
2012-08-22 8:32 ` Russell King - ARM Linux
2012-08-22 9:27 ` Shawn Guo
2012-08-21 12:53 ` Rob Herring
2012-08-22 22:50 ` Matt Sealey
2012-08-23 0:08 ` Matt Sealey
2012-08-23 1:32 ` Rob Herring
2012-08-22 2:47 ` Shawn Guo
2012-08-19 14:05 ` [PATCH 3/4] clk: mxs: replace imx28 " Shawn Guo
2012-08-19 14:05 ` [PATCH 4/4] clk: mxs: replace imx23 " Shawn Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).