[RFC PATCH 2/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[RFC PATCH 2/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

The CLK PM domain assumes that platform code should always provide
list of Connection IDs of the clock (con_id) in
pm_clk_notifier_block structure. Then CLK PM domain uses these con_ids
to setup list of clocks per device.

Such approach is inconvenient when devices can have different number
of clocks. For example, if maximum number of clocks used by
device is 4 the pm_clk_notifier_block structure will look like:

static struct pm_clk_notifier_block platform_domain_notifier = {
	.pm_domain = &keystone_pm_domain,
	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
};

More over, clocks in DT have to be named using only con_ids:
	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
	clock-names = "fck", "opt1", "opt2";

This patch allow to specify custom pm_clk_notifier callback from
platform code and in such way makes CLK PM domain initialization
more flexible. For example, Keystone 2 will provide custom callback
to fill list of clocks for Device with all clocks assigned to this
Device in DT.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/base/power/clock_ops.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 2d5c9c1..c103598 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -520,6 +520,7 @@ void pm_clk_add_notifier(struct bus_type *bus,
 	if (!bus || !clknb)
 		return;
 
-	clknb->nb.notifier_call = pm_clk_notify;
+	if (!clknb->nb.notifier_call)
+		clknb->nb.notifier_call = pm_clk_notify;
 	bus_register_notifier(bus, &clknb->nb);
 }
-- 
1.7.9.5

[RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT

[Grygorii Strashko]

This patch implements custom pm_clk_notifier callback for Keystone 2
CLK PM domain which fills list of clocks for Device with all
clocks assigned to this Device in DT.

After this patch .con_ids field in pm_clk_notifier_block is not
used and there are no limitation for clocks names in DT any more.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/mach-keystone/pm_domain.c |   46 ++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm/mach-keystone/pm_domain.c b/arch/arm/mach-keystone/pm_domain.c
index ca79dda..56542d2 100644
--- a/arch/arm/mach-keystone/pm_domain.c
+++ b/arch/arm/mach-keystone/pm_domain.c
@@ -47,6 +47,51 @@ static int keystone_pm_runtime_resume(struct device *dev)
 
 	return pm_generic_runtime_resume(dev);
 }
+
+static int keystone_pm_clk_notify(struct notifier_block *nb,
+				  unsigned long action, void *data)
+{
+	struct pm_clk_notifier_block *clknb;
+	struct device *dev = data;
+	int error;
+	unsigned int i;
+	struct clk *clk;
+	struct device_node *np = dev->of_node;
+
+	dev_dbg(dev, "%s() %ld\n", __func__, action);
+
+	clknb = container_of(nb, struct pm_clk_notifier_block, nb);
+
+	switch (action) {
+	case BUS_NOTIFY_BIND_DRIVER:
+		if (dev->pm_domain)
+			break;
+
+		error = pm_clk_create(dev);
+		if (error)
+			break;
+
+		dev->pm_domain = clknb->pm_domain;
+
+		for (i = 0; (clk = of_clk_get(np, i)) && !IS_ERR(clk); i++) {
+			if (pm_clk_add_clk(dev, clk))
+				clk_put(clk);
+		}
+
+		break;
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if (dev->pm_domain != clknb->pm_domain)
+			break;
+
+		dev->pm_domain = NULL;
+		pm_clk_destroy(dev);
+		break;
+	}
+
+	return 0;
+}
+#else
+
 #endif
 
 static struct dev_pm_domain keystone_pm_domain = {
@@ -59,6 +104,7 @@ static struct dev_pm_domain keystone_pm_domain = {
 
 static struct pm_clk_notifier_block platform_domain_notifier = {
 	.pm_domain = &keystone_pm_domain,
+	.nb = { .notifier_call = keystone_pm_clk_notify, },
 };
 
 static struct of_device_id of_keystone_table[] = {
-- 
1.7.9.5

[RFC PATCH 1/3] PM / clock_ops: Add pm_clk_add_clk()

[Grygorii Strashko]

From: Geert Uytterhoeven <geert+renesas@glider.be>

The existing pm_clk_add() allows to pass a clock by con_id. However,
when referring to a specific clock from DT, no con_id is available.

Add pm_clk_add_clk(), which allows to specify the struct clk * directly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/base/power/clock_ops.c |   40 ++++++++++++++++++++++++++++++----------
 include/linux/pm_clock.h       |    3 +++
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index b99e6c0..2d5c9c1 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -53,7 +53,8 @@ static inline int __pm_clk_enable(struct device *dev, struct clk *clk)
  */
 static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
 {
-	ce->clk = clk_get(dev, ce->con_id);
+	if (!ce->clk)
+		ce->clk = clk_get(dev, ce->con_id);
 	if (IS_ERR(ce->clk)) {
 		ce->status = PCE_STATUS_ERROR;
 	} else {
@@ -63,15 +64,8 @@ static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
 	}
 }
 
-/**
- * pm_clk_add - Start using a device clock for power management.
- * @dev: Device whose clock is going to be used for power management.
- * @con_id: Connection ID of the clock.
- *
- * Add the clock represented by @con_id to the list of clocks used for
- * the power management of @dev.
- */
-int pm_clk_add(struct device *dev, const char *con_id)
+static int __pm_clk_add(struct device *dev, const char *con_id,
+			struct clk *clk)
 {
 	struct pm_subsys_data *psd = dev_to_psd(dev);
 	struct pm_clock_entry *ce;
@@ -93,6 +87,8 @@ int pm_clk_add(struct device *dev, const char *con_id)
 			kfree(ce);
 			return -ENOMEM;
 		}
+	} else {
+		ce->clk = clk;
 	}
 
 	pm_clk_acquire(dev, ce);
@@ -102,6 +98,30 @@ int pm_clk_add(struct device *dev, const char *con_id)
 	spin_unlock_irq(&psd->lock);
 	return 0;
 }
+/**
+ * pm_clk_add - Start using a device clock for power management.
+ * @dev: Device whose clock is going to be used for power management.
+ * @con_id: Connection ID of the clock.
+ *
+ * Add the clock represented by @con_id to the list of clocks used for
+ * the power management of @dev.
+ */
+int pm_clk_add(struct device *dev, const char *con_id)
+{
+	return __pm_clk_add(dev, con_id, NULL);
+}
+
+/**
+ * pm_clk_add_clk - Start using a device clock for power management.
+ * @dev: Device whose clock is going to be used for power management.
+ * @clk: Clock pointer
+ *
+ * Add the clock to the list of clocks used for the power management of @dev.
+ */
+int pm_clk_add_clk(struct device *dev, struct clk *clk)
+{
+	return __pm_clk_add(dev, NULL, clk);
+}
 
 /**
  * __pm_clk_remove - Destroy PM clock entry.
diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
index 8348866..6981aa2 100644
--- a/include/linux/pm_clock.h
+++ b/include/linux/pm_clock.h
@@ -18,6 +18,8 @@ struct pm_clk_notifier_block {
 	char *con_ids[];
 };
 
+struct clk;
+
 #ifdef CONFIG_PM_CLK
 static inline bool pm_clk_no_clocks(struct device *dev)
 {
@@ -29,6 +31,7 @@ extern void pm_clk_init(struct device *dev);
 extern int pm_clk_create(struct device *dev);
 extern void pm_clk_destroy(struct device *dev);
 extern int pm_clk_add(struct device *dev, const char *con_id);
+extern int pm_clk_add_clk(struct device *dev, struct clk *clk);
 extern void pm_clk_remove(struct device *dev, const char *con_id);
 extern int pm_clk_suspend(struct device *dev);
 extern int pm_clk_resume(struct device *dev);
-- 
1.7.9.5

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi,

The CLK PM domain (clock_ops.c) assumes that platform code should always provide
list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
Then CLK PM domain uses these con_ids to setup list of clocks per device.

Such approach is inconvenient when devices can have different number
of clocks. For example, if maximum number of clocks used by
devices is 4 the pm_clk_notifier_block structure will look like:

static struct pm_clk_notifier_block platform_domain_notifier = {
	.pm_domain = &keystone_pm_domain,
	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
};

More over, clocks in DT have to be named using only above con_ids:
	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
	clock-names = "fck", "opt1", "opt2";

It makes hard to enable/support Runtime PM in case when some HW modules are used
by different SoCs or there are few version of the same SoC, because clock trees
can be changed significantly in all such cases.

This patch set allows to specify custom pm_clk_notifier callback from
platform code and in such way makes CLK PM domain initialization
more flexible. Also, It updates Keystone 2 platform code to provide custom
callback which fills list of clocks for Device with all clocks assigned to this
Device in DT.
More over, It's safe for Keystone 2 to perform CLK PM domain initialization
at device's binding time instead of device's creation time.

I've posted these patches because I need fully enable Runtime PM on Keystone 2
where all HW modules are controlled using clocks only. That's why CLK PM domain 
fits our needs perfectly. Also, on Keystone 2 clocks are initialized early and
they are available at device creation time. The main problems from my side are:
 - every time when support for new HW modules is added the ".con_ids" array 
   need to be checked and updated;
 - restriction for clock's names in DT - to be named using only above con_ids
   It's ugly.

I hope, this approach to be accepted at least as temporal solution until 
the generic solution is not found.

Thanks for your comments!

PS: patch 1 was reused from [1].

Regards,
-grygorii

[1] Previously, same problem was discussed in, but no final solution was accepted:
 "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
 https://lkml.org/lkml/2014/4/24/1118

Geert Uytterhoeven (1):
  PM / clock_ops: Add pm_clk_add_clk()

Grygorii Strashko (2):
  PM / clock_ops: allow to specify custom pm_clk_notifier callback
  ARM: keystone: pm_domain: setup clk pm domain clocks from DT

 arch/arm/mach-keystone/pm_domain.c |   46 ++++++++++++++++++++++++++++++++++++
 drivers/base/power/clock_ops.c     |   43 ++++++++++++++++++++++++---------
 include/linux/pm_clock.h           |    3 +++
 3 files changed, 81 insertions(+), 11 deletions(-)

-- 
1.7.9.5

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi,

The CLK PM domain (clock_ops.c) assumes that platform code should always provide
list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
Then CLK PM domain uses these con_ids to setup list of clocks per device.

Such approach is inconvenient when devices can have different number
of clocks. For example, if maximum number of clocks used by
devices is 4 the pm_clk_notifier_block structure will look like:

static struct pm_clk_notifier_block platform_domain_notifier = {
	.pm_domain = &keystone_pm_domain,
	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
};

More over, clocks in DT have to be named using only above con_ids:
	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
	clock-names = "fck", "opt1", "opt2";

It makes hard to enable/support Runtime PM in case when some HW modules are used
by different SoCs or there are few version of the same SoC, because clock trees
can be changed significantly in all such cases.

This patch set allows to specify custom pm_clk_notifier callback from
platform code and in such way makes CLK PM domain initialization
more flexible. Also, It updates Keystone 2 platform code to provide custom
callback which fills list of clocks for Device with all clocks assigned to this
Device in DT.
More over, It's safe for Keystone 2 to perform CLK PM domain initialization
at device's binding time instead of device's creation time.

I've posted these patches because I need fully enable Runtime PM on Keystone 2
where all HW modules are controlled using clocks only. That's why CLK PM domain 
fits our needs perfectly. Also, on Keystone 2 clocks are initialized early and
they are available at device creation time. The main problems from my side are:
 - every time when support for new HW modules is added the ".con_ids" array 
   need to be checked and updated;
 - restriction for clock's names in DT - to be named using only above con_ids
   It's ugly.

I hope, this approach to be accepted at least as temporal solution until 
the generic solution is not found.

Thanks for your comments!

PS: patch 1 was reused from [1].

Regards,
-grygorii

[1] Previously, same problem was discussed in, but no final solution was accepted:
 "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
 https://lkml.org/lkml/2014/4/24/1118

Geert Uytterhoeven (1):
  PM / clock_ops: Add pm_clk_add_clk()

Grygorii Strashko (2):
  PM / clock_ops: allow to specify custom pm_clk_notifier callback
  ARM: keystone: pm_domain: setup clk pm domain clocks from DT

 arch/arm/mach-keystone/pm_domain.c |   46 ++++++++++++++++++++++++++++++++++++
 drivers/base/power/clock_ops.c     |   43 ++++++++++++++++++++++++---------
 include/linux/pm_clock.h           |    3 +++
 3 files changed, 81 insertions(+), 11 deletions(-)

-- 
1.7.9.5

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi,

The CLK PM domain (clock_ops.c) assumes that platform code should always provide
list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
Then CLK PM domain uses these con_ids to setup list of clocks per device.

Such approach is inconvenient when devices can have different number
of clocks. For example, if maximum number of clocks used by
devices is 4 the pm_clk_notifier_block structure will look like:

static struct pm_clk_notifier_block platform_domain_notifier = {
	.pm_domain = &keystone_pm_domain,
	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
};

More over, clocks in DT have to be named using only above con_ids:
	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
	clock-names = "fck", "opt1", "opt2";

It makes hard to enable/support Runtime PM in case when some HW modules are used
by different SoCs or there are few version of the same SoC, because clock trees
can be changed significantly in all such cases.

This patch set allows to specify custom pm_clk_notifier callback from
platform code and in such way makes CLK PM domain initialization
more flexible. Also, It updates Keystone 2 platform code to provide custom
callback which fills list of clocks for Device with all clocks assigned to this
Device in DT.
More over, It's safe for Keystone 2 to perform CLK PM domain initialization
at device's binding time instead of device's creation time.

I've posted these patches because I need fully enable Runtime PM on Keystone 2
where all HW modules are controlled using clocks only. That's why CLK PM domain 
fits our needs perfectly. Also, on Keystone 2 clocks are initialized early and
they are available at device creation time. The main problems from my side are:
 - every time when support for new HW modules is added the ".con_ids" array 
   need to be checked and updated;
 - restriction for clock's names in DT - to be named using only above con_ids
   It's ugly.

I hope, this approach to be accepted at least as temporal solution until 
the generic solution is not found.

Thanks for your comments!

PS: patch 1 was reused from [1].

Regards,
-grygorii

[1] Previously, same problem was discussed in, but no final solution was accepted:
 "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
 https://lkml.org/lkml/2014/4/24/1118

Geert Uytterhoeven (1):
  PM / clock_ops: Add pm_clk_add_clk()

Grygorii Strashko (2):
  PM / clock_ops: allow to specify custom pm_clk_notifier callback
  ARM: keystone: pm_domain: setup clk pm domain clocks from DT

 arch/arm/mach-keystone/pm_domain.c |   46 ++++++++++++++++++++++++++++++++++++
 drivers/base/power/clock_ops.c     |   43 ++++++++++++++++++++++++---------
 include/linux/pm_clock.h           |    3 +++
 3 files changed, 81 insertions(+), 11 deletions(-)

-- 
1.7.9.5

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi,

The CLK PM domain (clock_ops.c) assumes that platform code should always provide
list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
Then CLK PM domain uses these con_ids to setup list of clocks per device.

Such approach is inconvenient when devices can have different number
of clocks. For example, if maximum number of clocks used by
devices is 4 the pm_clk_notifier_block structure will look like:

static struct pm_clk_notifier_block platform_domain_notifier = {
	.pm_domain = &keystone_pm_domain,
	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
};

More over, clocks in DT have to be named using only above con_ids:
	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
	clock-names = "fck", "opt1", "opt2";

It makes hard to enable/support Runtime PM in case when some HW modules are used
by different SoCs or there are few version of the same SoC, because clock trees
can be changed significantly in all such cases.

This patch set allows to specify custom pm_clk_notifier callback from
platform code and in such way makes CLK PM domain initialization
more flexible. Also, It updates Keystone 2 platform code to provide custom
callback which fills list of clocks for Device with all clocks assigned to this
Device in DT.
More over, It's safe for Keystone 2 to perform CLK PM domain initialization
at device's binding time instead of device's creation time.

I've posted these patches because I need fully enable Runtime PM on Keystone 2
where all HW modules are controlled using clocks only. That's why CLK PM domain 
fits our needs perfectly. Also, on Keystone 2 clocks are initialized early and
they are available at device creation time. The main problems from my side are:
 - every time when support for new HW modules is added the ".con_ids" array 
   need to be checked and updated;
 - restriction for clock's names in DT - to be named using only above con_ids
   It's ugly.

I hope, this approach to be accepted at least as temporal solution until 
the generic solution is not found.

Thanks for your comments!

PS: patch 1 was reused from [1].

Regards,
-grygorii

[1] Previously, same problem was discussed in, but no final solution was accepted:
 "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
 https://lkml.org/lkml/2014/4/24/1118

Geert Uytterhoeven (1):
  PM / clock_ops: Add pm_clk_add_clk()

Grygorii Strashko (2):
  PM / clock_ops: allow to specify custom pm_clk_notifier callback
  ARM: keystone: pm_domain: setup clk pm domain clocks from DT

 arch/arm/mach-keystone/pm_domain.c |   46 ++++++++++++++++++++++++++++++++++++
 drivers/base/power/clock_ops.c     |   43 ++++++++++++++++++++++++---------
 include/linux/pm_clock.h           |    3 +++
 3 files changed, 81 insertions(+), 11 deletions(-)

-- 
1.7.9.5

[RFC PATCH 1/3] PM / clock_ops: Add pm_clk_add_clk()

[Grygorii Strashko]

From: Geert Uytterhoeven <geert+renesas@glider.be>

The existing pm_clk_add() allows to pass a clock by con_id. However,
when referring to a specific clock from DT, no con_id is available.

Add pm_clk_add_clk(), which allows to specify the struct clk * directly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/base/power/clock_ops.c |   40 ++++++++++++++++++++++++++++++----------
 include/linux/pm_clock.h       |    3 +++
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index b99e6c0..2d5c9c1 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -53,7 +53,8 @@ static inline int __pm_clk_enable(struct device *dev, struct clk *clk)
  */
 static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
 {
-	ce->clk = clk_get(dev, ce->con_id);
+	if (!ce->clk)
+		ce->clk = clk_get(dev, ce->con_id);
 	if (IS_ERR(ce->clk)) {
 		ce->status = PCE_STATUS_ERROR;
 	} else {
@@ -63,15 +64,8 @@ static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
 	}
 }
 
-/**
- * pm_clk_add - Start using a device clock for power management.
- * @dev: Device whose clock is going to be used for power management.
- * @con_id: Connection ID of the clock.
- *
- * Add the clock represented by @con_id to the list of clocks used for
- * the power management of @dev.
- */
-int pm_clk_add(struct device *dev, const char *con_id)
+static int __pm_clk_add(struct device *dev, const char *con_id,
+			struct clk *clk)
 {
 	struct pm_subsys_data *psd = dev_to_psd(dev);
 	struct pm_clock_entry *ce;
@@ -93,6 +87,8 @@ int pm_clk_add(struct device *dev, const char *con_id)
 			kfree(ce);
 			return -ENOMEM;
 		}
+	} else {
+		ce->clk = clk;
 	}
 
 	pm_clk_acquire(dev, ce);
@@ -102,6 +98,30 @@ int pm_clk_add(struct device *dev, const char *con_id)
 	spin_unlock_irq(&psd->lock);
 	return 0;
 }
+/**
+ * pm_clk_add - Start using a device clock for power management.
+ * @dev: Device whose clock is going to be used for power management.
+ * @con_id: Connection ID of the clock.
+ *
+ * Add the clock represented by @con_id to the list of clocks used for
+ * the power management of @dev.
+ */
+int pm_clk_add(struct device *dev, const char *con_id)
+{
+	return __pm_clk_add(dev, con_id, NULL);
+}
+
+/**
+ * pm_clk_add_clk - Start using a device clock for power management.
+ * @dev: Device whose clock is going to be used for power management.
+ * @clk: Clock pointer
+ *
+ * Add the clock to the list of clocks used for the power management of @dev.
+ */
+int pm_clk_add_clk(struct device *dev, struct clk *clk)
+{
+	return __pm_clk_add(dev, NULL, clk);
+}
 
 /**
  * __pm_clk_remove - Destroy PM clock entry.
diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
index 8348866..6981aa2 100644
--- a/include/linux/pm_clock.h
+++ b/include/linux/pm_clock.h
@@ -18,6 +18,8 @@ struct pm_clk_notifier_block {
 	char *con_ids[];
 };
 
+struct clk;
+
 #ifdef CONFIG_PM_CLK
 static inline bool pm_clk_no_clocks(struct device *dev)
 {
@@ -29,6 +31,7 @@ extern void pm_clk_init(struct device *dev);
 extern int pm_clk_create(struct device *dev);
 extern void pm_clk_destroy(struct device *dev);
 extern int pm_clk_add(struct device *dev, const char *con_id);
+extern int pm_clk_add_clk(struct device *dev, struct clk *clk);
 extern void pm_clk_remove(struct device *dev, const char *con_id);
 extern int pm_clk_suspend(struct device *dev);
 extern int pm_clk_resume(struct device *dev);
-- 
1.7.9.5

[RFC PATCH 1/3] PM / clock_ops: Add pm_clk_add_clk()

[Grygorii Strashko]

From: Geert Uytterhoeven <geert+renesas@glider.be>

The existing pm_clk_add() allows to pass a clock by con_id. However,
when referring to a specific clock from DT, no con_id is available.

Add pm_clk_add_clk(), which allows to specify the struct clk * directly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/base/power/clock_ops.c |   40 ++++++++++++++++++++++++++++++----------
 include/linux/pm_clock.h       |    3 +++
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index b99e6c0..2d5c9c1 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -53,7 +53,8 @@ static inline int __pm_clk_enable(struct device *dev, struct clk *clk)
  */
 static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
 {
-	ce->clk = clk_get(dev, ce->con_id);
+	if (!ce->clk)
+		ce->clk = clk_get(dev, ce->con_id);
 	if (IS_ERR(ce->clk)) {
 		ce->status = PCE_STATUS_ERROR;
 	} else {
@@ -63,15 +64,8 @@ static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
 	}
 }
 
-/**
- * pm_clk_add - Start using a device clock for power management.
- * @dev: Device whose clock is going to be used for power management.
- * @con_id: Connection ID of the clock.
- *
- * Add the clock represented by @con_id to the list of clocks used for
- * the power management of @dev.
- */
-int pm_clk_add(struct device *dev, const char *con_id)
+static int __pm_clk_add(struct device *dev, const char *con_id,
+			struct clk *clk)
 {
 	struct pm_subsys_data *psd = dev_to_psd(dev);
 	struct pm_clock_entry *ce;
@@ -93,6 +87,8 @@ int pm_clk_add(struct device *dev, const char *con_id)
 			kfree(ce);
 			return -ENOMEM;
 		}
+	} else {
+		ce->clk = clk;
 	}
 
 	pm_clk_acquire(dev, ce);
