[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Chanwoo Choi]

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

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Rafael J. Wysocki]

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

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Russell King - ARM Linux]

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?
IOW, drivers _can_ (and should be) calling clk_disable() when they don't
need the clock running.

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Rafael J. Wysocki]

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.

Thanks,
Rafael

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Kukjin Kim]

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

Hi Chanwoo,

Basically looks ok.
I will review this in detail in the next week.
Then if no problems, will apply.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Sylwester Nawrocki]

On 08/13/2011 11:24 PM, 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

Are we going to have this patch set in 3.2 ? Does it need more work
or is it already merged in someone's -next repository ?

--
Thanks,
Sylwester

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Kukjin Kim]

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.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Rafael J. Wysocki]

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

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Russell King - ARM Linux]

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.

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Rafael J. Wysocki]

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

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Russell King - ARM Linux]

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.

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Sylwester Nawrocki]

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

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Rafael J. Wysocki]

On Sunday, October 02, 2011, Sylwester Nawrocki wrote:
> 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.

That's correct.
 
> 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 ?

Well, the idea is to use them as a driver's or subsystem's .runtime_suspend()
and .runtime_resume() callbacks, or a PM domain's .stop_device() and
.start_device() callbacks, but I don't see fundamental arguments against
calling them directly.

> 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.

Yes, it is gone now.

> 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.  

That sounds like a good idea.

Thanks,
Rafael

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Rafael J. Wysocki]

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.

Thanks,
Rafael

[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Kukjin Kim]

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.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.