| Message ID | 1336562022-24368-1-git-send-email-viresh.kumar@st.com |
|---|---|
| State | New |
| Headers | show |
Hi Viresh, On Wed, May 9, 2012 at 4:13 AM, Viresh Kumar <viresh.kumar@st.com> wrote: > Hi Arnd,Olof, > > This is rebased over a (merge of Mike's/clk-next & SPEAr's DT) + Russell's > patch: CLKDEV: provide helpers for common clock framework rebased over them. Russell has published a clkdev branch that you pull instead of applying his patch manually, that way there will be no surprises with respect to conflicts, etc: git://ftp.arm.linux.org.uk/~rmk/linux-next.git clkdev It is also in arm-soc as depends/rmk/clkdev now. Also, Mike, can you please confirm that your clk-next branch is truly stable and will never be rebased before it goes upstream? If it might be rebased, can you please provide a separate branch that is guaranteed to be stable that we can pull in as the dependency here? Thanks, -Olof
On Thu, May 10, 2012 at 12:14:39AM -0700, Olof Johansson wrote: > Russell has published a clkdev branch that you pull instead of > applying his patch manually, that way there will be no surprises with > respect to conflicts, etc: > > git://ftp.arm.linux.org.uk/~rmk/linux-next.git clkdev > > It is also in arm-soc as depends/rmk/clkdev now. I'll mention that I'm considering a patch on top of this to add __must_check to the return values for these clkdev clk registration functions. I've noticed that many users aren't checking that, so there's no real way to be sure that these things are being created. The alternative is we make these functions void and/or add pr_err() inside them to report the failures.
On 5/10/2012 12:44 PM, Olof Johansson wrote: > Also, Mike, can you please confirm that your clk-next branch is truly > stable and will never be rebased before it goes upstream? If it might > be rebased, can you please provide a separate branch that is > guaranteed to be stable that we can pull in as the dependency here? Mike already sent a pull request, in reply to a mail. That's why i sent my request.
On Thu, May 10, 2012 at 12:14 AM, Olof Johansson <olof@lixom.net> wrote: > Hi Viresh, > > On Wed, May 9, 2012 at 4:13 AM, Viresh Kumar <viresh.kumar@st.com> wrote: >> Hi Arnd,Olof, >> >> This is rebased over a (merge of Mike's/clk-next & SPEAr's DT) + Russell's >> patch: CLKDEV: provide helpers for common clock framework rebased over them. > > Russell has published a clkdev branch that you pull instead of > applying his patch manually, that way there will be no surprises with > respect to conflicts, etc: > > git://ftp.arm.linux.org.uk/~rmk/linux-next.git clkdev > > It is also in arm-soc as depends/rmk/clkdev now. > > Also, Mike, can you please confirm that your clk-next branch is truly > stable and will never be rebased before it goes upstream? If it might > be rebased, can you please provide a separate branch that is > guaranteed to be stable that we can pull in as the dependency here? clk-next is stable. It will not be rebased. Regards, Mike > > > Thanks, > > -Olof
On Thu, May 10, 2012 at 09:59:39AM +0100, Russell King - ARM Linux wrote: > On Thu, May 10, 2012 at 12:14:39AM -0700, Olof Johansson wrote: > > Russell has published a clkdev branch that you pull instead of > > applying his patch manually, that way there will be no surprises with > > respect to conflicts, etc: > > > > git://ftp.arm.linux.org.uk/~rmk/linux-next.git clkdev > > > > It is also in arm-soc as depends/rmk/clkdev now. > > I'll mention that I'm considering a patch on top of this to add __must_check > to the return values for these clkdev clk registration functions. I've > noticed that many users aren't checking that, so there's no real way to > be sure that these things are being created. > I have to admit I'm one of such users. Does that mean we have to revise the platform porting again? So ... > The alternative is we make these functions void and/or add pr_err() inside > them to report the failures. > ... I'd vote for this alternative. (Sorry for users that are checking the return right now. Any?)
On Thu, May 10, 2012 at 11:37:47PM +0800, Shawn Guo wrote: > On Thu, May 10, 2012 at 09:59:39AM +0100, Russell King - ARM Linux wrote: > > On Thu, May 10, 2012 at 12:14:39AM -0700, Olof Johansson wrote: > > > Russell has published a clkdev branch that you pull instead of > > > applying his patch manually, that way there will be no surprises with > > > respect to conflicts, etc: > > > > > > git://ftp.arm.linux.org.uk/~rmk/linux-next.git clkdev > > > > > > It is also in arm-soc as depends/rmk/clkdev now. > > > > I'll mention that I'm considering a patch on top of this to add __must_check > > to the return values for these clkdev clk registration functions. I've > > noticed that many users aren't checking that, so there's no real way to > > be sure that these things are being created. > > > I have to admit I'm one of such users. Does that mean we have to > revise the platform porting again? So ... > > > The alternative is we make these functions void and/or add pr_err() inside > > them to report the failures. > > > ... I'd vote for this alternative. (Sorry for users that are checking > the return right now. Any?) It's one reason why I mentioned it in this email, to find out how folk would like to deal with this. I have no particular preference as to how it gets addressed, just as long as we don't end up with silent failures causing hard to debug problems.
On Thu, May 10, 2012 at 8:53 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, May 10, 2012 at 11:37:47PM +0800, Shawn Guo wrote: >> On Thu, May 10, 2012 at 09:59:39AM +0100, Russell King - ARM Linux wrote: >> > On Thu, May 10, 2012 at 12:14:39AM -0700, Olof Johansson wrote: >> > > Russell has published a clkdev branch that you pull instead of >> > > applying his patch manually, that way there will be no surprises with >> > > respect to conflicts, etc: >> > > >> > > git://ftp.arm.linux.org.uk/~rmk/linux-next.git clkdev >> > > >> > > It is also in arm-soc as depends/rmk/clkdev now. >> > >> > I'll mention that I'm considering a patch on top of this to add __must_check >> > to the return values for these clkdev clk registration functions. I've >> > noticed that many users aren't checking that, so there's no real way to >> > be sure that these things are being created. >> > >> I have to admit I'm one of such users. Does that mean we have to >> revise the platform porting again? So ... >> >> > The alternative is we make these functions void and/or add pr_err() inside >> > them to report the failures. >> > >> ... I'd vote for this alternative. (Sorry for users that are checking >> the return right now. Any?) > > It's one reason why I mentioned it in this email, to find out how folk > would like to deal with this. I have no particular preference as to how > it gets addressed, just as long as we don't end up with silent failures > causing hard to debug problems. How about adding the __must_check patch on your for-next branch for a bit, so we can find and fix build fallout on the arm-soc branches through linux-next, maintainers can add build fixes that we pick up and then, after that, the __must_check fix can go upstream? That maintains bisectability on the branches we already have and should avoid more non-checking code to be introduced down the road. -Olof
On Thu, May 10, 2012 at 10:33:24PM -0700, Olof Johansson wrote: > How about adding the __must_check patch on your for-next branch for a > bit, so we can find and fix build fallout on the arm-soc branches > through linux-next, maintainers can add build fixes that we pick up > and then, after that, the __must_check fix can go upstream? That > maintains bisectability on the branches we already have and should > avoid more non-checking code to be introduced down the road. > The function clk_register_clkdev is created to run-time register clk lookup. Mostly the users of the function (platform clk code) will have a long list like the below (arch/arm/mach-imx/clk-imx6q.c). clk_register_clkdev(clk[mmdc_ch0_axi], NULL, "mmdc_ch0_axi"); clk_register_clkdev(clk[mmdc_ch1_axi], NULL, "mmdc_ch1_axi"); 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[usboh3], NULL, "usboh3"); 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); We probably do not want to make the whole clk initialization fail on individual clk_register_clkdev call, but just give an error message and continue trying to boot. In that case, clk_register_clkdev can just do the job to save users from return checking on every single call in that long list?
Hi Arnd,Olof, This is rebased over a (merge of Mike's/clk-next & SPEAr's DT) + Russell's patch: CLKDEV: provide helpers for common clock framework rebased over them. Other pull requests will be rebased over this one. The following changes since commit 82d8f4b7ea3ebaf7f6dfe2cd8a25045d9ff95d5b: CLKDEV: provide helpers for common clock framework (2012-05-09 14:43:39 +0530) are available in the git repository at: git://git.stlinux.com/spear/linux-2.6.git for-v3.5-spear-clk for you to fetch changes up to df8e37e5972dd4ea2a4a0e5dbfdab14b00e98b70: SPEAr: Switch to common clock framework (2012-05-09 14:44:57 +0530) ---------------------------------------------------------------- Viresh Kumar (6): SPEAr: clk: Add VCO-PLL Synthesizer clock SPEAr: clk: Add Auxiliary Synthesizer clock SPEAr: clk: Add Fractional Synthesizer clock SPEAr: clk: Add General Purpose Timer Synthesizer clock SPEAr: Call clk_prepare() before calling clk_enable SPEAr: Switch to common clock framework MAINTAINERS | 4 +- arch/arm/Kconfig | 1 + arch/arm/mach-spear3xx/Makefile | 2 +- arch/arm/mach-spear3xx/clock.c | 892 -------------------- arch/arm/mach-spear3xx/include/mach/generic.h | 4 +- arch/arm/mach-spear3xx/include/mach/misc_regs.h | 2 + arch/arm/mach-spear3xx/include/mach/spear.h | 13 + arch/arm/mach-spear3xx/spear300.c | 1 - arch/arm/mach-spear3xx/spear310.c | 1 - arch/arm/mach-spear3xx/spear320.c | 12 +- arch/arm/mach-spear3xx/spear3xx.c | 2 + arch/arm/mach-spear6xx/Makefile | 2 +- arch/arm/mach-spear6xx/clock.c | 789 ------------------ arch/arm/mach-spear6xx/include/mach/misc_regs.h | 2 + arch/arm/mach-spear6xx/spear6xx.c | 5 +- arch/arm/plat-spear/Makefile | 2 +- arch/arm/plat-spear/clock.c | 1005 ----------------------- arch/arm/plat-spear/include/plat/clock.h | 249 ------ arch/arm/plat-spear/time.c | 8 +- drivers/clk/Makefile | 3 + drivers/clk/spear/Makefile | 8 + drivers/clk/spear/clk-aux-synth.c | 198 +++++ drivers/clk/spear/clk-frac-synth.c | 165 ++++ drivers/clk/spear/clk-gpt-synth.c | 154 ++++ drivers/clk/spear/clk-vco-pll.c | 363 ++++++++ drivers/clk/spear/clk.c | 36 + drivers/clk/spear/clk.h | 134 +++ drivers/clk/spear/spear3xx_clock.c | 612 ++++++++++++++ drivers/clk/spear/spear6xx_clock.c | 342 ++++++++ 29 files changed, 2056 insertions(+), 2955 deletions(-) delete mode 100644 arch/arm/mach-spear3xx/clock.c delete mode 100644 arch/arm/mach-spear6xx/clock.c delete mode 100644 arch/arm/plat-spear/clock.c delete mode 100644 arch/arm/plat-spear/include/plat/clock.h create mode 100644 drivers/clk/spear/Makefile create mode 100644 drivers/clk/spear/clk-aux-synth.c create mode 100644 drivers/clk/spear/clk-frac-synth.c create mode 100644 drivers/clk/spear/clk-gpt-synth.c create mode 100644 drivers/clk/spear/clk-vco-pll.c create mode 100644 drivers/clk/spear/clk.c create mode 100644 drivers/clk/spear/clk.h create mode 100644 drivers/clk/spear/spear3xx_clock.c create mode 100644 drivers/clk/spear/spear6xx_clock.c