[PATCH 00/12] soc: samsung: Modify and enhance power domain driver

[PATCH 00/12] soc: samsung: Modify and enhance power domain driver

[Amit Daniel Kachhap]

This patch series[3 - 12] performs several implementations as described below,

1) Converts power domain driver to platform driver.
2) Registers this driver as MFD client driver.
3) Moves them into driver/soc/samsung folder.
4) Add new power domain notifier feature.
5) Uses those notfier from exynos clk.
6) Adds parent power domain and some other features mostly for exynos7 platforms.

This patch will change the DT representation of all the exynos power domain
DT nodes which will be posted as a new patch series.

This patch has dependency on following patches posted earlier and most of them are
already accepted or in final stages of review.
1) http://www.spinics.net/lists/linux-samsung-soc/msg37050.html
2) https://lkml.org/lkml/2014/10/6/581

Patches 1-2 are from Pankaj and have been posted earlier.
(http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/271555.html
They are kept in this patch series for completeness after rebase fixes.

This complete patch series is rebased on linux-next as it involves many
subsystems.

Amit Daniel Kachhap (10):
  PM / Domains: Add notifier support for power domain transitions
  mfd: exynos-pmu: Register exynos-pmu driver as a mfd driver
  arm: exynos: Add platform driver support for power domain driver
  soc: exynos: Move exynos power domain file to driver/soc/samsung
    folder
  soc: samsung: pm_domain: Use compatible name for power domain name
  soc: samsung: pm_domain: Add a new parameter for power configuration
  soc: samsung: pm_domain: Add support for parent power domain
  soc: samsung: pm_domain: Use the recently added PM Domain notifiers
  clk: samsung: save and restore clock registers for power domain
  arm64: Kconfig: Enable PM_GENERIC_DOMAINS for exynos7

Pankaj Dubey (2):
  ARM: EXYNOS: Move pmu specific header files under "linux/mfd/samsung"
  drivers: mfd: Add support for Exynos PMU driver

 .../bindings/arm/exynos/power_domain.txt           |   21 +++-
 arch/arm/mach-exynos/Kconfig                       |    1 +
 arch/arm/mach-exynos/Makefile                      |    3 +-
 arch/arm/mach-exynos/exynos.c                      |    2 +-
 arch/arm/mach-exynos/mcpm-exynos.c                 |    2 +-
 arch/arm/mach-exynos/platsmp.c                     |    2 +-
 arch/arm/mach-exynos/pm.c                          |    5 +-
 arch/arm/mach-exynos/suspend.c                     |    3 +-
 arch/arm64/Kconfig                                 |    1 +
 drivers/base/power/domain.c                        |  112 +++++++++++++++++-
 drivers/clk/samsung/clk.c                          |   66 +++++++++++
 drivers/clk/samsung/clk.h                          |   14 +++
 drivers/mfd/Kconfig                                |    9 ++
 drivers/mfd/Makefile                               |    1 +
 .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c  |   30 ++++-
 drivers/soc/Kconfig                                |    1 +
 drivers/soc/Makefile                               |    1 +
 drivers/soc/samsung/Kconfig                        |   11 ++
 drivers/soc/samsung/Makefile                       |    1 +
 .../soc/samsung}/pm_domains.c                      |  124 ++++++++++++++++----
 .../linux/mfd/samsung}/exynos-pmu.h                |    5 +
 .../linux/mfd/samsung/exynos-regs-pmu.h            |    0
 include/linux/pm_domain.h                          |   31 +++++
 23 files changed, 409 insertions(+), 37 deletions(-)
 rename arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (96%)
 create mode 100644 drivers/soc/samsung/Kconfig
 create mode 100644 drivers/soc/samsung/Makefile
 rename {arch/arm/mach-exynos => drivers/soc/samsung}/pm_domains.c (51%)
 rename {arch/arm/mach-exynos => include/linux/mfd/samsung}/exynos-pmu.h (86%)
 rename arch/arm/mach-exynos/regs-pmu.h => include/linux/mfd/samsung/exynos-regs-pmu.h (100%)

-- 
1.7.9.5

[PATCH 01/12] ARM: EXYNOS: Move pmu specific header files under "linux/mfd/samsung"

[Amit Daniel Kachhap]

From: Pankaj Dubey <pankaj.dubey@samsung.com>

Moving Exynos PMU specific header file into "include/linux/mfd/samsung"
thus updated affected files under "mach-exynos" to use new location of
these header files.

CC: Sangbeom Kim <sbkim73@samsung.com>
CC: Samuel Ortiz <sameo@linux.intel.com>
CC: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/exynos.c                      |    2 +-
 arch/arm/mach-exynos/mcpm-exynos.c                 |    2 +-
 arch/arm/mach-exynos/platsmp.c                     |    2 +-
 arch/arm/mach-exynos/pm.c                          |    5 +++--
 arch/arm/mach-exynos/pmu.c                         |    5 ++---
 arch/arm/mach-exynos/suspend.c                     |    3 ++-
 .../linux/mfd/samsung}/exynos-pmu.h                |    0
 .../linux/mfd/samsung/exynos-regs-pmu.h            |    0
 8 files changed, 10 insertions(+), 9 deletions(-)
 rename {arch/arm/mach-exynos => include/linux/mfd/samsung}/exynos-pmu.h (100%)
 rename arch/arm/mach-exynos/regs-pmu.h => include/linux/mfd/samsung/exynos-regs-pmu.h (100%)

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index a487e59..f6b8cb1 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -20,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/irqchip.h>
+#include <linux/mfd/samsung/exynos-regs-pmu.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -29,7 +30,6 @@
 
 #include "common.h"
 #include "mfc.h"
-#include "regs-pmu.h"
 #include "regs-sys.h"
 
 void __iomem *pmu_base_addr;
diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
index dc9a764..5326b67 100644
--- a/arch/arm/mach-exynos/mcpm-exynos.c
+++ b/arch/arm/mach-exynos/mcpm-exynos.c
@@ -15,12 +15,12 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/of_address.h>
+#include <linux/mfd/samsung/exynos-regs-pmu.h>
 
 #include <asm/cputype.h>
 #include <asm/cp15.h>
 #include <asm/mcpm.h>
 
-#include "regs-pmu.h"
 #include "common.h"
 
 #define EXYNOS5420_CPUS_PER_CLUSTER	4
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index adb36a8..16d6acd 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -20,6 +20,7 @@
 #include <linux/smp.h>
 #include <linux/io.h>
 #include <linux/of_address.h>
+#include <linux/mfd/samsung/exynos-regs-pmu.h>
 
 #include <asm/cacheflush.h>
 #include <asm/cp15.h>
@@ -30,7 +31,6 @@
 #include <mach/map.h>
 
 #include "common.h"
-#include "regs-pmu.h"
 
 extern void exynos4_secondary_startup(void);
 
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 86f3ecd..8e0ffc3 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -18,6 +18,9 @@
 #include <linux/cpu_pm.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/mfd/samsung/exynos-regs-pmu.h>
+#include <linux/mfd/samsung/exynos-pmu.h>
 
 #include <asm/firmware.h>
 #include <asm/smp_scu.h>
@@ -26,8 +29,6 @@
 #include <plat/pm-common.h>
 
 #include "common.h"
-#include "exynos-pmu.h"
-#include "regs-pmu.h"
 #include "regs-sys.h"
 
 static inline void __iomem *exynos_boot_vector_addr(void)
diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index 1993e08..07019f0 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -12,9 +12,8 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
-
-#include "exynos-pmu.h"
-#include "regs-pmu.h"
+#include <linux/mfd/samsung/exynos-regs-pmu.h>
+#include <linux/mfd/samsung/exynos-pmu.h>
 
 #define PMU_TABLE_END	(-1U)
 
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index f5d9773..df979f6 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -20,6 +20,8 @@
 #include <linux/io.h>
 #include <linux/irqchip/arm-gic.h>
 #include <linux/err.h>
+#include <linux/mfd/samsung/exynos-pmu.h>
+#include <linux/mfd/samsung/exynos-regs-pmu.h>
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -31,7 +33,6 @@
 #include <plat/regs-srom.h>
 
 #include "common.h"
-#include "regs-pmu.h"
 #include "regs-sys.h"
 
 #define S5P_CHECK_SLEEP 0x00000BAD
diff --git a/arch/arm/mach-exynos/exynos-pmu.h b/include/linux/mfd/samsung/exynos-pmu.h
similarity index 100%
rename from arch/arm/mach-exynos/exynos-pmu.h
rename to include/linux/mfd/samsung/exynos-pmu.h
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/include/linux/mfd/samsung/exynos-regs-pmu.h
similarity index 100%
rename from arch/arm/mach-exynos/regs-pmu.h
rename to include/linux/mfd/samsung/exynos-regs-pmu.h
-- 
1.7.9.5

[PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

[Amit Daniel Kachhap]

From: Pankaj Dubey <pankaj.dubey@samsung.com>

This patch moves Exynos PMU driver implementation from "arm/mach-exynos"
to "drivers/mfd". This driver is mainly used for setting misc bits of
register from PMU IP of Exynos SoC which will be required to configure
before Suspend/Resume. Currently all these settings are done in
"arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC support,
there is a need of DT based implementation of PMU driver.

This driver uses already existing DT binding information.

CC: Sangbeom Kim <sbkim73@samsung.com>
CC: Samuel Ortiz <sameo@linux.intel.com>
CC: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/Kconfig                       |    1 +
 arch/arm/mach-exynos/Makefile                      |    2 +-
 drivers/mfd/Kconfig                                |    9 +++++++++
 drivers/mfd/Makefile                               |    1 +
 .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c  |    0
 5 files changed, 12 insertions(+), 1 deletion(-)
 rename arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%)

diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 96d56ab..c2aef3f 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -25,6 +25,7 @@ menuconfig ARCH_EXYNOS
 	select S5P_DEV_MFC
 	select SRAM
 	select MFD_SYSCON
+	select MFD_EXYNOS_PMU
 	help
 	  Support for SAMSUNG EXYNOS SoCs (EXYNOS4/5)
 
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index bcefb54..b91b382 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -9,7 +9,7 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) += -I$(srctree)/$(src)/include -I$(srctree)
 
 # Core
 
-obj-$(CONFIG_ARCH_EXYNOS)	+= exynos.o pmu.o exynos-smc.o firmware.o
+obj-$(CONFIG_ARCH_EXYNOS)	+= exynos.o exynos-smc.o firmware.o
 
 obj-$(CONFIG_EXYNOS_CPU_SUSPEND) += pm.o sleep.o
 obj-$(CONFIG_PM_SLEEP)		+= suspend.o
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index cbdb109..619a9ba 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1323,6 +1323,15 @@ config MFD_STW481X
 	  in various ST Microelectronics and ST-Ericsson embedded
 	  Nomadik series.
 
+config MFD_EXYNOS_PMU
+	tristate "Support Exynos Power Management Unit"
+	depends on ARM || ARM64
+	help
+	  Exynos SoC have Power Management Unit (PMU) which controls power and
+	  operation state of Exynos SoC in two different ways. This driver
+	  provides implementation of PMU driver and provides basic functionality
+	  required during these operation state.
+
 menu "Multimedia Capabilities Port drivers"
 	depends on ARCH_SA1100
 
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8e679d6..33489b4 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -175,6 +175,7 @@ obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
 obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
 obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
 obj-$(CONFIG_MFD_HI6421_PMIC)	+= hi6421-pmic-core.o
+obj-$(CONFIG_MFD_EXYNOS_PMU)	+= exynos-pmu.o
 
 intel-soc-pmic-objs		:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
diff --git a/arch/arm/mach-exynos/pmu.c b/drivers/mfd/exynos-pmu.c
similarity index 100%
rename from arch/arm/mach-exynos/pmu.c
rename to drivers/mfd/exynos-pmu.c
-- 
1.7.9.5

Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

[Lee Jones]

On Mon, 03 Nov 2014, Amit Daniel Kachhap wrote:

> From: Pankaj Dubey <pankaj.dubey@samsung.com>
> 
> This patch moves Exynos PMU driver implementation from "arm/mach-exynos"
> to "drivers/mfd". This driver is mainly used for setting misc bits of
> register from PMU IP of Exynos SoC which will be required to configure
> before Suspend/Resume. Currently all these settings are done in
> "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC support,
> there is a need of DT based implementation of PMU driver.
> 
> This driver uses already existing DT binding information.
> 
> CC: Sangbeom Kim <sbkim73@samsung.com>
> CC: Samuel Ortiz <sameo@linux.intel.com>
> CC: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  arch/arm/mach-exynos/Kconfig                       |    1 +
>  arch/arm/mach-exynos/Makefile                      |    2 +-
>  drivers/mfd/Kconfig                                |    9 +++++++++
>  drivers/mfd/Makefile                               |    1 +
>  .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c  |    0
>  5 files changed, 12 insertions(+), 1 deletion(-)
>  rename arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%)

I thought we spoke about this already [1]?  What's changed?

[1] https://lkml.org/lkml/2014/4/29/193

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

RE: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

[Pankaj Dubey]

Hi Lee,

On Monday, November 03, 2014 8:56 PM, Lee Jones wrote,
> Subject: Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver
> 
> On Mon, 03 Nov 2014, Amit Daniel Kachhap wrote:
> 
> > From: Pankaj Dubey <pankaj.dubey@samsung.com>
> >
> > This patch moves Exynos PMU driver implementation from "arm/mach-exynos"
> > to "drivers/mfd". This driver is mainly used for setting misc bits of
> > register from PMU IP of Exynos SoC which will be required to configure
> > before Suspend/Resume. Currently all these settings are done in
> > "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC
> > support, there is a need of DT based implementation of PMU driver.
> >
> > This driver uses already existing DT binding information.
> >
> > CC: Sangbeom Kim <sbkim73@samsung.com>
> > CC: Samuel Ortiz <sameo@linux.intel.com>
> > CC: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > ---
> >  arch/arm/mach-exynos/Kconfig                       |    1 +
> >  arch/arm/mach-exynos/Makefile                      |    2 +-
> >  drivers/mfd/Kconfig                                |    9 +++++++++
> >  drivers/mfd/Makefile                               |    1 +
> >  .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c  |    0
> >  5 files changed, 12 insertions(+), 1 deletion(-)  rename
> > arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%)
> 
> I thought we spoke about this already [1]?  What's changed?
> 
> [1] https://lkml.org/lkml/2014/4/29/193
> 

Yes correct, we had discussion about this in above post. But at that time I was not having enough points to
put forward for keeping Exynos PMU driver in MFD folder.
At that point there was only one idea behind keeping Exynos PMU that in future it will be reused for ARM64
based SoC for PM related functionalities.
But in this series we have implemented Exynos Power Domain for Samsung's ARM64 based SoCs.

As far as I can see this will in following ways -

1: Moving out PM domain specific code from mach-exynos and reuse of same code for ARM64 based SoC also.
2: Since PM domain relies on PMU registers and does not have its own DT binding, MFD client and MFD device
is most suitable for making this kind of platform drivers.

> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software
> for ARM SoCs Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

[Lee Jones]

On Tue, 04 Nov 2014, Pankaj Dubey wrote:
> On Monday, November 03, 2014 8:56 PM, Lee Jones wrote,
> > Subject: Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver
> > 
> > On Mon, 03 Nov 2014, Amit Daniel Kachhap wrote:
> > 
> > > From: Pankaj Dubey <pankaj.dubey@samsung.com>
> > >
> > > This patch moves Exynos PMU driver implementation from "arm/mach-exynos"
> > > to "drivers/mfd". This driver is mainly used for setting misc bits of
> > > register from PMU IP of Exynos SoC which will be required to configure
> > > before Suspend/Resume. Currently all these settings are done in
> > > "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC
> > > support, there is a need of DT based implementation of PMU driver.
> > >
> > > This driver uses already existing DT binding information.
> > >
> > > CC: Sangbeom Kim <sbkim73@samsung.com>
> > > CC: Samuel Ortiz <sameo@linux.intel.com>
> > > CC: Lee Jones <lee.jones@linaro.org>
> > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> > > Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > > ---
> > >  arch/arm/mach-exynos/Kconfig                       |    1 +
> > >  arch/arm/mach-exynos/Makefile                      |    2 +-
> > >  drivers/mfd/Kconfig                                |    9 +++++++++
> > >  drivers/mfd/Makefile                               |    1 +
> > >  .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c  |    0
> > >  5 files changed, 12 insertions(+), 1 deletion(-)  rename
> > > arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%)
> > 
> > I thought we spoke about this already [1]?  What's changed?
> > 
> > [1] https://lkml.org/lkml/2014/4/29/193
> > 
> 
> Yes correct, we had discussion about this in above post. But at that time I was not having enough points to
> put forward for keeping Exynos PMU driver in MFD folder.

There are no valid points for keeping PMU code in MFD.

> At that point there was only one idea behind keeping Exynos PMU that in future it will be reused for ARM64
> based SoC for PM related functionalities.
> But in this series we have implemented Exynos Power Domain for Samsung's ARM64 based SoCs.
> 
> As far as I can see this will in following ways -
> 
> 1: Moving out PM domain specific code from mach-exynos and reuse of same code for ARM64 based SoC also.
> 2: Since PM domain relies on PMU registers and does not have its own DT binding, MFD client and MFD device
> is most suitable for making this kind of platform drivers.

MFD can be used to register the device, but the driver must not live
in MFD.  Please relocate it into drivers/power.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

[Sylwester Nawrocki]

On 04/11/14 04:18, Pankaj Dubey wrote:
> 2: Since PM domain relies on PMU registers and does not have 
> its own DT binding, MFD client and MFD device
> is most suitable for making this kind of platform drivers.

We have DT binding for the Exynos power domains:
Documentation/devicetree/bindings/arm/exynos/power_domain.txt
In fact the IO memory regions used in the power domain bindings
fall within the (sparse) PMU registers region.

There is also DT binding for the PMU subsystem:
Documentation/devicetree/bindings/arm/samsung/pmu.txt

I guess a platform device driver in drivers/power would be a better
fit than drivers/mfd. I doubt introducing an mfd driver now makes
much sense, we have already DT bindings for PHYs, the power domains,
etc.

--
Thanks,
Sylwester

[PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Amit Daniel Kachhap]

These power domain transition notifiers will assist in carrying
out some activity associated with domain power on/off such as
some registers which may lose its contents and need save/restore
across domain power off/on.

4 type of notifications are added,
GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
			enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd name.
The actual invoking of the notfiers will be done by the platform implementing
power domain enable/disable low level handlers according to the above
defined notification types. This approach will considerably reduce the
number of call to notifiers as compared to calling always from core
powerdomain subsystem.