@@ -102,6 +98,30 @@ int pm_clk_add(struct device *dev, const char *con_id)
 	spin_unlock_irq(&psd->lock);
 	return 0;
 }
+/**
+ * pm_clk_add - Start using a device clock for power management.
+ * @dev: Device whose clock is going to be used for power management.
+ * @con_id: Connection ID of the clock.
+ *
+ * Add the clock represented by @con_id to the list of clocks used for
+ * the power management of @dev.
+ */
+int pm_clk_add(struct device *dev, const char *con_id)
+{
+	return __pm_clk_add(dev, con_id, NULL);
+}
+
+/**
+ * pm_clk_add_clk - Start using a device clock for power management.
+ * @dev: Device whose clock is going to be used for power management.
+ * @clk: Clock pointer
+ *
+ * Add the clock to the list of clocks used for the power management of @dev.
+ */
+int pm_clk_add_clk(struct device *dev, struct clk *clk)
+{
+	return __pm_clk_add(dev, NULL, clk);
+}
 
 /**
  * __pm_clk_remove - Destroy PM clock entry.
diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
index 8348866..6981aa2 100644
--- a/include/linux/pm_clock.h
+++ b/include/linux/pm_clock.h
@@ -18,6 +18,8 @@ struct pm_clk_notifier_block {
 	char *con_ids[];
 };
 
+struct clk;
+
 #ifdef CONFIG_PM_CLK
 static inline bool pm_clk_no_clocks(struct device *dev)
 {
@@ -29,6 +31,7 @@ extern void pm_clk_init(struct device *dev);
 extern int pm_clk_create(struct device *dev);
 extern void pm_clk_destroy(struct device *dev);
 extern int pm_clk_add(struct device *dev, const char *con_id);
+extern int pm_clk_add_clk(struct device *dev, struct clk *clk);
 extern void pm_clk_remove(struct device *dev, const char *con_id);
 extern int pm_clk_suspend(struct device *dev);
 extern int pm_clk_resume(struct device *dev);
-- 
1.7.9.5

[RFC PATCH 1/3] PM / clock_ops: Add pm_clk_add_clk()

[Grygorii Strashko]

From: Geert Uytterhoeven <geert+renesas@glider.be>

The existing pm_clk_add() allows to pass a clock by con_id. However,
when referring to a specific clock from DT, no con_id is available.

Add pm_clk_add_clk(), which allows to specify the struct clk * directly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/base/power/clock_ops.c |   40 ++++++++++++++++++++++++++++++----------
 include/linux/pm_clock.h       |    3 +++
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index b99e6c0..2d5c9c1 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -53,7 +53,8 @@ static inline int __pm_clk_enable(struct device *dev, struct clk *clk)
  */
 static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
 {
-	ce->clk = clk_get(dev, ce->con_id);
+	if (!ce->clk)
+		ce->clk = clk_get(dev, ce->con_id);
 	if (IS_ERR(ce->clk)) {
 		ce->status = PCE_STATUS_ERROR;
 	} else {
@@ -63,15 +64,8 @@ static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
 	}
 }
 
