Message ID | 1345104526-14797-3-git-send-email-shawn.guo@linaro.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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@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@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@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@00110000 { > compatible = "fsl,imx6q-dma-apbh", "fsl,imx28-dma-apbh"; > reg = <0x00110000 0x2000>; > + clocks = <&clks 106>; > }; > > gpmi-nand@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@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@020c4000 { > + clks: ccm@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@020c8000 { > @@ -468,12 +679,14 @@ > compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy"; > reg = <0x020c9000 0x1000>; > interrupts = <0 44 0x04>; > + clocks = <&clks 182>; > }; > > usbphy2: usbphy@020ca000 { > compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy"; > reg = <0x020ca000 0x1000>; > interrupts = <0 45 0x04>; > + clocks = <&clks 183>; > }; > > snvs@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); >
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@020c4000 { ... my_clock: clock@0 { name = "my_clock_name"; } } uart@nnnnnnnn { ... clocks = <&my_clock>; } ?
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@020c4000 { > ... > my_clock: clock@0 { > name = "my_clock_name"; > } > } > > uart@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.
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@020c4000 { >> ... >> my_clock: clock@0 { >> name = "my_clock_name"; >> } >> } >> >> uart@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?
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.
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@020c4000 { >>> ... >>> my_clock: clock@0 { >>> name = "my_clock_name"; >>> } >>> } >>> >>> uart@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? >
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. >
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.
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.)
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@foo { clock-name = "uart"; fsl,clock-type = "ipg"; ... }; uart@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.
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.
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
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@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@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@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@00110000 { compatible = "fsl,imx6q-dma-apbh", "fsl,imx28-dma-apbh"; reg = <0x00110000 0x2000>; + clocks = <&clks 106>; }; gpmi-nand@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@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@020c4000 { + clks: ccm@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@020c8000 { @@ -468,12 +679,14 @@ compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy"; reg = <0x020c9000 0x1000>; interrupts = <0 44 0x04>; + clocks = <&clks 182>; }; usbphy2: usbphy@020ca000 { compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy"; reg = <0x020ca000 0x1000>; interrupts = <0 45 0x04>; + clocks = <&clks 183>; }; snvs@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);
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