Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Hi Grygorii,
I've found this mail deep inside my inbox :-)
On Wednesday 30 July 2014 16:25:31 Grygorii Strashko wrote:
On 07/30/2014 03:06 AM, Laurent Pinchart wrote:
On Monday 28 July 2014 23:52:34 Grant Likely wrote:
On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko wrote:
On 07/28/2014 05:05 PM, Grant Likely wrote:
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko wrote:
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct
device_node *np,
struct clk *clk;
int error;
-for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
-if (!clk_may_runtime_pm(clk)) {
-clk_put(clk);
-continue;
-}
+for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+ !IS_ERR(clk); i++) {
This really looks like an ABI break to me. What happens to all the
existing platforms who don't have this new clkops-clocks in their
device tree?
Agree. This patch as is will break such platforms.
As possible solution for above problem - the NULL can be used as clock's
prefix by default and platform code can configure new value of clock's
prefix during initialization.
In addition, to make this solution full the of_clk_get_by_name() will
need to be modified too.
But note pls, this is pure RFC patches which I did to find out the
answer on questions: - What is better: maintain Runtime PM clocks
configuration in DT or in code?
In code. I don't think it is workable to embed runtime PM behaviour
into the DT bindings. I think there will be too much variance in what
hardware requires. We can create helpers to make this simpler, but I
don't think it is a good idea to set it up automatically without any
control from the driver itself.
- Where and when to call of_clk_register_runtime_pm_clocks()?
Bus notifier/ platform core/ device drivers
I would say in device drivers.
I tend to agree with that.
It will help here to take a step back and remember what the problem we're
trying to solve is.
At the root is clock management. Our system comprise many clocks, and they
need to be handled. The Common Clock Framework nicely models the clocks,
and offers an API for drivers to retrieve device clocks and control them.
Drivers can thus implement clock management manually without much pain.
A clock can be managed in roughly three different ways :
- it can be enabled at probe time and disabled at remove time ;
- it can be enabled right before the device leaves its idle state and
disabled when the device goes back to idle ; or
- it can be enabled and disabled in a more fine-grained, device-specific
manner.
The selected clock management granularity depends on constraints specific
to the device and on how aggressive power saving needs to be. Enabling
the clocks at probe time and disabling them at remove time is enough for
most devices, but leads to a high power consumption. For that reason the
second clock management scheme is often desired.
Managing clocks manually in the driver is a valid option. However, when
adding runtime PM to the equation, and realizing that the clocks need to
be enabled in the runtime PM resume handler and disabled in the suspend
handler, the clock management code starts looking very similar in most
drivers. We're thus tempted to factorize it away from the drivers into a
shared location.
It's important to note at this point that the goal here is only to
simplify drivers. Moving clock management code out of the drivers doesn't
(unless I'm missing something) open the door to new possibilities, it just
serves as a simplification.
Now, as Grygorii mentioned, differences between how a given IP core is
integrated in various SoCs can make clock management SoC-dependent. In the
vast majority of cases (which is really what we need to target, given that
our target is simplifying drivers) SoC integration can be described as a
list of clocks that must be managed. That list can be common to all
devices in a given SoC, or can be device-dependent as well.
That's actually a problem - now we have static list of managed clocks
per-SoC and not per device.
Few locations can be used to express a per-device list of per-SoC clocks.
We can have clocks lists in a per-SoC and per-device location, per-device
clocks lists in an SoC-specific
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Hi Kevin, On Monday 08 September 2014 13:13:25 Kevin Hilman wrote: Laurent Pinchart laurent.pinch...@ideasonboard.com writes: On Monday 28 July 2014 23:52:34 Grant Likely wrote: On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko wrote: On 07/28/2014 05:05 PM, Grant Likely wrote: On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko wrote: [...] - Where and when to call of_clk_register_runtime_pm_clocks()? Bus notifier/ platform core/ device drivers I would say in device drivers. I tend to agree with that. It will help here to take a step back and remember what the problem we're trying to solve is. [jumping in late, after Grygorii ping'd me about looking at this] Laurent, thanks for summarizing the problem so well. It helped me catchup on the discussion. You're welcome. Sorry for the very late reply. At the root is clock management. Our system comprise many clocks, and they need to be handled. The Common Clock Framework nicely models the clocks, and offers an API for drivers to retrieve device clocks and control them. Drivers can thus implement clock management manually without much pain. A clock can be managed in roughly three different ways : - it can be enabled at probe time and disabled at remove time ; - it can be enabled right before the device leaves its idle state and disabled when the device goes back to idle ; or - it can be enabled and disabled in a more fine-grained, device-specific manner. The selected clock management granularity depends on constraints specific to the device and on how aggressive power saving needs to be. Enabling the clocks at probe time and disabling them at remove time is enough for most devices, but leads to a high power consumption. For that reason the second clock management scheme is often desired. Managing clocks manually in the driver is a valid option. However, when adding runtime PM to the equation, and realizing that the clocks need to be enabled in the runtime PM resume handler and disabled in the suspend handler, the clock management code starts looking very similar in most drivers. We're thus tempted to factorize it away from the drivers into a shared location. It's important to note at this point that the goal here is only to simplify drivers. Moving clock management code out of the drivers doesn't (unless I'm missing something) open the door to new possibilities, it just serves as a simplification. I disagree. Actually, it opens up the door to lots of new possibilities that are crucial for fine-grained PM with QoS. It is not just simplification. There are many good reasons that some SoCs have moved all the management of PM-related clocks *out* of device drivers. More on that below... Now, as Grygorii mentioned, differences between how a given IP core is integrated in various SoCs can make clock management SoC-dependent. In the vast majority of cases (which is really what we need to target, given that our target is simplifying drivers) SoC integration can be described as a list of clocks that must be managed. That list can be common to all devices in a given SoC, or can be device-dependent as well. If we care about fine-grained PM, this is a way-too oversimplified version of what SoC integragion means. There are lots of pieces which fall under SoC integration, for example: clock domains, power domains, voltage domains, SoC-specific wakeup capabilities, etc. etc. Then, for fun throw in QoS constraints, and things get really exciting. IOW, if you care about fine-grained PM and QoS, you simply can't reduce SoC integration down to a list of clocks to be managed. Of course. I was talking about SoC integration for clocks, not about SoC integration in general. QoS makes this interesting as well because a device driver's decision to gate its own clocks may have serious repercussions on the wakeup latency of *other* devices in the same power domain. For example, the clock gating of the last active device in a powerdomain may cause the enclosing power- domain to be power gated, having a major impact on the wakup latency of *all* devices in that power domain. So if we're going to manage the list of PM-related clocks in the device driver, we'll also keep track of all the other devices in the same power domain, whether or not they're active, whether or not they have QoS constraints, etc. etc. Hopefully you can see that we're quickly way outside the scope of the IP block that the device driver is intended to manage. All of this is SoC integration knowledge, and IMO doen't belong in the device drivers. It belongs at the SoC integration level, and in todays kernel frameworks that means pm_domain/genpd. Ok, there's more to it than I initially thought. Let's see how we can make this happen then :-) Few locations can be used to express a per-device list of per-SoC clocks. We can have clocks
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Laurent Pinchart laurent.pinch...@ideasonboard.com writes: Hi Grygorii and Grant, On Monday 28 July 2014 23:52:34 Grant Likely wrote: On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko wrote: On 07/28/2014 05:05 PM, Grant Likely wrote: On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko wrote: [...] - Where and when to call of_clk_register_runtime_pm_clocks()? Bus notifier/ platform core/ device drivers I would say in device drivers. I tend to agree with that. It will help here to take a step back and remember what the problem we're trying to solve is. [jumping in late, after Grygorii ping'd me about looking at this] Laurent, thanks for summarizing the problem so well. It helped me catchup on the discussion. At the root is clock management. Our system comprise many clocks, and they need to be handled. The Common Clock Framework nicely models the clocks, and offers an API for drivers to retrieve device clocks and control them. Drivers can thus implement clock management manually without much pain. A clock can be managed in roughly three different ways : - it can be enabled at probe time and disabled at remove time ; - it can be enabled right before the device leaves its idle state and disabled when the device goes back to idle ; or - it can be enabled and disabled in a more fine-grained, device-specific manner. The selected clock management granularity depends on constraints specific to the device and on how aggressive power saving needs to be. Enabling the clocks at probe time and disabling them at remove time is enough for most devices, but leads to a high power consumption. For that reason the second clock management scheme is often desired. Managing clocks manually in the driver is a valid option. However, when adding runtime PM to the equation, and realizing that the clocks need to be enabled in the runtime PM resume handler and disabled in the suspend handler, the clock management code starts looking very similar in most drivers. We're thus tempted to factorize it away from the drivers into a shared location. It's important to note at this point that the goal here is only to simplify drivers. Moving clock management code out of the drivers doesn't (unless I'm missing something) open the door to new possibilities, it just serves as a simplification. I disagree. Actually, it opens up the door to lots of new possibilities that are crucial for fine-grained PM with QoS. It is not just simplification. There are many good reasons that some SoCs have moved all the management of PM-related clocks *out* of device drivers. More on that below... Now, as Grygorii mentioned, differences between how a given IP core is integrated in various SoCs can make clock management SoC-dependent. In the vast majority of cases (which is really what we need to target, given that our target is simplifying drivers) SoC integration can be described as a list of clocks that must be managed. That list can be common to all devices in a given SoC, or can be device-dependent as well. If we care about fine-grained PM, this is a way-too oversimplified version of what SoC integragion means. There are lots of pieces which fall under SoC integration, for example: clock domains, power domains, voltage domains, SoC-specific wakeup capabilities, etc. etc. Then, for fun throw in QoS constraints, and things get really exciting. IOW, if you care about fine-grained PM and QoS, you simply can't reduce SoC integration down to a list of clocks to be managed. QoS makes this interesting as well because a device driver's decision to gate its own clocks may have serious repercussions on the wakeup latency of *other* devices in the same power domain. For example, the clock gating of the last active device in a powerdomain may cause the enclosing power-domain to be power gated, having a major impact on the wakup latency of *all* devices in that power domain. So if we're going to manage the list of PM-related clocks in the device driver, we'll also keep track of all the other devices in the same power domain, whether or not they're active, whether or not they have QoS constraints, etc. etc. Hopefully you can see that we're quickly way outside the scope of the IP block that the device driver is intended to manage. All of this is SoC integration knowledge, and IMO doen't belong in the device drivers. It belongs at the SoC integration level, and in todays kernel frameworks that means pm_domain/genpd. Few locations can be used to express a per-device list of per-SoC clocks. We can have clocks lists in a per-SoC and per-device location, per-device clocks lists in an SoC-specific location, or per-SoC clocks lists in a device- specific location. The first option would require listing clocks to be managed by runtime PM in DT nodes, as proposed by this patch set. I don't think this is the best option, as that information is a mix of
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Hi Laurent,
On Wed, Jul 30, 2014 at 2:06 AM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
The third option would require storing the clocks lists in device drivers. I
believe this is our best option, as a trade-off between simplicity and
versatility. Drivers that use runtime PM already need to enable it explicitly
when probing devices. Passing a list of clock names to runtime PM at that
point wouldn't complicate drivers much. When the clocks list isn't SoC-
dependent it could be stored as static information. Otherwise it could be
derived from DT (or any other source of hardware description) using C code,
offering all the versatility we need.
The only drawback of this solution I can think of right now is that the
runtime PM core couldn't manage device clocks before probing the device.
Specifically device clocks couldn't be managed if no driver is loaded for that
device. I somehow recall that someone raised this as being a problem, but I
can't remember why.
Perhaps you're thinking of clocks that were enabled (by the boot loader or
implicit reset state) before running Linux, and aren't disabled?
That was fixed by commit bb178da701382a230e26d90cf94e8a24b280e0d9
(clk: shmobile: mstp: Fix the is_enabled() operation).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Hi Geert, On Monday 04 August 2014 13:28:32 Geert Uytterhoeven wrote: On Wed, Jul 30, 2014 at 2:06 AM, Laurent Pinchart wrote: The third option would require storing the clocks lists in device drivers. I believe this is our best option, as a trade-off between simplicity and versatility. Drivers that use runtime PM already need to enable it explicitly when probing devices. Passing a list of clock names to runtime PM at that point wouldn't complicate drivers much. When the clocks list isn't SoC- dependent it could be stored as static information. Otherwise it could be derived from DT (or any other source of hardware description) using C code, offering all the versatility we need. The only drawback of this solution I can think of right now is that the runtime PM core couldn't manage device clocks before probing the device. Specifically device clocks couldn't be managed if no driver is loaded for that device. I somehow recall that someone raised this as being a problem, but I can't remember why. Perhaps you're thinking of clocks that were enabled (by the boot loader or implicit reset state) before running Linux, and aren't disabled? That wasn't the reason, I know that clk_disable_unused() takes care of that problem (provided the clock drivers behave correctly, the commit you mention below shows that's not always the case, but that's an unrelated issue). That was fixed by commit bb178da701382a230e26d90cf94e8a24b280e0d9 (clk: shmobile: mstp: Fix the is_enabled() operation). -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Hi Laurent,
On 07/30/2014 03:06 AM, Laurent Pinchart wrote:
Hi Grygorii and Grant,
On Monday 28 July 2014 23:52:34 Grant Likely wrote:
On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko wrote:
On 07/28/2014 05:05 PM, Grant Likely wrote:
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko wrote:
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct
device_node *np,
struct clk *clk;
int error;
-for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
-if (!clk_may_runtime_pm(clk)) {
-clk_put(clk);
-continue;
-}
+for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+ !IS_ERR(clk); i++) {
This really looks like an ABI break to me. What happens to all the
existing platforms who don't have this new clkops-clocks in their device
tree?
Agree. This patch as is will break such platforms.
As possible solution for above problem - the NULL can be used as clock's
prefix by default and platform code can configure new value of clock's
prefix during initialization.
In addition, to make this solution full the of_clk_get_by_name() will
need to be modified too.
But note pls, this is pure RFC patches which I did to find out the answer
on questions: - What is better: maintain Runtime PM clocks configuration
in DT or in code?
In code. I don't think it is workable to embed runtime PM behaviour
into the DT bindings. I think there will be too much variance in what
hardware requires. We can create helpers to make this simpler, but I
don't think it is a good idea to set it up automatically without any
control from the driver itself.
- Where and when to call of_clk_register_runtime_pm_clocks()?
Bus notifier/ platform core/ device drivers
I would say in device drivers.
I tend to agree with that.
It will help here to take a step back and remember what the problem we're
trying to solve is.
At the root is clock management. Our system comprise many clocks, and they
need to be handled. The Common Clock Framework nicely models the clocks, and
offers an API for drivers to retrieve device clocks and control them. Drivers
can thus implement clock management manually without much pain.
A clock can be managed in roughly three different ways :
- it can be enabled at probe time and disabled at remove time ;
- it can be enabled right before the device leaves its idle state and disabled
when the device goes back to idle ; or
- it can be enabled and disabled in a more fine-grained, device-specific
manner.
The selected clock management granularity depends on constraints specific to
the device and on how aggressive power saving needs to be. Enabling the clocks
at probe time and disabling them at remove time is enough for most devices,
but leads to a high power consumption. For that reason the second clock
management scheme is often desired.
Managing clocks manually in the driver is a valid option. However, when adding
runtime PM to the equation, and realizing that the clocks need to be enabled
in the runtime PM resume handler and disabled in the suspend handler, the
clock management code starts looking very similar in most drivers. We're thus
tempted to factorize it away from the drivers into a shared location.
It's important to note at this point that the goal here is only to simplify
drivers. Moving clock management code out of the drivers doesn't (unless I'm
missing something) open the door to new possibilities, it just serves as a
simplification.
Now, as Grygorii mentioned, differences between how a given IP core is
integrated in various SoCs can make clock management SoC-dependent. In the
vast majority of cases (which is really what we need to target, given that our
target is simplifying drivers) SoC integration can be described as a list of
clocks that must be managed. That list can be common to all devices in a given
SoC, or can be device-dependent as well.
That's actually a problem - now we have static list of managed clocks per-SoC
and
not per device.
Few locations can be used to express a per-device list of per-SoC clocks. We
can have clocks lists in a per-SoC and per-device location, per-device clocks
lists in an SoC-specific location, or per-SoC clocks lists in a device-
specific location.
The first option would require listing clocks to be managed by runtime PM in
DT nodes, as proposed by this patch set. I don't think
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Hi Grygorii and Grant,
On Monday 28 July 2014 23:52:34 Grant Likely wrote:
On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko wrote:
On 07/28/2014 05:05 PM, Grant Likely wrote:
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko wrote:
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct
device_node *np,
struct clk *clk;
int error;
-for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
-if (!clk_may_runtime_pm(clk)) {
-clk_put(clk);
-continue;
-}
+for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+ !IS_ERR(clk); i++) {
This really looks like an ABI break to me. What happens to all the
existing platforms who don't have this new clkops-clocks in their device
tree?
Agree. This patch as is will break such platforms.
As possible solution for above problem - the NULL can be used as clock's
prefix by default and platform code can configure new value of clock's
prefix during initialization.
In addition, to make this solution full the of_clk_get_by_name() will
need to be modified too.
But note pls, this is pure RFC patches which I did to find out the answer
on questions: - What is better: maintain Runtime PM clocks configuration
in DT or in code?
In code. I don't think it is workable to embed runtime PM behaviour
into the DT bindings. I think there will be too much variance in what
hardware requires. We can create helpers to make this simpler, but I
don't think it is a good idea to set it up automatically without any
control from the driver itself.
- Where and when to call of_clk_register_runtime_pm_clocks()?
Bus notifier/ platform core/ device drivers
I would say in device drivers.
I tend to agree with that.
It will help here to take a step back and remember what the problem we're
trying to solve is.
At the root is clock management. Our system comprise many clocks, and they
need to be handled. The Common Clock Framework nicely models the clocks, and
offers an API for drivers to retrieve device clocks and control them. Drivers
can thus implement clock management manually without much pain.
A clock can be managed in roughly three different ways :
- it can be enabled at probe time and disabled at remove time ;
- it can be enabled right before the device leaves its idle state and disabled
when the device goes back to idle ; or
- it can be enabled and disabled in a more fine-grained, device-specific
manner.
The selected clock management granularity depends on constraints specific to
the device and on how aggressive power saving needs to be. Enabling the clocks
at probe time and disabling them at remove time is enough for most devices,
but leads to a high power consumption. For that reason the second clock
management scheme is often desired.
Managing clocks manually in the driver is a valid option. However, when adding
runtime PM to the equation, and realizing that the clocks need to be enabled
in the runtime PM resume handler and disabled in the suspend handler, the
clock management code starts looking very similar in most drivers. We're thus
tempted to factorize it away from the drivers into a shared location.
It's important to note at this point that the goal here is only to simplify
drivers. Moving clock management code out of the drivers doesn't (unless I'm
missing something) open the door to new possibilities, it just serves as a
simplification.
Now, as Grygorii mentioned, differences between how a given IP core is
integrated in various SoCs can make clock management SoC-dependent. In the
vast majority of cases (which is really what we need to target, given that our
target is simplifying drivers) SoC integration can be described as a list of
clocks that must be managed. That list can be common to all devices in a given
SoC, or can be device-dependent as well.
Few locations can be used to express a per-device list of per-SoC clocks. We
can have clocks lists in a per-SoC and per-device location, per-device clocks
lists in an SoC-specific location, or per-SoC clocks lists in a device-
specific location.
The first option would require listing clocks to be managed by runtime PM in
DT nodes, as proposed by this patch set. I don't think this is the best
option, as that information is a mix of hardware description and software
policy, with the hardware description part
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct device_node
*np,
struct clk *clk;
int error;
- for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
- if (!clk_may_runtime_pm(clk)) {
- clk_put(clk);
- continue;
- }
+ for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+ !IS_ERR(clk); i++) {
This really looks like an ABI break to me. What happens to all the
existing platforms who don't have this new clkops-clocks in their device
tree?
g.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Hi Grant.
On 07/28/2014 05:05 PM, Grant Likely wrote:
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct device_node
*np,
struct clk *clk;
int error;
-for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
-if (!clk_may_runtime_pm(clk)) {
-clk_put(clk);
-continue;
-}
+for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+ !IS_ERR(clk); i++) {
This really looks like an ABI break to me. What happens to all the
existing platforms who don't have this new clkops-clocks in their device
tree?
Agree. This patch as is will break such platforms.
As possible solution for above problem - the NULL can be used as clock's prefix
by default and platform code can configure new value of clock's prefix during
initialization.
In addition, to make this solution full the of_clk_get_by_name() will
need to be modified too.
But note pls, this is pure RFC patches which I did to find out the answer on
questions:
- What is better: maintain Runtime PM clocks configuration in DT or in code?
- Where and when to call of_clk_register_runtime_pm_clocks()?
Bus notifier/ platform core/ device drivers
Also, May be platform dependent solution [1] can be acceptable for now?
[1] https://lkml.org/lkml/2014/7/25/630
Best regards,
-grygorii
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
On Mon, Jul 28, 2014 at 11:47 AM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Hi Grant.
On 07/28/2014 05:05 PM, Grant Likely wrote:
On Thu, 12 Jun 2014 19:53:43 +0300, Grygorii Strashko
grygorii.stras...@ti.com wrote:
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct device_node
*np,
struct clk *clk;
int error;
-for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
-if (!clk_may_runtime_pm(clk)) {
-clk_put(clk);
-continue;
-}
+for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+ !IS_ERR(clk); i++) {
This really looks like an ABI break to me. What happens to all the
existing platforms who don't have this new clkops-clocks in their device
tree?
Agree. This patch as is will break such platforms.
As possible solution for above problem - the NULL can be used as clock's
prefix
by default and platform code can configure new value of clock's prefix during
initialization.
In addition, to make this solution full the of_clk_get_by_name() will
need to be modified too.
But note pls, this is pure RFC patches which I did to find out the answer on
questions:
- What is better: maintain Runtime PM clocks configuration in DT or in code?
In code. I don't think it is workable to embed runtime PM behaviour
into the DT bindings. I think there will be too much variance in what
hardware requires. We can create helpers to make this simpler, but I
don't think it is a good idea to set it up automatically without any
control from the driver itself.
- Where and when to call of_clk_register_runtime_pm_clocks()?
Bus notifier/ platform core/ device drivers
I would say in device drivers.
Also, May be platform dependent solution [1] can be acceptable for now?
[1] https://lkml.org/lkml/2014/7/25/630
I need to look at the series before I comment. I've flagged it and
will hopefully look at it tomorrow.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 2/2] of/clk: use clkops-clocks to specify clocks handled by clock_ops domain
Use clkops-clocks property to specify clocks handled by
clock_ops domain PM domain. Only clocks defined in clkops-clocks
set of clocks will be handled by Runtime PM through clock_ops
Pm domain.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/of/of_clk.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/of/of_clk.c b/drivers/of/of_clk.c
index 35f5e9f..5f9b90e 100644
--- a/drivers/of/of_clk.c
+++ b/drivers/of/of_clk.c
@@ -86,11 +86,8 @@ int of_clk_register_runtime_pm_clocks(struct device_node *np,
struct clk *clk;
int error;
- for (i = 0; (clk = of_clk_get(np, i)) !IS_ERR(clk); i++) {
- if (!clk_may_runtime_pm(clk)) {
- clk_put(clk);
- continue;
- }
+ for (i = 0; (clk = of_clk_get_from_set(np, clkops, i))
+!IS_ERR(clk); i++) {
error = of_clk_register(dev, clk);
if (error) {
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
