| Message ID | 1351181518-11882-8-git-send-email-m-karicheri2@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On 10/25/2012 9:41 PM, Murali Karicheri wrote: > pll.h is added to migrate some of the PLL controller defines for sleep.S. > psc.h is modified to keep only PSC modules definitions needed by sleep.S > after migrating to common clock. The definitions under > ifdef CONFIG_COMMON_CLK will be removed in a subsequent patch. > davinci_watchdog_reset prototype is moved to time.h as clock.h is > being obsoleted. sleep.S and pm.c is modified to include the new header > file replacements. > > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> > --- > arch/arm/mach-davinci/devices.c | 2 ++ > arch/arm/mach-davinci/include/mach/pll.h | 46 +++++++++++++++++++++++++++++ > arch/arm/mach-davinci/include/mach/psc.h | 4 +++ > arch/arm/mach-davinci/include/mach/time.h | 4 ++- > arch/arm/mach-davinci/pm.c | 4 +++ > arch/arm/mach-davinci/sleep.S | 4 +++ > 6 files changed, 63 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/mach-davinci/include/mach/pll.h With this patch a _third_ copy of PLL definitions is created in kernel sources. The existing PLL definitions in clock.h inside mach-davinci should be moved to mach/pll.h and the pll.h you introduced inside drivers/clk in 5/11 should be removed (this patch should appear before 5/11). The biggest disadvantage of this approach is inclusion of mach/ includes in drivers/clk. But duplicating code is definitely not the fix for this. Anyway, mach/ includes are not uncommon in drivers/clk (they are all probably suffering from the same issue). $ grep -rl "include <mach/" drivers/clk/* drivers/clk/clk-u300.c drivers/clk/mmp/clk-pxa168.c drivers/clk/mmp/clk-mmp2.c drivers/clk/mmp/clk-pxa910.c drivers/clk/mxs/clk-imx23.c drivers/clk/mxs/clk-imx28.c drivers/clk/spear/spear6xx_clock.c drivers/clk/spear/spear3xx_clock.c drivers/clk/spear/spear1340_clock.c drivers/clk/spear/spear1310_clock.c drivers/clk/ux500/clk-prcc.c drivers/clk/versatile/clk-integrator.c drivers/clk/versatile/clk-realview.c pll.h can probably be moved to include/linux/clk/ to avoid this. Would like to hear from Mike on this before going ahead. Anyway, instead of just commenting, I though I will be more useful and went ahead and made some of the changes I have been talking about. I fixed the multiple PLL definitions issue, the build infrastructure issue and the commit ordering too. I pushed the patches I fixed to devel-common-clk branch of my git tree. It is build tested using davinci_all_defconfig but its not runtime tested. Can you start from here and provide me incremental changes on top of this? That way we can collaborate to finish this faster. Thanks, Sekhar
On 11/04/2012 09:05 AM, Sekhar Nori wrote: > > On 10/25/2012 9:41 PM, Murali Karicheri wrote: >> pll.h is added to migrate some of the PLL controller defines for sleep.S. >> psc.h is modified to keep only PSC modules definitions needed by sleep.S >> after migrating to common clock. The definitions under >> ifdef CONFIG_COMMON_CLK will be removed in a subsequent patch. >> davinci_watchdog_reset prototype is moved to time.h as clock.h is >> being obsoleted. sleep.S and pm.c is modified to include the new header >> file replacements. >> >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> --- >> arch/arm/mach-davinci/devices.c | 2 ++ >> arch/arm/mach-davinci/include/mach/pll.h | 46 +++++++++++++++++++++++++++++ >> arch/arm/mach-davinci/include/mach/psc.h | 4 +++ >> arch/arm/mach-davinci/include/mach/time.h | 4 ++- >> arch/arm/mach-davinci/pm.c | 4 +++ >> arch/arm/mach-davinci/sleep.S | 4 +++ >> 6 files changed, 63 insertions(+), 1 deletion(-) >> create mode 100644 arch/arm/mach-davinci/include/mach/pll.h > With this patch a _third_ copy of PLL definitions is created in kernel > sources. The existing PLL definitions in clock.h inside mach-davinci > should be moved to mach/pll.h and the pll.h you introduced inside > drivers/clk in 5/11 should be removed (this patch should appear before > 5/11). > > The biggest disadvantage of this approach is inclusion of mach/ includes > in drivers/clk. But duplicating code is definitely not the fix for this. > Anyway, mach/ includes are not uncommon in drivers/clk (they are all > probably suffering from the same issue). Sekhar, I have replied to patch 5/11 that also refers to this. The main reason we are not able to do this cleanly is the code in sleep.c and pm.c. That is something related to Power management. Could you take a look and see if you can do some clean up on this code? I believe It is more than just moving the header files. Murali > > $ grep -rl "include <mach/" drivers/clk/* > drivers/clk/clk-u300.c > drivers/clk/mmp/clk-pxa168.c > drivers/clk/mmp/clk-mmp2.c > drivers/clk/mmp/clk-pxa910.c > drivers/clk/mxs/clk-imx23.c > drivers/clk/mxs/clk-imx28.c > drivers/clk/spear/spear6xx_clock.c > drivers/clk/spear/spear3xx_clock.c > drivers/clk/spear/spear1340_clock.c > drivers/clk/spear/spear1310_clock.c > drivers/clk/ux500/clk-prcc.c > drivers/clk/versatile/clk-integrator.c > drivers/clk/versatile/clk-realview.c > > pll.h can probably be moved to include/linux/clk/ to avoid this. Would > like to hear from Mike on this before going ahead. > > Anyway, instead of just commenting, I though I will be more useful and > went ahead and made some of the changes I have been talking about. I > fixed the multiple PLL definitions issue, the build infrastructure issue > and the commit ordering too. > > I pushed the patches I fixed to devel-common-clk branch of my git tree. > It is build tested using davinci_all_defconfig but its not runtime tested. > > Can you start from here and provide me incremental changes on top of > this? That way we can collaborate to finish this faster. Thanks for offering some help. Yes I can provide you incremental patch. But then could you also help me to squash/rebase and send patches to the list for review? > Thanks, > Sekhar >
On 11/04/2012 09:05 AM, Sekhar Nori wrote: > > On 10/25/2012 9:41 PM, Murali Karicheri wrote: >> pll.h is added to migrate some of the PLL controller defines for sleep.S. >> psc.h is modified to keep only PSC modules definitions needed by sleep.S >> after migrating to common clock. The definitions under >> ifdef CONFIG_COMMON_CLK will be removed in a subsequent patch. >> davinci_watchdog_reset prototype is moved to time.h as clock.h is >> being obsoleted. sleep.S and pm.c is modified to include the new header >> file replacements. >> >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> --- >> arch/arm/mach-davinci/devices.c | 2 ++ >> arch/arm/mach-davinci/include/mach/pll.h | 46 +++++++++++++++++++++++++++++ >> arch/arm/mach-davinci/include/mach/psc.h | 4 +++ >> arch/arm/mach-davinci/include/mach/time.h | 4 ++- >> arch/arm/mach-davinci/pm.c | 4 +++ >> arch/arm/mach-davinci/sleep.S | 4 +++ >> 6 files changed, 63 insertions(+), 1 deletion(-) >> create mode 100644 arch/arm/mach-davinci/include/mach/pll.h > With this patch a _third_ copy of PLL definitions is created in kernel > sources. The existing PLL definitions in clock.h inside mach-davinci > should be moved to mach/pll.h and the pll.h you introduced inside > drivers/clk in 5/11 should be removed (this patch should appear before > 5/11). > > The biggest disadvantage of this approach is inclusion of mach/ includes > in drivers/clk. But duplicating code is definitely not the fix for this. > Anyway, mach/ includes are not uncommon in drivers/clk (they are all > probably suffering from the same issue). > > $ grep -rl "include <mach/" drivers/clk/* > drivers/clk/clk-u300.c > drivers/clk/mmp/clk-pxa168.c > drivers/clk/mmp/clk-mmp2.c > drivers/clk/mmp/clk-pxa910.c > drivers/clk/mxs/clk-imx23.c > drivers/clk/mxs/clk-imx28.c > drivers/clk/spear/spear6xx_clock.c > drivers/clk/spear/spear3xx_clock.c > drivers/clk/spear/spear1340_clock.c > drivers/clk/spear/spear1310_clock.c > drivers/clk/ux500/clk-prcc.c > drivers/clk/versatile/clk-integrator.c > drivers/clk/versatile/clk-realview.c > > pll.h can probably be moved to include/linux/clk/ to avoid this. Would > like to hear from Mike on this before going ahead. > > Anyway, instead of just commenting, I though I will be more useful and > went ahead and made some of the changes I have been talking about. I > fixed the multiple PLL definitions issue, the build infrastructure issue > and the commit ordering too. > > I pushed the patches I fixed to devel-common-clk branch of my git tree. > It is build tested using davinci_all_defconfig but its not runtime tested. > > Can you start from here and provide me incremental changes on top of > this? That way we can collaborate to finish this faster. > > Thanks, > Sekhar > I made a build from your branch and it doesn't boot up DM6446. I will debug this tomorrow. But what should I focus on? I thought it is a header file re-arrangement? Murali
On 11/6/2012 12:41 AM, Murali Karicheri wrote: > On 11/04/2012 09:05 AM, Sekhar Nori wrote: >> >> On 10/25/2012 9:41 PM, Murali Karicheri wrote: >>> pll.h is added to migrate some of the PLL controller defines for >>> sleep.S. >>> psc.h is modified to keep only PSC modules definitions needed by sleep.S >>> after migrating to common clock. The definitions under >>> ifdef CONFIG_COMMON_CLK will be removed in a subsequent patch. >>> davinci_watchdog_reset prototype is moved to time.h as clock.h is >>> being obsoleted. sleep.S and pm.c is modified to include the new header >>> file replacements. >>> >>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >>> --- >>> arch/arm/mach-davinci/devices.c | 2 ++ >>> arch/arm/mach-davinci/include/mach/pll.h | 46 >>> +++++++++++++++++++++++++++++ >>> arch/arm/mach-davinci/include/mach/psc.h | 4 +++ >>> arch/arm/mach-davinci/include/mach/time.h | 4 ++- >>> arch/arm/mach-davinci/pm.c | 4 +++ >>> arch/arm/mach-davinci/sleep.S | 4 +++ >>> 6 files changed, 63 insertions(+), 1 deletion(-) >>> create mode 100644 arch/arm/mach-davinci/include/mach/pll.h >> With this patch a _third_ copy of PLL definitions is created in kernel >> sources. The existing PLL definitions in clock.h inside mach-davinci >> should be moved to mach/pll.h and the pll.h you introduced inside >> drivers/clk in 5/11 should be removed (this patch should appear before >> 5/11). >> >> The biggest disadvantage of this approach is inclusion of mach/ includes >> in drivers/clk. But duplicating code is definitely not the fix for this. >> Anyway, mach/ includes are not uncommon in drivers/clk (they are all >> probably suffering from the same issue). > Sekhar, > > I have replied to patch 5/11 that also refers to this. The main reason > we are not able to do this cleanly is the code in sleep.c and pm.c. That > is something related to Power management. Could you take a look and see > if you can do some clean up on this code? I believe It is more than just > moving the header files. I am not sure what more is required than moving header files? Can you elaborate? > > Murali >> >> $ grep -rl "include <mach/" drivers/clk/* >> drivers/clk/clk-u300.c >> drivers/clk/mmp/clk-pxa168.c >> drivers/clk/mmp/clk-mmp2.c >> drivers/clk/mmp/clk-pxa910.c >> drivers/clk/mxs/clk-imx23.c >> drivers/clk/mxs/clk-imx28.c >> drivers/clk/spear/spear6xx_clock.c >> drivers/clk/spear/spear3xx_clock.c >> drivers/clk/spear/spear1340_clock.c >> drivers/clk/spear/spear1310_clock.c >> drivers/clk/ux500/clk-prcc.c >> drivers/clk/versatile/clk-integrator.c >> drivers/clk/versatile/clk-realview.c >> >> pll.h can probably be moved to include/linux/clk/ to avoid this. Would >> like to hear from Mike on this before going ahead. >> >> Anyway, instead of just commenting, I though I will be more useful and >> went ahead and made some of the changes I have been talking about. I >> fixed the multiple PLL definitions issue, the build infrastructure issue >> and the commit ordering too. >> >> I pushed the patches I fixed to devel-common-clk branch of my git tree. >> It is build tested using davinci_all_defconfig but its not runtime >> tested. >> >> Can you start from here and provide me incremental changes on top of >> this? That way we can collaborate to finish this faster. > Thanks for offering some help. Yes I can provide you incremental patch. > But then could you also help me to squash/rebase and send patches to the > list for review? No, no. I want you to own the submissions. Towards this, if it is easier for you to build upon my work in your own tree, by all means, go ahead and do it. Just share the branch/tree details so I can take a look and contribute as well. Thanks, Sekhar
On 11/6/2012 3:27 AM, Murali Karicheri wrote: > On 11/04/2012 09:05 AM, Sekhar Nori wrote: [...] >> I pushed the patches I fixed to devel-common-clk branch of my git tree. >> It is build tested using davinci_all_defconfig but its not runtime >> tested. >> >> Can you start from here and provide me incremental changes on top of >> this? That way we can collaborate to finish this faster. >> >> Thanks, >> Sekhar >> > I made a build from your branch and it doesn't boot up DM6446. I will Okay so there were some silly issues (as expected from untested work) like not even calling the DM644x common clock initializer function. I have been able to fix them and now the devel-common-clk branch on my tree boots on DM644x EVM. The work is still rough and I haven't fixed many of the comments I gave you before. You can use the updated branch to base your further work on and once you have something to share, please put it on a public repo so I can take a look. I will try and summarize all that I changed in a later mail. Wanted to get this message out to you as soon as I can. Thanks, Sekhar
diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c index d2f96662..96ee175 100644 --- a/arch/arm/mach-davinci/devices.c +++ b/arch/arm/mach-davinci/devices.c @@ -24,7 +24,9 @@ #include <mach/time.h> #include "davinci.h" +#ifndef CONFIG_COMMON_CLK #include "clock.h" +#endif #define DAVINCI_I2C_BASE 0x01C21000 #define DAVINCI_ATA_BASE 0x01C66000 diff --git a/arch/arm/mach-davinci/include/mach/pll.h b/arch/arm/mach-davinci/include/mach/pll.h new file mode 100644 index 0000000..fa51bc4 --- /dev/null +++ b/arch/arm/mach-davinci/include/mach/pll.h @@ -0,0 +1,46 @@ +/* + * TI DaVinci PLL definitions + * + * Copyright (C) 2006-2012 Texas Instruments. + * Copyright (C) 2008-2009 Deep Root Systems, LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#ifndef __ASM_ARCH_PLL_H +#define __ASM_ARCH_PLL_H + +/* PLL/Reset register offsets */ +#define PLLCTL 0x100 +#define PLLCTL_PLLEN BIT(0) +#define PLLCTL_PLLPWRDN BIT(1) +#define PLLCTL_PLLRST BIT(3) +#define PLLCTL_PLLDIS BIT(4) +#define PLLCTL_PLLENSRC BIT(5) +#define PLLCTL_CLKMODE BIT(8) +#define PLLDIV_EN BIT(15) +#define PLLDIV1 0x118 +/* + * OMAP-L138 system reference guide recommends a wait for 4 OSCIN/CLKIN + * cycles to ensure that the PLLC has switched to bypass mode. Delay of 1us + * ensures we are good for all > 4MHz OSCIN/CLKIN inputs. Typically the input + * is ~25MHz. Units are micro seconds. + */ +#define PLL_BYPASS_TIME 1 +/* From OMAP-L138 datasheet table 6-4. Units are micro seconds */ +#define PLL_RESET_TIME 1 +/* + * From OMAP-L138 datasheet table 6-4; assuming prediv = 1, sqrt(pllm) = 4 + * Units are micro seconds. + */ +#define PLL_LOCK_TIME 20 +#define PLLSTAT_GOSTAT BIT(0) +#define PLLCMD_GOSET BIT(0) + +#endif /* __ASM_ARCH_PLL_H */ diff --git a/arch/arm/mach-davinci/include/mach/psc.h b/arch/arm/mach-davinci/include/mach/psc.h index 405318e..eb464d3 100644 --- a/arch/arm/mach-davinci/include/mach/psc.h +++ b/arch/arm/mach-davinci/include/mach/psc.h @@ -27,6 +27,7 @@ #ifndef __ASM_ARCH_PSC_H #define __ASM_ARCH_PSC_H +#ifndef CONFIG_COMMON_CLK #define DAVINCI_PWR_SLEEP_CNTRL_BASE 0x01C41000 /* Power and Sleep Controller (PSC) Domains */ @@ -227,6 +228,7 @@ #define TNETV107X_LPSC_DDR2_EMIF1_VRST 42 #define TNETV107X_LPSC_DDR2_EMIF2_VCTL_RST 43 #define TNETV107X_LPSC_MAX 44 +#endif /* PSC register offsets */ #define EPCPR 0x070 @@ -251,9 +253,11 @@ #ifndef __ASSEMBLER__ +#ifndef CONFIG_COMMON_CLK extern int davinci_psc_is_clk_active(unsigned int ctlr, unsigned int id); extern void davinci_psc_config(unsigned int domain, unsigned int ctlr, unsigned int id, bool enable, u32 flags); +#endif #endif diff --git a/arch/arm/mach-davinci/include/mach/time.h b/arch/arm/mach-davinci/include/mach/time.h index 1c971d8..7faa530 100644 --- a/arch/arm/mach-davinci/include/mach/time.h +++ b/arch/arm/mach-davinci/include/mach/time.h @@ -31,5 +31,7 @@ enum { #define ID_TO_TIMER(id) (IS_TIMER1(id) != 0) extern struct davinci_timer_instance davinci_timer_instance[]; - +#ifdef CONFIG_COMMON_CLK +extern void davinci_watchdog_reset(struct platform_device *); +#endif #endif /* __ARCH_ARM_MACH_DAVINCI_TIME_H */ diff --git a/arch/arm/mach-davinci/pm.c b/arch/arm/mach-davinci/pm.c index eb8360b..8802fdc 100644 --- a/arch/arm/mach-davinci/pm.c +++ b/arch/arm/mach-davinci/pm.c @@ -23,7 +23,11 @@ #include <mach/sram.h> #include <mach/pm.h> +#ifndef CONFIG_COMMON_CLK #include "clock.h" +#else +#include <mach/pll.h> +#endif #define DEEPSLEEP_SLEEPCOUNT_MASK 0xFFFF diff --git a/arch/arm/mach-davinci/sleep.S b/arch/arm/mach-davinci/sleep.S index d4e9316..a3bd60d 100644 --- a/arch/arm/mach-davinci/sleep.S +++ b/arch/arm/mach-davinci/sleep.S @@ -24,7 +24,11 @@ #include <mach/psc.h> #include <mach/ddr2.h> +#ifndef CONFIG_COMMON_CLK #include "clock.h" +#else +#include <mach/pll.h> +#endif /* Arbitrary, hardware currently does not update PHYRDY correctly */ #define PHYRDY_CYCLES 0x1000
pll.h is added to migrate some of the PLL controller defines for sleep.S. psc.h is modified to keep only PSC modules definitions needed by sleep.S after migrating to common clock. The definitions under ifdef CONFIG_COMMON_CLK will be removed in a subsequent patch. davinci_watchdog_reset prototype is moved to time.h as clock.h is being obsoleted. sleep.S and pm.c is modified to include the new header file replacements. Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> --- arch/arm/mach-davinci/devices.c | 2 ++ arch/arm/mach-davinci/include/mach/pll.h | 46 +++++++++++++++++++++++++++++ arch/arm/mach-davinci/include/mach/psc.h | 4 +++ arch/arm/mach-davinci/include/mach/time.h | 4 ++- arch/arm/mach-davinci/pm.c | 4 +++ arch/arm/mach-davinci/sleep.S | 4 +++ 6 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-davinci/include/mach/pll.h