Also the registered domain's will be managed inside a cache list and not
part of the genpd structure. This will help in registration of notifiers from
subsystems (such as clock) even when the PD subsystem is still not initialised.
This list also filters out the unregistered pd's requesting notification.

Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |   31 ++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4..e05045e 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -46,10 +46,19 @@
 	__retval;								\
 })
 
+struct cache_notify_domains {
+	char *name;
+	/* Node in the cache pm domain name list */
+	struct list_head cache_list_node;
+};
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
+static LIST_HEAD(domain_notify_list);
+static DEFINE_MUTEX(domain_notify_list_lock);
+static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
 
-static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
 	struct generic_pm_domain *genpd = NULL, *gpd;
 
@@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 	mutex_unlock(&gpd_list_lock);
 	return genpd;
 }
+EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
 
 struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {
@@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	mutex_unlock(&gpd_list_lock);
 }
 
+/**
+ * genpd_register_notifier - Register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+	int ret;
+	struct cache_notify_domains *entry;
+
+	if (!pd_name)
+		return -EINVAL;
+
+	/* Search if the pd is already registered */
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, pd_name))
+			break;
+	}
+	mutex_unlock(&domain_notify_list_lock);
+
+	if (entry) {
+		pr_err("%s: pd already exists\n", __func__);
+		return -EINVAL;
+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->name = pd_name;
+
+	mutex_lock(&domain_notify_list_lock);
+	list_add(&entry->cache_list_node, &domain_notify_list);
+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_register(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_unregister_notifier - Un-register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+	int ret;
+	struct cache_notify_domains *entry;
+
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, pd_name))
+			break;
+	}
+	if (!entry) {
+		mutex_unlock(&domain_notify_list_lock);
+		pr_err("%s: Invalid pd name\n", __func__);
+		return -EINVAL;
+	}
+	list_del(&entry->cache_list_node);
+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_unregister(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_invoke_transition_notifier - Calls the matching notification handler.
+ * @genpd: generic power domain structure.
+ * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
+ */
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state)
+{
+	struct cache_notify_domains *entry;
+
+	if (!genpd) {
+		pr_err("Invalid genpd parameter\n");
+		return;
+	}
+
+	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
+			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
+		pr_err("Invalid state parameter\n");
+		return;
+	}
+
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, genpd->name))
+			break;
+	}
+	mutex_unlock(&domain_notify_list_lock);
+	if (!entry) /* Simply ignore */
+		return;
+
+	blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
+				genpd);
+}
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 73e938b..659997f 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -25,6 +25,13 @@ enum gpd_status {
 	GPD_STATE_POWER_OFF,	/* PM domain is off */
 };
 
+enum gpd_on_off_state {
+	GPD_OFF_PRE = 0,	/* GPD state before power off */
+	GPD_OFF_POST,		/* GPD state after power off */
+	GPD_ON_PRE,		/* GPD state before power on */
+	GPD_ON_POST,		/* GPD state after power on */
+};
+
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
 	bool (*stop_ok)(struct device *dev);
@@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
+
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name);
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state);
 #else
 
 static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
@@ -219,6 +232,24 @@ static inline int pm_genpd_name_poweron(const char *domain_name)
 {
 	return -ENOSYS;
 }
+static inline struct
+generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+{
+	return NULL;
+}
+static inline int
+genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+	return 0;
+}
+static inline int
+genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+	return 0;
+}
+static inline void
+genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state) { }
 #define simple_qos_governor NULL
 #define pm_domain_always_on_gov NULL
 #endif
-- 
1.7.9.5

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Rafael J. Wysocki]

CC: Kevin, Ulf, Geert.

On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
> These power domain transition notifiers will assist in carrying
> out some activity associated with domain power on/off such as
> some registers which may lose its contents and need save/restore
> across domain power off/on.
> 
> 4 type of notifications are added,
> GPD_OFF_PRE     - GPD state before power off
> GPD_OFF_POST    - GPD state after power off
> GPD_ON_PRE      - GPD state before power off
> GPD_ON_POST     - GPD state after power off
> 
> 3 notfication API's are exported.
> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> 			enum gpd_on_off_state state);
> 
> In this approach the notifiers are registered/unregistered with pd name.
> The actual invoking of the notfiers will be done by the platform implementing
> power domain enable/disable low level handlers according to the above
> defined notification types. This approach will considerably reduce the
> number of call to notifiers as compared to calling always from core
> powerdomain subsystem.
> 
> Also the registered domain's will be managed inside a cache list and not
> part of the genpd structure. This will help in registration of notifiers from
> subsystems (such as clock) even when the PD subsystem is still not initialised.
> This list also filters out the unregistered pd's requesting notification.
> 
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/pm_domain.h   |   31 ++++++++++++
>  2 files changed, 142 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4..e05045e 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -46,10 +46,19 @@
>  	__retval;								\
>  })
>  
> +struct cache_notify_domains {
> +	char *name;
> +	/* Node in the cache pm domain name list */
> +	struct list_head cache_list_node;
> +};
> +
>  static LIST_HEAD(gpd_list);
>  static DEFINE_MUTEX(gpd_list_lock);
> +static LIST_HEAD(domain_notify_list);
> +static DEFINE_MUTEX(domain_notify_list_lock);
> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>  
> -static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>  {
>  	struct generic_pm_domain *genpd = NULL, *gpd;
>  
> @@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>  	mutex_unlock(&gpd_list_lock);
>  	return genpd;
>  }
> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>  
>  struct generic_pm_domain *dev_to_genpd(struct device *dev)
>  {
> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>  	mutex_unlock(&gpd_list_lock);
>  }
>  
> +/**
> + * genpd_register_notifier - Register a PM domain for future notification.
> + * @nb: notification block containing notification handle.
> + * @pd_name: PM domain name.
> + */
> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
> +{
> +	int ret;
> +	struct cache_notify_domains *entry;
> +
> +	if (!pd_name)
> +		return -EINVAL;
> +
> +	/* Search if the pd is already registered */
> +	mutex_lock(&domain_notify_list_lock);
> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> +		if (!strcmp(entry->name, pd_name))
> +			break;
> +	}
> +	mutex_unlock(&domain_notify_list_lock);
> +
> +	if (entry) {
> +		pr_err("%s: pd already exists\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return -ENOMEM;
> +
> +	entry->name = pd_name;
> +
> +	mutex_lock(&domain_notify_list_lock);
> +	list_add(&entry->cache_list_node, &domain_notify_list);
> +	mutex_unlock(&domain_notify_list_lock);
> +	ret = blocking_notifier_chain_register(
> +				&genpd_transition_notifier_list, nb);
> +	return ret;
> +}
> +
> +/**
> + * genpd_unregister_notifier - Un-register a PM domain for future notification.
> + * @nb: notification block containing notification handle.
> + * @pd_name: PM domain name.
> + */
> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
> +{
> +	int ret;
> +	struct cache_notify_domains *entry;
> +
> +	mutex_lock(&domain_notify_list_lock);
> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> +		if (!strcmp(entry->name, pd_name))
> +			break;
> +	}
> +	if (!entry) {
> +		mutex_unlock(&domain_notify_list_lock);
> +		pr_err("%s: Invalid pd name\n", __func__);
> +		return -EINVAL;
> +	}
> +	list_del(&entry->cache_list_node);
> +	mutex_unlock(&domain_notify_list_lock);
> +	ret = blocking_notifier_chain_unregister(
> +				&genpd_transition_notifier_list, nb);
> +	return ret;
> +}
> +
> +/**
> + * genpd_invoke_transition_notifier - Calls the matching notification handler.
> + * @genpd: generic power domain structure.
> + * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
> + */
> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> +			enum gpd_on_off_state state)
> +{
> +	struct cache_notify_domains *entry;
> +
> +	if (!genpd) {
> +		pr_err("Invalid genpd parameter\n");
> +		return;
> +	}
> +
> +	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
> +			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
> +		pr_err("Invalid state parameter\n");
> +		return;
> +	}
> +
> +	mutex_lock(&domain_notify_list_lock);
> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> +		if (!strcmp(entry->name, genpd->name))
> +			break;
> +	}
> +	mutex_unlock(&domain_notify_list_lock);
> +	if (!entry) /* Simply ignore */
> +		return;
> +
> +	blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
> +				genpd);
> +}
>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>  /*
>   * Device Tree based PM domain providers.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 73e938b..659997f 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -25,6 +25,13 @@ enum gpd_status {
>  	GPD_STATE_POWER_OFF,	/* PM domain is off */
>  };
>  
> +enum gpd_on_off_state {
> +	GPD_OFF_PRE = 0,	/* GPD state before power off */
> +	GPD_OFF_POST,		/* GPD state after power off */
> +	GPD_ON_PRE,		/* GPD state before power on */
> +	GPD_ON_POST,		/* GPD state after power on */
> +};
> +
>  struct dev_power_governor {
>  	bool (*power_down_ok)(struct dev_pm_domain *domain);
>  	bool (*stop_ok)(struct device *dev);
> @@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
>  
>  extern struct dev_power_governor simple_qos_governor;
>  extern struct dev_power_governor pm_domain_always_on_gov;
> +
> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name);
> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> +			enum gpd_on_off_state state);
>  #else
>  
>  static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
> @@ -219,6 +232,24 @@ static inline int pm_genpd_name_poweron(const char *domain_name)
>  {
>  	return -ENOSYS;
>  }
> +static inline struct
> +generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
> +{
> +	return NULL;
> +}
> +static inline int
> +genpd_register_notifier(struct notifier_block *nb, char *pd_name)
> +{
> +	return 0;
> +}
> +static inline int
> +genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
> +{
> +	return 0;
> +}
> +static inline void
> +genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> +			enum gpd_on_off_state state) { }
>  #define simple_qos_governor NULL
>  #define pm_domain_always_on_gov NULL
>  #endif
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Ulf Hansson]

On 3 November 2014 15:54, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> CC: Kevin, Ulf, Geert.
>
> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>> These power domain transition notifiers will assist in carrying
>> out some activity associated with domain power on/off such as
>> some registers which may lose its contents and need save/restore
>> across domain power off/on.

Hi Amit,

This mechanism seems to be needed - somehow.
A similar approach has been posted earlier, I am not sure that
discussion was fully settled though.

http://marc.info/?l=linux-pm&m=136448756308203&w=2

Anyway, I plan to review this shortly.

Kind regards
Uffe

>>
>> 4 type of notifications are added,
>> GPD_OFF_PRE     - GPD state before power off
>> GPD_OFF_POST    - GPD state after power off
>> GPD_ON_PRE      - GPD state before power off
>> GPD_ON_POST     - GPD state after power off
>>
>> 3 notfication API's are exported.
>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>>                       enum gpd_on_off_state state);
>>
>> In this approach the notifiers are registered/unregistered with pd name.
>> The actual invoking of the notfiers will be done by the platform implementing
>> power domain enable/disable low level handlers according to the above
>> defined notification types. This approach will considerably reduce the
>> number of call to notifiers as compared to calling always from core
>> powerdomain subsystem.
>>
>> Also the registered domain's will be managed inside a cache list and not
>> part of the genpd structure. This will help in registration of notifiers from
>> subsystems (such as clock) even when the PD subsystem is still not initialised.
>> This list also filters out the unregistered pd's requesting notification.
>>
>> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
>>  include/linux/pm_domain.h   |   31 ++++++++++++
>>  2 files changed, 142 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 40bc2f4..e05045e 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -46,10 +46,19 @@
>>       __retval;                                                               \
>>  })
>>
>> +struct cache_notify_domains {
>> +     char *name;
>> +     /* Node in the cache pm domain name list */
>> +     struct list_head cache_list_node;
>> +};
>> +
>>  static LIST_HEAD(gpd_list);
>>  static DEFINE_MUTEX(gpd_list_lock);
>> +static LIST_HEAD(domain_notify_list);
>> +static DEFINE_MUTEX(domain_notify_list_lock);
>> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>>
>> -static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>  {
>>       struct generic_pm_domain *genpd = NULL, *gpd;
>>
>> @@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>       mutex_unlock(&gpd_list_lock);
>>       return genpd;
>>  }
>> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>>
>>  struct generic_pm_domain *dev_to_genpd(struct device *dev)
>>  {
>> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>>       mutex_unlock(&gpd_list_lock);
>>  }
>>
>> +/**
>> + * genpd_register_notifier - Register a PM domain for future notification.
>> + * @nb: notification block containing notification handle.
>> + * @pd_name: PM domain name.
>> + */
>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +     int ret;
>> +     struct cache_notify_domains *entry;
>> +
>> +     if (!pd_name)
>> +             return -EINVAL;
>> +
>> +     /* Search if the pd is already registered */
>> +     mutex_lock(&domain_notify_list_lock);
>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +             if (!strcmp(entry->name, pd_name))
>> +                     break;
>> +     }
>> +     mutex_unlock(&domain_notify_list_lock);
>> +
>> +     if (entry) {
>> +             pr_err("%s: pd already exists\n", __func__);
>> +             return -EINVAL;
>> +     }
>> +
>> +     entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>> +     if (!entry)
>> +             return -ENOMEM;
>> +
>> +     entry->name = pd_name;
>> +
>> +     mutex_lock(&domain_notify_list_lock);
>> +     list_add(&entry->cache_list_node, &domain_notify_list);
>> +     mutex_unlock(&domain_notify_list_lock);
>> +     ret = blocking_notifier_chain_register(
>> +                             &genpd_transition_notifier_list, nb);
>> +     return ret;
>> +}
>> +
>> +/**
>> + * genpd_unregister_notifier - Un-register a PM domain for future notification.
>> + * @nb: notification block containing notification handle.
>> + * @pd_name: PM domain name.
>> + */
>> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +     int ret;
>> +     struct cache_notify_domains *entry;
>> +
>> +     mutex_lock(&domain_notify_list_lock);
>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +             if (!strcmp(entry->name, pd_name))
>> +                     break;
>> +     }
>> +     if (!entry) {
>> +             mutex_unlock(&domain_notify_list_lock);
>> +             pr_err("%s: Invalid pd name\n", __func__);
>> +             return -EINVAL;
>> +     }
>> +     list_del(&entry->cache_list_node);
>> +     mutex_unlock(&domain_notify_list_lock);
>> +     ret = blocking_notifier_chain_unregister(
>> +                             &genpd_transition_notifier_list, nb);
>> +     return ret;
>> +}
>> +
>> +/**
>> + * genpd_invoke_transition_notifier - Calls the matching notification handler.
>> + * @genpd: generic power domain structure.
>> + * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
>> + */
>> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> +                     enum gpd_on_off_state state)
>> +{
>> +     struct cache_notify_domains *entry;
>> +
>> +     if (!genpd) {
>> +             pr_err("Invalid genpd parameter\n");
>> +             return;
>> +     }
>> +
>> +     if (state != GPD_OFF_PRE || state != GPD_OFF_POST
>> +                     || state != GPD_ON_PRE || state != GPD_ON_POST) {
>> +             pr_err("Invalid state parameter\n");
>> +             return;
>> +     }
>> +
>> +     mutex_lock(&domain_notify_list_lock);
>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +             if (!strcmp(entry->name, genpd->name))
>> +                     break;
>> +     }
>> +     mutex_unlock(&domain_notify_list_lock);
>> +     if (!entry) /* Simply ignore */
>> +             return;
>> +
>> +     blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
>> +                             genpd);
>> +}
>>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>>  /*
>>   * Device Tree based PM domain providers.
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index 73e938b..659997f 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -25,6 +25,13 @@ enum gpd_status {
>>       GPD_STATE_POWER_OFF,    /* PM domain is off */
>>  };
>>
>> +enum gpd_on_off_state {
>> +     GPD_OFF_PRE = 0,        /* GPD state before power off */
>> +     GPD_OFF_POST,           /* GPD state after power off */
>> +     GPD_ON_PRE,             /* GPD state before power on */
>> +     GPD_ON_POST,            /* GPD state after power on */
>> +};
>> +
>>  struct dev_power_governor {
>>       bool (*power_down_ok)(struct dev_pm_domain *domain);
>>       bool (*stop_ok)(struct device *dev);
>> @@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
>>
>>  extern struct dev_power_governor simple_qos_governor;
>>  extern struct dev_power_governor pm_domain_always_on_gov;
>> +
>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name);
>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> +                     enum gpd_on_off_state state);
>>  #else
>>
>>  static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
>> @@ -219,6 +232,24 @@ static inline int pm_genpd_name_poweron(const char *domain_name)
>>  {
>>       return -ENOSYS;
>>  }
>> +static inline struct
>> +generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>> +{
>> +     return NULL;
>> +}
>> +static inline int
>> +genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +     return 0;
>> +}
>> +static inline int
>> +genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +     return 0;
>> +}
>> +static inline void
>> +genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> +                     enum gpd_on_off_state state) { }
>>  #define simple_qos_governor NULL
>>  #define pm_domain_always_on_gov NULL
>>  #endif
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[amit daniel kachhap]

