Re: [PATCH 4/7] OMAP2+: powerdomain: add voltage domain lookup during register
On Wed, 23 Mar 2011, Cousson, Benoit wrote:
On 3/22/2011 11:08 PM, Paul Walmsley wrote:
On Tue, 22 Mar 2011, Cousson, Benoit wrote:
On the other hand not every clock belong to a clockdomain, that's why we
have clock without clockdomain on OMAP. A clockdomain is a just group of
IPs that share the same interface clock.
PRCM clockdomains also include functional clocks. See for example the
OMAP4 Public TRM Rev. O Section 3.1.1.1.3 Clock Domain ('The clockdomain
of CM_B is composed of two clocks: a functional clock (FCLK2) and an
interface clock (ICLK1)', and also later, 'The PRCM module lets software
check the status of the clock domain functional clocks').
True, but that does not change the definition of the clock domain (same
paragraph):
A clock domain is a group of modules fed by clock signals controlled by the
same clock manager in the PRCM module (see Figure 3-2).
My point was that a clock domain is related to modules that share some clocks.
So yes, a clock domain will contains modules and their related iclk and fclk.
But that does not means that every clocks will belong to a clockdomain.
All the non leaf clocks will not have any clock domain since they will not be
attached to any module directly.
In the OMAP Linux code, we use clockdomains for three purposes:
1. as a way to control the PRCM clockdomain hardware
2. as a way to associate clocks with powerdomains
3. as a way to group clocks that have similar idle behavior
So yes. All non-leaf clocks should be associated with OMAP Linux
clockdomains also, since those clocks will be associated with powerdomains
and also will be grouped by idle behavior.
Both the TRM and the OMAP4 functional specification clearly link PRCM
clockdomain idle management to the state of the clockdomain's functional
clocks. See for example OMAP4 Public TRM Rev. O Table 3-11 Clock Domain
Clock States ('INACTIVE: ... Every optional functional clock in the clock
domain is gated.')
That just means that each clock that supplies a module must be idled in order
to allow the clock domain to transition.
The clock domain FSM is using both iclk and fclk state to trigger the domain
transition, but that same clock could be used in another clock domain too.
When the same clock is used in another clockdomain, another struct clk is
created for that clock with a different clockdomain. We already have this
situation with OMAP3's 54MHz clock, for example.
Beyond the PRCM, the Linux-OMAP clockdomain code is not only concerned
with PRCM-controllable clockdomains. It is intended to be a generically
useful way to connect clocks to the powerdomain and voltagedomain that the
clock exists in, even if there are no explicit PRCM registers associated
with the clockdomain. These powerdomains and voltagedomains also may
not be directly PRCM controllable.
In that case, you cannot call that clockdomain anymore since this is not the
proper definition used by the PRCM. If you do not have a module, you cannot
have a clockdomain.
That may be the way that TI considers it in the AutoPRCM data or the
system PRCM, but the OMAP Linux clockdomain code uses a superset
definition and has done so for a few years now.
OMAP4 partitioning is following this hierarchy: module clockdomain
powerdomain voltagedomain device. The clocks are not part of the
hierarchy and for a good reason, the can be shared across the whole
device.
As mentioned before, each instance of a clock in a different clockdomain
should have a different struct clk associated with it.
Every clock that is in the Linux-OMAP clock tree should have a clockdomain
associated with it.
Mmm... What domain will you use for sys_clk
sys_clk and related clocks should go into a sys_clkdm or hf_sys_clkdm
or 32k_clk?
The external 32KiHZ oscillator should be associated with wkup_clkdm.
or for any DPLL or HS divider?
Those go into separate DPLL clockdomains. This is already there in OMAP3.
And what for?
2. as a way to associate clocks with powerdomains
3. as a way to group clocks that have similar idle behavior
Also, the original idea was to put autoidle control for the DPLLs and the
external HF clock LDO into the clockdomain layer, since it already had a
convenient concept for hardware automatic idle, vs. software controlled
idle. That part may not be needed any longer, now that we've added
autoidle support back into the clock code.
I know that the clockdomain name is confusing, but why do you want to change
its definition?
- Paul
--
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: [PATCH 4/7] OMAP2+: powerdomain: add voltage domain lookup during register
Hi Kevin,
a couple of comments, now that i have the chance to look this over
closely.
Probably we should require every powerdomain to have a voltagedomain.
This will avoid the problems we're having with the clocks, in which not
every clock has a clockdomain pointer.
So I'd suggest splitting this patch up into two patches -- one to make the
changes the powerdomain structure, and the second to add the code into the
powerdomain.c. Between these two patches, we'd need patches to the
powerdomain data to add the voltagedomain names in.
then...
On Fri, 18 Mar 2011, Kevin Hilman wrote:
When a powerdomain is registered, lookup the voltage domain by name
and keep a pointer to the containing voltagedomain in the powerdomain
structure.
Modeled after similar method between powerdomain and clockdomain layers.
Signed-off-by: Kevin Hilman khil...@ti.com
---
arch/arm/mach-omap2/powerdomain.c | 26 ++
arch/arm/mach-omap2/powerdomain.h |8
arch/arm/mach-omap2/voltage.h |1 +
3 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index 49c6513..da86c72 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -77,6 +77,7 @@ static struct powerdomain *_pwrdm_lookup(const char *name)
static int _pwrdm_register(struct powerdomain *pwrdm)
{
int i;
+ struct voltagedomain *voltdm;
if (!pwrdm || !pwrdm-name)
return -EINVAL;
@@ -94,6 +95,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
if (_pwrdm_lookup(pwrdm-name))
return -EEXIST;
+ if (pwrdm-voltdm.name) {
we'd drop this
+ voltdm = voltdm_lookup(pwrdm-voltdm.name);
+ if (!voltdm) {
+ pr_err(powerdomain: %s: voltagedomain %s does not
exist\n,
+pwrdm-name, pwrdm-voltdm.name);
+ return -EINVAL;
+ }
+ pwrdm-voltdm.ptr = voltdm;
+ }
+
list_add(pwrdm-node, pwrdm_list);
/* Initialize the powerdomain's state counter */
@@ -383,6 +394,21 @@ int pwrdm_for_each_clkdm(struct powerdomain *pwrdm,
}
/**
+ * pwrdm_get_voltdm - return a ptr to the voltdm that this pwrdm resides in
+ * @pwrdm: struct powerdomain *
+ *
+ * Return a pointer to the struct voltageomain that the specified powerdomain
+ * @pwrdm exists in, or returns NULL if @pwrdm is NULL.
+ */
+struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm)
+{
+ if (!pwrdm)
+ return NULL;
and this
+
+ return pwrdm-voltdm.ptr;
+}
+
+/**
* pwrdm_get_mem_bank_count - get number of memory banks in this powerdomain
* @pwrdm: struct powerdomain *
*
diff --git a/arch/arm/mach-omap2/powerdomain.h
b/arch/arm/mach-omap2/powerdomain.h
index 027f40b..3a1ec37 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -24,6 +24,8 @@
#include plat/cpu.h
+#include voltage.h
+
/* Powerdomain basic power states */
#define PWRDM_POWER_OFF 0x0
#define PWRDM_POWER_RET 0x1
@@ -78,6 +80,7 @@ struct powerdomain;
/**
* struct powerdomain - OMAP powerdomain
* @name: Powerdomain name
+ * @voltdm: voltagedomain containing this powerdomain
* @omap_chip: represents the OMAP chip types containing this pwrdm
* @prcm_offs: the address offset from CM_BASE/PRM_BASE
* @prcm_partition: (OMAP4 only) the PRCM partition ID containing @prcm_offs
@@ -98,6 +101,10 @@ struct powerdomain;
*/
struct powerdomain {
const char *name;
+ union {
+ const char *name;
+ struct voltagedomain *ptr;
+ } voltdm;
const struct omap_chip_id omap_chip;
const s16 prcm_offs;
const u8 pwrsts;
@@ -176,6 +183,7 @@ int pwrdm_del_clkdm(struct powerdomain *pwrdm, struct
clockdomain *clkdm);
int pwrdm_for_each_clkdm(struct powerdomain *pwrdm,
int (*fn)(struct powerdomain *pwrdm,
struct clockdomain *clkdm));
+struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm);
int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm);
diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
index 122d8c1..13e5ed9 100644
--- a/arch/arm/mach-omap2/voltage.h
+++ b/arch/arm/mach-omap2/voltage.h
@@ -182,4 +182,5 @@ extern void omap44xx_voltagedomains_init(void);
struct voltagedomain *voltdm_lookup(const char *name);
void voltdm_init(struct voltagedomain **voltdm_list);
+int voltdm_add_pwrdm(struct voltagedomain *voltdm, struct powerdomain
*pwrdm);
#endif
--
1.7.4
--
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: [PATCH 4/7] OMAP2+: powerdomain: add voltage domain lookup during register
Hi Paul,
On 3/22/2011 8:23 PM, Paul Walmsley wrote:
Hi Kevin,
a couple of comments, now that i have the chance to look this over
closely.
Probably we should require every powerdomain to have a voltagedomain.
This will avoid the problems we're having with the clocks, in which not
every clock has a clockdomain pointer.
I do agree with you, but not for the same reason :-)
Every power domain belongs to a voltage domain, scalable or not, so
that's why we should always have it.
On the other hand not every clock belong to a clockdomain, that's why we
have clock without clockdomain on OMAP.
A clockdomain is a just group of IPs that share the same interface clock.
Benoit
So I'd suggest splitting this patch up into two patches -- one to make the
changes the powerdomain structure, and the second to add the code into the
powerdomain.c. Between these two patches, we'd need patches to the
powerdomain data to add the voltagedomain names in.
then...
On Fri, 18 Mar 2011, Kevin Hilman wrote:
When a powerdomain is registered, lookup the voltage domain by name
and keep a pointer to the containing voltagedomain in the powerdomain
structure.
Modeled after similar method between powerdomain and clockdomain layers.
Signed-off-by: Kevin Hilmankhil...@ti.com
---
arch/arm/mach-omap2/powerdomain.c | 26 ++
arch/arm/mach-omap2/powerdomain.h |8
arch/arm/mach-omap2/voltage.h |1 +
3 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index 49c6513..da86c72 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -77,6 +77,7 @@ static struct powerdomain *_pwrdm_lookup(const char *name)
static int _pwrdm_register(struct powerdomain *pwrdm)
{
int i;
+ struct voltagedomain *voltdm;
if (!pwrdm || !pwrdm-name)
return -EINVAL;
@@ -94,6 +95,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
if (_pwrdm_lookup(pwrdm-name))
return -EEXIST;
+ if (pwrdm-voltdm.name) {
we'd drop this
+ voltdm = voltdm_lookup(pwrdm-voltdm.name);
+ if (!voltdm) {
+ pr_err(powerdomain: %s: voltagedomain %s does not
exist\n,
+ pwrdm-name, pwrdm-voltdm.name);
+ return -EINVAL;
+ }
+ pwrdm-voltdm.ptr = voltdm;
+ }
+
list_add(pwrdm-node,pwrdm_list);
/* Initialize the powerdomain's state counter */
@@ -383,6 +394,21 @@ int pwrdm_for_each_clkdm(struct powerdomain *pwrdm,
}
/**
+ * pwrdm_get_voltdm - return a ptr to the voltdm that this pwrdm resides in
+ * @pwrdm: struct powerdomain *
+ *
+ * Return a pointer to the struct voltageomain that the specified powerdomain
+ * @pwrdm exists in, or returns NULL if @pwrdm is NULL.
+ */
+struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm)
+{
+ if (!pwrdm)
+ return NULL;
and this
+
+ return pwrdm-voltdm.ptr;
+}
+
+/**
* pwrdm_get_mem_bank_count - get number of memory banks in this powerdomain
* @pwrdm: struct powerdomain *
*
diff --git a/arch/arm/mach-omap2/powerdomain.h
b/arch/arm/mach-omap2/powerdomain.h
index 027f40b..3a1ec37 100644
--- a/arch/arm/mach-omap2/powerdomain.h
+++ b/arch/arm/mach-omap2/powerdomain.h
@@ -24,6 +24,8 @@
#includeplat/cpu.h
+#include voltage.h
+
/* Powerdomain basic power states */
#define PWRDM_POWER_OFF 0x0
#define PWRDM_POWER_RET 0x1
@@ -78,6 +80,7 @@ struct powerdomain;
/**
* struct powerdomain - OMAP powerdomain
* @name: Powerdomain name
+ * @voltdm: voltagedomain containing this powerdomain
* @omap_chip: represents the OMAP chip types containing this pwrdm
* @prcm_offs: the address offset from CM_BASE/PRM_BASE
* @prcm_partition: (OMAP4 only) the PRCM partition ID containing @prcm_offs
@@ -98,6 +101,10 @@ struct powerdomain;
*/
struct powerdomain {
const char *name;
+ union {
+ const char *name;
+ struct voltagedomain *ptr;
+ } voltdm;
const struct omap_chip_id omap_chip;
const s16 prcm_offs;
const u8 pwrsts;
@@ -176,6 +183,7 @@ int pwrdm_del_clkdm(struct powerdomain *pwrdm, struct
clockdomain *clkdm);
int pwrdm_for_each_clkdm(struct powerdomain *pwrdm,
int (*fn)(struct powerdomain *pwrdm,
struct clockdomain *clkdm));
+struct voltagedomain *pwrdm_get_voltdm(struct powerdomain *pwrdm);
int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm);
diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
index 122d8c1..13e5ed9 100644
--- a/arch/arm/mach-omap2/voltage.h
+++ b/arch/arm/mach-omap2/voltage.h
@@ -182,4 +182,5 @@ extern void omap44xx_voltagedomains_init(void);
struct voltagedomain *voltdm_lookup(const char
Re: [PATCH 4/7] OMAP2+: powerdomain: add voltage domain lookup during register
Hi Benoît,
On Tue, 22 Mar 2011, Cousson, Benoit wrote:
On the other hand not every clock belong to a clockdomain, that's why we
have clock without clockdomain on OMAP. A clockdomain is a just group of
IPs that share the same interface clock.
PRCM clockdomains also include functional clocks. See for example the
OMAP4 Public TRM Rev. O Section 3.1.1.1.3 Clock Domain ('The clockdomain
of CM_B is composed of two clocks: a functional clock (FCLK2) and an
interface clock (ICLK1)', and also later, 'The PRCM module lets software
check the status of the clock domain functional clocks').
Both the TRM and the OMAP4 functional specification clearly link PRCM
clockdomain idle management to the state of the clockdomain's functional
clocks. See for example OMAP4 Public TRM Rev. O Table 3-11 Clock Domain
Clock States ('INACTIVE: ... Every optional functional clock in the clock
domain is gated.')
Beyond the PRCM, the Linux-OMAP clockdomain code is not only concerned
with PRCM-controllable clockdomains. It is intended to be a generically
useful way to connect clocks to the powerdomain and voltagedomain that the
clock exists in, even if there are no explicit PRCM registers associated
with the clockdomain. These powerdomains and voltagedomains also may
not be directly PRCM controllable.
Every clock that is in the Linux-OMAP clock tree should have a clockdomain
associated with it.
- Paul
Re: [PATCH 4/7] OMAP2+: powerdomain: add voltage domain lookup during register
I knew you will answer that one :-)
On 3/22/2011 11:08 PM, Paul Walmsley wrote:
Hi Benoît,
On Tue, 22 Mar 2011, Cousson, Benoit wrote:
On the other hand not every clock belong to a clockdomain, that's why we
have clock without clockdomain on OMAP. A clockdomain is a just group of
IPs that share the same interface clock.
PRCM clockdomains also include functional clocks. See for example the
OMAP4 Public TRM Rev. O Section 3.1.1.1.3 Clock Domain ('The clockdomain
of CM_B is composed of two clocks: a functional clock (FCLK2) and an
interface clock (ICLK1)', and also later, 'The PRCM module lets software
check the status of the clock domain functional clocks').
True, but that does not change the definition of the clock domain (same
paragraph):
A clock domain is a group of modules fed by clock signals controlled by
the same clock manager in the PRCM module (see Figure 3-2).
My point was that a clock domain is related to modules that share some
clocks.
So yes, a clock domain will contains modules and their related iclk and
fclk. But that does not means that every clocks will belong to a
clockdomain.
All the non leaf clocks will not have any clock domain since they will
not be attached to any module directly.
Both the TRM and the OMAP4 functional specification clearly link PRCM
clockdomain idle management to the state of the clockdomain's functional
clocks. See for example OMAP4 Public TRM Rev. O Table 3-11 Clock Domain
Clock States ('INACTIVE: ... Every optional functional clock in the clock
domain is gated.')
That just means that each clock that supplies a module must be idled in
order to allow the clock domain to transition.
The clock domain FSM is using both iclk and fclk state to trigger the
domain transition, but that same clock could be used in another clock
domain too.
Beyond the PRCM, the Linux-OMAP clockdomain code is not only concerned
with PRCM-controllable clockdomains. It is intended to be a generically
useful way to connect clocks to the powerdomain and voltagedomain that the
clock exists in, even if there are no explicit PRCM registers associated
with the clockdomain. These powerdomains and voltagedomains also may
not be directly PRCM controllable.
In that case, you cannot call that clockdomain anymore since this is not
the proper definition used by the PRCM. If you do not have a module, you
cannot have a clockdomain.
OMAP4 partitioning is following this hierarchy: module clockdomain
powerdomain voltagedomain device.
The clocks are not part of the hierarchy and for a good reason, the can
be shared across the whole device.
Every clock that is in the Linux-OMAP clock tree should have a clockdomain
associated with it.
Mmm... What domain will you use for sys_clk or 32k_clk? or for any DPLL
or HS divider?
And what for?
I know that the clockdomain name is confusing, but why do you want to
change its definition?
Benoit
--
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: [PATCH 4/7] OMAP2+: powerdomain: add voltage domain lookup during register
Paul Walmsley p...@pwsan.com writes: Hi Kevin, a couple of comments, now that i have the chance to look this over closely. Probably we should require every powerdomain to have a voltagedomain. This will avoid the problems we're having with the clocks, in which not every clock has a clockdomain pointer. So I'd suggest splitting this patch up into two patches -- one to make the changes the powerdomain structure, and the second to add the code into the powerdomain.c. Between these two patches, we'd need patches to the powerdomain data to add the voltagedomain names in. OK, Benoit and I talked earlier today and agreed on this as well. He has already generated the OMAP4 data as well, so split up, and drop the checks below as you suggest. Kevin -- 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
