Re: [PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
Hello Rafael, On 08/11/2011 06:55 AM, Chanwoo Choi wrote: The following patch set use the generic Power domain Framework instead of power domain code depend of Samsung SoC. Chanwoo Choi (4): ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210 ARM: EXYNOS4: Support for generic Clock manipulation PM callbacks ARM: EXYNOS4: Delete the power-domain code depend on Samsung SoC ARM: EXYNOS4: Add power domain to use generic Power domain Framework arch/arm/mach-exynos4/Kconfig | 10 +- arch/arm/mach-exynos4/Makefile |4 +- arch/arm/mach-exynos4/dev-pd.c | 139 -- arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 52 ++ arch/arm/mach-exynos4/include/mach/regs-clock.h|8 + arch/arm/mach-exynos4/mach-nuri.c | 21 ++- arch/arm/mach-exynos4/mach-smdkc210.c | 26 ++- arch/arm/mach-exynos4/mach-smdkv310.c | 23 ++- arch/arm/mach-exynos4/mach-universal_c210.c| 21 ++- arch/arm/mach-exynos4/pm-exynos4210.c | 189 arch/arm/mach-exynos4/pm-runtime.c | 56 ++ arch/arm/plat-samsung/Kconfig |8 - arch/arm/plat-samsung/Makefile |4 - arch/arm/plat-samsung/include/plat/pd.h| 30 --- arch/arm/plat-samsung/pd.c | 95 -- 15 files changed, 377 insertions(+), 309 deletions(-) delete mode 100644 arch/arm/mach-exynos4/dev-pd.c create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c create mode 100644 arch/arm/mach-exynos4/pm-runtime.c delete mode 100644 arch/arm/plat-samsung/include/plat/pd.h delete mode 100644 arch/arm/plat-samsung/pd.c I'm having some trouble after applying these patches, with s5p_device_fimc0..3 devices added to exynos4210_pd_cam power domain. When pm_runtime_get_sync() is called in s5p_device_fimc0 device driver probe() the driver's runtime_resume helper isn't called. In __pm_genpd_restore_device() dle-need_restore is 0 and thus the function exits, not calling drv-pm-runtime_resume(). I would expect the driver's runtime_resume helper to be invoked since the device is initially in suspended state. In rpm_resume() if dev-pm_domain is not null its ops.runtime_resume will get called, and nothing else. I observed that after fimc0 device is probed, it gets suspended and then all other devices are attempted to be suspended as well, through __pm_genpd_save_device(). But at that time only fimc0 device has non null dev-driver, and its driver runtime_suspend helper is called (without prior runtime_resume call). Remaining devices in the power domain (fimc1..3) only get 'need_restore' set to 1. So at the time the fimc1 device probe() is invoked, its corresponding need_restore is set to 1, and driver's runtime_resume helper is called as expected. Then the runtime_suspend follows. Is this intended behaviour ? I'd like to avoid any reference counters for runtime_suspend/resume helpers. As for now the clock is controlled there and the driver need to ensure balanced calls to clk_enable()/clk_disable(). Thanks, -- Sylwester Nawrocki Samsung Poland RD Center -- 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 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
Rafael J. Wysocki wrote: On Saturday, August 13, 2011, Russell King - ARM Linux wrote: On Sat, Aug 13, 2011 at 11:24:07PM +0200, Rafael J. Wysocki wrote: On Thursday, August 11, 2011, Chanwoo Choi wrote: The following patch set use the generic Power domain Framework instead of power domain code depend of Samsung SoC. Chanwoo Choi (4): ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210 ARM: EXYNOS4: Support for generic Clock manipulation PM callbacks ARM: EXYNOS4: Delete the power-domain code depend on Samsung SoC ARM: EXYNOS4: Add power domain to use generic Power domain Framework arch/arm/mach-exynos4/Kconfig | 10 +- arch/arm/mach-exynos4/Makefile |4 +- arch/arm/mach-exynos4/dev-pd.c | 139 -- arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 52 ++ arch/arm/mach-exynos4/include/mach/regs-clock.h|8 + arch/arm/mach-exynos4/mach-nuri.c | 21 ++- arch/arm/mach-exynos4/mach-smdkc210.c | 26 ++- arch/arm/mach-exynos4/mach-smdkv310.c | 23 ++- arch/arm/mach-exynos4/mach-universal_c210.c| 21 ++- arch/arm/mach-exynos4/pm-exynos4210.c | 189 arch/arm/mach-exynos4/pm-runtime.c | 56 ++ arch/arm/plat-samsung/Kconfig |8 - arch/arm/plat-samsung/Makefile |4 - arch/arm/plat-samsung/include/plat/pd.h| 30 --- arch/arm/plat-samsung/pd.c | 95 -- 15 files changed, 377 insertions(+), 309 deletions(-) delete mode 100644 arch/arm/mach-exynos4/dev-pd.c create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c create mode 100644 arch/arm/mach-exynos4/pm-runtime.c delete mode 100644 arch/arm/plat-samsung/include/plat/pd.h delete mode 100644 arch/arm/plat-samsung/pd.c The patchset looks good to me, but please note that some code it is based on will most likely change in 3.2 due to this patchset: https://lkml.org/lkml/2011/8/8/420 Err, isn't all that pm_clk stuff just duplicating what the clk API does? I'm not sure it's duplicating anything. Maybe it does, but it came into being by moving some code that were duplicated in a few places throughout the ARM and sh trees into one place. IOW, drivers _can_ (and should be) calling clk_disable() when they don't need the clock running. Drivers may not know about what to do in a given situation. For example, if the system has power domains, it may be better to switch a power domain off instead of or in addition to disabling the clock and the driver usually doesn't know about that. Hmm... Even though each driver cannot know the given situation, the driver can know each own clock should be alive or not. I think, if clock gating (enable, disable clock) is required, it should be handled in each driver. In addition, the clock and power are not always one-on-one match. Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- 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 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
On Sunday, October 02, 2011, Kukjin Kim wrote: Rafael J. Wysocki wrote: On Saturday, August 13, 2011, Russell King - ARM Linux wrote: On Sat, Aug 13, 2011 at 11:24:07PM +0200, Rafael J. Wysocki wrote: On Thursday, August 11, 2011, Chanwoo Choi wrote: The following patch set use the generic Power domain Framework instead of power domain code depend of Samsung SoC. Chanwoo Choi (4): ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210 ARM: EXYNOS4: Support for generic Clock manipulation PM callbacks ARM: EXYNOS4: Delete the power-domain code depend on Samsung SoC ARM: EXYNOS4: Add power domain to use generic Power domain Framework arch/arm/mach-exynos4/Kconfig | 10 +- arch/arm/mach-exynos4/Makefile |4 +- arch/arm/mach-exynos4/dev-pd.c | 139 -- arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 52 ++ arch/arm/mach-exynos4/include/mach/regs-clock.h|8 + arch/arm/mach-exynos4/mach-nuri.c | 21 ++- arch/arm/mach-exynos4/mach-smdkc210.c | 26 ++- arch/arm/mach-exynos4/mach-smdkv310.c | 23 ++- arch/arm/mach-exynos4/mach-universal_c210.c| 21 ++- arch/arm/mach-exynos4/pm-exynos4210.c | 189 arch/arm/mach-exynos4/pm-runtime.c | 56 ++ arch/arm/plat-samsung/Kconfig |8 - arch/arm/plat-samsung/Makefile |4 - arch/arm/plat-samsung/include/plat/pd.h| 30 --- arch/arm/plat-samsung/pd.c | 95 -- 15 files changed, 377 insertions(+), 309 deletions(-) delete mode 100644 arch/arm/mach-exynos4/dev-pd.c create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c create mode 100644 arch/arm/mach-exynos4/pm-runtime.c delete mode 100644 arch/arm/plat-samsung/include/plat/pd.h delete mode 100644 arch/arm/plat-samsung/pd.c The patchset looks good to me, but please note that some code it is based on will most likely change in 3.2 due to this patchset: https://lkml.org/lkml/2011/8/8/420 Err, isn't all that pm_clk stuff just duplicating what the clk API does? I'm not sure it's duplicating anything. Maybe it does, but it came into being by moving some code that were duplicated in a few places throughout the ARM and sh trees into one place. IOW, drivers _can_ (and should be) calling clk_disable() when they don't need the clock running. Drivers may not know about what to do in a given situation. For example, if the system has power domains, it may be better to switch a power domain off instead of or in addition to disabling the clock and the driver usually doesn't know about that. Hmm... Even though each driver cannot know the given situation, the driver can know each own clock should be alive or not. I think, if clock gating (enable, disable clock) is required, it should be handled in each driver. In addition, the clock and power are not always one-on-one match. If the driver is to be portable and there's no guarantee that the clocks configuration on all systems it's supposed to be working with will always be the same, the driver shouldn't handle clocks directly. Thanks, Rafael -- 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 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
On Sun, Oct 02, 2011 at 01:47:01PM +0200, Rafael J. Wysocki wrote: On Sunday, October 02, 2011, Kukjin Kim wrote: Hmm... Even though each driver cannot know the given situation, the driver can know each own clock should be alive or not. I think, if clock gating (enable, disable clock) is required, it should be handled in each driver. In addition, the clock and power are not always one-on-one match. If the driver is to be portable and there's no guarantee that the clocks configuration on all systems it's supposed to be working with will always be the same, the driver shouldn't handle clocks directly. How do these misconceptions start? The clock API. Drivers are supposed to get a clock (source) when they initialize. Drivers then enable and disable the clock as they _themselves_ require the use of that clock. The clock enable is counted such that if there is at least one user of the clock, it will be enabled. It is not a forced 'off' when disable is called - the number of enable calls must be balanced by the same number of disable calls for the clock itself to be disabled. Drivers are _expected_ to do this themselves. Or the runtime PM stuff if that's what they're doing. -- 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 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
On Sunday, October 02, 2011, Russell King - ARM Linux wrote: On Sun, Oct 02, 2011 at 01:47:01PM +0200, Rafael J. Wysocki wrote: On Sunday, October 02, 2011, Kukjin Kim wrote: Hmm... Even though each driver cannot know the given situation, the driver can know each own clock should be alive or not. I think, if clock gating (enable, disable clock) is required, it should be handled in each driver. In addition, the clock and power are not always one-on-one match. If the driver is to be portable and there's no guarantee that the clocks configuration on all systems it's supposed to be working with will always be the same, the driver shouldn't handle clocks directly. How do these misconceptions start? The clock API. Drivers are supposed to get a clock (source) when they initialize. Drivers then enable and disable the clock as they _themselves_ require the use of that clock. OK Now think of a driver that should handle the same device on both ARM and x86-based SoCs. Is the clock API available on x86? The clock enable is counted such that if there is at least one user of the clock, it will be enabled. It is not a forced 'off' when disable is called - the number of enable calls must be balanced by the same number of disable calls for the clock itself to be disabled. Drivers are _expected_ to do this themselves. Or the runtime PM stuff if that's what they're doing. That I can agree with. Thanks, Rafael -- 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 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
On Sun, Oct 02, 2011 at 03:08:43PM +0200, Rafael J. Wysocki wrote: On Sunday, October 02, 2011, Russell King - ARM Linux wrote: How do these misconceptions start? The clock API. Drivers are supposed to get a clock (source) when they initialize. Drivers then enable and disable the clock as they _themselves_ require the use of that clock. OK Now think of a driver that should handle the same device on both ARM and x86-based SoCs. Is the clock API available on x86? No one's bothered yet. -- 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 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
On 10/02/2011 09:07 AM, Kukjin Kim wrote: Rafael J. Wysocki wrote: On Saturday, August 13, 2011, Russell King - ARM Linux wrote: On Sat, Aug 13, 2011 at 11:24:07PM +0200, Rafael J. Wysocki wrote: On Thursday, August 11, 2011, Chanwoo Choi wrote: The following patch set use the generic Power domain Framework instead of power domain code depend of Samsung SoC. Chanwoo Choi (4): ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210 ARM: EXYNOS4: Support for generic Clock manipulation PM callbacks ARM: EXYNOS4: Delete the power-domain code depend on Samsung SoC ARM: EXYNOS4: Add power domain to use generic Power domain Framework arch/arm/mach-exynos4/Kconfig | 10 +- arch/arm/mach-exynos4/Makefile |4 +- arch/arm/mach-exynos4/dev-pd.c | 139 -- arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 52 ++ arch/arm/mach-exynos4/include/mach/regs-clock.h|8 + arch/arm/mach-exynos4/mach-nuri.c | 21 ++- arch/arm/mach-exynos4/mach-smdkc210.c | 26 ++- arch/arm/mach-exynos4/mach-smdkv310.c | 23 ++- arch/arm/mach-exynos4/mach-universal_c210.c| 21 ++- arch/arm/mach-exynos4/pm-exynos4210.c | 189 arch/arm/mach-exynos4/pm-runtime.c | 56 ++ arch/arm/plat-samsung/Kconfig |8 - arch/arm/plat-samsung/Makefile |4 - arch/arm/plat-samsung/include/plat/pd.h| 30 --- arch/arm/plat-samsung/pd.c | 95 -- 15 files changed, 377 insertions(+), 309 deletions(-) delete mode 100644 arch/arm/mach-exynos4/dev-pd.c create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c create mode 100644 arch/arm/mach-exynos4/pm-runtime.c delete mode 100644 arch/arm/plat-samsung/include/plat/pd.h delete mode 100644 arch/arm/plat-samsung/pd.c The patchset looks good to me, but please note that some code it is based on will most likely change in 3.2 due to this patchset: https://lkml.org/lkml/2011/8/8/420 Err, isn't all that pm_clk stuff just duplicating what the clk API does? I'm not sure it's duplicating anything. Maybe it does, but it came into being by moving some code that were duplicated in a few places throughout the ARM and sh trees into one place. IOW, drivers _can_ (and should be) calling clk_disable() when they don't need the clock running. Drivers may not know about what to do in a given situation. For example, if the system has power domains, it may be better to switch a power domain off instead of or in addition to disabling the clock and the driver usually doesn't know about that. Hmm... Even though each driver cannot know the given situation, the driver can know each own clock should be alive or not. I think, if clock gating (enable, disable clock) is required, it should be handled in each driver. In addition, the clock and power are not always one-on-one match. AFAICS the genpd API doesn't for an architecture to move the clocks' control out from drivers, it can be decided on individual basis for each clock. With Chanwoo's patches at least exynos power domains and global clock gates for devices (S5P_CLKGATE_BLOCK) are handled through the common API. Also, it's not quite clear to me, can't the pm_clk_* calls be used by the drivers to control the clocks as they need ? Last time I looked at the genpd API, an important drawback was that pm_clk_suspend/ pm_clk_resume couldn't be used in interrupt context. Now it seems the mutexes in the core functions has been replaced with spinlocks and such limitation is gone. I'm considering using pm_clk* calls to implement clock gating in a video codec driver. If, for instance, frame period is 30 ms and processing in the device takes only 10 ms, significant power savings could be achieved by turning the clocks off for the 20 ms idle period. -- Thanks, Sylwester -- 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 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
Rafael J. Wysocki wrote: On Sunday, October 02, 2011, Russell King - ARM Linux wrote: On Sun, Oct 02, 2011 at 03:08:43PM +0200, Rafael J. Wysocki wrote: On Sunday, October 02, 2011, Russell King - ARM Linux wrote: How do these misconceptions start? The clock API. Drivers are supposed to get a clock (source) when they initialize. Drivers then enable and disable the clock as they _themselves_ require the use of that clock. OK Now think of a driver that should handle the same device on both ARM and x86-based SoCs. Is the clock API available on x86? No one's bothered yet. Prehaps because x86 doesn't allow us to control device clocks directly. They are controlled through PCI PM or through ACPI methods, both of which hide the clocks behind abstract low-power states we're supposed to use. Well, I think Embedded drivers especially ARM sometimes need to control its own clock directly and each embedded drivers know when clock controlling is required or not. Actually, Samsung driver stuff have calling clk_enable() in probe() or open() and clk_disable() when clock is not needed more. But I'm not sure we _really_ can get some benefits when matching clock and power control on ARM devices. Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- 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 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
On Thursday, August 11, 2011, Chanwoo Choi wrote: The following patch set use the generic Power domain Framework instead of power domain code depend of Samsung SoC. Chanwoo Choi (4): ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210 ARM: EXYNOS4: Support for generic Clock manipulation PM callbacks ARM: EXYNOS4: Delete the power-domain code depend on Samsung SoC ARM: EXYNOS4: Add power domain to use generic Power domain Framework arch/arm/mach-exynos4/Kconfig | 10 +- arch/arm/mach-exynos4/Makefile |4 +- arch/arm/mach-exynos4/dev-pd.c | 139 -- arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 52 ++ arch/arm/mach-exynos4/include/mach/regs-clock.h|8 + arch/arm/mach-exynos4/mach-nuri.c | 21 ++- arch/arm/mach-exynos4/mach-smdkc210.c | 26 ++- arch/arm/mach-exynos4/mach-smdkv310.c | 23 ++- arch/arm/mach-exynos4/mach-universal_c210.c| 21 ++- arch/arm/mach-exynos4/pm-exynos4210.c | 189 arch/arm/mach-exynos4/pm-runtime.c | 56 ++ arch/arm/plat-samsung/Kconfig |8 - arch/arm/plat-samsung/Makefile |4 - arch/arm/plat-samsung/include/plat/pd.h| 30 --- arch/arm/plat-samsung/pd.c | 95 -- 15 files changed, 377 insertions(+), 309 deletions(-) delete mode 100644 arch/arm/mach-exynos4/dev-pd.c create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c create mode 100644 arch/arm/mach-exynos4/pm-runtime.c delete mode 100644 arch/arm/plat-samsung/include/plat/pd.h delete mode 100644 arch/arm/plat-samsung/pd.c The patchset looks good to me, but please note that some code it is based on will most likely change in 3.2 due to this patchset: https://lkml.org/lkml/2011/8/8/420 Thanks, Rafael -- 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
[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210
The following patch set use the generic Power domain Framework instead of power domain code depend of Samsung SoC. Chanwoo Choi (4): ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210 ARM: EXYNOS4: Support for generic Clock manipulation PM callbacks ARM: EXYNOS4: Delete the power-domain code depend on Samsung SoC ARM: EXYNOS4: Add power domain to use generic Power domain Framework arch/arm/mach-exynos4/Kconfig | 10 +- arch/arm/mach-exynos4/Makefile |4 +- arch/arm/mach-exynos4/dev-pd.c | 139 -- arch/arm/mach-exynos4/include/mach/pm-exynos4210.h | 52 ++ arch/arm/mach-exynos4/include/mach/regs-clock.h|8 + arch/arm/mach-exynos4/mach-nuri.c | 21 ++- arch/arm/mach-exynos4/mach-smdkc210.c | 26 ++- arch/arm/mach-exynos4/mach-smdkv310.c | 23 ++- arch/arm/mach-exynos4/mach-universal_c210.c| 21 ++- arch/arm/mach-exynos4/pm-exynos4210.c | 189 arch/arm/mach-exynos4/pm-runtime.c | 56 ++ arch/arm/plat-samsung/Kconfig |8 - arch/arm/plat-samsung/Makefile |4 - arch/arm/plat-samsung/include/plat/pd.h| 30 --- arch/arm/plat-samsung/pd.c | 95 -- 15 files changed, 377 insertions(+), 309 deletions(-) delete mode 100644 arch/arm/mach-exynos4/dev-pd.c create mode 100644 arch/arm/mach-exynos4/include/mach/pm-exynos4210.h create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c create mode 100644 arch/arm/mach-exynos4/pm-runtime.c delete mode 100644 arch/arm/plat-samsung/include/plat/pd.h delete mode 100644 arch/arm/plat-samsung/pd.c -- 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