On Mon, Nov 3, 2014 at 8:22 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 3 November 2014 15:54, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> CC: Kevin, Ulf, Geert.
>>
>> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>>> These power domain transition notifiers will assist in carrying
>>> out some activity associated with domain power on/off such as
>>> some registers which may lose its contents and need save/restore
>>> across domain power off/on.
>
> Hi Amit,
>
> This mechanism seems to be needed - somehow.
> A similar approach has been posted earlier, I am not sure that
> discussion was fully settled though.
>
> http://marc.info/?l=linux-pm&m=136448756308203&w=2
Missed looking into it. Thanks for the pointer.
>
> Anyway, I plan to review this shortly.
Sure.
>
> Kind regards
> Uffe
>
>>>
>>> 4 type of notifications are added,
>>> GPD_OFF_PRE     - GPD state before power off
>>> GPD_OFF_POST    - GPD state after power off
>>> GPD_ON_PRE      - GPD state before power off
>>> GPD_ON_POST     - GPD state after power off
>>>
>>> 3 notfication API's are exported.
>>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>>>                       enum gpd_on_off_state state);
>>>
>>> In this approach the notifiers are registered/unregistered with pd name.
>>> The actual invoking of the notfiers will be done by the platform implementing
>>> power domain enable/disable low level handlers according to the above
>>> defined notification types. This approach will considerably reduce the
>>> number of call to notifiers as compared to calling always from core
>>> powerdomain subsystem.
>>>
>>> Also the registered domain's will be managed inside a cache list and not
>>> part of the genpd structure. This will help in registration of notifiers from
>>> subsystems (such as clock) even when the PD subsystem is still not initialised.
>>> This list also filters out the unregistered pd's requesting notification.
>>>
>>> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>>> ---
>>>  drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
>>>  include/linux/pm_domain.h   |   31 ++++++++++++
>>>  2 files changed, 142 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>> index 40bc2f4..e05045e 100644
>>> --- a/drivers/base/power/domain.c
>>> +++ b/drivers/base/power/domain.c
>>> @@ -46,10 +46,19 @@
>>>       __retval;                                                               \
>>>  })
>>>
>>> +struct cache_notify_domains {
>>> +     char *name;
>>> +     /* Node in the cache pm domain name list */
>>> +     struct list_head cache_list_node;
>>> +};
>>> +
>>>  static LIST_HEAD(gpd_list);
>>>  static DEFINE_MUTEX(gpd_list_lock);
>>> +static LIST_HEAD(domain_notify_list);
>>> +static DEFINE_MUTEX(domain_notify_list_lock);
>>> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>>>
>>> -static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>>  {
>>>       struct generic_pm_domain *genpd = NULL, *gpd;
>>>
>>> @@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>>       mutex_unlock(&gpd_list_lock);
>>>       return genpd;
>>>  }
>>> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>>>
>>>  struct generic_pm_domain *dev_to_genpd(struct device *dev)
>>>  {
>>> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>>>       mutex_unlock(&gpd_list_lock);
>>>  }
>>>
>>> +/**
>>> + * genpd_register_notifier - Register a PM domain for future notification.
>>> + * @nb: notification block containing notification handle.
>>> + * @pd_name: PM domain name.
>>> + */
>>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>>> +{
>>> +     int ret;
>>> +     struct cache_notify_domains *entry;
>>> +
>>> +     if (!pd_name)
>>> +             return -EINVAL;
>>> +
>>> +     /* Search if the pd is already registered */
>>> +     mutex_lock(&domain_notify_list_lock);
>>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>>> +             if (!strcmp(entry->name, pd_name))
>>> +                     break;
>>> +     }
>>> +     mutex_unlock(&domain_notify_list_lock);
>>> +
>>> +     if (entry) {
>>> +             pr_err("%s: pd already exists\n", __func__);
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>>> +     if (!entry)
>>> +             return -ENOMEM;
>>> +
>>> +     entry->name = pd_name;
>>> +
>>> +     mutex_lock(&domain_notify_list_lock);
>>> +     list_add(&entry->cache_list_node, &domain_notify_list);
>>> +     mutex_unlock(&domain_notify_list_lock);
>>> +     ret = blocking_notifier_chain_register(
>>> +                             &genpd_transition_notifier_list, nb);
>>> +     return ret;
>>> +}
>>> +
>>> +/**
>>> + * genpd_unregister_notifier - Un-register a PM domain for future notification.
>>> + * @nb: notification block containing notification handle.
>>> + * @pd_name: PM domain name.
>>> + */
>>> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
>>> +{
>>> +     int ret;
>>> +     struct cache_notify_domains *entry;
>>> +
>>> +     mutex_lock(&domain_notify_list_lock);
>>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>>> +             if (!strcmp(entry->name, pd_name))
>>> +                     break;
>>> +     }
>>> +     if (!entry) {
>>> +             mutex_unlock(&domain_notify_list_lock);
>>> +             pr_err("%s: Invalid pd name\n", __func__);
>>> +             return -EINVAL;
>>> +     }
>>> +     list_del(&entry->cache_list_node);
>>> +     mutex_unlock(&domain_notify_list_lock);
>>> +     ret = blocking_notifier_chain_unregister(
>>> +                             &genpd_transition_notifier_list, nb);
>>> +     return ret;
>>> +}
>>> +
>>> +/**
>>> + * genpd_invoke_transition_notifier - Calls the matching notification handler.
>>> + * @genpd: generic power domain structure.
>>> + * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
>>> + */
>>> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>>> +                     enum gpd_on_off_state state)
>>> +{
>>> +     struct cache_notify_domains *entry;
>>> +
>>> +     if (!genpd) {
>>> +             pr_err("Invalid genpd parameter\n");
>>> +             return;
>>> +     }
>>> +
>>> +     if (state != GPD_OFF_PRE || state != GPD_OFF_POST
>>> +                     || state != GPD_ON_PRE || state != GPD_ON_POST) {
>>> +             pr_err("Invalid state parameter\n");
>>> +             return;
>>> +     }
>>> +
>>> +     mutex_lock(&domain_notify_list_lock);
>>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>>> +             if (!strcmp(entry->name, genpd->name))
>>> +                     break;
>>> +     }
>>> +     mutex_unlock(&domain_notify_list_lock);
>>> +     if (!entry) /* Simply ignore */
>>> +             return;
>>> +
>>> +     blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
>>> +                             genpd);
>>> +}
>>>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>>>  /*
>>>   * Device Tree based PM domain providers.
>>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>>> index 73e938b..659997f 100644
>>> --- a/include/linux/pm_domain.h
>>> +++ b/include/linux/pm_domain.h
>>> @@ -25,6 +25,13 @@ enum gpd_status {
>>>       GPD_STATE_POWER_OFF,    /* PM domain is off */
>>>  };
>>>
>>> +enum gpd_on_off_state {
>>> +     GPD_OFF_PRE = 0,        /* GPD state before power off */
>>> +     GPD_OFF_POST,           /* GPD state after power off */
>>> +     GPD_ON_PRE,             /* GPD state before power on */
>>> +     GPD_ON_POST,            /* GPD state after power on */
>>> +};
>>> +
>>>  struct dev_power_governor {
>>>       bool (*power_down_ok)(struct dev_pm_domain *domain);
>>>       bool (*stop_ok)(struct device *dev);
>>> @@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
>>>
>>>  extern struct dev_power_governor simple_qos_governor;
>>>  extern struct dev_power_governor pm_domain_always_on_gov;
>>> +
>>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name);
>>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>>> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>>> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>>> +                     enum gpd_on_off_state state);
>>>  #else
>>>
>>>  static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
>>> @@ -219,6 +232,24 @@ static inline int pm_genpd_name_poweron(const char *domain_name)
>>>  {
>>>       return -ENOSYS;
>>>  }
>>> +static inline struct
>>> +generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>> +{
>>> +     return NULL;
>>> +}
>>> +static inline int
>>> +genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>>> +{
>>> +     return 0;
>>> +}
>>> +static inline int
>>> +genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
>>> +{
>>> +     return 0;
>>> +}
>>> +static inline void
>>> +genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>>> +                     enum gpd_on_off_state state) { }
>>>  #define simple_qos_governor NULL
>>>  #define pm_domain_always_on_gov NULL
>>>  #endif
>>>
>>
>> --
>> I speak only for myself.
>> Rafael J. Wysocki, Intel Open Source Technology Center.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Kevin Hilman]

+Mike Turquette

Hi Amit,

"Rafael J. Wysocki" <rjw@rjwysocki.net> writes:

> CC: Kevin, Ulf, Geert.
>
> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>> These power domain transition notifiers will assist in carrying
>> out some activity associated with domain power on/off such as
>> some registers which may lose its contents and need save/restore
>> across domain power off/on.

The runtime PM framework already provides callbacks that are useful for
context save/restore for devices.  Could you please describe in more
detail which registers in which kind of devices need to be
saved/restored, and why they cannot be saved/restored using existing
mechanisms.

Personally, I'm uncomfortable with notifiers like this because it
suggests that underlying frameworks are not doing the right thing, or
are not being used.  (I also don't like the implementation here where a
single global notifier list is maintained by the core, but the notifiers
are actually triggered by SoC specific code.)

IIUC, the usage in this series seems to be that certain clock related
registers need to be saved/restored across a power domain transition.

Wouldn't an alternative solution be to add a feature to the clock driver
such that the state of each clock is saved when the clock is disabled,
and restored when the clock is enabled?   That would allow any clock
context to survive any power domain transtion also, correct?

I have some issues with the implementaion as well, but I think we need
to first sort out the real need for this before going into those
details.

Kevin

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[amit daniel kachhap]

On Mon, Nov 3, 2014 at 11:53 PM, Kevin Hilman <khilman@kernel.org> wrote:
> +Mike Turquette
>
> Hi Amit,
>
> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>
>> CC: Kevin, Ulf, Geert.
>>
>> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>>> These power domain transition notifiers will assist in carrying
>>> out some activity associated with domain power on/off such as
>>> some registers which may lose its contents and need save/restore
>>> across domain power off/on.
>
> The runtime PM framework already provides callbacks that are useful for
> context save/restore for devices.  Could you please describe in more
> detail which registers in which kind of devices need to be
> saved/restored, and why they cannot be saved/restored using existing
> mechanisms.
Basically the requirement is mandated by exynos7 manual. It tells that
before turning off the power domain, some clock registers need to saved
and should be restored just after turning the power domain. These clock
registers are not necessarily gate clocks but could be mux clocks etc.
The driver may not have all information of these clocks also. I suppose
these are Soc specific changes but drivers should work across Socs.
This behavior is almost similar to suspend/resume case where a whole
list of clock registers are saved/restored.

Even earlier post by Sylwester (https://lkml.org/lkml/2014/8/5/182) also points
to the need of this feature.
>
> Personally, I'm uncomfortable with notifiers like this because it
> suggests that underlying frameworks are not doing the right thing, or
> are not being used.  (I also don't like the implementation here where a
> single global notifier list is maintained by the core, but the notifiers
> are actually triggered by SoC specific code.)
Yes right the global notifier block can be moved to per genpd structure.
Also SoC trigger can be moved to core files.
>
> IIUC, the usage in this series seems to be that certain clock related
> registers need to be saved/restored across a power domain transition.
>
> Wouldn't an alternative solution be to add a feature to the clock driver
> such that the state of each clock is saved when the clock is disabled,
> and restored when the clock is enabled?   That would allow any clock
> context to survive any power domain transtion also, correct?
I also thought about same. But the trigger point for this would be
driver calling
clk disable/enable and not the power domain. so this will lead to lot of
save/restore for each power domain child.
>
> I have some issues with the implementaion as well, but I think we need
> to first sort out the real need for this before going into those
> details.
right :( may be I should have posted this as RFC
Thanks for the review.

Regards,
Amit D
>
> Kevin
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Sylwester Nawrocki]

On 04/11/14 07:44, amit daniel kachhap wrote:
> On Mon, Nov 3, 2014 at 11:53 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>>> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>>>> These power domain transition notifiers will assist in carrying
>>>> out some activity associated with domain power on/off such as
>>>> some registers which may lose its contents and need save/restore
>>>> across domain power off/on.
>>
>> The runtime PM framework already provides callbacks that are useful for
>> context save/restore for devices.  Could you please describe in more
>> detail which registers in which kind of devices need to be
>> saved/restored, and why they cannot be saved/restored using existing
>> mechanisms.
>
> Basically the requirement is mandated by exynos7 manual. It tells that
> before turning off the power domain, some clock registers need to saved
> and should be restored just after turning the power domain. These clock
> registers are not necessarily gate clocks but could be mux clocks etc.
> The driver may not have all information of these clocks also. I suppose
> these are Soc specific changes but drivers should work across Socs.
> This behavior is almost similar to suspend/resume case where a whole
> list of clock registers are saved/restored.

Indeed, the somehow complicated power domain power on/off sequences
are SoC specific.  They involve not only groups of clocks (usually
gate, mux clock registers of all devices in a power domain) but also
SoC-specific PMU (Power Management Unit) registers.
I assume it would be inappropriate to push such details to device 
drivers.  Moreover, a device driver could not be even loaded.

Since the clocks' state is already maintained by clk driver we came 
up with an idea of having generic calls from power domain driver back 
to the clock controller driver.

It might not to be the prettiest solution, nevertheless I couldn't come 
up with a better one which would satisfy all the requirements.  The power
domain should only be provided for use with all the clk/PMU sequences 
handling in place.

Clocks need to also be touched before a power domain switch on or off,
so attaching the clock controller to some power domain wouldn't help,
unless we modify/add to existing power domain related callbacks for 
devices.  Another issue is the clock controller device would need to 
be attached to multiple power domains, and for each power domain the 
power on/off sequence is usually slightly different.

There are also hierarchical power domains where each: the master and
the sub-domain need they own sequence and device usually is attached
only to a sub-domain.  

> Even earlier post by Sylwester (https://lkml.org/lkml/2014/8/5/182) 
> also points to the need of this feature.
>>
>> Personally, I'm uncomfortable with notifiers like this because it
>> suggests that underlying frameworks are not doing the right thing, or
>> are not being used.  (I also don't like the implementation here where a
>> single global notifier list is maintained by the core, but the notifiers
>> are actually triggered by SoC specific code.)
>
> Yes right the global notifier block can be moved to per genpd structure.
> Also SoC trigger can be moved to core files.
>>
>> IIUC, the usage in this series seems to be that certain clock related
>> registers need to be saved/restored across a power domain transition.
>>
>> Wouldn't an alternative solution be to add a feature to the clock driver
>> such that the state of each clock is saved when the clock is disabled,
>> and restored when the clock is enabled?   That would allow any clock
>> context to survive any power domain transtion also, correct?
>
> I also thought about same. But the trigger point for this would be
> driver calling clk disable/enable and not the power domain. so this 
> will lead to lot of save/restore for each power domain child.

Even though we would have saved/restored at that points still the power
domain driver would need to enforce some specific clock/PMU registers 
state before/after a power domain state transition. And this is what I 
found difficult with the existing APIs.

--
Thanks,
Sylwester

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Kevin Hilman]

Sylwester Nawrocki <s.nawrocki@samsung.com> writes:

> On 04/11/14 07:44, amit daniel kachhap wrote:
>> On Mon, Nov 3, 2014 at 11:53 PM, Kevin Hilman <khilman@kernel.org> wrote:
>>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>>>> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>>>>> These power domain transition notifiers will assist in carrying
>>>>> out some activity associated with domain power on/off such as
>>>>> some registers which may lose its contents and need save/restore
>>>>> across domain power off/on.
>>>
>>> The runtime PM framework already provides callbacks that are useful for
>>> context save/restore for devices.  Could you please describe in more
>>> detail which registers in which kind of devices need to be
>>> saved/restored, and why they cannot be saved/restored using existing
>>> mechanisms.
>>
>> Basically the requirement is mandated by exynos7 manual. It tells that
>> before turning off the power domain, some clock registers need to saved
>> and should be restored just after turning the power domain. These clock
>> registers are not necessarily gate clocks but could be mux clocks etc.
>> The driver may not have all information of these clocks also. I suppose
>> these are Soc specific changes but drivers should work across Socs.
>> This behavior is almost similar to suspend/resume case where a whole
>> list of clock registers are saved/restored.
>
> Indeed, the somehow complicated power domain power on/off sequences
> are SoC specific.  They involve not only groups of clocks (usually
> gate, mux clock registers of all devices in a power domain) but also
> SoC-specific PMU (Power Management Unit) registers.
> I assume it would be inappropriate to push such details to device 
> drivers.  Moreover, a device driver could not be even loaded.
>
> Since the clocks' state is already maintained by clk driver we came 
> up with an idea of having generic calls from power domain driver back 
> to the clock controller driver.

For the clock tree, it still seems to me that this is better handled in
the SoC clock driver.  For example, when a power domain is about to be
gated, all the devices in that domain are runtime suspended, and
presumably all of their gate clocks are disabled.  Now, doesn't the
clock driver know the clock tree parent-child hierarchy and shouldn't it
be capable of saving the state of parent clocks (like mux clocks) etc?

Stated diffrently, it still seems to me like we're pushing functionality
in PM core notifiers that should be the responsibility of subsystem
drivers. 

> It might not to be the prettiest solution, nevertheless I couldn't come 
> up with a better one which would satisfy all the requirements.  The power
> domain should only be provided for use with all the clk/PMU sequences 
> handling in place.
>
> Clocks need to also be touched before a power domain switch on or off,
> so attaching the clock controller to some power domain wouldn't help,
> unless we modify/add to existing power domain related callbacks for 
> devices.  Another issue is the clock controller device would need to 
> be attached to multiple power domains, and for each power domain the 
> power on/off sequence is usually slightly different.
>
> There are also hierarchical power domains where each: the master and
> the sub-domain need they own sequence and device usually is attached
> only to a sub-domain.  
>
>> Even earlier post by Sylwester (https://lkml.org/lkml/2014/8/5/182) 
>> also points to the need of this feature.
>>>
>>> Personally, I'm uncomfortable with notifiers like this because it
>>> suggests that underlying frameworks are not doing the right thing, or
>>> are not being used.  (I also don't like the implementation here where a
>>> single global notifier list is maintained by the core, but the notifiers
>>> are actually triggered by SoC specific code.)
>>
>> Yes right the global notifier block can be moved to per genpd structure.
>> Also SoC trigger can be moved to core files.
>>>
>>> IIUC, the usage in this series seems to be that certain clock related
>>> registers need to be saved/restored across a power domain transition.
>>>
>>> Wouldn't an alternative solution be to add a feature to the clock driver
>>> such that the state of each clock is saved when the clock is disabled,
>>> and restored when the clock is enabled?   That would allow any clock
>>> context to survive any power domain transtion also, correct?
>>
>> I also thought about same. But the trigger point for this would be
>> driver calling clk disable/enable and not the power domain. so this 
>> will lead to lot of save/restore for each power domain child.
>
> Even though we would have saved/restored at that points still the power
> domain driver would need to enforce some specific clock/PMU registers 
> state before/after a power domain state transition. And this is what I 
> found difficult with the existing APIs.

This is what I'm not understanding.

Why can't the power domain driver's power_on/power_off callback just
call the PMU APIs and/or the clk_enable/_disable calls it needs?

Kevin

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[amit daniel kachhap]

On Sat, Nov 8, 2014 at 12:15 AM, Kevin Hilman <khilman@kernel.org> wrote:
> Sylwester Nawrocki <s.nawrocki@samsung.com> writes:
>
>> On 04/11/14 07:44, amit daniel kachhap wrote:
>>> On Mon, Nov 3, 2014 at 11:53 PM, Kevin Hilman <khilman@kernel.org> wrote:
>>>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>>>>> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>>>>>> These power domain transition notifiers will assist in carrying
>>>>>> out some activity associated with domain power on/off such as
>>>>>> some registers which may lose its contents and need save/restore
>>>>>> across domain power off/on.
>>>>
>>>> The runtime PM framework already provides callbacks that are useful for
>>>> context save/restore for devices.  Could you please describe in more
>>>> detail which registers in which kind of devices need to be
>>>> saved/restored, and why they cannot be saved/restored using existing
>>>> mechanisms.
>>>
>>> Basically the requirement is mandated by exynos7 manual. It tells that
>>> before turning off the power domain, some clock registers need to saved
>>> and should be restored just after turning the power domain. These clock
>>> registers are not necessarily gate clocks but could be mux clocks etc.
>>> The driver may not have all information of these clocks also. I suppose
>>> these are Soc specific changes but drivers should work across Socs.
>>> This behavior is almost similar to suspend/resume case where a whole
>>> list of clock registers are saved/restored.
>>
>> Indeed, the somehow complicated power domain power on/off sequences
>> are SoC specific.  They involve not only groups of clocks (usually
>> gate, mux clock registers of all devices in a power domain) but also
>> SoC-specific PMU (Power Management Unit) registers.
>> I assume it would be inappropriate to push such details to device
>> drivers.  Moreover, a device driver could not be even loaded.
>>
>> Since the clocks' state is already maintained by clk driver we came
>> up with an idea of having generic calls from power domain driver back
>> to the clock controller driver.
>
> For the clock tree, it still seems to me that this is better handled in
> the SoC clock driver.  For example, when a power domain is about to be
> gated, all the devices in that domain are runtime suspended, and
> presumably all of their gate clocks are disabled.  Now, doesn't the
> clock driver know the clock tree parent-child hierarchy and shouldn't it
> be capable of saving the state of parent clocks (like mux clocks) etc?
>
> Stated diffrently, it still seems to me like we're pushing functionality
> in PM core notifiers that should be the responsibility of subsystem
> drivers.
>
>> It might not to be the prettiest solution, nevertheless I couldn't come
>> up with a better one which would satisfy all the requirements.  The power
>> domain should only be provided for use with all the clk/PMU sequences
>> handling in place.
>>
>> Clocks need to also be touched before a power domain switch on or off,
>> so attaching the clock controller to some power domain wouldn't help,
>> unless we modify/add to existing power domain related callbacks for
>> devices.  Another issue is the clock controller device would need to
>> be attached to multiple power domains, and for each power domain the
>> power on/off sequence is usually slightly different.
>>
>> There are also hierarchical power domains where each: the master and
>> the sub-domain need they own sequence and device usually is attached
>> only to a sub-domain.
>>
>>> Even earlier post by Sylwester (https://lkml.org/lkml/2014/8/5/182)
>>> also points to the need of this feature.
>>>>
>>>> Personally, I'm uncomfortable with notifiers like this because it
>>>> suggests that underlying frameworks are not doing the right thing, or
>>>> are not being used.  (I also don't like the implementation here where a
>>>> single global notifier list is maintained by the core, but the notifiers
>>>> are actually triggered by SoC specific code.)
>>>
>>> Yes right the global notifier block can be moved to per genpd structure.
>>> Also SoC trigger can be moved to core files.
>>>>
>>>> IIUC, the usage in this series seems to be that certain clock related
>>>> registers need to be saved/restored across a power domain transition.
>>>>
>>>> Wouldn't an alternative solution be to add a feature to the clock driver
>>>> such that the state of each clock is saved when the clock is disabled,
>>>> and restored when the clock is enabled?   That would allow any clock
>>>> context to survive any power domain transtion also, correct?
>>>
>>> I also thought about same. But the trigger point for this would be
>>> driver calling clk disable/enable and not the power domain. so this
>>> will lead to lot of save/restore for each power domain child.
>>
>> Even though we would have saved/restored at that points still the power
>> domain driver would need to enforce some specific clock/PMU registers
>> state before/after a power domain state transition. And this is what I
>> found difficult with the existing APIs.
>
> This is what I'm not understanding.
>
> Why can't the power domain driver's power_on/power_off callback just
> call the PMU APIs and/or the clk_enable/_disable calls it needs?
This looks possible for clock enable/disable at least. Will further
check the implementation
feasibility and reply.

Regards,
Amit
>
> Kevin
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Sylwester Nawrocki]

On 07/11/14 19:45, Kevin Hilman wrote:
> Sylwester Nawrocki <s.nawrocki@samsung.com> writes:
>> On 04/11/14 07:44, amit daniel kachhap wrote:
>>> On Mon, Nov 3, 2014 at 11:53 PM, Kevin Hilman <khilman@kernel.org> wrote:
>>>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>>>>> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
[...]
>> Indeed, the somehow complicated power domain power on/off sequences
>> are SoC specific.  They involve not only groups of clocks (usually
>> gate, mux clock registers of all devices in a power domain) but also
>> SoC-specific PMU (Power Management Unit) registers.
>> I assume it would be inappropriate to push such details to device 
>> drivers.  Moreover, a device driver could not be even loaded.
>>
>> Since the clocks' state is already maintained by clk driver we came 
>> up with an idea of having generic calls from power domain driver back 
>> to the clock controller driver.
> 
> For the clock tree, it still seems to me that this is better handled in
> the SoC clock driver.  For example, when a power domain is about to be
> gated, all the devices in that domain are runtime suspended, and
> presumably all of their gate clocks are disabled.  Now, doesn't the
> clock driver know the clock tree parent-child hierarchy and shouldn't it
> be capable of saving the state of parent clocks (like mux clocks) etc?
> 
> Stated diffrently, it still seems to me like we're pushing functionality
> in PM core notifiers that should be the responsibility of subsystem
> drivers. 

My apologies for not replying earlier, I got distracted by other 
activities.

I'd prefer not adding anything new to the PM core, however there are
dependencies between the power domain and clock controller driver 
which are hard to model in the current APIs.  I assume resorting to 
inter-exynos-driver API is not a good idea either.

Saving/restoring the clock hierarchy in the clock controller driver
during the power domain state transitions has a caveat that the clock
turn off/on sequences are IP/SoC subsystem specific.  So simply 
restoring saved registers from memory is not going to work.

The other detail I might have forgotten to mention is that the whole
clock controller may be in same power domain as the consumer devices.
That means the clock controller's registers must not be touched when
a related power domain is turned off.  Naturally when a power domain 
gets switched off all the clock controller's registers reset to their 
default values.  If we decided to use pm_runtime_{get_sync, put} in
the clock controller driver I'm not sure how it would need to interact
with the clk API.
In current mainline there is an issue with exynos4x12 that the system
may hang if clk_summary is attempted to read as the clock controller
driver doesn't take the ISP power domain into account.

[...]
>>>> Personally, I'm uncomfortable with notifiers like this because it
>>>> suggests that underlying frameworks are not doing the right thing, or
>>>> are not being used.  (I also don't like the implementation here where a
>>>> single global notifier list is maintained by the core, but the notifiers
>>>> are actually triggered by SoC specific code.)
>>>
>>> Yes right the global notifier block can be moved to per genpd structure.
>>> Also SoC trigger can be moved to core files.
>>>>
>>>> IIUC, the usage in this series seems to be that certain clock related
>>>> registers need to be saved/restored across a power domain transition.
>>>>
>>>> Wouldn't an alternative solution be to add a feature to the clock driver
>>>> such that the state of each clock is saved when the clock is disabled,
>>>> and restored when the clock is enabled?   That would allow any clock
>>>> context to survive any power domain transtion also, correct?
>>>
>>> I also thought about same. But the trigger point for this would be
>>> driver calling clk disable/enable and not the power domain. so this 
>>> will lead to lot of save/restore for each power domain child.
>>
>> Even though we would have saved/restored at that points still the power
>> domain driver would need to enforce some specific clock/PMU registers 
>> state before/after a power domain state transition. And this is what I 
>> found difficult with the existing APIs.
> 
> This is what I'm not understanding.
> 
> Why can't the power domain driver's power_on/power_off callback just
> call the PMU APIs and/or the clk_enable/_disable calls it needs?

I was concerned that it would not have been reliable by using the clk 
API due to the clk enable refcounting.  But that might not be a valid 
argument, since as you pointed out when a power domain is about to 
be gated related the clocks should be already disabled.

The other concern was atomicity in enabling/disabling groups of clocks,
i.e. setting group of bits in a clock gate register at once, rather
than a bit for each clock one by one.  But I'm not 100% sure about 
such a hardware requirement myself, would need to do some more testing 
and/or find a hardware engineer who could explain this. 

Additionally specifying clocks in device tree would be a bit messy and
there would very likely anyway be an additional information required 
in the power domain driver per each power domain regarding the clock 
handling sequence. 

-- 
Regards,
Sylwester

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Sylwester Nawrocki]

Hi,

Cc: Ulf, Kevin, Geert.

On 03/11/14 04:53, Amit Daniel Kachhap wrote:
> These power domain transition notifiers will assist in carrying
> out some activity associated with domain power on/off such as
> some registers which may lose its contents and need save/restore
> across domain power off/on.

Other specific use cases I could add here is gating selected clocks
to ensure proper signal propagation for devices attached to a power
domain, e.g. ungating selected clocks before the power domain on and
restoring them to previous state after the power domain was switched
on.

> 4 type of notifications are added,
> GPD_OFF_PRE     - GPD state before power off
> GPD_OFF_POST    - GPD state after power off
> GPD_ON_PRE      - GPD state before power off
> GPD_ON_POST     - GPD state after power off
> 
> 3 notfication API's are exported.
> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> 			enum gpd_on_off_state state);
> 
> In this approach the notifiers are registered/unregistered with pd name.
> The actual invoking of the notfiers will be done by the platform implementing
> power domain enable/disable low level handlers according to the above
> defined notification types. This approach will considerably reduce the
> number of call to notifiers as compared to calling always from core
> powerdomain subsystem.
> 
> Also the registered domain's will be managed inside a cache list and not
> part of the genpd structure. This will help in registration of notifiers from
> subsystems (such as clock) even when the PD subsystem is still not initialised.
> This list also filters out the unregistered pd's requesting notification.