-/**
- * pm_clk_add - Start using a device clock for power management.
- * @dev: Device whose clock is going to be used for power management.
- * @con_id: Connection ID of the clock.
- *
- * Add the clock represented by @con_id to the list of clocks used for
- * the power management of @dev.
- */
-int pm_clk_add(struct device *dev, const char *con_id)
+static int __pm_clk_add(struct device *dev, const char *con_id,
+			struct clk *clk)
 {
 	struct pm_subsys_data *psd = dev_to_psd(dev);
 	struct pm_clock_entry *ce;
@@ -93,6 +87,8 @@ int pm_clk_add(struct device *dev, const char *con_id)
 			kfree(ce);
 			return -ENOMEM;
 		}
+	} else {
+		ce->clk = clk;
 	}
 
 	pm_clk_acquire(dev, ce);
@@ -102,6 +98,30 @@ int pm_clk_add(struct device *dev, const char *con_id)
 	spin_unlock_irq(&psd->lock);
 	return 0;
 }
+/**
+ * pm_clk_add - Start using a device clock for power management.
+ * @dev: Device whose clock is going to be used for power management.
+ * @con_id: Connection ID of the clock.
+ *
+ * Add the clock represented by @con_id to the list of clocks used for
+ * the power management of @dev.
+ */
+int pm_clk_add(struct device *dev, const char *con_id)
+{
+	return __pm_clk_add(dev, con_id, NULL);
+}
+
+/**
+ * pm_clk_add_clk - Start using a device clock for power management.
+ * @dev: Device whose clock is going to be used for power management.
+ * @clk: Clock pointer
+ *
+ * Add the clock to the list of clocks used for the power management of @dev.
+ */
+int pm_clk_add_clk(struct device *dev, struct clk *clk)
+{
+	return __pm_clk_add(dev, NULL, clk);
+}
 
 /**
  * __pm_clk_remove - Destroy PM clock entry.
diff --git a/include/linux/pm_clock.h b/include/linux/pm_clock.h
index 8348866..6981aa2 100644
--- a/include/linux/pm_clock.h
+++ b/include/linux/pm_clock.h
@@ -18,6 +18,8 @@ struct pm_clk_notifier_block {
 	char *con_ids[];
 };
 
+struct clk;
+
 #ifdef CONFIG_PM_CLK
 static inline bool pm_clk_no_clocks(struct device *dev)
 {
@@ -29,6 +31,7 @@ extern void pm_clk_init(struct device *dev);
 extern int pm_clk_create(struct device *dev);
 extern void pm_clk_destroy(struct device *dev);
 extern int pm_clk_add(struct device *dev, const char *con_id);
+extern int pm_clk_add_clk(struct device *dev, struct clk *clk);
 extern void pm_clk_remove(struct device *dev, const char *con_id);
 extern int pm_clk_suspend(struct device *dev);
 extern int pm_clk_resume(struct device *dev);
-- 
1.7.9.5

[RFC PATCH 2/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

The CLK PM domain assumes that platform code should always provide
list of Connection IDs of the clock (con_id) in
pm_clk_notifier_block structure. Then CLK PM domain uses these con_ids
to setup list of clocks per device.

Such approach is inconvenient when devices can have different number
of clocks. For example, if maximum number of clocks used by
device is 4 the pm_clk_notifier_block structure will look like:

static struct pm_clk_notifier_block platform_domain_notifier = {
	.pm_domain = &keystone_pm_domain,
	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
};

More over, clocks in DT have to be named using only con_ids:
	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
	clock-names = "fck", "opt1", "opt2";

This patch allow to specify custom pm_clk_notifier callback from
platform code and in such way makes CLK PM domain initialization
more flexible. For example, Keystone 2 will provide custom callback
to fill list of clocks for Device with all clocks assigned to this
Device in DT.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/base/power/clock_ops.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 2d5c9c1..c103598 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -520,6 +520,7 @@ void pm_clk_add_notifier(struct bus_type *bus,
 	if (!bus || !clknb)
 		return;
 
-	clknb->nb.notifier_call = pm_clk_notify;
+	if (!clknb->nb.notifier_call)
+		clknb->nb.notifier_call = pm_clk_notify;
 	bus_register_notifier(bus, &clknb->nb);
 }
-- 
1.7.9.5

[RFC PATCH 2/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

The CLK PM domain assumes that platform code should always provide
list of Connection IDs of the clock (con_id) in
pm_clk_notifier_block structure. Then CLK PM domain uses these con_ids
to setup list of clocks per device.

Such approach is inconvenient when devices can have different number
of clocks. For example, if maximum number of clocks used by
device is 4 the pm_clk_notifier_block structure will look like:

static struct pm_clk_notifier_block platform_domain_notifier = {
	.pm_domain = &keystone_pm_domain,
	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
};

More over, clocks in DT have to be named using only con_ids:
	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
	clock-names = "fck", "opt1", "opt2";

This patch allow to specify custom pm_clk_notifier callback from
platform code and in such way makes CLK PM domain initialization
more flexible. For example, Keystone 2 will provide custom callback
to fill list of clocks for Device with all clocks assigned to this
Device in DT.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/base/power/clock_ops.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 2d5c9c1..c103598 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -520,6 +520,7 @@ void pm_clk_add_notifier(struct bus_type *bus,
 	if (!bus || !clknb)
 		return;
 
-	clknb->nb.notifier_call = pm_clk_notify;
+	if (!clknb->nb.notifier_call)
+		clknb->nb.notifier_call = pm_clk_notify;
 	bus_register_notifier(bus, &clknb->nb);
 }
-- 
1.7.9.5

[RFC PATCH 2/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

The CLK PM domain assumes that platform code should always provide
list of Connection IDs of the clock (con_id) in
pm_clk_notifier_block structure. Then CLK PM domain uses these con_ids
to setup list of clocks per device.

Such approach is inconvenient when devices can have different number
of clocks. For example, if maximum number of clocks used by
device is 4 the pm_clk_notifier_block structure will look like:

static struct pm_clk_notifier_block platform_domain_notifier = {
	.pm_domain = &keystone_pm_domain,
	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
};

More over, clocks in DT have to be named using only con_ids:
	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
	clock-names = "fck", "opt1", "opt2";

This patch allow to specify custom pm_clk_notifier callback from
platform code and in such way makes CLK PM domain initialization
more flexible. For example, Keystone 2 will provide custom callback
to fill list of clocks for Device with all clocks assigned to this
Device in DT.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/base/power/clock_ops.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 2d5c9c1..c103598 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -520,6 +520,7 @@ void pm_clk_add_notifier(struct bus_type *bus,
 	if (!bus || !clknb)
 		return;
 
-	clknb->nb.notifier_call = pm_clk_notify;
+	if (!clknb->nb.notifier_call)
+		clknb->nb.notifier_call = pm_clk_notify;
 	bus_register_notifier(bus, &clknb->nb);
 }
-- 
1.7.9.5

[RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT

[Grygorii Strashko]

This patch implements custom pm_clk_notifier callback for Keystone 2
CLK PM domain which fills list of clocks for Device with all
clocks assigned to this Device in DT.

After this patch .con_ids field in pm_clk_notifier_block is not
used and there are no limitation for clocks names in DT any more.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/mach-keystone/pm_domain.c |   46 ++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm/mach-keystone/pm_domain.c b/arch/arm/mach-keystone/pm_domain.c
index ca79dda..56542d2 100644
--- a/arch/arm/mach-keystone/pm_domain.c
+++ b/arch/arm/mach-keystone/pm_domain.c
@@ -47,6 +47,51 @@ static int keystone_pm_runtime_resume(struct device *dev)
 
 	return pm_generic_runtime_resume(dev);
 }
+
+static int keystone_pm_clk_notify(struct notifier_block *nb,
+				  unsigned long action, void *data)
+{
+	struct pm_clk_notifier_block *clknb;
+	struct device *dev = data;
+	int error;
+	unsigned int i;
+	struct clk *clk;
+	struct device_node *np = dev->of_node;
+
+	dev_dbg(dev, "%s() %ld\n", __func__, action);
+
+	clknb = container_of(nb, struct pm_clk_notifier_block, nb);
+
+	switch (action) {
+	case BUS_NOTIFY_BIND_DRIVER:
+		if (dev->pm_domain)
+			break;
+
+		error = pm_clk_create(dev);
+		if (error)
+			break;
+
+		dev->pm_domain = clknb->pm_domain;
+
+		for (i = 0; (clk = of_clk_get(np, i)) && !IS_ERR(clk); i++) {
+			if (pm_clk_add_clk(dev, clk))
+				clk_put(clk);
+		}
+
+		break;
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if (dev->pm_domain != clknb->pm_domain)
+			break;
+
+		dev->pm_domain = NULL;
+		pm_clk_destroy(dev);
+		break;
+	}
+
+	return 0;
+}
+#else
+
 #endif
 
 static struct dev_pm_domain keystone_pm_domain = {
@@ -59,6 +104,7 @@ static struct dev_pm_domain keystone_pm_domain = {
 
 static struct pm_clk_notifier_block platform_domain_notifier = {
 	.pm_domain = &keystone_pm_domain,
+	.nb = { .notifier_call = keystone_pm_clk_notify, },
 };
 
 static struct of_device_id of_keystone_table[] = {
-- 
1.7.9.5

[RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT

[Grygorii Strashko]

This patch implements custom pm_clk_notifier callback for Keystone 2
CLK PM domain which fills list of clocks for Device with all
clocks assigned to this Device in DT.

After this patch .con_ids field in pm_clk_notifier_block is not
used and there are no limitation for clocks names in DT any more.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/mach-keystone/pm_domain.c |   46 ++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm/mach-keystone/pm_domain.c b/arch/arm/mach-keystone/pm_domain.c
index ca79dda..56542d2 100644
--- a/arch/arm/mach-keystone/pm_domain.c
+++ b/arch/arm/mach-keystone/pm_domain.c
@@ -47,6 +47,51 @@ static int keystone_pm_runtime_resume(struct device *dev)
 
 	return pm_generic_runtime_resume(dev);
 }
+
+static int keystone_pm_clk_notify(struct notifier_block *nb,
+				  unsigned long action, void *data)
+{
+	struct pm_clk_notifier_block *clknb;
+	struct device *dev = data;
+	int error;
+	unsigned int i;
+	struct clk *clk;
+	struct device_node *np = dev->of_node;
+
+	dev_dbg(dev, "%s() %ld\n", __func__, action);
+
+	clknb = container_of(nb, struct pm_clk_notifier_block, nb);
+
+	switch (action) {
+	case BUS_NOTIFY_BIND_DRIVER:
+		if (dev->pm_domain)
+			break;
+
+		error = pm_clk_create(dev);
+		if (error)
+			break;
+
+		dev->pm_domain = clknb->pm_domain;
+
+		for (i = 0; (clk = of_clk_get(np, i)) && !IS_ERR(clk); i++) {
+			if (pm_clk_add_clk(dev, clk))
+				clk_put(clk);
+		}
+
+		break;
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if (dev->pm_domain != clknb->pm_domain)
+			break;
+
+		dev->pm_domain = NULL;
+		pm_clk_destroy(dev);
+		break;
+	}
+
+	return 0;
+}
+#else
+
 #endif
 
 static struct dev_pm_domain keystone_pm_domain = {
@@ -59,6 +104,7 @@ static struct dev_pm_domain keystone_pm_domain = {
 
 static struct pm_clk_notifier_block platform_domain_notifier = {
 	.pm_domain = &keystone_pm_domain,
+	.nb = { .notifier_call = keystone_pm_clk_notify, },
 };
 
 static struct of_device_id of_keystone_table[] = {
-- 
1.7.9.5

[RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT

[Grygorii Strashko]

This patch implements custom pm_clk_notifier callback for Keystone 2
CLK PM domain which fills list of clocks for Device with all
clocks assigned to this Device in DT.

After this patch .con_ids field in pm_clk_notifier_block is not
used and there are no limitation for clocks names in DT any more.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/mach-keystone/pm_domain.c |   46 ++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm/mach-keystone/pm_domain.c b/arch/arm/mach-keystone/pm_domain.c
index ca79dda..56542d2 100644
--- a/arch/arm/mach-keystone/pm_domain.c
+++ b/arch/arm/mach-keystone/pm_domain.c
@@ -47,6 +47,51 @@ static int keystone_pm_runtime_resume(struct device *dev)
 
 	return pm_generic_runtime_resume(dev);
 }
+
+static int keystone_pm_clk_notify(struct notifier_block *nb,
+				  unsigned long action, void *data)
+{
+	struct pm_clk_notifier_block *clknb;
+	struct device *dev = data;
+	int error;
+	unsigned int i;
+	struct clk *clk;
+	struct device_node *np = dev->of_node;
+
+	dev_dbg(dev, "%s() %ld\n", __func__, action);
+
+	clknb = container_of(nb, struct pm_clk_notifier_block, nb);
+
+	switch (action) {
+	case BUS_NOTIFY_BIND_DRIVER:
+		if (dev->pm_domain)
+			break;
+
+		error = pm_clk_create(dev);
+		if (error)
+			break;
+
+		dev->pm_domain = clknb->pm_domain;
+
+		for (i = 0; (clk = of_clk_get(np, i)) && !IS_ERR(clk); i++) {
+			if (pm_clk_add_clk(dev, clk))
+				clk_put(clk);
+		}
+
+		break;
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if (dev->pm_domain != clknb->pm_domain)
+			break;
+
+		dev->pm_domain = NULL;
+		pm_clk_destroy(dev);
+		break;
+	}
+
+	return 0;
+}
+#else
+
 #endif
 
 static struct dev_pm_domain keystone_pm_domain = {
@@ -59,6 +104,7 @@ static struct dev_pm_domain keystone_pm_domain = {
 
 static struct pm_clk_notifier_block platform_domain_notifier = {
 	.pm_domain = &keystone_pm_domain,
+	.nb = { .notifier_call = keystone_pm_clk_notify, },
 };
 
 static struct of_device_id of_keystone_table[] = {
-- 
1.7.9.5

Re: [RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT

[Kevin Hilman]

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> This patch implements custom pm_clk_notifier callback for Keystone 2
> CLK PM domain which fills list of clocks for Device with all
> clocks assigned to this Device in DT.
>
> After this patch .con_ids field in pm_clk_notifier_block is not
> used and there are no limitation for clocks names in DT any more.

OK, but this also assumes you want *every* clock associated with a
device node managed by runtime PM.  Is that really what you want?  

I may have misunderstood, but your previous attempts at solving this
problem suggest you wanted finer grained control over which device
clocks are runtime PM managed and which ones arent.

Kevin

Re: [RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT

[Kevin Hilman]

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> This patch implements custom pm_clk_notifier callback for Keystone 2
> CLK PM domain which fills list of clocks for Device with all
> clocks assigned to this Device in DT.
>
> After this patch .con_ids field in pm_clk_notifier_block is not
> used and there are no limitation for clocks names in DT any more.

OK, but this also assumes you want *every* clock associated with a
device node managed by runtime PM.  Is that really what you want?  

I may have misunderstood, but your previous attempts at solving this
problem suggest you wanted finer grained control over which device
clocks are runtime PM managed and which ones arent.

Kevin

Re: [RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT

[Kevin Hilman]

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> This patch implements custom pm_clk_notifier callback for Keystone 2
> CLK PM domain which fills list of clocks for Device with all
> clocks assigned to this Device in DT.
>
> After this patch .con_ids field in pm_clk_notifier_block is not
> used and there are no limitation for clocks names in DT any more.

OK, but this also assumes you want *every* clock associated with a
device node managed by runtime PM.  Is that really what you want?  

I may have misunderstood, but your previous attempts at solving this
problem suggest you wanted finer grained control over which device
clocks are runtime PM managed and which ones arent.

Kevin

[RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT

[Kevin Hilman]

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> This patch implements custom pm_clk_notifier callback for Keystone 2
> CLK PM domain which fills list of clocks for Device with all
> clocks assigned to this Device in DT.
>
> After this patch .con_ids field in pm_clk_notifier_block is not
> used and there are no limitation for clocks names in DT any more.

OK, but this also assumes you want *every* clock associated with a
device node managed by runtime PM.  Is that really what you want?  

I may have misunderstood, but your previous attempts at solving this
problem suggest you wanted finer grained control over which device
clocks are runtime PM managed and which ones arent.

Kevin

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Kevin Hilman]

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>
> Such approach is inconvenient when devices can have different number
> of clocks. For example, if maximum number of clocks used by
> devices is 4 the pm_clk_notifier_block structure will look like:
>
> static struct pm_clk_notifier_block platform_domain_notifier = {
> 	.pm_domain = &keystone_pm_domain,
> 	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
> };
>
> More over, clocks in DT have to be named using only above con_ids:
> 	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
> 	clock-names = "fck", "opt1", "opt2";
>
> It makes hard to enable/support Runtime PM in case when some HW modules are used
> by different SoCs or there are few version of the same SoC, because clock trees
> can be changed significantly in all such cases.
>
> This patch set allows to specify custom pm_clk_notifier callback from
> platform code and in such way makes CLK PM domain initialization
> more flexible. 

Replacing the pm_clk notifier is essentially replacing the guts of the
clock_ops code.

I think your platform may have left the realm of "simple" platforms that
the clock_ops was intended for.

Since you're essentially gutting clock_ops, have you considered
migrating to genpd and having your own pm_domain ops that manage your clocks?

> Also, It updates Keystone 2 platform code to provide custom
> callback which fills list of clocks for Device with all clocks assigned to this
> Device in DT.
> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
> at device's binding time instead of device's creation time.
>
> I've posted these patches because I need fully enable Runtime PM on Keystone 2
> where all HW modules are controlled using clocks only. That's why CLK PM domain 
> fits our needs perfectly. 

Heh, doesn't seem like a perfect fit to me ;)

> Also, on Keystone 2 clocks are initialized early and
> they are available at device creation time. The main problems from my side are:
>  - every time when support for new HW modules is added the ".con_ids" array 
>    need to be checked and updated;
>  - restriction for clock's names in DT - to be named using only above con_ids
>    It's ugly.
>
> I hope, this approach to be accepted at least as temporal solution until 
> the generic solution is not found.

Sorry if I missed it, but is there ongoing discussion of a more generic
solution other than this one:

> [1] Previously, same problem was discussed in, but no final solution was accepted:
>  "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>  https://lkml.org/lkml/2014/4/24/1118

Personally, I still like Geert's approach better.

Kevin

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Kevin Hilman]

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>
> Such approach is inconvenient when devices can have different number
> of clocks. For example, if maximum number of clocks used by
> devices is 4 the pm_clk_notifier_block structure will look like:
>
> static struct pm_clk_notifier_block platform_domain_notifier = {
> 	.pm_domain = &keystone_pm_domain,
> 	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
> };
>
> More over, clocks in DT have to be named using only above con_ids:
> 	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
> 	clock-names = "fck", "opt1", "opt2";
>
> It makes hard to enable/support Runtime PM in case when some HW modules are used
> by different SoCs or there are few version of the same SoC, because clock trees
> can be changed significantly in all such cases.
>
> This patch set allows to specify custom pm_clk_notifier callback from
> platform code and in such way makes CLK PM domain initialization
> more flexible. 

Replacing the pm_clk notifier is essentially replacing the guts of the
clock_ops code.

I think your platform may have left the realm of "simple" platforms that
the clock_ops was intended for.

Since you're essentially gutting clock_ops, have you considered
migrating to genpd and having your own pm_domain ops that manage your clocks?

> Also, It updates Keystone 2 platform code to provide custom
> callback which fills list of clocks for Device with all clocks assigned to this
> Device in DT.
> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
> at device's binding time instead of device's creation time.
>
> I've posted these patches because I need fully enable Runtime PM on Keystone 2
> where all HW modules are controlled using clocks only. That's why CLK PM domain 
> fits our needs perfectly. 

Heh, doesn't seem like a perfect fit to me ;)

> Also, on Keystone 2 clocks are initialized early and
> they are available at device creation time. The main problems from my side are:
>  - every time when support for new HW modules is added the ".con_ids" array 
>    need to be checked and updated;
>  - restriction for clock's names in DT - to be named using only above con_ids
>    It's ugly.
>
> I hope, this approach to be accepted at least as temporal solution until 
> the generic solution is not found.

Sorry if I missed it, but is there ongoing discussion of a more generic
solution other than this one:

> [1] Previously, same problem was discussed in, but no final solution was accepted:
>  "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>  https://lkml.org/lkml/2014/4/24/1118

Personally, I still like Geert's approach better.

Kevin

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Kevin Hilman]

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>
> Such approach is inconvenient when devices can have different number
> of clocks. For example, if maximum number of clocks used by
> devices is 4 the pm_clk_notifier_block structure will look like:
>
> static struct pm_clk_notifier_block platform_domain_notifier = {
> 	.pm_domain = &keystone_pm_domain,
> 	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
> };
>
> More over, clocks in DT have to be named using only above con_ids:
> 	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
> 	clock-names = "fck", "opt1", "opt2";
>
> It makes hard to enable/support Runtime PM in case when some HW modules are used
> by different SoCs or there are few version of the same SoC, because clock trees
> can be changed significantly in all such cases.
>
> This patch set allows to specify custom pm_clk_notifier callback from
> platform code and in such way makes CLK PM domain initialization
> more flexible. 

Replacing the pm_clk notifier is essentially replacing the guts of the
clock_ops code.

I think your platform may have left the realm of "simple" platforms that
the clock_ops was intended for.

Since you're essentially gutting clock_ops, have you considered
migrating to genpd and having your own pm_domain ops that manage your clocks?

> Also, It updates Keystone 2 platform code to provide custom
> callback which fills list of clocks for Device with all clocks assigned to this
> Device in DT.
> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
> at device's binding time instead of device's creation time.
>
> I've posted these patches because I need fully enable Runtime PM on Keystone 2
> where all HW modules are controlled using clocks only. That's why CLK PM domain 
> fits our needs perfectly. 

Heh, doesn't seem like a perfect fit to me ;)

> Also, on Keystone 2 clocks are initialized early and
> they are available at device creation time. The main problems from my side are:
>  - every time when support for new HW modules is added the ".con_ids" array 
>    need to be checked and updated;
>  - restriction for clock's names in DT - to be named using only above con_ids
>    It's ugly.
>
> I hope, this approach to be accepted at least as temporal solution until 
> the generic solution is not found.

Sorry if I missed it, but is there ongoing discussion of a more generic
solution other than this one:

> [1] Previously, same problem was discussed in, but no final solution was accepted:
>  "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>  https://lkml.org/lkml/2014/4/24/1118

Personally, I still like Geert's approach better.

Kevin

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Kevin Hilman]

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>
> Such approach is inconvenient when devices can have different number
> of clocks. For example, if maximum number of clocks used by
> devices is 4 the pm_clk_notifier_block structure will look like:
>
> static struct pm_clk_notifier_block platform_domain_notifier = {
> 	.pm_domain = &keystone_pm_domain,
> 	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
> };
>
> More over, clocks in DT have to be named using only above con_ids:
> 	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
> 	clock-names = "fck", "opt1", "opt2";
>
> It makes hard to enable/support Runtime PM in case when some HW modules are used
> by different SoCs or there are few version of the same SoC, because clock trees
> can be changed significantly in all such cases.
>
> This patch set allows to specify custom pm_clk_notifier callback from
> platform code and in such way makes CLK PM domain initialization
> more flexible. 

Replacing the pm_clk notifier is essentially replacing the guts of the
clock_ops code.

I think your platform may have left the realm of "simple" platforms that
the clock_ops was intended for.

Since you're essentially gutting clock_ops, have you considered
migrating to genpd and having your own pm_domain ops that manage your clocks?

> Also, It updates Keystone 2 platform code to provide custom
> callback which fills list of clocks for Device with all clocks assigned to this
> Device in DT.
> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
> at device's binding time instead of device's creation time.
>
> I've posted these patches because I need fully enable Runtime PM on Keystone 2
> where all HW modules are controlled using clocks only. That's why CLK PM domain 
> fits our needs perfectly. 

Heh, doesn't seem like a perfect fit to me ;)

> Also, on Keystone 2 clocks are initialized early and
> they are available at device creation time. The main problems from my side are:
>  - every time when support for new HW modules is added the ".con_ids" array 
>    need to be checked and updated;
>  - restriction for clock's names in DT - to be named using only above con_ids
>    It's ugly.
>
> I hope, this approach to be accepted at least as temporal solution until 
> the generic solution is not found.

Sorry if I missed it, but is there ongoing discussion of a more generic
solution other than this one:

> [1] Previously, same problem was discussed in, but no final solution was accepted:
>  "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>  https://lkml.org/lkml/2014/4/24/1118

Personally, I still like Geert's approach better.

Kevin

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi Kevin,

On 09/09/2014 12:37 AM, Kevin Hilman wrote:
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
> 
>> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
>> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
>> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>>
>> Such approach is inconvenient when devices can have different number
>> of clocks. For example, if maximum number of clocks used by
>> devices is 4 the pm_clk_notifier_block structure will look like:
>>
>> static struct pm_clk_notifier_block platform_domain_notifier = {
>> 	.pm_domain = &keystone_pm_domain,
>> 	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
>> };
>>
>> More over, clocks in DT have to be named using only above con_ids:
>> 	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
>> 	clock-names = "fck", "opt1", "opt2";
>>
>> It makes hard to enable/support Runtime PM in case when some HW modules are used
>> by different SoCs or there are few version of the same SoC, because clock trees
>> can be changed significantly in all such cases.
>>
>> This patch set allows to specify custom pm_clk_notifier callback from
>> platform code and in such way makes CLK PM domain initialization
>> more flexible.
> 
> Replacing the pm_clk notifier is essentially replacing the guts of the
> clock_ops code.
> 
> I think your platform may have left the realm of "simple" platforms that
> the clock_ops was intended for.
> 
> Since you're essentially gutting clock_ops, have you considered
> migrating to genpd and having your own pm_domain ops that manage your clocks?

Yes. I've thought about using genpd. But:
- PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
http://www.spinics.net/lists/arm-kernel/msg357003.html

- To switch using PM domains I will need to create PM domain node PER EACH device,
(if I understand PM domain bindings right). For example:

+power_netcp: power-controller@0 {
+		compatible = "keystone,power-controller";
+		#power-domain-cells = <0>;
+		clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
+	};

netcp: netcp@2090000 {
		reg = <0x2620110 0x8>;
+		power-domains = <&power_netcp>;
...
}

- I'm not sure that such DT- definition will be accepted - 
It's not obviously a HW definitions.

Looks like, I'll have no choice as only switch to genpd even if it will 
introduce code & data over head.

> 
>> Also, It updates Keystone 2 platform code to provide custom
>> callback which fills list of clocks for Device with all clocks assigned to this
>> Device in DT.
>> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
>> at device's binding time instead of device's creation time.
>>
>> I've posted these patches because I need fully enable Runtime PM on Keystone 2
>> where all HW modules are controlled using clocks only. That's why CLK PM domain
>> fits our needs perfectly.
> 
> Heh, doesn't seem like a perfect fit to me ;)
> 
>> Also, on Keystone 2 clocks are initialized early and
>> they are available at device creation time. The main problems from my side are:
>>   - every time when support for new HW modules is added the ".con_ids" array
>>     need to be checked and updated;
>>   - restriction for clock's names in DT - to be named using only above con_ids
>>     It's ugly.
>>
>> I hope, this approach to be accepted at least as temporal solution until
>> the generic solution is not found.
> 
> Sorry if I missed it, but is there ongoing discussion of a more generic
> solution other than this one:

Yep. :( But It's ongoing more than 2 months already and seems there is no
light at the end of tunnel.

> 
>> [1] Previously, same problem was discussed in, but no final solution was accepted:
>>   "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>>   https://lkml.org/lkml/2014/4/24/1118
> 
> Personally, I still like Geert's approach better.
> 

Unfortunately, Geert is not going to continue working on it (as I know).

Thanks for your comments.
I agree. This solution isn't good - it's just fast attempt to solve problem
with minimal changes in common code.


Regards,
-grygorii

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi Kevin,

On 09/09/2014 12:37 AM, Kevin Hilman wrote:
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
> 
>> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
>> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
>> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>>
>> Such approach is inconvenient when devices can have different number
>> of clocks. For example, if maximum number of clocks used by
>> devices is 4 the pm_clk_notifier_block structure will look like:
>>
>> static struct pm_clk_notifier_block platform_domain_notifier = {
>> 	.pm_domain = &keystone_pm_domain,
>> 	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
>> };
>>
>> More over, clocks in DT have to be named using only above con_ids:
>> 	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
>> 	clock-names = "fck", "opt1", "opt2";
>>
>> It makes hard to enable/support Runtime PM in case when some HW modules are used
>> by different SoCs or there are few version of the same SoC, because clock trees
>> can be changed significantly in all such cases.
>>
>> This patch set allows to specify custom pm_clk_notifier callback from
>> platform code and in such way makes CLK PM domain initialization
>> more flexible.
> 
> Replacing the pm_clk notifier is essentially replacing the guts of the
> clock_ops code.
> 
> I think your platform may have left the realm of "simple" platforms that
> the clock_ops was intended for.
> 
> Since you're essentially gutting clock_ops, have you considered
> migrating to genpd and having your own pm_domain ops that manage your clocks?

Yes. I've thought about using genpd. But:
- PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
http://www.spinics.net/lists/arm-kernel/msg357003.html

- To switch using PM domains I will need to create PM domain node PER EACH device,
(if I understand PM domain bindings right). For example:

+power_netcp: power-controller@0 {
+		compatible = "keystone,power-controller";
+		#power-domain-cells = <0>;
+		clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
+	};

netcp: netcp@2090000 {
		reg = <0x2620110 0x8>;
+		power-domains = <&power_netcp>;
...
}

- I'm not sure that such DT- definition will be accepted - 
It's not obviously a HW definitions.

Looks like, I'll have no choice as only switch to genpd even if it will 
introduce code & data over head.

> 
>> Also, It updates Keystone 2 platform code to provide custom
>> callback which fills list of clocks for Device with all clocks assigned to this
>> Device in DT.
>> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
>> at device's binding time instead of device's creation time.
>>
>> I've posted these patches because I need fully enable Runtime PM on Keystone 2
>> where all HW modules are controlled using clocks only. That's why CLK PM domain
>> fits our needs perfectly.
> 
> Heh, doesn't seem like a perfect fit to me ;)
> 
>> Also, on Keystone 2 clocks are initialized early and
>> they are available at device creation time. The main problems from my side are:
>>   - every time when support for new HW modules is added the ".con_ids" array
>>     need to be checked and updated;
>>   - restriction for clock's names in DT - to be named using only above con_ids
>>     It's ugly.
>>
>> I hope, this approach to be accepted at least as temporal solution until
>> the generic solution is not found.
> 
> Sorry if I missed it, but is there ongoing discussion of a more generic
> solution other than this one:

Yep. :( But It's ongoing more than 2 months already and seems there is no
light at the end of tunnel.

> 
>> [1] Previously, same problem was discussed in, but no final solution was accepted:
>>   "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>>   https://lkml.org/lkml/2014/4/24/1118
> 
> Personally, I still like Geert's approach better.
> 

Unfortunately, Geert is not going to continue working on it (as I know).

Thanks for your comments.
I agree. This solution isn't good - it's just fast attempt to solve problem
with minimal changes in common code.


Regards,
-grygorii

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi Kevin,

On 09/09/2014 12:37 AM, Kevin Hilman wrote:
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
> 
>> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
>> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
>> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>>
>> Such approach is inconvenient when devices can have different number
>> of clocks. For example, if maximum number of clocks used by
>> devices is 4 the pm_clk_notifier_block structure will look like:
>>
>> static struct pm_clk_notifier_block platform_domain_notifier = {
>> 	.pm_domain = &keystone_pm_domain,
>> 	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
>> };
>>
>> More over, clocks in DT have to be named using only above con_ids:
>> 	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
>> 	clock-names = "fck", "opt1", "opt2";
>>
>> It makes hard to enable/support Runtime PM in case when some HW modules are used
>> by different SoCs or there are few version of the same SoC, because clock trees
>> can be changed significantly in all such cases.
>>
>> This patch set allows to specify custom pm_clk_notifier callback from
>> platform code and in such way makes CLK PM domain initialization
>> more flexible.
> 
> Replacing the pm_clk notifier is essentially replacing the guts of the
> clock_ops code.
> 
> I think your platform may have left the realm of "simple" platforms that
> the clock_ops was intended for.
> 
> Since you're essentially gutting clock_ops, have you considered
> migrating to genpd and having your own pm_domain ops that manage your clocks?

Yes. I've thought about using genpd. But:
- PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
http://www.spinics.net/lists/arm-kernel/msg357003.html

- To switch using PM domains I will need to create PM domain node PER EACH device,
(if I understand PM domain bindings right). For example:

+power_netcp: power-controller@0 {
+		compatible = "keystone,power-controller";
+		#power-domain-cells = <0>;
+		clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
+	};

netcp: netcp@2090000 {
		reg = <0x2620110 0x8>;
+		power-domains = <&power_netcp>;
...
}

- I'm not sure that such DT- definition will be accepted - 
It's not obviously a HW definitions.

Looks like, I'll have no choice as only switch to genpd even if it will 
introduce code & data over head.

> 
>> Also, It updates Keystone 2 platform code to provide custom
>> callback which fills list of clocks for Device with all clocks assigned to this
>> Device in DT.
>> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
>> at device's binding time instead of device's creation time.
>>
>> I've posted these patches because I need fully enable Runtime PM on Keystone 2
>> where all HW modules are controlled using clocks only. That's why CLK PM domain
>> fits our needs perfectly.
> 
> Heh, doesn't seem like a perfect fit to me ;)
> 
>> Also, on Keystone 2 clocks are initialized early and
>> they are available at device creation time. The main problems from my side are:
>>   - every time when support for new HW modules is added the ".con_ids" array
>>     need to be checked and updated;
>>   - restriction for clock's names in DT - to be named using only above con_ids
>>     It's ugly.
>>
>> I hope, this approach to be accepted at least as temporal solution until
>> the generic solution is not found.
> 
> Sorry if I missed it, but is there ongoing discussion of a more generic
> solution other than this one:

Yep. :( But It's ongoing more than 2 months already and seems there is no
light at the end of tunnel.

> 
>> [1] Previously, same problem was discussed in, but no final solution was accepted:
>>   "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>>   https://lkml.org/lkml/2014/4/24/1118
> 
> Personally, I still like Geert's approach better.
> 

Unfortunately, Geert is not going to continue working on it (as I know).

Thanks for your comments.
I agree. This solution isn't good - it's just fast attempt to solve problem
with minimal changes in common code.


Regards,
-grygorii

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi Kevin,

On 09/09/2014 12:37 AM, Kevin Hilman wrote:
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
> 
>> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
>> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
>> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>>
>> Such approach is inconvenient when devices can have different number
>> of clocks. For example, if maximum number of clocks used by
>> devices is 4 the pm_clk_notifier_block structure will look like:
>>
>> static struct pm_clk_notifier_block platform_domain_notifier = {
>> 	.pm_domain = &keystone_pm_domain,
>> 	.con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
>> };
>>
>> More over, clocks in DT have to be named using only above con_ids:
>> 	clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
>> 	clock-names = "fck", "opt1", "opt2";
>>
>> It makes hard to enable/support Runtime PM in case when some HW modules are used
>> by different SoCs or there are few version of the same SoC, because clock trees
>> can be changed significantly in all such cases.
>>
>> This patch set allows to specify custom pm_clk_notifier callback from
>> platform code and in such way makes CLK PM domain initialization
>> more flexible.
> 
> Replacing the pm_clk notifier is essentially replacing the guts of the
> clock_ops code.
> 
> I think your platform may have left the realm of "simple" platforms that
> the clock_ops was intended for.
> 
> Since you're essentially gutting clock_ops, have you considered
> migrating to genpd and having your own pm_domain ops that manage your clocks?

Yes. I've thought about using genpd. But:
- PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
http://www.spinics.net/lists/arm-kernel/msg357003.html

- To switch using PM domains I will need to create PM domain node PER EACH device,
(if I understand PM domain bindings right). For example:

+power_netcp: power-controller at 0 {
+		compatible = "keystone,power-controller";
+		#power-domain-cells = <0>;
+		clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
+	};

netcp: netcp at 2090000 {
		reg = <0x2620110 0x8>;
+		power-domains = <&power_netcp>;
...
}

- I'm not sure that such DT- definition will be accepted - 
It's not obviously a HW definitions.

Looks like, I'll have no choice as only switch to genpd even if it will 
introduce code & data over head.

> 
>> Also, It updates Keystone 2 platform code to provide custom
>> callback which fills list of clocks for Device with all clocks assigned to this
>> Device in DT.
>> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
>> at device's binding time instead of device's creation time.
>>
>> I've posted these patches because I need fully enable Runtime PM on Keystone 2
>> where all HW modules are controlled using clocks only. That's why CLK PM domain
>> fits our needs perfectly.
> 
> Heh, doesn't seem like a perfect fit to me ;)
> 
>> Also, on Keystone 2 clocks are initialized early and
>> they are available at device creation time. The main problems from my side are:
>>   - every time when support for new HW modules is added the ".con_ids" array
>>     need to be checked and updated;
>>   - restriction for clock's names in DT - to be named using only above con_ids
>>     It's ugly.
>>
>> I hope, this approach to be accepted at least as temporal solution until
>> the generic solution is not found.
> 
> Sorry if I missed it, but is there ongoing discussion of a more generic
> solution other than this one:

Yep. :( But It's ongoing more than 2 months already and seems there is no
light at the end of tunnel.

> 
>> [1] Previously, same problem was discussed in, but no final solution was accepted:
>>   "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>>   https://lkml.org/lkml/2014/4/24/1118
> 
> Personally, I still like Geert's approach better.
> 

Unfortunately, Geert is not going to continue working on it (as I know).

Thanks for your comments.
I agree. This solution isn't good - it's just fast attempt to solve problem
with minimal changes in common code.


Regards,
-grygorii

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Geert Uytterhoeven]

Hi Grygorii,

On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>>> Also, It updates Keystone 2 platform code to provide custom
>>> callback which fills list of clocks for Device with all clocks assigned to this
>>> Device in DT.
>>> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
>>> at device's binding time instead of device's creation time.
>>>
>>> I've posted these patches because I need fully enable Runtime PM on Keystone 2
>>> where all HW modules are controlled using clocks only. That's why CLK PM domain
>>> fits our needs perfectly.
>>
>> Heh, doesn't seem like a perfect fit to me ;)
>>
>>> Also, on Keystone 2 clocks are initialized early and
>>> they are available at device creation time. The main problems from my side are:
>>>   - every time when support for new HW modules is added the ".con_ids" array
>>>     need to be checked and updated;
>>>   - restriction for clock's names in DT - to be named using only above con_ids
>>>     It's ugly.
>>>
>>> I hope, this approach to be accepted at least as temporal solution until
>>> the generic solution is not found.
>>
>> Sorry if I missed it, but is there ongoing discussion of a more generic
>> solution other than this one:
>
> Yep. :( But It's ongoing more than 2 months already and seems there is no
> light at the end of tunnel.
>
>>
>>> [1] Previously, same problem was discussed in, but no final solution was accepted:
>>>   "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>>>   https://lkml.org/lkml/2014/4/24/1118
>>
>> Personally, I still like Geert's approach better.
>>
>
> Unfortunately, Geert is not going to continue working on it (as I know).

For the record, I had postponed working on it, as we had an interim solution for
multiplatform shmobile being broken in mainline (drivers/sh/pm_runtime.c).

Currently I'm tackling "real" power domains. As this is incompatible with
the above hack (you can attach only one power domain to a device, as Ulf
had pointed out before --- Hm, why is the DT property called "power-domains"
(plural) then?), I'll have to revisit anyway, and get back to it.
Furthermore, not all shmobile SoCs have such fine-grained power domains
as sh7372/sh73a0/r8a7740/r8a73a4, so we still need something better there.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Geert Uytterhoeven]

Hi Grygorii,

On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>>> Also, It updates Keystone 2 platform code to provide custom
>>> callback which fills list of clocks for Device with all clocks assigned to this
>>> Device in DT.
>>> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
>>> at device's binding time instead of device's creation time.
>>>
>>> I've posted these patches because I need fully enable Runtime PM on Keystone 2
>>> where all HW modules are controlled using clocks only. That's why CLK PM domain
>>> fits our needs perfectly.
>>
>> Heh, doesn't seem like a perfect fit to me ;)
>>
>>> Also, on Keystone 2 clocks are initialized early and
>>> they are available at device creation time. The main problems from my side are:
>>>   - every time when support for new HW modules is added the ".con_ids" array
>>>     need to be checked and updated;
>>>   - restriction for clock's names in DT - to be named using only above con_ids
>>>     It's ugly.
>>>
>>> I hope, this approach to be accepted at least as temporal solution until
>>> the generic solution is not found.
>>
>> Sorry if I missed it, but is there ongoing discussion of a more generic
>> solution other than this one:
>
> Yep. :( But It's ongoing more than 2 months already and seems there is no
> light at the end of tunnel.
>
>>
>>> [1] Previously, same problem was discussed in, but no final solution was accepted:
>>>   "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>>>   https://lkml.org/lkml/2014/4/24/1118
>>
>> Personally, I still like Geert's approach better.
>>
>
> Unfortunately, Geert is not going to continue working on it (as I know).

For the record, I had postponed working on it, as we had an interim solution for
multiplatform shmobile being broken in mainline (drivers/sh/pm_runtime.c).

Currently I'm tackling "real" power domains. As this is incompatible with
the above hack (you can attach only one power domain to a device, as Ulf
had pointed out before --- Hm, why is the DT property called "power-domains"
(plural) then?), I'll have to revisit anyway, and get back to it.
Furthermore, not all shmobile SoCs have such fine-grained power domains
as sh7372/sh73a0/r8a7740/r8a73a4, so we still need something better there.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Geert Uytterhoeven]

Hi Grygorii,

On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>>> Also, It updates Keystone 2 platform code to provide custom
>>> callback which fills list of clocks for Device with all clocks assigned to this
>>> Device in DT.
>>> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
>>> at device's binding time instead of device's creation time.
>>>
>>> I've posted these patches because I need fully enable Runtime PM on Keystone 2
>>> where all HW modules are controlled using clocks only. That's why CLK PM domain
>>> fits our needs perfectly.
>>
>> Heh, doesn't seem like a perfect fit to me ;)
>>
>>> Also, on Keystone 2 clocks are initialized early and
>>> they are available at device creation time. The main problems from my side are:
>>>   - every time when support for new HW modules is added the ".con_ids" array
>>>     need to be checked and updated;
>>>   - restriction for clock's names in DT - to be named using only above con_ids
>>>     It's ugly.
>>>
>>> I hope, this approach to be accepted at least as temporal solution until
>>> the generic solution is not found.
>>
>> Sorry if I missed it, but is there ongoing discussion of a more generic
>> solution other than this one:
>
> Yep. :( But It's ongoing more than 2 months already and seems there is no
> light at the end of tunnel.
>
>>
>>> [1] Previously, same problem was discussed in, but no final solution was accepted:
>>>   "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>>>   https://lkml.org/lkml/2014/4/24/1118
>>
>> Personally, I still like Geert's approach better.
>>
>
> Unfortunately, Geert is not going to continue working on it (as I know).

For the record, I had postponed working on it, as we had an interim solution for
multiplatform shmobile being broken in mainline (drivers/sh/pm_runtime.c).

Currently I'm tackling "real" power domains. As this is incompatible with
the above hack (you can attach only one power domain to a device, as Ulf
had pointed out before --- Hm, why is the DT property called "power-domains"
(plural) then?), I'll have to revisit anyway, and get back to it.
Furthermore, not all shmobile SoCs have such fine-grained power domains
as sh7372/sh73a0/r8a7740/r8a73a4, so we still need something better there.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Geert Uytterhoeven]

Hi Grygorii,

On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>>> Also, It updates Keystone 2 platform code to provide custom
>>> callback which fills list of clocks for Device with all clocks assigned to this
>>> Device in DT.
>>> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
>>> at device's binding time instead of device's creation time.
>>>
>>> I've posted these patches because I need fully enable Runtime PM on Keystone 2
>>> where all HW modules are controlled using clocks only. That's why CLK PM domain
>>> fits our needs perfectly.
>>
>> Heh, doesn't seem like a perfect fit to me ;)
>>
>>> Also, on Keystone 2 clocks are initialized early and
>>> they are available at device creation time. The main problems from my side are:
>>>   - every time when support for new HW modules is added the ".con_ids" array
>>>     need to be checked and updated;
>>>   - restriction for clock's names in DT - to be named using only above con_ids
>>>     It's ugly.
>>>
>>> I hope, this approach to be accepted at least as temporal solution until
>>> the generic solution is not found.
>>
>> Sorry if I missed it, but is there ongoing discussion of a more generic
>> solution other than this one:
>
> Yep. :( But It's ongoing more than 2 months already and seems there is no
> light at the end of tunnel.
>
>>
>>> [1] Previously, same problem was discussed in, but no final solution was accepted:
>>>   "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
>>>   https://lkml.org/lkml/2014/4/24/1118
>>
>> Personally, I still like Geert's approach better.
>>
>
> Unfortunately, Geert is not going to continue working on it (as I know).

For the record, I had postponed working on it, as we had an interim solution for
multiplatform shmobile being broken in mainline (drivers/sh/pm_runtime.c).

Currently I'm tackling "real" power domains. As this is incompatible with
the above hack (you can attach only one power domain to a device, as Ulf
had pointed out before --- Hm, why is the DT property called "power-domains"
(plural) then?), I'll have to revisit anyway, and get back to it.
Furthermore, not all shmobile SoCs have such fine-grained power domains
as sh7372/sh73a0/r8a7740/r8a73a4, so we still need something better there.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Geert Uytterhoeven]

Hi Grygorii,

On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>> Since you're essentially gutting clock_ops, have you considered
>> migrating to genpd and having your own pm_domain ops that manage your clocks?
>
> Yes. I've thought about using genpd. But:
> - PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
> http://www.spinics.net/lists/arm-kernel/msg357003.html

Let's wait and see...

> - To switch using PM domains I will need to create PM domain node PER EACH device,
> (if I understand PM domain bindings right). For example:
>
> +power_netcp: power-controller@0 {
> +               compatible = "keystone,power-controller";
> +               #power-domain-cells = <0>;
> +               clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
> +       };
>
> netcp: netcp@2090000 {
>                 reg = <0x2620110 0x8>;
> +               power-domains = <&power_netcp>;
> ...
> }

I think you can have one power-controller, and add the clocks and
power-domains properties to the individual device nodes.

Then the power-controller driver can use the attach callback
introduced in "[PATCH/RFC v2 04/12] PM / Domains: Add genpd attach/detach
callbacks" (https://lkml.org/lkml/2014/9/16/429) to set up the clocks?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Geert Uytterhoeven]

Hi Grygorii,

On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>> Since you're essentially gutting clock_ops, have you considered
>> migrating to genpd and having your own pm_domain ops that manage your clocks?
>
> Yes. I've thought about using genpd. But:
> - PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
> http://www.spinics.net/lists/arm-kernel/msg357003.html

Let's wait and see...

> - To switch using PM domains I will need to create PM domain node PER EACH device,
> (if I understand PM domain bindings right). For example:
>
> +power_netcp: power-controller@0 {
> +               compatible = "keystone,power-controller";
> +               #power-domain-cells = <0>;
> +               clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
> +       };
>
> netcp: netcp@2090000 {
>                 reg = <0x2620110 0x8>;
> +               power-domains = <&power_netcp>;
> ...
> }

I think you can have one power-controller, and add the clocks and
power-domains properties to the individual device nodes.

Then the power-controller driver can use the attach callback
introduced in "[PATCH/RFC v2 04/12] PM / Domains: Add genpd attach/detach
callbacks" (https://lkml.org/lkml/2014/9/16/429) to set up the clocks?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Geert Uytterhoeven]

Hi Grygorii,

On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>> Since you're essentially gutting clock_ops, have you considered
>> migrating to genpd and having your own pm_domain ops that manage your clocks?
>
> Yes. I've thought about using genpd. But:
> - PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
> http://www.spinics.net/lists/arm-kernel/msg357003.html

Let's wait and see...

> - To switch using PM domains I will need to create PM domain node PER EACH device,
> (if I understand PM domain bindings right). For example:
>
> +power_netcp: power-controller@0 {
> +               compatible = "keystone,power-controller";
> +               #power-domain-cells = <0>;
> +               clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
> +       };
>
> netcp: netcp@2090000 {
>                 reg = <0x2620110 0x8>;
> +               power-domains = <&power_netcp>;
> ...
> }

I think you can have one power-controller, and add the clocks and
power-domains properties to the individual device nodes.

Then the power-controller driver can use the attach callback
introduced in "[PATCH/RFC v2 04/12] PM / Domains: Add genpd attach/detach
callbacks" (https://lkml.org/lkml/2014/9/16/429) to set up the clocks?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Geert Uytterhoeven]

Hi Grygorii,

On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>> Since you're essentially gutting clock_ops, have you considered
>> migrating to genpd and having your own pm_domain ops that manage your clocks?
>
> Yes. I've thought about using genpd. But:
> - PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
> http://www.spinics.net/lists/arm-kernel/msg357003.html

Let's wait and see...

> - To switch using PM domains I will need to create PM domain node PER EACH device,
> (if I understand PM domain bindings right). For example:
>
> +power_netcp: power-controller at 0 {
> +               compatible = "keystone,power-controller";
> +               #power-domain-cells = <0>;
> +               clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
> +       };
>
> netcp: netcp at 2090000 {
>                 reg = <0x2620110 0x8>;
> +               power-domains = <&power_netcp>;
> ...
> }

I think you can have one power-controller, and add the clocks and
power-domains properties to the individual device nodes.

Then the power-controller driver can use the attach callback
introduced in "[PATCH/RFC v2 04/12] PM / Domains: Add genpd attach/detach
callbacks" (https://lkml.org/lkml/2014/9/16/429) to set up the clocks?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi Geert,

On 09/17/2014 06:03 PM, Geert Uytterhoeven wrote:
> 
> On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>>> Since you're essentially gutting clock_ops, have you considered
>>> migrating to genpd and having your own pm_domain ops that manage your clocks?
>>
>> Yes. I've thought about using genpd. But:
>> - PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
>> http://www.spinics.net/lists/arm-kernel/msg357003.html
> 
> Let's wait and see...
> 
>> - To switch using PM domains I will need to create PM domain node PER EACH device,
>> (if I understand PM domain bindings right). For example:
>>
>> +power_netcp: power-controller@0 {
>> +               compatible = "keystone,power-controller";
>> +               #power-domain-cells = <0>;
>> +               clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
>> +       };
>>
>> netcp: netcp@2090000 {
>>                  reg = <0x2620110 0x8>;
>> +               power-domains = <&power_netcp>;
>> ...
>> }
> 
> I think you can have one power-controller, and add the clocks and
> power-domains properties to the individual device nodes.
> 
> Then the power-controller driver can use the attach callback
> introduced in "[PATCH/RFC v2 04/12] PM / Domains: Add genpd attach/detach
> callbacks" (https://lkml.org/lkml/2014/9/16/429) to set up the clocks?

Thanks for the advice. I saw your series :)
Unfortunately, I think, It can't be used as is, because there still no way
 to separate "functional" and "optional" clocks for the device in DT.
So, I'll need to define some sort of "fck-clocks" or "pm-clocks" properties for each device -
but, it seems to be mostly impossible to obtain the approval of such a decision  :(
 (taking into account current state of discussion
  https://lkml.org/lkml/2014/6/12/441).

There no such problems with Gen PD, but, as drawback:
- I have to define one Gen PD per-device.
- some CLK data duplication is present if Driver code need
  to operate with clocks in legacy way (for example to get current
  CLK  frequency).

Actually, I've done RFC implementation already and I hope to post it soon.
My DT looks like:
+               netcp_domain: netcp_pm_controller {
+                       compatible = "ti,keystone-pm-controller";
+                       clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
+                       #power-domain-cells = <0>;
+               };
+
                netcp: netcp@2090000 {
                        reg = <0x2620110 0x8>;
                        reg-names = "efuse";
@@ -229,6 +252,7 @@
                        #address-cells = <1>;
                        #size-cells = <1>;
                        ranges;
+                       power-domains = <&netcp_domain>;
 
                        clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
                        dma-coherent;


By the way, looks like you did the same thing as I:
I've hacked cock_ops, You - Generic PD ;P
but the goal is the same - add hooks for platform's specific initialization code
to workaround imperfection of clock_ops framework :)

regards,
-grygorii

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi Geert,

On 09/17/2014 06:03 PM, Geert Uytterhoeven wrote:
> 
> On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>>> Since you're essentially gutting clock_ops, have you considered
>>> migrating to genpd and having your own pm_domain ops that manage your clocks?
>>
>> Yes. I've thought about using genpd. But:
>> - PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
>> http://www.spinics.net/lists/arm-kernel/msg357003.html
> 
> Let's wait and see...
> 
>> - To switch using PM domains I will need to create PM domain node PER EACH device,
>> (if I understand PM domain bindings right). For example:
>>
>> +power_netcp: power-controller@0 {
>> +               compatible = "keystone,power-controller";
>> +               #power-domain-cells = <0>;
>> +               clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
>> +       };
>>
>> netcp: netcp@2090000 {
>>                  reg = <0x2620110 0x8>;
>> +               power-domains = <&power_netcp>;
>> ...
>> }
> 
> I think you can have one power-controller, and add the clocks and
> power-domains properties to the individual device nodes.
> 
> Then the power-controller driver can use the attach callback
> introduced in "[PATCH/RFC v2 04/12] PM / Domains: Add genpd attach/detach
> callbacks" (https://lkml.org/lkml/2014/9/16/429) to set up the clocks?

Thanks for the advice. I saw your series :)
Unfortunately, I think, It can't be used as is, because there still no way
 to separate "functional" and "optional" clocks for the device in DT.
So, I'll need to define some sort of "fck-clocks" or "pm-clocks" properties for each device -
but, it seems to be mostly impossible to obtain the approval of such a decision  :(
 (taking into account current state of discussion
  https://lkml.org/lkml/2014/6/12/441).

There no such problems with Gen PD, but, as drawback:
- I have to define one Gen PD per-device.
- some CLK data duplication is present if Driver code need
  to operate with clocks in legacy way (for example to get current
  CLK  frequency).

Actually, I've done RFC implementation already and I hope to post it soon.
My DT looks like:
+               netcp_domain: netcp_pm_controller {
+                       compatible = "ti,keystone-pm-controller";
+                       clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
+                       #power-domain-cells = <0>;
+               };
+
                netcp: netcp@2090000 {
                        reg = <0x2620110 0x8>;
                        reg-names = "efuse";
@@ -229,6 +252,7 @@
                        #address-cells = <1>;
                        #size-cells = <1>;
                        ranges;
+                       power-domains = <&netcp_domain>;
 
                        clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
                        dma-coherent;


By the way, looks like you did the same thing as I:
I've hacked cock_ops, You - Generic PD ;P
but the goal is the same - add hooks for platform's specific initialization code
to workaround imperfection of clock_ops framework :)

regards,
-grygorii

Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi Geert,

On 09/17/2014 06:03 PM, Geert Uytterhoeven wrote:
> 
> On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>>> Since you're essentially gutting clock_ops, have you considered
>>> migrating to genpd and having your own pm_domain ops that manage your clocks?
>>
>> Yes. I've thought about using genpd. But:
>> - PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
>> http://www.spinics.net/lists/arm-kernel/msg357003.html
> 
> Let's wait and see...
> 
>> - To switch using PM domains I will need to create PM domain node PER EACH device,
>> (if I understand PM domain bindings right). For example:
>>
>> +power_netcp: power-controller@0 {
>> +               compatible = "keystone,power-controller";
>> +               #power-domain-cells = <0>;
>> +               clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
>> +       };
>>
>> netcp: netcp@2090000 {
>>                  reg = <0x2620110 0x8>;
>> +               power-domains = <&power_netcp>;
>> ...
>> }
> 
> I think you can have one power-controller, and add the clocks and
> power-domains properties to the individual device nodes.
> 
> Then the power-controller driver can use the attach callback
> introduced in "[PATCH/RFC v2 04/12] PM / Domains: Add genpd attach/detach
> callbacks" (https://lkml.org/lkml/2014/9/16/429) to set up the clocks?

Thanks for the advice. I saw your series :)
Unfortunately, I think, It can't be used as is, because there still no way
 to separate "functional" and "optional" clocks for the device in DT.
So, I'll need to define some sort of "fck-clocks" or "pm-clocks" properties for each device -
but, it seems to be mostly impossible to obtain the approval of such a decision  :(
 (taking into account current state of discussion
  https://lkml.org/lkml/2014/6/12/441).

There no such problems with Gen PD, but, as drawback:
- I have to define one Gen PD per-device.
- some CLK data duplication is present if Driver code need
  to operate with clocks in legacy way (for example to get current
  CLK  frequency).

Actually, I've done RFC implementation already and I hope to post it soon.
My DT looks like:
+               netcp_domain: netcp_pm_controller {
+                       compatible = "ti,keystone-pm-controller";
+                       clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
+                       #power-domain-cells = <0>;
+               };
+
                netcp: netcp@2090000 {
                        reg = <0x2620110 0x8>;
                        reg-names = "efuse";
@@ -229,6 +252,7 @@
                        #address-cells = <1>;
                        #size-cells = <1>;
                        ranges;
+                       power-domains = <&netcp_domain>;
 
                        clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
                        dma-coherent;


By the way, looks like you did the same thing as I:
I've hacked cock_ops, You - Generic PD ;P
but the goal is the same - add hooks for platform's specific initialization code
to workaround imperfection of clock_ops framework :)

regards,
-grygorii

[RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback

[Grygorii Strashko]

Hi Geert,

On 09/17/2014 06:03 PM, Geert Uytterhoeven wrote:
> 
> On Tue, Sep 9, 2014 at 3:41 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>>> Since you're essentially gutting clock_ops, have you considered
>>> migrating to genpd and having your own pm_domain ops that manage your clocks?
>>
>> Yes. I've thought about using genpd. But:
>> - PM domain is not merged in 3.17 and I don't know if it will be merged in 3.18 :(
>> http://www.spinics.net/lists/arm-kernel/msg357003.html
> 
> Let's wait and see...
> 
>> - To switch using PM domains I will need to create PM domain node PER EACH device,
>> (if I understand PM domain bindings right). For example:
>>
>> +power_netcp: power-controller at 0 {
>> +               compatible = "keystone,power-controller";
>> +               #power-domain-cells = <0>;
>> +               clocks = <&clkcpgmac>, <&clkpa>, <&chipclk12>;
>> +       };
>>
>> netcp: netcp at 2090000 {
>>                  reg = <0x2620110 0x8>;
>> +               power-domains = <&power_netcp>;
>> ...
>> }
> 
> I think you can have one power-controller, and add the clocks and
> power-domains properties to the individual device nodes.
> 
> Then the power-controller driver can use the attach callback
> introduced in "[PATCH/RFC v2 04/12] PM / Domains: Add genpd attach/detach
> callbacks" (https://lkml.org/lkml/2014/9/16/429) to set up the clocks?

Thanks for the advice. I saw your series :)
Unfortunately, I think, It can't be used as is, because there still no way
 to separate "functional" and "optional" clocks for the device in DT.
So, I'll need to define some sort of "fck-clocks" or "pm-clocks" properties for each device -
but, it seems to be mostly impossible to obtain the approval of such a decision  :(
 (taking into account current state of discussion
  https://lkml.org/lkml/2014/6/12/441).

There no such problems with Gen PD, but, as drawback:
- I have to define one Gen PD per-device.
- some CLK data duplication is present if Driver code need
  to operate with clocks in legacy way (for example to get current
  CLK  frequency).

Actually, I've done RFC implementation already and I hope to post it soon.
My DT looks like:
+               netcp_domain: netcp_pm_controller {
+                       compatible = "ti,keystone-pm-controller";
+                       clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
+                       #power-domain-cells = <0>;
+               };
+
                netcp: netcp at 2090000 {
                        reg = <0x2620110 0x8>;
                        reg-names = "efuse";
@@ -229,6 +252,7 @@
                        #address-cells = <1>;
                        #size-cells = <1>;
                        ranges;
+                       power-domains = <&netcp_domain>;
 
                        clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
                        dma-coherent;


By the way, looks like you did the same thing as I:
I've hacked cock_ops, You - Generic PD ;P
but the goal is the same - add hooks for platform's specific initialization code
to workaround imperfection of clock_ops framework :)

regards,
-grygorii