Re: [PATCH v3 06/10] clk: exynos5420: register clocks using common clock framework
On Tuesday 18 June 2013, Tomasz Figa wrote: > This is a binding that has been defined for Samsung Common Clock Framework > drivers. Exynos4 and Exynos5250 use the same convention. The numbers are > defined in a way that should allow adding further clocks of particular types > in > future as need for such shows up. I see, it's probably too late to change that then. Maybe if there is an Exynos6 this should be done differently though. > Physically there is one clock controller (CMU) which has a lot of dividers, > muxes and gates and so it is represented as a single device node. Some platforms just put device nodes for their clocks under the clock controller node, which would have helped here. Another option would be to use #clock-cells=<2> and make the first cell the type of the clock, and the second one the number. Also, if I understand things correctly, simple fixed rate clocks would normally not go into the clock driver at all when they are not controller by it. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/10] clk: exynos5420: register clocks using common clock framework
Hi Arnd, On Tuesday 18 of June 2013 16:01:16 Arnd Bergmann wrote: > On Tuesday 18 June 2013, Chander Kashyap wrote: > > >> + [Core Clocks] > > >> + > > >> + Clock ID > > >> + > > >> + > > >> + fin_pll1 > > >> + > > >> + [Clock Gate for Special Clocks] > > >> + > > >> + Clock ID > > >> + > > >> + sclk_uart0 128 > > >> + sclk_uart1 129 > > >> + sclk_uart2 130 > > >> > > >> + > > >> + [Peripheral Clock Gates] > > >> + > > >> + Clock ID > > >> + > > >> + > > >> + aclk66_peric 256 > > >> + uart0 257 > > >> + uart1 258 > > > > > > It looks like these are actually separate things. Wouldn't it be more > > > sensible to have separate device nodes for each of the lists and use a > > > local index?> > > I have listed the parent clock first, then the child clocks, to > > maintain readability. > > > > > What numbers are used in the data sheet? > > > > I didn't get your point? > > I would have expected three clock device nodes, one for fin_pll (presumably > a fixed-rate clock?), one for "special clocks" and one for "peripheral > clock gates", and a number space starting at '1' for each of them, rather > than having a shared node and numbers starting at '1', '128' and '256', > which looks a bit clumsy. > > Did you take the ID number definitions from a data sheet, or did you make up > the numbers yourself for the purpose of defining a binding? This is a binding that has been defined for Samsung Common Clock Framework drivers. Exynos4 and Exynos5250 use the same convention. The numbers are defined in a way that should allow adding further clocks of particular types in future as need for such shows up. Physically there is one clock controller (CMU) which has a lot of dividers, muxes and gates and so it is represented as a single device node. Best regards, Tomasz > Arnd > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/10] clk: exynos5420: register clocks using common clock framework
On Tuesday 18 June 2013, Chander Kashyap wrote: > >> + [Core Clocks] > >> + > >> + Clock ID > >> + > >> + > >> + fin_pll1 > >> + > >> + [Clock Gate for Special Clocks] > >> + > >> + Clock ID > >> + > >> + sclk_uart0 128 > >> + sclk_uart1 129 > >> + sclk_uart2 130 > > > >> + > >> + [Peripheral Clock Gates] > >> + > >> + Clock ID > >> + > >> + > >> + aclk66_peric 256 > >> + uart0 257 > >> + uart1 258 > > > > It looks like these are actually separate things. Wouldn't it be more > > sensible > > to have separate device nodes for each of the lists and use a local index? > I have listed the parent clock first, then the child clocks, to > maintain readability. > > > > What numbers are used in the data sheet? > I didn't get your point? I would have expected three clock device nodes, one for fin_pll (presumably a fixed-rate clock?), one for "special clocks" and one for "peripheral clock gates", and a number space starting at '1' for each of them, rather than having a shared node and numbers starting at '1', '128' and '256', which looks a bit clumsy. Did you take the ID number definitions from a data sheet, or did you make up the numbers yourself for the purpose of defining a binding? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/10] clk: exynos5420: register clocks using common clock framework
On 17 June 2013 17:57, Arnd Bergmann wrote: > On Monday 17 June 2013 16:30:31 Chander Kashyap wrote: >> diff --git a/Documentation/devicetree/bindings/clock/exynos5420-clock.txt >> b/Documentation/devicetree/bindings/clock/exynos5420-clock.txt >> new file mode 100644 >> index 000..9bcc4b1 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/exynos5420-clock.txt >> @@ -0,0 +1,201 @@ >> +* Samsung Exynos5420 Clock Controller >> + >> +The Exynos5420 clock controller generates and supplies clock to various >> +controllers within the Exynos5420 SoC. >> + >> +Required Properties: >> + >> +- comptible: should be one of the following. >> + - "samsung,exynos5420-clock" - controller compatible with Exynos5420 SoC. >> + >> +- reg: physical base address of the controller and length of memory mapped >> + region. >> + >> +- #clock-cells: should be 1. >> + >> +The following is the list of clocks generated by the controller. Each clock >> is >> +assigned an identifier and client nodes use this identifier to specify the >> +clock which they consume. >> + >> + >> + [Core Clocks] >> + >> + Clock ID >> + >> + >> + fin_pll1 >> + >> + [Clock Gate for Special Clocks] >> + >> + Clock ID >> + >> + sclk_uart0 128 >> + sclk_uart1 129 >> + sclk_uart2 130 > >> + >> + [Peripheral Clock Gates] >> + >> + Clock ID >> + >> + >> + aclk66_peric 256 >> + uart0 257 >> + uart1 258 > > It looks like these are actually separate things. Wouldn't it be more sensible > to have separate device nodes for each of the lists and use a local index? I have listed the parent clock first, then the child clocks, to maintain readability. > > What numbers are used in the data sheet? I didn't get your point? Thanks. > > Arnd -- with warm regards, Chander Kashyap -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/10] clk: exynos5420: register clocks using common clock framework
On Monday 17 June 2013 16:30:31 Chander Kashyap wrote: > diff --git a/Documentation/devicetree/bindings/clock/exynos5420-clock.txt > b/Documentation/devicetree/bindings/clock/exynos5420-clock.txt > new file mode 100644 > index 000..9bcc4b1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/exynos5420-clock.txt > @@ -0,0 +1,201 @@ > +* Samsung Exynos5420 Clock Controller > + > +The Exynos5420 clock controller generates and supplies clock to various > +controllers within the Exynos5420 SoC. > + > +Required Properties: > + > +- comptible: should be one of the following. > + - "samsung,exynos5420-clock" - controller compatible with Exynos5420 SoC. > + > +- reg: physical base address of the controller and length of memory mapped > + region. > + > +- #clock-cells: should be 1. > + > +The following is the list of clocks generated by the controller. Each clock > is > +assigned an identifier and client nodes use this identifier to specify the > +clock which they consume. > + > + > + [Core Clocks] > + > + Clock ID > + > + > + fin_pll1 > + > + [Clock Gate for Special Clocks] > + > + Clock ID > + > + sclk_uart0 128 > + sclk_uart1 129 > + sclk_uart2 130 > + > + [Peripheral Clock Gates] > + > + Clock ID > + > + > + aclk66_peric 256 > + uart0 257 > + uart1 258 It looks like these are actually separate things. Wouldn't it be more sensible to have separate device nodes for each of the lists and use a local index? What numbers are used in the data sheet? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 06/10] clk: exynos5420: register clocks using common clock framework
Chander Kashyap wrote: > > The Exynos5420 clocks are statically listed and registered using the > Samsung specific common clock helper functions. > > Signed-off-by: Chander Kashyap > Signed-off-by: Thomas Abraham (+ Mike) Mike, if you're OK on this, please let me know so that I could take this with others. Thanks, - Kukjin > --- > .../devicetree/bindings/clock/exynos5420-clock.txt | 201 ++ > drivers/clk/samsung/Makefile |1 + > drivers/clk/samsung/clk-exynos5420.c | 762 > > 3 files changed, 964 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/exynos5420- > clock.txt > create mode 100644 drivers/clk/samsung/clk-exynos5420.c > > diff --git a/Documentation/devicetree/bindings/clock/exynos5420-clock.txt > b/Documentation/devicetree/bindings/clock/exynos5420-clock.txt > new file mode 100644 > index 000..9bcc4b1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/exynos5420-clock.txt > @@ -0,0 +1,201 @@ > +* Samsung Exynos5420 Clock Controller > + > +The Exynos5420 clock controller generates and supplies clock to various > +controllers within the Exynos5420 SoC. > + > +Required Properties: > + > +- comptible: should be one of the following. > + - "samsung,exynos5420-clock" - controller compatible with Exynos5420 > SoC. > + > +- reg: physical base address of the controller and length of memory > mapped > + region. > + > +- #clock-cells: should be 1. > + > +The following is the list of clocks generated by the controller. Each > clock is > +assigned an identifier and client nodes use this identifier to specify > the > +clock which they consume. > + > + > + [Core Clocks] > + > + Clock ID > + > + > + fin_pll1 > + > + [Clock Gate for Special Clocks] > + > + Clock ID > + > + sclk_uart0 128 > + sclk_uart1 129 > + sclk_uart2 130 > + sclk_uart3 131 > + sclk_mmc0 132 > + sclk_mmc1 133 > + sclk_mmc2 134 > + sclk_spi0 135 > + sclk_spi1 136 > + sclk_spi2 137 > + sclk_i2s1 138 > + sclk_i2s2 139 > + sclk_pcm1 140 > + sclk_pcm2 141 > + sclk_spdif 142 > + sclk_hdmi 143 > + sclk_pixel 144 > + sclk_dp1 145 > + sclk_mipi1 146 > + sclk_fimd1 147 > + sclk_maudio0 148 > + sclk_maupcm0 149 > + sclk_usbd300 150 > + sclk_usbd301 151 > + sclk_usbphy300 152 > + sclk_usbphy301 153 > + sclk_unipro154 > + sclk_pwm 155 > + sclk_gscl_wa 156 > + sclk_gscl_wb 157 > + > + [Peripheral Clock Gates] > + > + Clock ID > + > + > + aclk66_peric 256 > + uart0 257 > + uart1 258 > + uart2 259 > + uart3 260 > + i2c0 261 > + i2c1 262 > + i2c2 263 > + i2c3 264 > + i2c4 265 > + i2c5 266 > + i2c6 267 > + i2c7 268 > + i2c_hdmi 269 > + tsadc 270 > + spi0 271 > + spi1 272 > + spi2 273 > + keyif 274 > + i2s1 275 > + i2s2 276 > + pcm1 277 > + pcm2 278 > + pwm279 > + spdif 280 > + i2c8 281 > + i2c9 282 > + i2c10 283 > + aclk66_psgen 300 > + chipid 301 > + sysreg 302 > + tzpc0 303 > + tzpc1 304 > + tzpc2 305 > + tzpc3 306 > + tzpc4 307 > + tzpc5 308 > + tzpc6 309 > + tzpc7 310 > + tzpc8 311 > + tzpc9 312 > + hdmi_cec 313 > + seckey 314 > + mct315 > + wdt316 > + rtc317 > + tmu318 > + tmu_gpu319 > + pclk66_gpio330 > + aclk200_fsys2 350 > + mmc0 351 > + mmc1 352 > + mmc2 353 > + sromc 354 > + ufs355 > + aclk200_fsys 360 > + tsi361 > + pdma0 362 > + pdma1