This patch is somewhat similar my patch adding power domain state change
notifications [1].  I have already a reworked version of that patch, with the
notifier list moved to struct generic_pm_domain and using genpd->lock instead
of dev->power.lock.  Anyway, while I'd leave the decision about the location
from where the notifier chains are supposed to be called to the subsystem's
maintainers preference, I'm rather reluctant to having one global notifiers
list for all possible power domains and all the notification clients.
The possibly long list needs to be traversed at each notifier call and it
seems in your implementation any registered user of the notification gets
notifications for all possible power domains.

[1] https://lkml.org/lkml/2014/8/5/182

> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/pm_domain.h   |   31 ++++++++++++
>  2 files changed, 142 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4..e05045e 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -46,10 +46,19 @@
>  	__retval;								\
>  })
>  
> +struct cache_notify_domains {
> +	char *name;
> +	/* Node in the cache pm domain name list */
> +	struct list_head cache_list_node;
> +};
> +
>  static LIST_HEAD(gpd_list);
>  static DEFINE_MUTEX(gpd_list_lock);
> +static LIST_HEAD(domain_notify_list);
> +static DEFINE_MUTEX(domain_notify_list_lock);
> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>  
> -static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>  {
>  	struct generic_pm_domain *genpd = NULL, *gpd;
>  
> @@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>  	mutex_unlock(&gpd_list_lock);
>  	return genpd;
>  }
> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>  
>  struct generic_pm_domain *dev_to_genpd(struct device *dev)
>  {
> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>  	mutex_unlock(&gpd_list_lock);
>  }
>  
> +/**
> + * genpd_register_notifier - Register a PM domain for future notification.
> + * @nb: notification block containing notification handle.
> + * @pd_name: PM domain name.
> + */
> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
> +{
> +	int ret;
> +	struct cache_notify_domains *entry;
> +
> +	if (!pd_name)
> +		return -EINVAL;
> +
> +	/* Search if the pd is already registered */
> +	mutex_lock(&domain_notify_list_lock);
> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> +		if (!strcmp(entry->name, pd_name))
> +			break;
> +	}
> +	mutex_unlock(&domain_notify_list_lock);
> +
> +	if (entry) {
> +		pr_err("%s: pd already exists\n", __func__);
> +		return -EINVAL;

I suspect this code doesn't work as expected. 'entry' will be NULL only
in case of an empty domain_notify_list list. Have you tested multiple
calls to genpd_register_notifier() ? It looks like only the first call
would work.

> +	}
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return -ENOMEM;
> +
> +	entry->name = pd_name;
> +
> +	mutex_lock(&domain_notify_list_lock);
> +	list_add(&entry->cache_list_node, &domain_notify_list);
> +	mutex_unlock(&domain_notify_list_lock);
> +	ret = blocking_notifier_chain_register(
> +				&genpd_transition_notifier_list, nb);
> +	return ret;
> +}
> +
> +/**
> + * genpd_unregister_notifier - Un-register a PM domain for future notification.
> + * @nb: notification block containing notification handle.
> + * @pd_name: PM domain name.
> + */
> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
> +{
> +	int ret;
> +	struct cache_notify_domains *entry;
> +
> +	mutex_lock(&domain_notify_list_lock);
> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> +		if (!strcmp(entry->name, pd_name))
> +			break;
> +	}
> +	if (!entry) {
> +		mutex_unlock(&domain_notify_list_lock);
> +		pr_err("%s: Invalid pd name\n", __func__);
> +		return -EINVAL;
> +	}
> +	list_del(&entry->cache_list_node);

'entry' will not be NULL even when the requested notification entry
was not found above. In such case it looks like last entry in the
domain_notify_list list will be removed, not something we would expect.

> +	mutex_unlock(&domain_notify_list_lock);
> +	ret = blocking_notifier_chain_unregister(
> +				&genpd_transition_notifier_list, nb);
> +	return ret;
> +}
> +
> +/**
> + * genpd_invoke_transition_notifier - Calls the matching notification handler.
> + * @genpd: generic power domain structure.
> + * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
> + */
> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> +			enum gpd_on_off_state state)
> +{
> +	struct cache_notify_domains *entry;
> +
> +	if (!genpd) {
> +		pr_err("Invalid genpd parameter\n");
> +		return;
> +	}
> +
> +	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
> +			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
> +		pr_err("Invalid state parameter\n");
> +		return;
> +	}
> +
> +	mutex_lock(&domain_notify_list_lock);
> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> +		if (!strcmp(entry->name, genpd->name))
> +			break;
> +	}
> +	mutex_unlock(&domain_notify_list_lock);
> +	if (!entry) /* Simply ignore */

Similar issue here, this condition will only be true when the notifications
list is empty.

> +		return;
> +
> +	blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
> +				genpd);

And finally regardless of to what power domain the notification user has
registered it will get notification for each possible power domain in
the system?  Are the notification users supposed to test the 'genpd'
argument to react on a specific power domain? If so, how? I guess not by
nother strcmp() ?

> +}
>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>  /*
>   * Device Tree based PM domain providers.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 73e938b..659997f 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -25,6 +25,13 @@ enum gpd_status {
>  	GPD_STATE_POWER_OFF,	/* PM domain is off */
>  };
>  
> +enum gpd_on_off_state {
> +	GPD_OFF_PRE = 0,	/* GPD state before power off */

The assignment is not needed.

> +	GPD_OFF_POST,		/* GPD state after power off */
> +	GPD_ON_PRE,		/* GPD state before power on */
> +	GPD_ON_POST,		/* GPD state after power on */
> +};
> +
>  struct dev_power_governor {
>  	bool (*power_down_ok)(struct dev_pm_domain *domain);
>  	bool (*stop_ok)(struct device *dev);
> @@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);

--
Regards,
Sylwester

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Sylwester Nawrocki]

Now really filling in the CC list, apologies for duplicate
post.

