Re: [PATCH 1/2] ARM: OMAP2+: hwmod data: Fix the wrong clkdm assigned to PRCM modules
Benoît and I discussed this today, at length. Our views still differ.
In an attempt to move on to more useful work, a compromise was agreed.
The following patch removes the requirement for hwmods to have a
clkdm_name, and removes the PRM and CM clockdomains from the data.
It is intended for 3.6.
- Paul
From: Paul Walmsley p...@pwsan.com
Date: Wed, 13 Jun 2012 17:16:05 -0600
Subject: [PATCH] ARM: OMAP2+: hwmod: remove prm_clkdm, cm_clkdm; allow hwmods
to have no clockdomain
Remove prm_clkdm and cm_clkdm and allow hwmods to have no clockdomain.
Signed-off-by: Paul Walmsley p...@pwsan.com
Cc: Benoît Cousson b-cous...@ti.com
---
arch/arm/mach-omap2/Makefile |1 -
arch/arm/mach-omap2/clockdomain.h |2 --
arch/arm/mach-omap2/clockdomains2420_data.c|2 --
arch/arm/mach-omap2/clockdomains2430_data.c|2 --
arch/arm/mach-omap2/clockdomains3xxx_data.c|2 --
arch/arm/mach-omap2/clockdomains44xx_data.c|2 --
arch/arm/mach-omap2/clockdomains_common_data.c | 24
arch/arm/mach-omap2/omap_hwmod.c |9 -
arch/arm/mach-omap2/omap_hwmod_44xx_data.c |3 ---
9 files changed, 4 insertions(+), 43 deletions(-)
delete mode 100644 arch/arm/mach-omap2/clockdomains_common_data.c
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index fa742f3..bc7d239 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -116,7 +116,6 @@ obj-$(CONFIG_ARCH_OMAP4)+=
powerdomains44xx_data.o
# PRCM clockdomain control
clockdomain-common += clockdomain.o
-clockdomain-common += clockdomains_common_data.o
obj-$(CONFIG_ARCH_OMAP2) += $(clockdomain-common)
obj-$(CONFIG_ARCH_OMAP2) += clockdomain2xxx_3xxx.o
obj-$(CONFIG_ARCH_OMAP2) += clockdomains2xxx_3xxx_data.o
diff --git a/arch/arm/mach-omap2/clockdomain.h
b/arch/arm/mach-omap2/clockdomain.h
index f7b5860..349dcbb 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -206,7 +206,5 @@ extern struct clkdm_ops omap4_clkdm_operations;
extern struct clkdm_dep gfx_24xx_wkdeps[];
extern struct clkdm_dep dsp_24xx_wkdeps[];
extern struct clockdomain wkup_common_clkdm;
-extern struct clockdomain prm_common_clkdm;
-extern struct clockdomain cm_common_clkdm;
#endif
diff --git a/arch/arm/mach-omap2/clockdomains2420_data.c
b/arch/arm/mach-omap2/clockdomains2420_data.c
index 0ab8e46..5c74185 100644
--- a/arch/arm/mach-omap2/clockdomains2420_data.c
+++ b/arch/arm/mach-omap2/clockdomains2420_data.c
@@ -131,8 +131,6 @@ static struct clockdomain dss_2420_clkdm = {
static struct clockdomain *clockdomains_omap242x[] __initdata = {
wkup_common_clkdm,
- cm_common_clkdm,
- prm_common_clkdm,
mpu_2420_clkdm,
iva1_2420_clkdm,
dsp_2420_clkdm,
diff --git a/arch/arm/mach-omap2/clockdomains2430_data.c
b/arch/arm/mach-omap2/clockdomains2430_data.c
index 3645ed0..f096175 100644
--- a/arch/arm/mach-omap2/clockdomains2430_data.c
+++ b/arch/arm/mach-omap2/clockdomains2430_data.c
@@ -157,8 +157,6 @@ static struct clockdomain dss_2430_clkdm = {
static struct clockdomain *clockdomains_omap243x[] __initdata = {
wkup_common_clkdm,
- cm_common_clkdm,
- prm_common_clkdm,
mpu_2430_clkdm,
mdm_clkdm,
dsp_2430_clkdm,
diff --git a/arch/arm/mach-omap2/clockdomains3xxx_data.c
b/arch/arm/mach-omap2/clockdomains3xxx_data.c
index 6038adb..2cdc17c 100644
--- a/arch/arm/mach-omap2/clockdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/clockdomains3xxx_data.c
@@ -347,8 +347,6 @@ static struct clkdm_autodep clkdm_autodeps[] = {
static struct clockdomain *clockdomains_omap3430_common[] __initdata = {
wkup_common_clkdm,
- cm_common_clkdm,
- prm_common_clkdm,
mpu_3xxx_clkdm,
neon_clkdm,
iva2_clkdm,
diff --git a/arch/arm/mach-omap2/clockdomains44xx_data.c
b/arch/arm/mach-omap2/clockdomains44xx_data.c
index c534258..bd7ed13 100644
--- a/arch/arm/mach-omap2/clockdomains44xx_data.c
+++ b/arch/arm/mach-omap2/clockdomains44xx_data.c
@@ -430,8 +430,6 @@ static struct clockdomain *clockdomains_omap44xx[]
__initdata = {
l4_wkup_44xx_clkdm,
emu_sys_44xx_clkdm,
l3_dma_44xx_clkdm,
- prm_common_clkdm,
- cm_common_clkdm,
NULL
};
diff --git a/arch/arm/mach-omap2/clockdomains_common_data.c
b/arch/arm/mach-omap2/clockdomains_common_data.c
deleted file mode 100644
index 615b1f0..000
--- a/arch/arm/mach-omap2/clockdomains_common_data.c
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * OMAP2+-common clockdomain data
- *
- * Copyright (C) 2008-2012 Texas Instruments, Inc.
- * Copyright (C) 2008-2010 Nokia Corporation
- *
- * Paul Walmsley, Jouni Högander
- */
-
-#include linux/kernel.h
-#include linux/io.h
-
-#include clockdomain.h
-
-/* These are implicit clockdomains -
Re: [PATCH 1/2] ARM: OMAP2+: hwmod data: Fix the wrong clkdm assigned to PRCM modules
Hi Paul, On 6/11/2012 11:11 PM, Cousson, Benoit wrote: On 6/11/2012 7:26 PM, Paul Walmsley wrote: On Mon, 11 Jun 2012, Benoit Cousson wrote: The following commit (794b480a37e3d284d6ee7344d8a737ef60476ed5) was adding the PRCM IPs data for PRCM, CM, PRCM_MPU and SCRM. The clkdm entry are not the correct ones and does not exist in the system. Replace them with the proper wkup, l4_ao and l4_cfg. This does not match either the publicly avaiable documentation and the internal NDA hardware specifications[1]. That's almost true, until I realized that the clock domain modules list contain this information (clock.py/clock_domain['l4_wkup']['modules']). Now, I'm wondering as well why this is not documented better. I'll check that tomorrow. OK, I've just spent 2 hours with HW folks trying to get the real truth... In fact, this information is not even accurate :-( There is no such thing as clock domain for PRCM IPs. These IPs are considered as clock generator or reset generator and thus does not belong to any clock domain. This information was used to capture the power domain where they belong and thus the closest clock domain was used for that, but this is purely artificial. Their first question was, why do you guys need that at the first place??? I guess this question is valid. Since we do not have any SW control, this information is completely useless except to potentially avoid de-referencing null clkdm. So I guess it is much better to just take care of that and avoid populating with wrong or fake clock domains. Going a little bit further, these IPs do not even have modulemode, or whatever SW control. Even the hwmod it not applicable for them. It should just be a regular platform_device and thus deserve a simple entry in DT without any hwmod attribute. Bottom-line, I think we should simply get rid of these PRCM hwmod entries and the corresponding fake clock domains. Does that make sense to you? Regards, 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
[PATCH 1/2] ARM: OMAP2+: hwmod data: Fix the wrong clkdm assigned to PRCM modules
The following commit (794b480a37e3d284d6ee7344d8a737ef60476ed5) was adding
the PRCM IPs data for PRCM, CM, PRCM_MPU and SCRM.
The clkdm entry are not the correct ones and does not exist in the system.
Replace them with the proper wkup, l4_ao and l4_cfg.
Fix as well the wrong OCP port used by the cm_core_aon. It uses the
l4_cfg interconnect and not the l4_wkup.
Re-order the entries by address value.
Signed-off-by: Benoit Cousson b-cous...@ti.com
---
arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 52 ++--
1 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 950454a..03fc705 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2529,27 +2529,6 @@ static struct omap_hwmod_class omap44xx_prcm_hwmod_class
= {
.name = prcm,
};
-/* prcm_mpu */
-static struct omap_hwmod omap44xx_prcm_mpu_hwmod = {
- .name = prcm_mpu,
- .class = omap44xx_prcm_hwmod_class,
- .clkdm_name = l4_wkup_clkdm,
-};
-
-/* cm_core_aon */
-static struct omap_hwmod omap44xx_cm_core_aon_hwmod = {
- .name = cm_core_aon,
- .class = omap44xx_prcm_hwmod_class,
- .clkdm_name = cm_clkdm,
-};
-
-/* cm_core */
-static struct omap_hwmod omap44xx_cm_core_hwmod = {
- .name = cm_core,
- .class = omap44xx_prcm_hwmod_class,
- .clkdm_name = cm_clkdm,
-};
-
/* prm */
static struct omap_hwmod_irq_info omap44xx_prm_irqs[] = {
{ .irq = 11 + OMAP44XX_IRQ_GIC_START },
@@ -2564,12 +2543,33 @@ static struct omap_hwmod_rst_info omap44xx_prm_resets[]
= {
static struct omap_hwmod omap44xx_prm_hwmod = {
.name = prm,
.class = omap44xx_prcm_hwmod_class,
- .clkdm_name = prm_clkdm,
+ .clkdm_name = l4_wkup_clkdm,
.mpu_irqs = omap44xx_prm_irqs,
.rst_lines = omap44xx_prm_resets,
.rst_lines_cnt = ARRAY_SIZE(omap44xx_prm_resets),
};
+/* cm_core */
+static struct omap_hwmod omap44xx_cm_core_hwmod = {
+ .name = cm_core,
+ .class = omap44xx_prcm_hwmod_class,
+ .clkdm_name = l4_cfg_clkdm,
+};
+
+/* cm_core_aon */
+static struct omap_hwmod omap44xx_cm_core_aon_hwmod = {
+ .name = cm_core_aon,
+ .class = omap44xx_prcm_hwmod_class,
+ .clkdm_name = l4_ao_clkdm,
+};
+
+/* prcm_mpu */
+static struct omap_hwmod omap44xx_prcm_mpu_hwmod = {
+ .name = prcm_mpu,
+ .class = omap44xx_prcm_hwmod_class,
+ .clkdm_name = l4_wkup_clkdm,
+};
+
/*
* 'scrm' class
* system clock and reset manager
@@ -5305,10 +5305,10 @@ static struct omap_hwmod_addr_space
omap44xx_cm_core_aon_addrs[] = {
};
/* l4_wkup - cm_core_aon */
-static struct omap_hwmod_ocp_if omap44xx_l4_wkup__cm_core_aon = {
- .master = omap44xx_l4_wkup_hwmod,
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__cm_core_aon = {
+ .master = omap44xx_l4_cfg_hwmod,
.slave = omap44xx_cm_core_aon_hwmod,
- .clk= l4_wkup_clk_mux_ck,
+ .clk= l4_div_ck,
.addr = omap44xx_cm_core_aon_addrs,
.user = OCP_USER_MPU | OCP_USER_SDMA,
};
@@ -6101,7 +6101,7 @@ static struct omap_hwmod_ocp_if *omap44xx_hwmod_ocp_ifs[]
__initdata = {
omap44xx_l3_main_2__ocmc_ram,
omap44xx_l4_cfg__ocp2scp_usb_phy,
omap44xx_mpu_private__prcm_mpu,
- omap44xx_l4_wkup__cm_core_aon,
+ omap44xx_l4_cfg__cm_core_aon,
omap44xx_l4_cfg__cm_core,
omap44xx_l4_wkup__prm,
omap44xx_l4_wkup__scrm,
--
1.7.0.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 1/2] ARM: OMAP2+: hwmod data: Fix the wrong clkdm assigned to PRCM modules
On Mon, 11 Jun 2012, Benoit Cousson wrote:
The following commit (794b480a37e3d284d6ee7344d8a737ef60476ed5) was adding
the PRCM IPs data for PRCM, CM, PRCM_MPU and SCRM.
The clkdm entry are not the correct ones and does not exist in the system.
Replace them with the proper wkup, l4_ao and l4_cfg.
This does not match either the publicly avaiable documentation and the
internal NDA hardware specifications[1].
Nor does it make sense from a logical perspective. To take an example
from your patch:
@@ -2564,12 +2543,33 @@ static struct omap_hwmod_rst_info
omap44xx_prm_resets[] = {
static struct omap_hwmod omap44xx_prm_hwmod = {
.name = prm,
.class = omap44xx_prcm_hwmod_class,
- .clkdm_name = prm_clkdm,
+ .clkdm_name = l4_wkup_clkdm,
.mpu_irqs = omap44xx_prm_irqs,
.rst_lines = omap44xx_prm_resets,
.rst_lines_cnt = ARRAY_SIZE(omap44xx_prm_resets),
};
There is no possible way that the PRM can exist in the L4_WKUP
clockdomain. The L4_WKUP clockdomain must be able to go inactive for the
chip to enter idle, as we just discussed[1]. But the PRM is the
entity that supervises chip wakeup from off-mode, so for off-mode
to work, there's no way that the PRM can be clock-gated.
Frustrating :-(
Fix as well the wrong OCP port used by the cm_core_aon. It uses the
l4_cfg interconnect and not the l4_wkup.
Re-order the entries by address value.
If you split this part off into a separate patch, I'll take it.
- Paul
1. Cousson, Benoît. _Re: [PATCH] ARM: OMAP2+: hwmod code/data: fix 32K
sync timer_. linux-omap@vger.kernel.org mailing list. Available from
http://www.spinics.net/lists/arm-kernel/msg177128.html (among others).
Re: [PATCH 1/2] ARM: OMAP2+: hwmod data: Fix the wrong clkdm assigned to PRCM modules
On 6/11/2012 7:26 PM, Paul Walmsley wrote:
On Mon, 11 Jun 2012, Benoit Cousson wrote:
The following commit (794b480a37e3d284d6ee7344d8a737ef60476ed5) was adding
the PRCM IPs data for PRCM, CM, PRCM_MPU and SCRM.
The clkdm entry are not the correct ones and does not exist in the system.
Replace them with the proper wkup, l4_ao and l4_cfg.
This does not match either the publicly avaiable documentation and the
internal NDA hardware specifications[1].
That's almost true, until I realized that the clock domain modules list
contain this information (clock.py/clock_domain['l4_wkup']['modules']).
Now, I'm wondering as well why this is not documented better. I'll check
that tomorrow.
Nor does it make sense from a logical perspective.
I do think it does.
To take an example from your patch:
@@ -2564,12 +2543,33 @@ static struct omap_hwmod_rst_info omap44xx_prm_resets[]
= {
static struct omap_hwmod omap44xx_prm_hwmod = {
.name = prm,
.class = omap44xx_prcm_hwmod_class,
- .clkdm_name = prm_clkdm,
+ .clkdm_name = l4_wkup_clkdm,
.mpu_irqs = omap44xx_prm_irqs,
.rst_lines = omap44xx_prm_resets,
.rst_lines_cnt = ARRAY_SIZE(omap44xx_prm_resets),
};
There is no possible way that the PRM can exist in the L4_WKUP
clockdomain. The L4_WKUP clockdomain must be able to go inactive for the
chip to enter idle, as we just discussed[1]. But the PRM is the
entity that supervises chip wakeup from off-mode, so for off-mode
to work, there's no way that the PRM can be clock-gated.
OK, so let's replace PRM by GPT1, without the part that is not
applicable in that case:
There is no possible way that the GPT1 can exist in the L4_WKUP
clockdomain. The L4_WKUP clockdomain must be able to go inactive for the
chip to enter idle, as we just discussed[1] ... so for off-mode
to work, there's no way that the GPT1 can be clock-gated.
Don't you think that this sentence looks wrong now?
Why can the GPT1 run during OFF mode and not the PRM?
The wkup domain is far from being a standard domain. The 32k is always
running even in OFF mode. So for the same reason, that domain can go to
inactive without gating the 32k. This is clearly not a standard domain
but that does not make it a bad one either.
All the IPs that belongs to the l4_wkup are all active during OFF mode,
so is the PRM. The point discussed in [1] clearly show that the domain
inactive state in the case of the wakeup is just used for the OCP clock.
The functional 32k is still active during OFF mode.
Frustrating :-(
You should not, I'm happy we were able to figured out where these PRCM
IPs are located after all these years :-)
Fix as well the wrong OCP port used by the cm_core_aon. It uses the
l4_cfg interconnect and not the l4_wkup.
Re-order the entries by address value.
If you split this part off into a separate patch, I'll take it.
You should take both since this patch is perfectly valid as explained
previously.
cm_clkdm does not exist either whereas l4_aon / l4_cfg does. And
honestly considering that the core_cm_aon is inside a domain names
l4_aon does not seems stupid at all.
The more or less accurate definition of the PRCM clock domain since ever
is that a domain is mostly a group of IPs and the related clocks that
does belong to the same interconnect. So it makes sense that the cm_core
(CM2), cm_core_aon (CM1) and prm does belong to the domain that contains
its respective interconnect.
Regards,
Benoit
- Paul
1. Cousson, Benoît. _Re: [PATCH] ARM: OMAP2+: hwmod code/data: fix 32K
sync timer_. linux-omap@vger.kernel.org mailing list. Available from
http://www.spinics.net/lists/arm-kernel/msg177128.html (among others).
--
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