On 03/11/14 19:21, Sylwester Nawrocki wrote:
> Hi,
> 
> Cc: Ulf, Kevin, Geert.
> 
> On 03/11/14 04:53, Amit Daniel Kachhap wrote:
>> These power domain transition notifiers will assist in carrying
>> out some activity associated with domain power on/off such as
>> some registers which may lose its contents and need save/restore
>> across domain power off/on.
> 
> Other specific use cases I could add here is gating selected clocks
> to ensure proper signal propagation for devices attached to a power
> domain, e.g. ungating selected clocks before the power domain on and
> restoring them to previous state after the power domain was switched
> on.
> 
>> 4 type of notifications are added,
>> GPD_OFF_PRE     - GPD state before power off
>> GPD_OFF_POST    - GPD state after power off
>> GPD_ON_PRE      - GPD state before power off
>> GPD_ON_POST     - GPD state after power off
>>
>> 3 notfication API's are exported.
>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> 			enum gpd_on_off_state state);
>>
>> In this approach the notifiers are registered/unregistered with pd name.
>> The actual invoking of the notfiers will be done by the platform implementing
>> power domain enable/disable low level handlers according to the above
>> defined notification types. This approach will considerably reduce the
>> number of call to notifiers as compared to calling always from core
>> powerdomain subsystem.
>>
>> Also the registered domain's will be managed inside a cache list and not
>> part of the genpd structure. This will help in registration of notifiers from
>> subsystems (such as clock) even when the PD subsystem is still not initialised.
>> This list also filters out the unregistered pd's requesting notification.
> 
> This patch is somewhat similar my patch adding power domain state change
> notifications [1].  I have already a reworked version of that patch, with the
> notifier list moved to struct generic_pm_domain and using genpd->lock instead
> of dev->power.lock.  Anyway, while I'd leave the decision about the location
> from where the notifier chains are supposed to be called to the subsystem's
> maintainers preference, I'm rather reluctant to having one global notifiers
> list for all possible power domains and all the notification clients.
> The possibly long list needs to be traversed at each notifier call and it
> seems in your implementation any registered user of the notification gets
> notifications for all possible power domains.
> 
> [1] https://lkml.org/lkml/2014/8/5/182
> 
>> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
>>  include/linux/pm_domain.h   |   31 ++++++++++++
>>  2 files changed, 142 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 40bc2f4..e05045e 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -46,10 +46,19 @@
>>  	__retval;								\
>>  })
>>  
>> +struct cache_notify_domains {
>> +	char *name;
>> +	/* Node in the cache pm domain name list */
>> +	struct list_head cache_list_node;
>> +};
>> +
>>  static LIST_HEAD(gpd_list);
>>  static DEFINE_MUTEX(gpd_list_lock);
>> +static LIST_HEAD(domain_notify_list);
>> +static DEFINE_MUTEX(domain_notify_list_lock);
>> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>>  
>> -static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>  {
>>  	struct generic_pm_domain *genpd = NULL, *gpd;
>>  
>> @@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>  	mutex_unlock(&gpd_list_lock);
>>  	return genpd;
>>  }
>> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>>  
>>  struct generic_pm_domain *dev_to_genpd(struct device *dev)
>>  {
>> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>>  	mutex_unlock(&gpd_list_lock);
>>  }
>>  
>> +/**
>> + * genpd_register_notifier - Register a PM domain for future notification.
>> + * @nb: notification block containing notification handle.
>> + * @pd_name: PM domain name.
>> + */
>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +	int ret;
>> +	struct cache_notify_domains *entry;
>> +
>> +	if (!pd_name)
>> +		return -EINVAL;
>> +
>> +	/* Search if the pd is already registered */
>> +	mutex_lock(&domain_notify_list_lock);
>> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +		if (!strcmp(entry->name, pd_name))
>> +			break;
>> +	}
>> +	mutex_unlock(&domain_notify_list_lock);
>> +
>> +	if (entry) {
>> +		pr_err("%s: pd already exists\n", __func__);
>> +		return -EINVAL;
> 
> I suspect this code doesn't work as expected. 'entry' will be NULL only
> in case of an empty domain_notify_list list. Have you tested multiple
> calls to genpd_register_notifier() ? It looks like only the first call
> would work.
> 
>> +	}
>> +
>> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>> +	if (!entry)
>> +		return -ENOMEM;
>> +
>> +	entry->name = pd_name;
>> +
>> +	mutex_lock(&domain_notify_list_lock);
>> +	list_add(&entry->cache_list_node, &domain_notify_list);
>> +	mutex_unlock(&domain_notify_list_lock);
>> +	ret = blocking_notifier_chain_register(
>> +				&genpd_transition_notifier_list, nb);
>> +	return ret;
>> +}
>> +
>> +/**
>> + * genpd_unregister_notifier - Un-register a PM domain for future notification.
>> + * @nb: notification block containing notification handle.
>> + * @pd_name: PM domain name.
>> + */
>> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +	int ret;
>> +	struct cache_notify_domains *entry;
>> +
>> +	mutex_lock(&domain_notify_list_lock);
>> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +		if (!strcmp(entry->name, pd_name))
>> +			break;
>> +	}
>> +	if (!entry) {
>> +		mutex_unlock(&domain_notify_list_lock);
>> +		pr_err("%s: Invalid pd name\n", __func__);
>> +		return -EINVAL;
>> +	}
>> +	list_del(&entry->cache_list_node);
> 
> 'entry' will not be NULL even when the requested notification entry
> was not found above. In such case it looks like last entry in the
> domain_notify_list list will be removed, not something we would expect.
> 
>> +	mutex_unlock(&domain_notify_list_lock);
>> +	ret = blocking_notifier_chain_unregister(
>> +				&genpd_transition_notifier_list, nb);
>> +	return ret;
>> +}
>> +
>> +/**
>> + * genpd_invoke_transition_notifier - Calls the matching notification handler.
>> + * @genpd: generic power domain structure.
>> + * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
>> + */
>> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> +			enum gpd_on_off_state state)
>> +{
>> +	struct cache_notify_domains *entry;
>> +
>> +	if (!genpd) {
>> +		pr_err("Invalid genpd parameter\n");
>> +		return;
>> +	}
>> +
>> +	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
>> +			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
>> +		pr_err("Invalid state parameter\n");
>> +		return;
>> +	}
>> +
>> +	mutex_lock(&domain_notify_list_lock);
>> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +		if (!strcmp(entry->name, genpd->name))
>> +			break;
>> +	}
>> +	mutex_unlock(&domain_notify_list_lock);
>> +	if (!entry) /* Simply ignore */
> 
> Similar issue here, this condition will only be true when the notifications
> list is empty.
> 
>> +		return;
>> +
>> +	blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
>> +				genpd);
> 
> And finally regardless of to what power domain the notification user has
> registered it will get notification for each possible power domain in
> the system?  Are the notification users supposed to test the 'genpd'
> argument to react on a specific power domain? If so, how? I guess not by
> nother strcmp() ?
> 
>> +}
>>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>>  /*
>>   * Device Tree based PM domain providers.
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index 73e938b..659997f 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -25,6 +25,13 @@ enum gpd_status {
>>  	GPD_STATE_POWER_OFF,	/* PM domain is off */
>>  };
>>  
>> +enum gpd_on_off_state {
>> +	GPD_OFF_PRE = 0,	/* GPD state before power off */
> 
> The assignment is not needed.
> 
>> +	GPD_OFF_POST,		/* GPD state after power off */
>> +	GPD_ON_PRE,		/* GPD state before power on */
>> +	GPD_ON_POST,		/* GPD state after power on */
>> +};
>> +
>>  struct dev_power_governor {
>>  	bool (*power_down_ok)(struct dev_pm_domain *domain);
>>  	bool (*stop_ok)(struct device *dev);
>> @@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
> 
> --
> Regards,
> Sylwester

RE: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Pankaj Dubey]

Hi,

On  Tuesday, November 04, 2014 12:11 AM, Sylwester Nawrocki wrote,
> To: linux-arm-kernel@lists.infradead.org
> Cc: Amit Daniel Kachhap; linux-samsung-soc@vger.kernel.org; linux-
> pm@vger.kernel.org; kgene.kim@samsung.com; pankaj.dubey@samsung.com;
Rafael
> J. Wysocki; Beata Michalska; geert@linux-m68k.org; Kevin Hilman; Ulf
Hansson
> Subject: Re: [PATCH 03/12] PM / Domains: Add notifier support for power
domain
> transitions
> 
> 
> Now really filling in the CC list, apologies for duplicate post.
> 
> On 03/11/14 19:21, Sylwester Nawrocki wrote:
> > Hi,
> >
> > Cc: Ulf, Kevin, Geert.
> >
> > On 03/11/14 04:53, Amit Daniel Kachhap wrote:
> >> These power domain transition notifiers will assist in carrying out
> >> some activity associated with domain power on/off such as some
> >> registers which may lose its contents and need save/restore across
> >> domain power off/on.
> >
> > Other specific use cases I could add here is gating selected clocks to
> > ensure proper signal propagation for devices attached to a power
> > domain, e.g. ungating selected clocks before the power domain on and
> > restoring them to previous state after the power domain was switched
> > on.
> >
> >> 4 type of notifications are added,
> >> GPD_OFF_PRE     - GPD state before power off
> >> GPD_OFF_POST    - GPD state after power off
> >> GPD_ON_PRE      - GPD state before power off
> >> GPD_ON_POST     - GPD state after power off
> >>
> >> 3 notfication API's are exported.
> >> 1) int genpd_register_notifier(struct notifier_block *nb, char
> >> *pd_name);
> >> 2) int genpd_unregister_notifier(struct notifier_block *nb, char
> >> *pd_name);
> >> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain
*genpd,
> >> 			enum gpd_on_off_state state);
> >>
> >> In this approach the notifiers are registered/unregistered with pd
name.
> >> The actual invoking of the notfiers will be done by the platform
> >> implementing power domain enable/disable low level handlers according
> >> to the above defined notification types. This approach will
> >> considerably reduce the number of call to notifiers as compared to
> >> calling always from core powerdomain subsystem.
> >>
> >> Also the registered domain's will be managed inside a cache list and
> >> not part of the genpd structure. This will help in registration of
> >> notifiers from subsystems (such as clock) even when the PD subsystem is
still not
> initialised.
> >> This list also filters out the unregistered pd's requesting
notification.
> >
> > This patch is somewhat similar my patch adding power domain state
> > change notifications [1].  I have already a reworked version of that
> > patch, with the notifier list moved to struct generic_pm_domain and
> > using genpd->lock instead of dev->power.lock.  Anyway, while I'd leave
> > the decision about the location from where the notifier chains are
> > supposed to be called to the subsystem's maintainers preference, I'm
> > rather reluctant to having one global notifiers list for all possible
power domains
> and all the notification clients.
> > The possibly long list needs to be traversed at each notifier call and
> > it seems in your implementation any registered user of the
> > notification gets notifications for all possible power domains.
> >
> > [1] https://lkml.org/lkml/2014/8/5/182
> >
> >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> >> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> >> ---
> >>  drivers/base/power/domain.c |  112
> ++++++++++++++++++++++++++++++++++++++++++-
> >>  include/linux/pm_domain.h   |   31 ++++++++++++
> >>  2 files changed, 142 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/power/domain.c
> >> b/drivers/base/power/domain.c index 40bc2f4..e05045e 100644
> >> --- a/drivers/base/power/domain.c
> >> +++ b/drivers/base/power/domain.c
> >> @@ -46,10 +46,19 @@
> >>  	__retval;
\
> >>  })
> >>
> >> +struct cache_notify_domains {
> >> +	char *name;
> >> +	/* Node in the cache pm domain name list */
> >> +	struct list_head cache_list_node;
> >> +};
> >> +
> >>  static LIST_HEAD(gpd_list);
> >>  static DEFINE_MUTEX(gpd_list_lock);
> >> +static LIST_HEAD(domain_notify_list); static
> >> +DEFINE_MUTEX(domain_notify_list_lock);
> >> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
> >>
> >> -static struct generic_pm_domain *pm_genpd_lookup_name(const char
> >> *domain_name)
> >> +struct generic_pm_domain *pm_genpd_lookup_name(const char
> >> +*domain_name)
> >>  {
> >>  	struct generic_pm_domain *genpd = NULL, *gpd;
> >>
> >> @@ -66,6 +75,7 @@ static struct generic_pm_domain
> *pm_genpd_lookup_name(const char *domain_name)
> >>  	mutex_unlock(&gpd_list_lock);
> >>  	return genpd;
> >>  }
> >> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
> >>
> >>  struct generic_pm_domain *dev_to_genpd(struct device *dev)  { @@
> >> -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain
*genpd,
> >>  	mutex_unlock(&gpd_list_lock);
> >>  }
> >>
> >> +/**
> >> + * genpd_register_notifier - Register a PM domain for future
notification.
> >> + * @nb: notification block containing notification handle.
> >> + * @pd_name: PM domain name.
> >> + */
> >> +int genpd_register_notifier(struct notifier_block *nb, char
> >> +*pd_name) {
> >> +	int ret;
> >> +	struct cache_notify_domains *entry;
> >> +
> >> +	if (!pd_name)
> >> +		return -EINVAL;
> >> +
> >> +	/* Search if the pd is already registered */
> >> +	mutex_lock(&domain_notify_list_lock);
> >> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> >> +		if (!strcmp(entry->name, pd_name))
> >> +			break;
> >> +	}
> >> +	mutex_unlock(&domain_notify_list_lock);
> >> +
> >> +	if (entry) {
> >> +		pr_err("%s: pd already exists\n", __func__);
> >> +		return -EINVAL;
> >
> > I suspect this code doesn't work as expected. 'entry' will be NULL
> > only in case of an empty domain_notify_list list. Have you tested
> > multiple calls to genpd_register_notifier() ? It looks like only the
> > first call would work.
> >

You are correct it should fail for multiple calls. 
Somehow we missed this in internal review. Thanks for pointing out.

> >> +	}
> >> +
> >> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> >> +	if (!entry)
> >> +		return -ENOMEM;
> >> +
> >> +	entry->name = pd_name;
> >> +
> >> +	mutex_lock(&domain_notify_list_lock);
> >> +	list_add(&entry->cache_list_node, &domain_notify_list);
> >> +	mutex_unlock(&domain_notify_list_lock);
> >> +	ret = blocking_notifier_chain_register(
> >> +				&genpd_transition_notifier_list, nb);
> >> +	return ret;
> >> +}
> >> +
> >> +/**
> >> + * genpd_unregister_notifier - Un-register a PM domain for future
notification.
> >> + * @nb: notification block containing notification handle.
> >> + * @pd_name: PM domain name.
> >> + */
> >> +int genpd_unregister_notifier(struct notifier_block *nb, char
> >> +*pd_name) {
> >> +	int ret;
> >> +	struct cache_notify_domains *entry;
> >> +
> >> +	mutex_lock(&domain_notify_list_lock);
> >> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> >> +		if (!strcmp(entry->name, pd_name))
> >> +			break;
> >> +	}
> >> +	if (!entry) {
> >> +		mutex_unlock(&domain_notify_list_lock);
> >> +		pr_err("%s: Invalid pd name\n", __func__);
> >> +		return -EINVAL;
> >> +	}
> >> +	list_del(&entry->cache_list_node);
> >
> > 'entry' will not be NULL even when the requested notification entry
> > was not found above. In such case it looks like last entry in the
> > domain_notify_list list will be removed, not something we would expect.
> >
> >> +	mutex_unlock(&domain_notify_list_lock);
> >> +	ret = blocking_notifier_chain_unregister(
> >> +				&genpd_transition_notifier_list, nb);
> >> +	return ret;
> >> +}
> >> +
> >> +/**
> >> + * genpd_invoke_transition_notifier - Calls the matching notification
handler.
> >> + * @genpd: generic power domain structure.
> >> + * @state: can be of type -
> GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
> >> + */
> >> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> >> +			enum gpd_on_off_state state)
> >> +{
> >> +	struct cache_notify_domains *entry;
> >> +
> >> +	if (!genpd) {
> >> +		pr_err("Invalid genpd parameter\n");
> >> +		return;
> >> +	}
> >> +
> >> +	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
> >> +			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
> >> +		pr_err("Invalid state parameter\n");
> >> +		return;
> >> +	}
> >> +
> >> +	mutex_lock(&domain_notify_list_lock);
> >> +	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> >> +		if (!strcmp(entry->name, genpd->name))
> >> +			break;
> >> +	}
> >> +	mutex_unlock(&domain_notify_list_lock);
> >> +	if (!entry) /* Simply ignore */
> >
> > Similar issue here, this condition will only be true when the
> > notifications list is empty.

I also think so.

> >
> >> +		return;
> >> +
> >> +	blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
> >> +				genpd);
> >
> > And finally regardless of to what power domain the notification user
> > has registered it will get notification for each possible power domain
> > in the system?  Are the notification users supposed to test the 'genpd'
> > argument to react on a specific power domain? If so, how? I guess not
> > by nother strcmp() ?
> >
> >> +}
> >>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
> >>  /*
> >>   * Device Tree based PM domain providers.
> >> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> >> index 73e938b..659997f 100644
> >> --- a/include/linux/pm_domain.h
> >> +++ b/include/linux/pm_domain.h
> >> @@ -25,6 +25,13 @@ enum gpd_status {
> >>  	GPD_STATE_POWER_OFF,	/* PM domain is off */
> >>  };
> >>
> >> +enum gpd_on_off_state {
> >> +	GPD_OFF_PRE = 0,	/* GPD state before power off */
> >
> > The assignment is not needed.
> >
> >> +	GPD_OFF_POST,		/* GPD state after power off */
> >> +	GPD_ON_PRE,		/* GPD state before power on */
> >> +	GPD_ON_POST,		/* GPD state after power on */
> >> +};
> >> +
> >>  struct dev_power_governor {
> >>  	bool (*power_down_ok)(struct dev_pm_domain *domain);
> >>  	bool (*stop_ok)(struct device *dev); @@ -148,6 +155,12 @@ extern
> >> int pm_genpd_name_poweron(const char *domain_name);
> >
> > --
> > Regards,
> > Sylwester

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[amit daniel kachhap]

On Mon, Nov 3, 2014 at 11:51 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Hi,
>
> Cc: Ulf, Kevin, Geert.
>
> On 03/11/14 04:53, Amit Daniel Kachhap wrote:
>> These power domain transition notifiers will assist in carrying
>> out some activity associated with domain power on/off such as
>> some registers which may lose its contents and need save/restore
>> across domain power off/on.
>
> Other specific use cases I could add here is gating selected clocks
> to ensure proper signal propagation for devices attached to a power
> domain, e.g. ungating selected clocks before the power domain on and
> restoring them to previous state after the power domain was switched
> on.
yes correct.
>
>> 4 type of notifications are added,
>> GPD_OFF_PRE     - GPD state before power off
>> GPD_OFF_POST    - GPD state after power off
>> GPD_ON_PRE      - GPD state before power off
>> GPD_ON_POST     - GPD state after power off
>>
>> 3 notfication API's are exported.
>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>>                       enum gpd_on_off_state state);
>>
>> In this approach the notifiers are registered/unregistered with pd name.
>> The actual invoking of the notfiers will be done by the platform implementing
>> power domain enable/disable low level handlers according to the above
>> defined notification types. This approach will considerably reduce the
>> number of call to notifiers as compared to calling always from core
>> powerdomain subsystem.
>>
>> Also the registered domain's will be managed inside a cache list and not
>> part of the genpd structure. This will help in registration of notifiers from
>> subsystems (such as clock) even when the PD subsystem is still not initialised.
>> This list also filters out the unregistered pd's requesting notification.
>
> This patch is somewhat similar my patch adding power domain state change
> notifications [1].  I have already a reworked version of that patch, with the
> notifier list moved to struct generic_pm_domain and using genpd->lock instead

Yes this will be correct as others also suggested to make per genpd
notifier block.

> of dev->power.lock.  Anyway, while I'd leave the decision about the location
> from where the notifier chains are supposed to be called to the subsystem's
> maintainers preference, I'm rather reluctant to having one global notifiers
> list for all possible power domains and all the notification clients.
> The possibly long list needs to be traversed at each notifier call and it
> seems in your implementation any registered user of the notification gets
> notifications for all possible power domains.
>
> [1] https://lkml.org/lkml/2014/8/5/182
My fault, I somehow missed this link earlier. After going through
this, I found it registers genpd from the platform driver, so the
function signature is
int pm_genpd_register_notifier(struct device *dev, struct notifier_block *nb);
I suggest to make the function signature to be like,
int pm_genpd_register_notifier(struct device_node *np, struct
notifier_block *nb)

In this way this function should should be able to support both
platform devices and non platform devices like clk.

The function may work like,

pdev = of_find_device_by_node(np);
if (pdev) {
         //  get genpd from device and go ahead with notfier registration.
         // blocking_notifier_chain_register(genpd->pd_notifier, nb)
} else
{
        // get pd_handle from np
        // get the pd_name from phandle and try registering this gen pd
        // if the genpd subsystem is not initialised then add this in
a temporary list and register the notifier later
}

Can you post your implementation with 1st part ? Later I can post the
else part with my changes.

>
>> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>  drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
>>  include/linux/pm_domain.h   |   31 ++++++++++++
>>  2 files changed, 142 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 40bc2f4..e05045e 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -46,10 +46,19 @@
>>       __retval;                                                               \
>>  })
>>
>> +struct cache_notify_domains {
>> +     char *name;
>> +     /* Node in the cache pm domain name list */
>> +     struct list_head cache_list_node;
>> +};
>> +
>>  static LIST_HEAD(gpd_list);
>>  static DEFINE_MUTEX(gpd_list_lock);
>> +static LIST_HEAD(domain_notify_list);
>> +static DEFINE_MUTEX(domain_notify_list_lock);
>> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>>
>> -static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>  {
>>       struct generic_pm_domain *genpd = NULL, *gpd;
>>
>> @@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>       mutex_unlock(&gpd_list_lock);
>>       return genpd;
>>  }
>> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>>
>>  struct generic_pm_domain *dev_to_genpd(struct device *dev)
>>  {
>> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>>       mutex_unlock(&gpd_list_lock);
>>  }
>>
>> +/**
>> + * genpd_register_notifier - Register a PM domain for future notification.
>> + * @nb: notification block containing notification handle.
>> + * @pd_name: PM domain name.
>> + */
>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +     int ret;
>> +     struct cache_notify_domains *entry;
>> +
>> +     if (!pd_name)
>> +             return -EINVAL;
>> +
>> +     /* Search if the pd is already registered */
>> +     mutex_lock(&domain_notify_list_lock);
>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +             if (!strcmp(entry->name, pd_name))
>> +                     break;
>> +     }
>> +     mutex_unlock(&domain_notify_list_lock);
>> +
>> +     if (entry) {
>> +             pr_err("%s: pd already exists\n", __func__);
>> +             return -EINVAL;
>
> I suspect this code doesn't work as expected. 'entry' will be NULL only
> in case of an empty domain_notify_list list. Have you tested multiple
> calls to genpd_register_notifier() ? It looks like only the first call
> would work.
Yes this list check is not correct. Will update this. Thanks.
>
>> +     }
>> +
>> +     entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>> +     if (!entry)
>> +             return -ENOMEM;
>> +
>> +     entry->name = pd_name;
>> +
>> +     mutex_lock(&domain_notify_list_lock);
>> +     list_add(&entry->cache_list_node, &domain_notify_list);
>> +     mutex_unlock(&domain_notify_list_lock);
>> +     ret = blocking_notifier_chain_register(
>> +                             &genpd_transition_notifier_list, nb);
>> +     return ret;
>> +}
>> +
>> +/**
>> + * genpd_unregister_notifier - Un-register a PM domain for future notification.
>> + * @nb: notification block containing notification handle.
>> + * @pd_name: PM domain name.
>> + */
>> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +     int ret;
>> +     struct cache_notify_domains *entry;
>> +
>> +     mutex_lock(&domain_notify_list_lock);
>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +             if (!strcmp(entry->name, pd_name))
>> +                     break;
>> +     }
>> +     if (!entry) {
>> +             mutex_unlock(&domain_notify_list_lock);
>> +             pr_err("%s: Invalid pd name\n", __func__);
>> +             return -EINVAL;
>> +     }
>> +     list_del(&entry->cache_list_node);
>
> 'entry' will not be NULL even when the requested notification entry
> was not found above. In such case it looks like last entry in the
> domain_notify_list list will be removed, not something we would expect.
>
>> +     mutex_unlock(&domain_notify_list_lock);
>> +     ret = blocking_notifier_chain_unregister(
>> +                             &genpd_transition_notifier_list, nb);
>> +     return ret;
>> +}
>> +
>> +/**
>> + * genpd_invoke_transition_notifier - Calls the matching notification handler.
>> + * @genpd: generic power domain structure.
>> + * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
>> + */
>> +void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> +                     enum gpd_on_off_state state)
>> +{
>> +     struct cache_notify_domains *entry;
>> +
>> +     if (!genpd) {
>> +             pr_err("Invalid genpd parameter\n");
>> +             return;
>> +     }
>> +
>> +     if (state != GPD_OFF_PRE || state != GPD_OFF_POST
>> +                     || state != GPD_ON_PRE || state != GPD_ON_POST) {
>> +             pr_err("Invalid state parameter\n");
>> +             return;
>> +     }
>> +
>> +     mutex_lock(&domain_notify_list_lock);
>> +     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> +             if (!strcmp(entry->name, genpd->name))
>> +                     break;
>> +     }
>> +     mutex_unlock(&domain_notify_list_lock);
>> +     if (!entry) /* Simply ignore */
>
> Similar issue here, this condition will only be true when the notifications
> list is empty.
>
>> +             return;
>> +
>> +     blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
>> +                             genpd);
>
> And finally regardless of to what power domain the notification user has
> registered it will get notification for each possible power domain in
> the system?  Are the notification users supposed to test the 'genpd'
> argument to react on a specific power domain? If so, how? I guess not by
> nother strcmp() ?
>
>> +}
>>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>>  /*
>>   * Device Tree based PM domain providers.
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index 73e938b..659997f 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -25,6 +25,13 @@ enum gpd_status {
>>       GPD_STATE_POWER_OFF,    /* PM domain is off */
>>  };
>>
>> +enum gpd_on_off_state {
>> +     GPD_OFF_PRE = 0,        /* GPD state before power off */
>
> The assignment is not needed.
yes correct.

Regards,
Amit
>
>> +     GPD_OFF_POST,           /* GPD state after power off */
>> +     GPD_ON_PRE,             /* GPD state before power on */
>> +     GPD_ON_POST,            /* GPD state after power on */
>> +};
>> +
>>  struct dev_power_governor {
>>       bool (*power_down_ok)(struct dev_pm_domain *domain);
>>       bool (*stop_ok)(struct device *dev);
>> @@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
>
> --
> Regards,
> Sylwester
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions

[Sylwester Nawrocki]

On 04/11/14 07:16, amit daniel kachhap wrote:
> On Mon, Nov 3, 2014 at 11:51 PM, Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
>> On 03/11/14 04:53, Amit Daniel Kachhap wrote:
[...]
>>> 4 type of notifications are added,
>>> GPD_OFF_PRE     - GPD state before power off
>>> GPD_OFF_POST    - GPD state after power off
>>> GPD_ON_PRE      - GPD state before power off
>>> GPD_ON_POST     - GPD state after power off
>>>
>>> 3 notfication API's are exported.
>>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>>>                       enum gpd_on_off_state state);
>>>
>>> In this approach the notifiers are registered/unregistered with pd name.
>>> The actual invoking of the notfiers will be done by the platform implementing
>>> power domain enable/disable low level handlers according to the above
>>> defined notification types. This approach will considerably reduce the
>>> number of call to notifiers as compared to calling always from core
>>> powerdomain subsystem.
>>>
>>> Also the registered domain's will be managed inside a cache list and not
>>> part of the genpd structure. This will help in registration of notifiers from
>>> subsystems (such as clock) even when the PD subsystem is still not initialised.
>>> This list also filters out the unregistered pd's requesting notification.
>>
>> This patch is somewhat similar my patch adding power domain state change
>> notifications [1].  I have already a reworked version of that patch, with the
>> notifier list moved to struct generic_pm_domain and using genpd->lock instead
> 
> Yes this will be correct as others also suggested to make per genpd
> notifier block.
> 
>> of dev->power.lock.  Anyway, while I'd leave the decision about the location
>> from where the notifier chains are supposed to be called to the subsystem's
>> maintainers preference, I'm rather reluctant to having one global notifiers
>> list for all possible power domains and all the notification clients.
>> The possibly long list needs to be traversed at each notifier call and it
>> seems in your implementation any registered user of the notification gets
>> notifications for all possible power domains.
>>
>> [1] https://lkml.org/lkml/2014/8/5/182
>
> My fault, I somehow missed this link earlier. After going through
> this, I found it registers genpd from the platform driver, so the
> function signature is
> int pm_genpd_register_notifier(struct device *dev, struct notifier_block *nb);
> I suggest to make the function signature to be like,
> int pm_genpd_register_notifier(struct device_node *np, struct
> notifier_block *nb)
> 
> In this way this function should should be able to support both
> platform devices and non platform devices like clk.
> 
> The function may work like,
> 
> pdev = of_find_device_by_node(np);
> if (pdev) {
>          //  get genpd from device and go ahead with notfier registration.
>          // blocking_notifier_chain_register(genpd->pd_notifier, nb)
> } else
> {
>         // get pd_handle from np
>         // get the pd_name from phandle and try registering this gen pd
>         // if the genpd subsystem is not initialised then add this in
> a temporary list and register the notifier later
> }
> 
> Can you post your implementation with 1st part ? Later I can post the
> else part with my changes.

Hmm, I doubt making the first argument of the notifier registration
function 'struct device_node *' is a good idea. AFAIK we shouldn't be
introducing and requiring device tree in generic interfaces.

We already tried an approach with pending notifier registration list.
I.e. when the power domain is not yet available we put the notification
registration request onto a separate list, which is then traversed
right before the power domain is registered with PM core.
Then we had a separate notification type for the notification client,
to let it know if his registration request has finally failed
permanently (invoked at some late initcall).

It all works, but it's not something we're really happy with. I hope
we can work out some reasonable solution for mainline. I will post
my PD transition notification patch, however it's mostly to demonstrate
the problem. In v2 PD can be also referred to by name, beside allowing
to just pass a struct device * to a device which belongs to a power
domain.

--
Thanks,
Sylwester

[RFC PATCH] pm: Add PM domain state transition notifications

[Sylwester Nawrocki]

This patch adds support for the power domain state transition notifiers.
The notifiers are mostly intended to be used in SoC-specific subsystem,
e.g. clock controller, power management unit, etc. drivers.

It is an attempt to address dependencies between power domains in an SoC
and other subsystems like clocks controller, where the power domain on/off
sequences are relatively complex and involve subsystems which are split
in the kernel across various frameworks/subsystems.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Beata Michalska <b.michalska@samsung.com>
---
The deferred notifier registration is not included in this patch.
---
 Documentation/power/notifiers.txt |   14 ++++
 drivers/base/power/domain.c       |  136 +++++++++++++++++++++++++++++++++++--
 include/linux/pm_domain.h         |   46 +++++++++++++
 3 files changed, 192 insertions(+), 4 deletions(-)

diff --git a/Documentation/power/notifiers.txt b/Documentation/power/notifiers.txt
index a81fa25..e70957d 100644
--- a/Documentation/power/notifiers.txt
+++ b/Documentation/power/notifiers.txt
@@ -53,3 +53,17 @@ NULL).  To register and/or unregister a suspend notifier use the functions
 register_pm_notifier() and unregister_pm_notifier(), respectively, defined in
 include/linux/suspend.h .  If you don't need to unregister the notifier, you can
 also use the pm_notifier() macro defined in include/linux/suspend.h .
+
+Power Domain notifiers
+----------------------
+
+The power domain notifiers allow subsystems or drivers to register for power
+domain on/off notifications, should they need to perform any actions right
+before or right after the power domain on/off.  The device must be already
+added to a power domain before its subsystem or driver registers the notifier.
+Following events are supported:
+
+GPD_NOTIFY_POWER_ON_PREPARE	The power domain is about to turn on.
+GPD_NOTIFY_POST_POWER_ON	The power domain has just turned on.
+GPD_NOTIFY_POWER_OFF_PREPARE	The power domain is about to turn off.
+GPD_NOTIFY_POST_POWER_OFF	The power domain has just turned off.
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4..2cf1b88 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -67,6 +67,110 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 	return genpd;
 }
 
+static int __pm_genpd_do_register_notifier(struct generic_pm_domain *genpd,
+				   struct gpd_notifier_block *gpd_nb)
+{
+	int ret;
+
+	if (IS_ERR_OR_NULL(genpd) || gpd_nb == NULL)
+		return -EINVAL;
+
+	mutex_lock(&genpd->lock);
+	ret = blocking_notifier_chain_cond_register(&genpd->notify_chain_head,
+						&gpd_nb->nb);
+	mutex_unlock(&genpd->lock);
+	return ret;
+}
+
+/**
+ * pm_genpd_register_notifier - Register notifier for the power domain
+ *				state transitions.
+ *
+ * @dev: Device attached to a power domain the state transitions are to be
+ *	 notified for.
+ * @gpd_nb: The notifier chain entry to be registered
+ */
+int pm_genpd_register_notifier(struct device *dev,
+			       struct gpd_notifier_block *gpd_nb)
+{
+	struct generic_pm_domain *genpd = dev_to_genpd(dev);
+
+	return __pm_genpd_do_register_notifier(genpd, gpd_nb);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_register_notifier);
+
+/**
+ * pm_genpd_name_register_notifier - Register a notifier for the power domain
+ *				     state transitions.
+ *
+ * @domain_name: Name of the power domain the notifier is to be registered for.
+ * @gpd_nb: The notifier chain entry to be registered.
+ *
+ */
+int pm_genpd_name_register_notifier(const char *domain_name,
+				    struct gpd_notifier_block *gpd_nb)
+{
+	struct generic_pm_domain *genpd = pm_genpd_lookup_name(domain_name);
+
+	return __pm_genpd_do_register_notifier(genpd, gpd_nb);
+}
+EXPORT_SYMBOL(pm_genpd_name_register_notifier);
+
+static void __pm_genpd_unregister_notifier(struct generic_pm_domain *genpd,
+					   struct gpd_notifier_block *gpd_nb)
+{
+	if (IS_ERR_OR_NULL(genpd) || gpd_nb == NULL)
+		return;
+
+	mutex_lock(&genpd->lock);
+	blocking_notifier_chain_unregister(&genpd->notify_chain_head,
+					  &gpd_nb->nb);
+	mutex_unlock(&genpd->lock);
+}
+
+/**
+ * pm_genpd_unregister_notifier - Unregister a power domain state notifier
+ *
+ * @dev: Device bound to a power domain the notifier is to be unregistered for.
+ * @gpd_nb: The notifier chain entry to be unregistered.
+ */
+void pm_genpd_unregister_notifier(struct device *dev,
+				  struct gpd_notifier_block *gpd_nb)
+{
+	struct generic_pm_domain *genpd =  dev_to_genpd(dev);
+
+	__pm_genpd_unregister_notifier(genpd, gpd_nb);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_unregister_notifier);
+
+/**
+ * pm_genpd_name_unregister_notifier - Unregister a power domain state notifier
+ *
+ * @domain_name: Name of a power domain of which notifier is to be
+ *		 unregistered.
+ * @gpd_nb: The notifier chain entry to be unregistered.
+ */
+void pm_genpd_name_unregister_notifier(const char *domain_name,
+				       struct gpd_notifier_block *gpd_nb)
+{
+	struct generic_pm_domain *genpd = pm_genpd_lookup_name(domain_name);
+
+	__pm_genpd_unregister_notifier(genpd, gpd_nb);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_name_unregister_notifier);
+
+/**
+ * pm_genpd_notifier_call - Call the power domain state transition notify chain
+ *
+ * @event: The state transition event type.
+ * @genpd: The power domain to notify state transition for.
+ */
+static inline void pm_genpd_notifier_call(unsigned long event,
+				   struct generic_pm_domain *genpd)
+{
+	blocking_notifier_call_chain(&genpd->notify_chain_head, event, genpd);
+}
+
 struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {
 	if (IS_ERR_OR_NULL(dev->pm_domain))
@@ -226,10 +330,14 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
 		ktime_t time_start = ktime_get();
 		s64 elapsed_ns;
 
+		pm_genpd_notifier_call(GPD_NOTIFY_POWER_ON_PREPARE, genpd);
+
 		ret = genpd->power_on(genpd);
 		if (ret)
 			goto err;
 
+		pm_genpd_notifier_call(GPD_NOTIFY_POST_POWER_ON, genpd);
+
 		elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
 		if (elapsed_ns > genpd->power_on_latency_ns) {
 			genpd->power_on_latency_ns = elapsed_ns;
@@ -547,6 +655,8 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 		 * the pm_genpd_poweron() restore power for us (this shouldn't
 		 * happen very often).
 		 */
+		pm_genpd_notifier_call(GPD_NOTIFY_POWER_OFF_PREPARE, genpd);
+
 		ret = genpd->power_off(genpd);
 		if (ret == -EBUSY) {
 			genpd_set_active(genpd);
@@ -554,6 +664,9 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 		}
 
 		elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
+
+		pm_genpd_notifier_call(GPD_NOTIFY_POST_POWER_OFF, genpd);
+
 		if (elapsed_ns > genpd->power_off_latency_ns) {
 			genpd->power_off_latency_ns = elapsed_ns;
 			genpd->max_off_time_changed = true;
@@ -796,9 +909,13 @@ static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd)
 	    || atomic_read(&genpd->sd_count) > 0)
 		return;
 
-	if (genpd->power_off)
+	if (genpd->power_off) {
+		pm_genpd_notifier_call(GPD_NOTIFY_POWER_OFF_PREPARE, genpd);
 		genpd->power_off(genpd);
 
+		pm_genpd_notifier_call(GPD_NOTIFY_POST_POWER_OFF, genpd);
+	}
+
 	genpd->status = GPD_STATE_POWER_OFF;
 
 	list_for_each_entry(link, &genpd->slave_links, slave_node) {
@@ -828,8 +945,11 @@ static void pm_genpd_sync_poweron(struct generic_pm_domain *genpd)
 		genpd_sd_counter_inc(link->master);
 	}
 
-	if (genpd->power_on)
+	if (genpd->power_on) {
+		pm_genpd_notifier_call(GPD_NOTIFY_POWER_ON_PREPARE, genpd);
 		genpd->power_on(genpd);
+		pm_genpd_notifier_call(GPD_NOTIFY_POST_POWER_ON, genpd);
+	}
 
 	genpd->status = GPD_STATE_ACTIVE;
 }
@@ -1251,9 +1371,16 @@ static int pm_genpd_restore_noirq(struct device *dev)
 			 * If the domain was off before the hibernation, make
 			 * sure it will be off going forward.
 			 */
-			if (genpd->power_off)
-				genpd->power_off(genpd);
+			if (!genpd->power_off)
+				return 0;
+
+			pm_genpd_notifier_call(GPD_NOTIFY_POWER_OFF_PREPARE,
+					       genpd);
+
+			genpd->power_off(genpd);
 
+			pm_genpd_notifier_call(GPD_NOTIFY_POST_POWER_OFF,
+					       genpd);
 			return 0;
 		}
 	}
@@ -1867,6 +1994,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	INIT_LIST_HEAD(&genpd->master_links);
 	INIT_LIST_HEAD(&genpd->slave_links);
 	INIT_LIST_HEAD(&genpd->dev_list);
+	BLOCKING_INIT_NOTIFIER_HEAD(&genpd->notify_chain_head);
 	mutex_init(&genpd->lock);
 	genpd->gov = gov;
 	INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 73e938b..917494f 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -17,6 +17,12 @@
 #include <linux/notifier.h>
 #include <linux/cpuidle.h>
 
+/* PM domain state transition notifications */
+#define GPD_NOTIFY_POWER_ON_PREPARE	0x01
+#define GPD_NOTIFY_POST_POWER_ON	0x02
+#define GPD_NOTIFY_POWER_OFF_PREPARE	0x03
+#define GPD_NOTIFY_POST_POWER_OFF	0x04
+
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
 	GPD_STATE_WAIT_MASTER,	/* PM domain's master is being waited for */
@@ -49,6 +55,7 @@ struct generic_pm_domain {
 	struct list_head master_links;	/* Links with PM domain as a master */
 	struct list_head slave_links;	/* Links with PM domain as a slave */
 	struct list_head dev_list;	/* List of devices */
+	struct blocking_notifier_head notify_chain_head;
 	struct mutex lock;
 	struct dev_power_governor *gov;
 	struct work_struct power_off_work;
@@ -107,6 +114,20 @@ struct generic_pm_domain_data {
 	bool need_restore;
 };
 
+struct gpd_notifier_block {
+	struct notifier_block nb;
+	/* The notification's user private data */
+	void *data;
+};
+
+static inline void *gpd_get_nb_data(struct notifier_block *nb)
+{
+	struct gpd_notifier_block *genpd_nb;
+
+	genpd_nb = container_of(nb, struct gpd_notifier_block, nb);
+	return genpd_nb->data;
+}
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS
 static inline struct generic_pm_domain_data *to_gpd_data(struct pm_domain_data *pdd)
 {
@@ -146,6 +167,15 @@ extern void pm_genpd_init(struct generic_pm_domain *genpd,
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
 extern int pm_genpd_name_poweron(const char *domain_name);
 
+extern int pm_genpd_register_notifier(struct device *dev,
+				      struct gpd_notifier_block *gpd_nb);
+extern int pm_genpd_name_register_notifier(const char *domain_name,
+				   struct gpd_notifier_block *gpd_nb);
+extern void pm_genpd_unregister_notifier(struct device *dev,
+					struct gpd_notifier_block *gpd_nb);
+extern void pm_genpd_name_unregister_notifier(const char *domain_name,
+					struct gpd_notifier_block *gpd_nb);
+
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
 #else
@@ -219,6 +249,22 @@ static inline int pm_genpd_name_poweron(const char *domain_name)
 {
 	return -ENOSYS;
 }
+static inline int pm_genpd_register_notifier(struct device *dev,
+					struct gpd_notifier_block *genpd_nb)
+{
+	return -ENOSYS;
+}
+
+static int pm_genpd_name_register_notifier(const char *domain_name,
+				    struct gpd_notifier_block *genpd_nb)
+{
+	return -ENOSYS;
+}
+
+static inline void pm_genpd_unregister_notifier(struct device *dev,
+				 struct gpd_notifier_block *genpd_nb) {}
+static inline void pm_genpd_name_unregister_notifier(const char *domain_name,
+				 struct gpd_notifier_block *genpd_nb) {}
 #define simple_qos_governor NULL
 #define pm_domain_always_on_gov NULL
 #endif
-- 
1.7.9.5

[PATCH 04/12] mfd: exynos-pmu: Register exynos-pmu driver as a mfd driver

[Amit Daniel Kachhap]

This can be used later to probe and configure PMU client drivers
like pm_domain etc.
A global structure pmu_dev_client_data is created to pass exynos-pmu
platform data to all the clients. Currently the data passed is register
base addresses.
Although the exynos-pmu driver also provides the sysreg interfaces to
provide register base addresses but this is costlier than readl/writel,
so both the interfaces are provided to mfd based exynos-pmu client
drivers.

Cc: Lee Jones <lee.jones@linaro.org>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/mfd/exynos-pmu.c               |   25 +++++++++++++++++++++++++
 include/linux/mfd/samsung/exynos-pmu.h |    5 +++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/mfd/exynos-pmu.c b/drivers/mfd/exynos-pmu.c
index 07019f0..c46f3f5 100644
--- a/drivers/mfd/exynos-pmu.c
+++ b/drivers/mfd/exynos-pmu.c
@@ -11,12 +11,19 @@
 
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
+#include <linux/mfd/core.h>
 #include <linux/mfd/samsung/exynos-regs-pmu.h>
 #include <linux/mfd/samsung/exynos-pmu.h>
 
 #define PMU_TABLE_END	(-1U)
 
+enum pmu_mfd_list {
+	MFD_POWER_DOMAIN,
+	MFD_MAX,
+};
+
 struct exynos_pmu_conf {
 	unsigned int offset;
 	unsigned int val[NUM_SYS_POWERDOWN];
@@ -33,6 +40,8 @@ struct exynos_pmu_data {
 struct exynos_pmu_context {
 	struct device *dev;
 	const struct exynos_pmu_data *pmu_data;
+	struct mfd_cell cells[MFD_MAX];
+	struct pmu_dev_client_data mfd_data;
 };
 
 static void __iomem *pmu_base_addr;
@@ -482,6 +491,8 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
+	struct mfd_cell *cell;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	pmu_base_addr = devm_ioremap_resource(dev, res);
@@ -506,6 +517,20 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pmu_context);
 
+	/* Initialize and invoke mfd clients */
+	cell = &pmu_context->cells[MFD_POWER_DOMAIN];
+	cell->name = "exynos-pmu-domain";
+	pmu_context->mfd_data.mem_base_addr = pmu_base_addr;
+	pmu_context->mfd_data.mem_size = resource_size(res);
+	cell->platform_data = &pmu_context->mfd_data;
+	cell->pdata_size = sizeof(pmu_context->mfd_data);
+	ret = mfd_add_devices(&pdev->dev, pdev->id, pmu_context->cells, MFD_MAX,
+				NULL, 0, NULL);
+	if (ret) {
+		dev_err(&pdev->dev, "fail to register client devices\n");
+		return ret;
+	}
+
 	dev_dbg(dev, "Exynos PMU Driver probe done\n");
 	return 0;
 }
diff --git a/include/linux/mfd/samsung/exynos-pmu.h b/include/linux/mfd/samsung/exynos-pmu.h
index a2ab0d5..d2f4083 100644
--- a/include/linux/mfd/samsung/exynos-pmu.h
+++ b/include/linux/mfd/samsung/exynos-pmu.h
@@ -19,6 +19,11 @@ enum sys_powerdown {
 	NUM_SYS_POWERDOWN,
 };
 
+struct pmu_dev_client_data {
+	void __iomem *mem_base_addr;
+	unsigned int mem_size;
+};
+
 extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
 
 #endif /* __EXYNOS_PMU_H */
-- 
1.7.9.5

[PATCH 05/12] arm: exynos: Add platform driver support for power domain driver

[Amit Daniel Kachhap]

This patch modifies Exynos Power Domain driver initialization
implementation in following way:

   - Added platform driver support and probe function where Exynos
     PM Domain driver will register itself as MFD PMU client
     driver.
   - This driver will now use the PMU base address with certain offset
     for power domain register address so reg property is not required.
     This change will avoid unnecessary ioremap of the power register.
   - PMU base address is directly accessed and syscon interface is
     not used as PM Domain driver is quite critical and syscon based
     lock protected register access(regmap) is not used.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 .../bindings/arm/exynos/power_domain.txt           |    8 +--
 arch/arm/mach-exynos/pm_domains.c                  |   59 ++++++++++++++------
 2 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index abde1ea..5599017 100644
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
@@ -6,8 +6,8 @@ to gate power to one or more peripherals on the processor.
 Required Properties:
 - compatible: should be one of the following.
     * samsung,exynos4210-pd - for exynos4210 type power domain.
-- reg: physical base address of the controller and length of memory mapped
-    region.
+- pd-offset: this gives the offset of PM power domain register from
+	     the PMU base address.
 - #power-domain-cells: number of cells in power domain specifier;
     must be 0.
 
@@ -30,13 +30,13 @@ Example:
 
 	lcd0: power-domain-lcd0 {
 		compatible = "samsung,exynos4210-pd";
-		reg = <0x10023C00 0x10>;
+		pd-offset = <0x3C00>;
 		#power-domain-cells = <0>;
 	};
 
 	mfc_pd: power-domain@10044060 {
 		compatible = "samsung,exynos4210-pd";
-		reg = <0x10044060 0x20>;
+		pd-offset = <0x4060>;
 		clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MOUT_SW_ACLK333>,
 			<&clock CLK_MOUT_USER_ACLK333>;
 		clock-names = "oscclk", "pclk0", "clk0";
diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c
index 20f2671..923eb57 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -22,6 +22,7 @@
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
 #include <linux/sched.h>
+#include <linux/mfd/samsung/exynos-pmu.h>
 
 #define INT_LOCAL_PWR_EN	0x7
 #define MAX_CLK_PER_DOMAIN	4
@@ -105,33 +106,41 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain)
 	return exynos_pd_power(domain, false);
 }
 
-static __init int exynos4_pm_init_power_domain(void)
+static int exynos_power_domain_probe(struct platform_device *pdev)
 {
-	struct platform_device *pdev;
 	struct device_node *np;
+	struct pmu_dev_client_data *pdata = NULL;
+	void __iomem *pmu_base_addr;
+	unsigned int offset;
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data passed\n");
+		return -EINVAL;
+	}
+
+	pmu_base_addr = pdata->mem_base_addr;
 
 	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
 		struct exynos_pm_domain *pd;
 		int on, i;
-		struct device *dev;
 
-		pdev = of_find_device_by_node(np);
-		dev = &pdev->dev;
-
-		pd = kzalloc(sizeof(*pd), GFP_KERNEL);
-		if (!pd) {
-			pr_err("%s: failed to allocate memory for domain\n",
-					__func__);
+		pd = devm_kzalloc(&pdev->dev, sizeof(*pd), GFP_KERNEL);
+		if (!pd)
 			return -ENOMEM;
-		}
 
 		pd->pd.name = kstrdup(np->name, GFP_KERNEL);
 		pd->name = pd->pd.name;
-		pd->base = of_iomap(np, 0);
+		if (of_property_read_u32(np, "pd-offset", &offset)) {
+			pr_err("%s: failed to find offset for power domain\n",
+					__func__);
+			return -EINVAL;
+		}
+		pd->base = pmu_base_addr + offset;
 		pd->pd.power_off = exynos_pd_power_off;
 		pd->pd.power_on = exynos_pd_power_on;
 
-		pd->oscclk = clk_get(dev, "oscclk");
+		pd->oscclk = of_clk_get_by_name(np, "oscclk");
 		if (IS_ERR(pd->oscclk))
 			goto no_clk;
 
@@ -139,11 +148,11 @@ static __init int exynos4_pm_init_power_domain(void)
 			char clk_name[8];
 
 			snprintf(clk_name, sizeof(clk_name), "clk%d", i);
-			pd->clk[i] = clk_get(dev, clk_name);
+			pd->clk[i] = of_clk_get_by_name(np, clk_name);
 			if (IS_ERR(pd->clk[i]))
 				break;
 			snprintf(clk_name, sizeof(clk_name), "pclk%d", i);
-			pd->pclk[i] = clk_get(dev, clk_name);
+			pd->pclk[i] = of_clk_get_by_name(np, clk_name);
 			if (IS_ERR(pd->pclk[i])) {
 				clk_put(pd->clk[i]);
 				pd->clk[i] = ERR_PTR(-EINVAL);
@@ -163,4 +172,22 @@ no_clk:
 
 	return 0;
 }
-arch_initcall(exynos4_pm_init_power_domain);
+
+static const struct platform_device_id exynos_power_domain_id[] = {
+	{ "exynos-pmu-domain"},
+	{ },
+};
+
+static struct platform_driver exynos_power_domain_driver = {
+	.driver  = {
+		.name   = "exynos-pd",
+	},
+	.probe = exynos_power_domain_probe,
+	.id_table = exynos_power_domain_id,
+};
+
+static int __init exynos_power_domain_init(void)
+{
+	return platform_driver_register(&exynos_power_domain_driver);
+}
+postcore_initcall(exynos_power_domain_init);
-- 
1.7.9.5

[PATCH 06/12] soc: exynos: Move exynos power domain file to driver/soc/samsung folder

[Amit Daniel Kachhap]

This patch moves arch/arm/mach-exynos/pm_domains.c to driver/soc/samsung
folder. This will be useful to use it for both arm and arm64 platforms.
This file is moved in this directory as this driver is soc specific and
only used by exynos platforms.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm/mach-exynos/Makefile                      |    1 -
 drivers/soc/Kconfig                                |    1 +
 drivers/soc/Makefile                               |    1 +
 drivers/soc/samsung/Kconfig                        |   11 +++++++++++
 drivers/soc/samsung/Makefile                       |    1 +
 .../soc/samsung}/pm_domains.c                      |    0
 6 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 drivers/soc/samsung/Kconfig
 create mode 100644 drivers/soc/samsung/Makefile
 rename {arch/arm/mach-exynos => drivers/soc/samsung}/pm_domains.c (100%)

diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index b91b382..9ea6c54 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -13,7 +13,6 @@ obj-$(CONFIG_ARCH_EXYNOS)	+= exynos.o exynos-smc.o firmware.o
 
 obj-$(CONFIG_EXYNOS_CPU_SUSPEND) += pm.o sleep.o
 obj-$(CONFIG_PM_SLEEP)		+= suspend.o
-obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
 
 obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
 
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 76d6bd4..90f33b9 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -3,5 +3,6 @@ menu "SOC (System On Chip) specific Drivers"
 source "drivers/soc/qcom/Kconfig"
 source "drivers/soc/ti/Kconfig"
 source "drivers/soc/versatile/Kconfig"
+source "drivers/soc/samsung/Kconfig"
 
 endmenu
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 063113d..44d220d 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_ARCH_QCOM)		+= qcom/
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
 obj-$(CONFIG_SOC_TI)		+= ti/
 obj-$(CONFIG_PLAT_VERSATILE)	+= versatile/
+obj-$(CONFIG_ARCH_EXYNOS)	+= samsung/
diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
new file mode 100644
index 0000000..1dc6a35
--- /dev/null
+++ b/drivers/soc/samsung/Kconfig
@@ -0,0 +1,11 @@
+#
+# Samsung Soc drivers
+#
+config EXYNOS_POWER_DOMAIN
+	tristate "Support Exynos PM Power Domain"
+	depends on ARCH_EXYNOS && PM_GENERIC_DOMAINS
+	default y
+	help
+	  Exynos SOC have power domain gating support. This driver implements that
+	  functionality and registers all the necessary hooks to the generic power domain
+	  core kernel infrastructure.
diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
new file mode 100644
index 0000000..f4efce7
--- /dev/null
+++ b/drivers/soc/samsung/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_EXYNOS_POWER_DOMAIN)	+=	pm_domains.o
diff --git a/arch/arm/mach-exynos/pm_domains.c b/drivers/soc/samsung/pm_domains.c
similarity index 100%
rename from arch/arm/mach-exynos/pm_domains.c
rename to drivers/soc/samsung/pm_domains.c
-- 
1.7.9.5

[PATCH 07/12] soc: samsung: pm_domain: Use compatible name for power domain name

[Amit Daniel Kachhap]

This patch adds support for second optional compatible complate name. If
this compatible name is present then this name will be used to create
the Power Domain and not the DT node name.
The benefit of this approach is to hook the notifier to the correct
Power Domain.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 .../bindings/arm/exynos/power_domain.txt           |    8 ++++++++
 drivers/soc/samsung/pm_domains.c                   |    7 ++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index 5599017..7250a5c 100644
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
@@ -12,6 +12,8 @@ Required Properties:
     must be 0.
 
 Optional Properties:
+- compatible: This is a second compatible name and gives the complete Power Domain
+	name like "samsung,exynos7-pd-mfc"
 - clocks: List of clock handles. The parent clocks of the input clocks to the
 	devices in this power domain are set to oscclk before power gating
 	and restored back after powering on a domain. This is required for
@@ -43,5 +45,11 @@ Example:
 		#power-domain-cells = <0>;
 	};
 
+	mfc_pd: power-domain@10044060 {
+		compatible = "samsung,exynos4210-pd", "samsung,exynos7-pd-mfc";
+		pd-offset = <0x4060>;
+		#power-domain-cells = <0>;
+	};
+
 See Documentation/devicetree/bindings/power/power_domain.txt for description
 of consumer-side bindings.
diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index 923eb57..e63d129 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -124,12 +124,17 @@ static int exynos_power_domain_probe(struct platform_device *pdev)
 	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
 		struct exynos_pm_domain *pd;
 		int on, i;
+		const char *name;
 
 		pd = devm_kzalloc(&pdev->dev, sizeof(*pd), GFP_KERNEL);
 		if (!pd)
 			return -ENOMEM;
 
-		pd->pd.name = kstrdup(np->name, GFP_KERNEL);
+		if (of_property_read_string_index(np, "compatible", 1, &name)) {
+			/* Second entry not found, use the node name*/
+			name = np->name;
+		}
+		pd->pd.name = kstrdup(name, GFP_KERNEL);
 		pd->name = pd->pd.name;
 		if (of_property_read_u32(np, "pd-offset", &offset)) {
 			pr_err("%s: failed to find offset for power domain\n",
-- 
1.7.9.5

[PATCH 08/12] soc: samsung: pm_domain: Add a new parameter for power configuration

[Amit Daniel Kachhap]

This patch adds a new optional parameter "pd-power-conf" to fetch the
correct power configuration value. In exynos7 soc, this field is
different from previous exynos soc's.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 .../bindings/arm/exynos/power_domain.txt           |    3 +++
 drivers/soc/samsung/pm_domains.c                   |   13 +++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index 7250a5c..07e4ffe 100644
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
@@ -24,6 +24,8 @@ Optional Properties:
 	- pclkN, clkN: Pairs of parent of input clock and input clock to the
 		devices in this power domain. Maximum of 4 pairs (N = 0 to 3)
 		are supported currently.
+- pd-power-conf: This optional parameter provides the local power configuration
+	value for the power domain.
 
 Node of a device using power domains must have a samsung,power-domain property
 defined with a phandle to respective power domain.
@@ -48,6 +50,7 @@ Example:
 	mfc_pd: power-domain@10044060 {
 		compatible = "samsung,exynos4210-pd", "samsung,exynos7-pd-mfc";
 		pd-offset = <0x4060>;
+		pd-power-conf = <0xf>;
 		#power-domain-cells = <0>;
 	};
 
diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index e63d129..84a3f1f 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -34,6 +34,7 @@ struct exynos_pm_domain {
 	void __iomem *base;
 	char const *name;
 	bool is_off;
+	unsigned int pwr_conf_val;
 	struct generic_pm_domain pd;
 	struct clk *oscclk;
 	struct clk *clk[MAX_CLK_PER_DOMAIN];
@@ -63,13 +64,13 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
 		}
 	}
 
-	pwr = power_on ? INT_LOCAL_PWR_EN : 0;
+	pwr = power_on ? pd->pwr_conf_val : 0;
 	__raw_writel(pwr, base);
 
 	/* Wait max 1ms */
 	timeout = 10;
 
-	while ((__raw_readl(base + 0x4) & INT_LOCAL_PWR_EN) != pwr) {
+	while ((__raw_readl(base + 0x4) & pd->pwr_conf_val) != pwr) {
 		if (!timeout) {
 			op = (power_on) ? "enable" : "disable";
 			pr_err("Power domain %s %s failed\n", domain->name, op);
@@ -111,7 +112,7 @@ static int exynos_power_domain_probe(struct platform_device *pdev)
 	struct device_node *np;
 	struct pmu_dev_client_data *pdata = NULL;
 	void __iomem *pmu_base_addr;
-	unsigned int offset;
+	unsigned int offset, pwr_conf;
 
 	pdata = pdev->dev.platform_data;
 	if (!pdata) {
@@ -141,9 +142,13 @@ static int exynos_power_domain_probe(struct platform_device *pdev)
 					__func__);
 			return -EINVAL;
 		}
+		if (of_property_read_u32(np, "pd-power-conf", &pwr_conf))
+			pwr_conf = INT_LOCAL_PWR_EN;
+
 		pd->base = pmu_base_addr + offset;
 		pd->pd.power_off = exynos_pd_power_off;
 		pd->pd.power_on = exynos_pd_power_on;
+		pd->pwr_conf_val = pwr_conf;
 
 		pd->oscclk = of_clk_get_by_name(np, "oscclk");
 		if (IS_ERR(pd->oscclk))
@@ -169,7 +174,7 @@ static int exynos_power_domain_probe(struct platform_device *pdev)
 			clk_put(pd->oscclk);
 
 no_clk:
-		on = __raw_readl(pd->base + 0x4) & INT_LOCAL_PWR_EN;
+		on = __raw_readl(pd->base + 0x4) & pwr_conf;
 
 		pm_genpd_init(&pd->pd, NULL, !on);
 		of_genpd_add_provider_simple(np, &pd->pd);
-- 
1.7.9.5

[PATCH 09/12] soc: samsung: pm_domain: Add support for parent power domain

[Amit Daniel Kachhap]

This patch adds supports for parent power domain. This will ensure
invoking of parent/child power domain on/off in a correct sequence.
In exynos7 SOC's, power domain controllers have parent and child
hierarchy.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 .../bindings/arm/exynos/power_domain.txt           |    2 +
 drivers/soc/samsung/pm_domains.c                   |   42 +++++++++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index 07e4ffe..07752a4 100644
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
@@ -26,6 +26,7 @@ Optional Properties:
 		are supported currently.
 - pd-power-conf: This optional parameter provides the local power configuration
 	value for the power domain.
+- parent: phandle of parent power domain.
 
 Node of a device using power domains must have a samsung,power-domain property
 defined with a phandle to respective power domain.
@@ -51,6 +52,7 @@ Example:
 		compatible = "samsung,exynos4210-pd", "samsung,exynos7-pd-mfc";
 		pd-offset = <0x4060>;
 		pd-power-conf = <0xf>;
+		parent = <&pd_top>;
 		#power-domain-cells = <0>;
 	};
 
diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index 84a3f1f..f14f182 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -26,7 +26,7 @@
 
 #define INT_LOCAL_PWR_EN	0x7
 #define MAX_CLK_PER_DOMAIN	4
-
+#define MAX_PARENT_POWER_DOMAIN	10
 /*
  * Exynos specific wrapper around the generic power domain
  */
@@ -179,7 +179,47 @@ no_clk:
 		pm_genpd_init(&pd->pd, NULL, !on);
 		of_genpd_add_provider_simple(np, &pd->pd);
 	}
+	/* Assign the child power domains to their parents */
+	for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
+		struct device_node *parent_np;
+		int i;
+		struct generic_pm_domain *child_domain, *parent_domain;
+		const char *name;
+
+		if (of_property_read_string_index(np, "compatible", 1,
+					&name)) {
+			/* Second entry not found, use the node name*/
+			name = np->name;
+		}
 
+		child_domain = pm_genpd_lookup_name(name);
+		if (!child_domain)
+			continue;
+		/* search parents in device tree */
+		for (i = 0; i < MAX_PARENT_POWER_DOMAIN; i++) {
+			parent_np = of_parse_phandle(np, "parent", i);
+			if (!parent_np)
+				break;
+
+			if (of_property_read_string_index(parent_np,
+						"compatible", 1, &name)) {
+				/* Second entry not found, use the node name*/
+				name = parent_np->name;
+			}
+
+			parent_domain = pm_genpd_lookup_name(name);
+			if (!parent_domain)
+				break;
+			if (pm_genpd_add_subdomain(parent_domain, child_domain))
+				pr_err("%s failed to add subdomain: %s\n",
+						parent_domain->name,
+						child_domain->name);
+			else
+				pr_info("%s has as child subdomain: %s.\n",
+						parent_domain->name,
+						child_domain->name);
+		}
+	}
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 10/12] soc: samsung: pm_domain: Use the recently added PM Domain notifiers

[Amit Daniel Kachhap]

Invoke the GPD_OFF_PRE/GPD_ON_POST power Domain notifiers. This will
allow some clocks to be saved and restored. These notifiers can also
be invoked from the core power domain files but is not done as exynos is
the only user of these notifiers as of now.
In exynos7 SOC's only GPD_OFF_PRE/GPD_ON_POST are required.

Cc: Kukjin Kim <kgene.kim@samsung.com>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/soc/samsung/pm_domains.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c
index f14f182..11633c2 100644
--- a/drivers/soc/samsung/pm_domains.c
+++ b/drivers/soc/samsung/pm_domains.c
@@ -99,11 +99,16 @@ static int exynos_pd_power(struct generic_pm_domain *domain, bool power_on)
 
 static int exynos_pd_power_on(struct generic_pm_domain *domain)
 {
-	return exynos_pd_power(domain, true);
+	int ret;
+
+	ret = exynos_pd_power(domain, true);
+	genpd_invoke_transition_notifier(domain, GPD_ON_POST);
+	return ret;
 }
 
 static int exynos_pd_power_off(struct generic_pm_domain *domain)
 {
+	genpd_invoke_transition_notifier(domain, GPD_OFF_PRE);
 	return exynos_pd_power(domain, false);
 }
 
-- 
1.7.9.5

[PATCH 11/12] clk: samsung: save and restore clock registers for power domain

[Amit Daniel Kachhap]

This patch adds support for save and restore for clock registers
which loses register contents across power domain off/on sequence.

Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Mike Turquette <mturquette@linaro.org>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/clk/samsung/clk.c |   66 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/samsung/clk.h |   14 ++++++++++
 2 files changed, 80 insertions(+)

diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index dd1f7c9..9fd1369 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -11,10 +11,12 @@
  * clock framework for Samsung platforms.
 */
 
+#include <linux/pm_domain.h>
 #include <linux/syscore_ops.h>
 #include "clk.h"
 
 static LIST_HEAD(clock_reg_cache_list);
+static LIST_HEAD(clock_pd_reg_cache_list);
 
 void samsung_clk_save(void __iomem *base,
 				    struct samsung_clk_reg_dump *rd,
@@ -370,6 +372,67 @@ static void samsung_clk_sleep_init(void __iomem *reg_base,
 		unsigned long nr_rdump) {}
 #endif
 
+static int samsung_power_domain_notifier(struct notifier_block *nb,
+				    unsigned long event, void *data)
+{
+	struct generic_pm_domain *genpd = data;
+	struct samsung_clock_pd_reg_cache *reg_cache;
+
+	switch (event) {
+	case GPD_OFF_PRE:
+		/* save all the pd domain clock registers */
+		list_for_each_entry(reg_cache, &clock_pd_reg_cache_list, node)
+			if (!strcmp(reg_cache->pd_name, genpd->name))
+				samsung_clk_save(reg_cache->reg_base,
+						reg_cache->rdump,
+						reg_cache->rd_num);
+		break;
+	case GPD_ON_POST:
+		/* restore all the pd domain clock registers */
+		list_for_each_entry(reg_cache, &clock_pd_reg_cache_list, node)
+			if (!strcmp(reg_cache->pd_name, genpd->name))
+				samsung_clk_restore(reg_cache->reg_base,
+						reg_cache->rdump,
+						reg_cache->rd_num);
+		break;
+	case GPD_OFF_POST:
+	case GPD_ON_PRE:
+		break;
+	default:
+		pr_err("%s: Invalid pm domain event\n", __func__);
+	}
+	return 0;
+}
+
+/* Notifier for power domain on/off changes */
+static struct notifier_block power_domain_notifier_block = {
+	.notifier_call = samsung_power_domain_notifier,
+};
+
+static void samsung_clk_pd_init(void __iomem *reg_base,
+		const unsigned long *rdump, unsigned long nr_rdump,
+		char *pd_name)
+{
+	struct samsung_clock_pd_reg_cache *pd_reg_cache;
+
+	pd_reg_cache = kzalloc(sizeof(struct samsung_clock_pd_reg_cache),
+			GFP_KERNEL);
+	if (!pd_reg_cache)
+		panic("could not allocate register reg_cache.\n");
+
+	pd_reg_cache->rdump = samsung_clk_alloc_reg_dump(rdump, nr_rdump);
+
+	if (!pd_reg_cache->rdump)
+		panic("could not allocate register dump storage.\n");
+
+	pd_reg_cache->reg_base = reg_base;
+	pd_reg_cache->rd_num = nr_rdump;
+	pd_reg_cache->pd_name = pd_name;
+	list_add_tail(&pd_reg_cache->node, &clock_pd_reg_cache_list);
+
+	genpd_register_notifier(&power_domain_notifier_block, pd_name);
+}
+
 /*
  * Common function which registers plls, muxes, dividers and gates
  * for each CMU. It also add CMU register list to register cache.
@@ -408,6 +471,9 @@ void __init samsung_cmu_register_one(struct device_node *np,
 	if (cmu->clk_regs)
 		samsung_clk_sleep_init(reg_base, cmu->clk_regs,
 			cmu->nr_clk_regs);
+	if (cmu->pd_clk_regs)
+		samsung_clk_pd_init(reg_base, cmu->pd_clk_regs,
+			cmu->nr_clk_regs, cmu->pd_name);
 
 	samsung_clk_of_add_provider(np, ctx);
 }
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 3f471e9..f67f5e5 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -331,6 +331,14 @@ struct samsung_clock_reg_cache {
 	unsigned int rd_num;
 };
 
+struct samsung_clock_pd_reg_cache {
+	struct list_head node;
+	void __iomem *reg_base;
+	struct samsung_clk_reg_dump *rdump;
+	unsigned int rd_num;
+	char *pd_name;
+};
+
 struct samsung_cmu_info {
 	/* list of pll clocks and respective count */
 	struct samsung_pll_clock *pll_clks;
@@ -356,6 +364,12 @@ struct samsung_cmu_info {
 	/* list and number of clocks registers */
 	unsigned long *clk_regs;
 	unsigned int nr_clk_regs;
+
+	/* list and number of clocks to be saved/restored during
+	 * power domain shutdown */
+	char *pd_name;
+	unsigned long *pd_clk_regs;
+	unsigned int nr_pd_clk_regs;
 };
 
 extern struct samsung_clk_provider *__init samsung_clk_init(
-- 
1.7.9.5

[PATCH 12/12] arm64: Kconfig: Enable PM_GENERIC_DOMAINS for exynos7

[Amit Daniel Kachhap]

Use power domain driver for Exynos7 SOC's

Cc: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 arch/arm64/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 693db3e..f537558 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -152,6 +152,7 @@ config ARCH_EXYNOS7
 	bool "ARMv8 based Samsung Exynos7"
 	select ARCH_EXYNOS
 	select COMMON_CLK_SAMSUNG
+	select PM_GENERIC_DOMAINS
 	help
 	  This enables support for Samsung Exynos7 SoC family
 
-- 
1.7.9.5

RE: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

[Pankaj Dubey]

On Wednesday, November 05, 2014 7:18 PM, Sylwester Nawrocki wrote:
> On 04/11/14 04:18, Pankaj Dubey wrote:
> > 2: Since PM domain relies on PMU registers and does not have its own 
> > DT binding, MFD client and MFD device is most suitable for making 
> > this kind of platform drivers.
> 
> We have DT binding for the Exynos power domains:
> Documentation/devicetree/bindings/arm/exynos/power_domain.txt
> In fact the IO memory regions used in the power domain bindings fall 
> within the
> (sparse) PMU registers region.
> 

Correct. But currently there is no platform driver for those DT bindings so we have two options:
1) Register a new platform driver for each of those DT nodes, which are using a very small subset of PMU register region. And we end up getting platform driver's probe for each pd nodes.
2) Register an Exynos power domain driver which will be a client driver of main Exynos PMU driver, and will get platform data from it. And in this probe parse all pd device nodes and populate all related information from those nodes into Exynos_pm_domain struct instances and keep list of these instances. Same has been proposed in Amit's patch.

I would prefer second method, as next, we can move pm sleep functionality (which also uses PMU register sets) also into such small mfd client driver. And as far as I know for pm sleep currently there is no DT nodes.

> There is also DT binding for the PMU subsystem:
> Documentation/devicetree/bindings/arm/samsung/pmu.txt

Yes, and currently I have used same binding to convert PMU implementation into a platform driver [1]. Now what we trying to address is same PMU implementation can be reused for ARM64 based SoC if we have to move it into "drivers/mfd/" or "drivers/power/" or may be "drivers/soc".

[1]: http://www.spinics.net/lists/linux-samsung-soc/msg37572.html

Currently we moved into "drivers/mfd" and patches from Amit shows an use case where power domain implementation has been converted into platform driver, but its probe will be called by MFD subsystem and will not be bind to DT nodes of pd  as it will be a client device to main PMU device. Please check this [2], [3].

[2]: http://www.spinics.net/lists/linux-samsung-soc/msg38446.html
[3]: http://www.spinics.net/lists/linux-samsung-soc/msg38447.html

> 
> I guess a platform device driver in drivers/power would be a better 
> fit than drivers/mfd. I doubt introducing an mfd driver now makes much 
> sense, we have already DT bindings for PHYs, the power domains, etc.
> 

The reason behind making it a mfd driver is, to make following design for pm domain and pm sleep functionality.
			 ----------------------
			| Exynos - PMU  |
			| MFD device      |
			| drivers/mfd/     |
			------------------------
				|
		  -----------------------------------------------
		|					|
	  -------------------------------		------------------------------------
	|  Exynos -pd		      |		|   Exynos - pm-sleep	      |
	|  PMU's client device	      |		|   PMU's client device      |  
	| (drivers/soc/samsung/) |		| (drivers/soc/samsung/) |	
	--------------------------------		---------------------------------
 
As Lee mentioned MFD can be used to register the device ( I am assuming he is referring to use of " mfd_add_devices" from Exynos -PMU driver, to make it a mfd device) but it should be kept outside of "drivers/mfd". I just looked for current users of "mfd_add_devices" but only drivers from "drivers/mfd" only are calling this API, also once we call this API it will become an MFD device then why not keep into "drivers/mfd" or maybe I have little understanding of purpose of "drivers/mfd".

But still if it's OK to register Exynos -PMU as MFD device even if it's not in "drivers/mfd" folder, then I would suggest better to keep it in "drivers/soc/samsung/" as it's very much SoC specific. As "drivers/power" looks like place to keep external power supply drivers such as batteries, AC, USB etc.

Thanks,
Pankaj Dubey

> --
> Thanks,
> Sylwester

Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

[amit daniel kachhap]

On Thu, Nov 6, 2014 at 11:18 AM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> On Wednesday, November 05, 2014 7:18 PM, Sylwester Nawrocki wrote:
>> On 04/11/14 04:18, Pankaj Dubey wrote:
>> > 2: Since PM domain relies on PMU registers and does not have its own
>> > DT binding, MFD client and MFD device is most suitable for making
>> > this kind of platform drivers.
>>
>> We have DT binding for the Exynos power domains:
>> Documentation/devicetree/bindings/arm/exynos/power_domain.txt
>> In fact the IO memory regions used in the power domain bindings fall
>> within the
>> (sparse) PMU registers region.
>>
>
> Correct. But currently there is no platform driver for those DT bindings so we have two options:
> 1) Register a new platform driver for each of those DT nodes, which are using a very small subset of PMU register region. And we end up getting platform driver's probe for each pd nodes.
> 2) Register an Exynos power domain driver which will be a client driver of main Exynos PMU driver, and will get platform data from it. And in this probe parse all pd device nodes and populate all related information from those nodes into Exynos_pm_domain struct instances and keep list of these instances. Same has been proposed in Amit's patch.
>
> I would prefer second method, as next, we can move pm sleep functionality (which also uses PMU register sets) also into such small mfd client driver. And as far as I know for pm sleep currently there is no DT nodes.
>
>> There is also DT binding for the PMU subsystem:
>> Documentation/devicetree/bindings/arm/samsung/pmu.txt
>
> Yes, and currently I have used same binding to convert PMU implementation into a platform driver [1]. Now what we trying to address is same PMU implementation can be reused for ARM64 based SoC if we have to move it into "drivers/mfd/" or "drivers/power/" or may be "drivers/soc".
>
> [1]: http://www.spinics.net/lists/linux-samsung-soc/msg37572.html
>
> Currently we moved into "drivers/mfd" and patches from Amit shows an use case where power domain implementation has been converted into platform driver, but its probe will be called by MFD subsystem and will not be bind to DT nodes of pd  as it will be a client device to main PMU device. Please check this [2], [3].
>
> [2]: http://www.spinics.net/lists/linux-samsung-soc/msg38446.html
> [3]: http://www.spinics.net/lists/linux-samsung-soc/msg38447.html
>
>>
>> I guess a platform device driver in drivers/power would be a better
>> fit than drivers/mfd. I doubt introducing an mfd driver now makes much
>> sense, we have already DT bindings for PHYs, the power domains, etc.
>>
>
> The reason behind making it a mfd driver is, to make following design for pm domain and pm sleep functionality.
>                          ----------------------
>                         | Exynos - PMU  |
>                         | MFD device      |
>                         | drivers/mfd/     |
>                         ------------------------
>                                 |
>                   -----------------------------------------------
>                 |                                       |
>           -------------------------------               ------------------------------------
>         |  Exynos -pd                 |         |   Exynos - pm-sleep         |
>         |  PMU's client device        |         |   PMU's client device      |
>         | (drivers/soc/samsung/) |              | (drivers/soc/samsung/) |
>         --------------------------------                ---------------------------------
I will post this sleep driver shortly. I also suggest to keep
everything in drivers/soc/samsung as all these drivers very much SoC
specific.
>
> As Lee mentioned MFD can be used to register the device ( I am assuming he is referring to use of " mfd_add_devices" from Exynos -PMU driver, to make it a mfd device) but it should be kept outside of "drivers/mfd". I just looked for current users of "mfd_add_devices" but only drivers from "drivers/mfd" only are calling this API, also once we call this API it will become an MFD device then why not keep into "drivers/mfd" or maybe I have little understanding of purpose of "drivers/mfd".
>
> But still if it's OK to register Exynos -PMU as MFD device even if it's not in "drivers/mfd" folder, then I would suggest better to keep it in "drivers/soc/samsung/" as it's very much SoC specific. As "drivers/power" looks like place to keep external power supply drivers such as batteries, AC, USB etc.
>
> Thanks,
> Pankaj Dubey
>
>> --
>> Thanks,
>> Sylwester
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html