Skip to contentSkip to conversation

OMAP: Add DT bindings to specify when devices should not be idled or reset

PATCH Superseded Rajendra Nayak 4 participants 19 messages Aug 20, 2013 – Oct 9, 2013

Subsystems (6)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover OMAP: Add DT bindings to specify when devices should not be idled or reset

We have devices like co-processors, SoC internal busses, memory controllers etc which should not be idled or reset. In some cases erratas around IP blocks prevent them from either being idled or reset. Have a way to pass this information from Device tree, and get rid of similar information that exists as part of the omap_hwmod data files for various DT only OMAP SoCs.

Patches are tested on am335x beagleboneblack and omap4 panda es boards.

Rajendra Nayak (3):
ARM: OMAP2+: hwmod: cleanup HWMOD_INIT_NO_RESET usage
ARM: OMAP2+: Add new bindings for OMAP
ARM: OMAP2+: Let DT say what devices should not to idled or reset

 .../devicetree/bindings/arm/omap/omap.txt          |    3 ++-
 arch/arm/boot/dts/am33xx.dtsi                      |    2 ++
 arch/arm/boot/dts/omap4.dtsi                       |    3 +++
 arch/arm/boot/dts/omap5.dtsi                       |    2 ++
 arch/arm/mach-omap2/omap_hwmod.c                   |   22 +++++++++++++-------
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c         |   22 ++++++++++----------
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    6 ++----
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c         |    4 +---
 8 files changed, 38 insertions(+), 26 deletions(-)
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/3 ARM: OMAP2+: hwmod: cleanup HWMOD_INIT_NO_RESET usage

For modules/IPs/hwmods which do not have -1- sys->class->reset()
and
-2- hardreset lines
and
-3- No way to do an ocp reset (no sysc control) the flag 'HWMOD_INIT_NO_RESET' is not much useful.

Cleanup all such instances across various hwmod data files.

---
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c |   18 +++++++++---------
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    6 +++---
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    6 +++---
 3 files changed, 15 insertions(+), 15 deletions(-)
arch/arm/mach-omap2/omap_hwmod_33xx_data.c +9 -9
index eb2f3b9..d4f0531 100644
@@ -52,7 +52,7 @@ static struct omap_hwmod am33xx_emif_hwmod = {
 	.name		= "emif",
 	.class		= &am33xx_emif_hwmod_class,
 	.clkdm_name	= "l3_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_ddr_m2_div2_ck",
 	.prcm		= {
 		.omap4	= {
@@ -74,7 +74,7 @@ static struct omap_hwmod am33xx_l3_main_hwmod = {
 	.name		= "l3_main",
 	.class		= &am33xx_l3_hwmod_class,
 	.clkdm_name	= "l3_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "l3_gclk",
 	.prcm		= {
 		.omap4	= {
@@ -96,7 +96,7 @@ static struct omap_hwmod am33xx_l3_instr_hwmod = {
 	.name		= "l3_instr",
 	.class		= &am33xx_l3_hwmod_class,
 	.clkdm_name	= "l3_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "l3_gclk",
 	.prcm		= {
 		.omap4	= {
@@ -119,7 +119,7 @@ static struct omap_hwmod am33xx_l4_ls_hwmod = {
 	.name		= "l4_ls",
 	.class		= &am33xx_l4_hwmod_class,
 	.clkdm_name	= "l4ls_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "l4ls_gclk",
 	.prcm		= {
 		.omap4	= {
@@ -134,7 +134,7 @@ static struct omap_hwmod am33xx_l4_hs_hwmod = {
 	.name		= "l4_hs",
 	.class		= &am33xx_l4_hwmod_class,
 	.clkdm_name	= "l4hs_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "l4hs_gclk",
 	.prcm		= {
 		.omap4	= {
@@ -150,7 +150,7 @@ static struct omap_hwmod am33xx_l4_wkup_hwmod = {
 	.name		= "l4_wkup",
 	.class		= &am33xx_l4_hwmod_class,
 	.clkdm_name	= "l4_wkup_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.prcm		= {
 		.omap4	= {
 			.clkctrl_offs	= AM33XX_CM_WKUP_L4WKUP_CLKCTRL_OFFSET,
@@ -170,7 +170,7 @@ static struct omap_hwmod am33xx_mpu_hwmod = {
 	.name		= "mpu",
 	.class		= &am33xx_mpu_hwmod_class,
 	.clkdm_name	= "mpu_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_mpu_m2_ck",
 	.prcm		= {
 		.omap4	= {
@@ -472,7 +472,7 @@ static struct omap_hwmod am33xx_ocmcram_hwmod = {
 	.name		= "ocmcram",
 	.class		= &am33xx_ocmcram_hwmod_class,
 	.clkdm_name	= "l3_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "l3_gclk",
 	.prcm		= {
 		.omap4	= {
@@ -526,7 +526,7 @@ static struct omap_hwmod am33xx_control_hwmod = {
 	.name		= "control",
 	.class		= &am33xx_control_hwmod_class,
 	.clkdm_name	= "l4_wkup_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_core_m4_div2_ck",
 	.prcm		= {
 		.omap4	= {
arch/arm/mach-omap2/omap_hwmod_44xx_data.c +3 -3
index 9c3b504..1e5b12c 100644
@@ -914,7 +914,7 @@ static struct omap_hwmod omap44xx_emif1_hwmod = {
 	.name		= "emif1",
 	.class		= &omap44xx_emif_hwmod_class,
 	.clkdm_name	= "l3_emif_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "ddrphy_ck",
 	.prcm = {
 		.omap4 = {
@@ -930,7 +930,7 @@ static struct omap_hwmod omap44xx_emif2_hwmod = {
 	.name		= "emif2",
 	.class		= &omap44xx_emif_hwmod_class,
 	.clkdm_name	= "l3_emif_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "ddrphy_ck",
 	.prcm = {
 		.omap4 = {
@@ -2193,7 +2193,7 @@ static struct omap_hwmod omap44xx_mpu_hwmod = {
 	.name		= "mpu",
 	.class		= &omap44xx_mpu_hwmod_class,
 	.clkdm_name	= "mpuss_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_mpu_m2_ck",
 	.prcm = {
 		.omap4 = {
arch/arm/mach-omap2/omap_hwmod_54xx_data.c +3 -3
index 3c70f5c..5483c6d 100644
@@ -353,7 +353,7 @@ static struct omap_hwmod omap54xx_emif1_hwmod = {
 	.name		= "emif1",
 	.class		= &omap54xx_emif_hwmod_class,
 	.clkdm_name	= "emif_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_core_h11x2_ck",
 	.prcm = {
 		.omap4 = {
@@ -369,7 +369,7 @@ static struct omap_hwmod omap54xx_emif2_hwmod = {
 	.name		= "emif2",
 	.class		= &omap54xx_emif_hwmod_class,
 	.clkdm_name	= "emif_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_core_h11x2_ck",
 	.prcm = {
 		.omap4 = {
@@ -1103,7 +1103,7 @@ static struct omap_hwmod omap54xx_mpu_hwmod = {
 	.name		= "mpu",
 	.class		= &omap54xx_mpu_hwmod_class,
 	.clkdm_name	= "mpu_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_mpu_m2_ck",
 	.prcm = {
 		.omap4 = {
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

In reply to Rajendra Nayak

On OMAP we have co-processor IPs, memory controllers, GPIOs which control regulators and power switches to PMIC, and SoC internal Bus IPs, some or most of which should either not be reset or idled or both. Have a way to pass this information from DT.
(In some cases there are erratas which prevent an IPs from being reset)

Also update omap_hwmod to extract this from DT.

---
 .../devicetree/bindings/arm/omap/omap.txt          |    3 ++-
 arch/arm/mach-omap2/omap_hwmod.c                   |   22 +++++++++++++-------
 2 files changed, 17 insertions(+), 8 deletions(-)
Documentation/devicetree/bindings/arm/omap/omap.txt +2 -1
index 6d498c7..a08647e 100644
@@ -21,7 +21,8 @@ Required properties:
 Optional properties:
 - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
   during suspend.
-
+- ti,no-reset: When present, the module should not be reset
+- ti,no-idle: When present, the module should not be idled
 
 Example:
 
arch/arm/mach-omap2/omap_hwmod.c +15 -7
index 7f4db12..4c66a5e 100644
@@ -2361,11 +2361,11 @@ static struct device_node *of_dev_hwmod_lookup(struct device_node *np,
  * are part of the device's address space can be ioremapped properly.
  * No return value.
  */
-static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
+static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
+				     struct device_node *np)
 {
 	struct omap_hwmod_addr_space *mem;
 	void __iomem *va_start = NULL;
-	struct device_node *np;
 
 	if (!oh)
 		return;
@@ -2381,12 +2381,10 @@ static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
 			 oh->name);
 
 		/* Extract the IO space from device tree blob */
-		if (!of_have_populated_dt())
+		if (!np)
 			return;
 
-		np = of_dev_hwmod_lookup(of_find_node_by_name(NULL, "ocp"), oh);
-		if (np)
-			va_start = of_iomap(np, oh->mpu_rt_idx);
+		va_start = of_iomap(np, oh->mpu_rt_idx);
 	} else {
 		va_start = ioremap(mem->pa_start, mem->pa_end - mem->pa_start);
 	}
@@ -2418,12 +2416,16 @@ static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
 static int __init _init(struct omap_hwmod *oh, void *data)
 {
 	int r;
+	struct device_node *np = NULL;
 
 	if (oh->_state != _HWMOD_STATE_REGISTERED)
 		return 0;
 
+	if (of_have_populated_dt())
+		np = of_dev_hwmod_lookup(of_find_node_by_name(NULL, "ocp"), oh);
+
 	if (oh->class->sysc)
-		_init_mpu_rt_base(oh, NULL);
+		_init_mpu_rt_base(oh, NULL, np);
 
 	r = _init_clocks(oh, NULL);
 	if (r < 0) {
@@ -2431,6 +2433,12 @@ static int __init _init(struct omap_hwmod *oh, void *data)
 		return -EINVAL;
 	}
 
+	if (np)
+		if (of_find_property(np, "ti,no-reset", NULL))
+			oh->flags |= HWMOD_INIT_NO_RESET;
+		if (of_find_property(np, "ti,no-idle", NULL))
+			oh->flags |= HWMOD_INIT_NO_IDLE;
+
 	oh->_state = _HWMOD_STATE_INITIALIZED;
 
 	return 0;
Signature · 1 line
1.7.9.5
maintainer
#3 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 21 lines · Rajendra Nayak

On OMAP we have co-processor IPs, memory controllers, GPIOs which control regulators and power switches to PMIC, and SoC internal Bus IPs, some or most of which should either not be reset or idled or both. Have a way to pass this information from DT.
(In some cases there are erratas which prevent an IPs from being reset)

Also update omap_hwmod to extract this from DT.

---
 .../devicetree/bindings/arm/omap/omap.txt          |    3 ++-
 arch/arm/mach-omap2/omap_hwmod.c                   |   22 +++++++++++++-------
 2 files changed, 17 insertions(+), 8 deletions(-)
Documentation/devicetree/bindings/arm/omap/omap.txt +0 -0
index 6d498c7..a08647e 100644

Documentation/devicetree/bindings/arm/omap/omap.txt Required properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/arm/omap/omap.txt +2 -1
@@ -21,7 +21,8 @@ Required properties:
 Optional properties:
 - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
   during suspend.
-
+- ti,no-reset: When present, the module should not be reset
+- ti,no-idle: When present, the module should not be idled

This naming is a bit confusing as people may think that the hardware has no reset support or no idle support. Let's try to make this to describe the hardware a bit more instead.

Then ideally we'd not map individual bits of data to properties, but describe few basic types of hardware instead and build lists of things instead of tagging things. Or maybe we can get this data from the bus hierarchy instead?

If these options don't work, and the choice may be board specific, then how about ti,skip-reset-on-init, and ti,skip-idle-on-init?

Regards,

Tony

#4 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 33 lines · Tony Lindgren
Quoted 21 lines · Rajendra Nayak

On OMAP we have co-processor IPs, memory controllers, GPIOs which control regulators and power switches to PMIC, and SoC internal Bus IPs, some or most of which should either not be reset or idled or both. Have a way to pass this information from DT.
(In some cases there are erratas which prevent an IPs from being reset)

Also update omap_hwmod to extract this from DT.

---
 .../devicetree/bindings/arm/omap/omap.txt          |    3 ++-
 arch/arm/mach-omap2/omap_hwmod.c                   |   22 +++++++++++++-------
 2 files changed, 17 insertions(+), 8 deletions(-)
Documentation/devicetree/bindings/arm/omap/omap.txt +0 -0
index 6d498c7..a08647e 100644

Documentation/devicetree/bindings/arm/omap/omap.txt Required properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/arm/omap/omap.txt +2 -1
@@ -21,7 +21,8 @@ Required properties:
 Optional properties:
 - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
   during suspend.
-
+- ti,no-reset: When present, the module should not be reset
+- ti,no-idle: When present, the module should not be idled

This naming is a bit confusing as people may think that the hardware has no reset support or no idle support. Let's try to make this to describe the hardware a bit more instead.

Quoted 4 lines

Then ideally we'd not map individual bits of data to properties, but describe few basic types of hardware instead and build lists of things instead of tagging things. Or maybe we can get this data from the bus hierarchy instead?

Yeah, I thought if I could do this without any new bindings. For instance, never reset or idle something which is a "cpu" or an "interrupt-controller" or a "memory-controller" or a "bus". For some like memory-controllers though there are no bindings which describe them as memory-controllers.

Quoted 4 lines

If these options don't work, and the choice may be board specific, then how about ti,skip-reset-on-init, and ti,skip-idle-on-init?

This looks fine too.

Quoted 5 lines

Regards,

Tony

maintainer
#5 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 46 lines · Rajendra Nayak
Quoted 33 lines · Tony Lindgren
Quoted 21 lines · Rajendra Nayak

On OMAP we have co-processor IPs, memory controllers, GPIOs which control regulators and power switches to PMIC, and SoC internal Bus IPs, some or most of which should either not be reset or idled or both. Have a way to pass this information from DT.
(In some cases there are erratas which prevent an IPs from being reset)

Also update omap_hwmod to extract this from DT.

---
 .../devicetree/bindings/arm/omap/omap.txt          |    3 ++-
 arch/arm/mach-omap2/omap_hwmod.c                   |   22 +++++++++++++-------
 2 files changed, 17 insertions(+), 8 deletions(-)
Documentation/devicetree/bindings/arm/omap/omap.txt +0 -0
index 6d498c7..a08647e 100644

Documentation/devicetree/bindings/arm/omap/omap.txt Required properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/arm/omap/omap.txt +2 -1
@@ -21,7 +21,8 @@ Required properties:
 Optional properties:
 - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
   during suspend.
-
+- ti,no-reset: When present, the module should not be reset
+- ti,no-idle: When present, the module should not be idled

This naming is a bit confusing as people may think that the hardware has no reset support or no idle support. Let's try to make this to describe the hardware a bit more instead.

Quoted 4 lines

Then ideally we'd not map individual bits of data to properties, but describe few basic types of hardware instead and build lists of things instead of tagging things. Or maybe we can get this data from the bus hierarchy instead?

Yeah, I thought if I could do this without any new bindings. For instance, never reset or idle something which is a "cpu" or an "interrupt-controller" or a "memory-controller" or a "bus". For some like memory-controllers though there are no bindings which describe them as memory-controllers.

Quoted 1 line
Quoted 5 lines
Quoted 3 lines

If these options don't work, and the choice may be board specific, then how about ti,skip-reset-on-init, and ti,skip-idle-on-init?

This looks fine too.

Or you could also have various bus specific bindings for the ocp with lists of phandles?

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,reset-on-init = <&module1, &module2>;
	...
};

Or something similar.

Tony

#6 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 56 lines · Tony Lindgren
Quoted 46 lines · Rajendra Nayak
Quoted 33 lines · Tony Lindgren
Quoted 21 lines · Rajendra Nayak

On OMAP we have co-processor IPs, memory controllers, GPIOs which control regulators and power switches to PMIC, and SoC internal Bus IPs, some or most of which should either not be reset or idled or both. Have a way to pass this information from DT.
(In some cases there are erratas which prevent an IPs from being reset)

Also update omap_hwmod to extract this from DT.

---
 .../devicetree/bindings/arm/omap/omap.txt          |    3 ++-
 arch/arm/mach-omap2/omap_hwmod.c                   |   22 +++++++++++++-------
 2 files changed, 17 insertions(+), 8 deletions(-)
Documentation/devicetree/bindings/arm/omap/omap.txt +0 -0
index 6d498c7..a08647e 100644

Documentation/devicetree/bindings/arm/omap/omap.txt Required properties: · line 25

Quoted 6 lines
Documentation/devicetree/bindings/arm/omap/omap.txt +2 -1
@@ -21,7 +21,8 @@ Required properties:
 Optional properties:
 - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
   during suspend.
-
+- ti,no-reset: When present, the module should not be reset
+- ti,no-idle: When present, the module should not be idled

This naming is a bit confusing as people may think that the hardware has no reset support or no idle support. Let's try to make this to describe the hardware a bit more instead.

Quoted 4 lines

Then ideally we'd not map individual bits of data to properties, but describe few basic types of hardware instead and build lists of things instead of tagging things. Or maybe we can get this data from the bus hierarchy instead?

Yeah, I thought if I could do this without any new bindings. For instance, never reset or idle something which is a "cpu" or an "interrupt-controller" or a "memory-controller" or a "bus". For some like memory-controllers though there are no bindings which describe them as memory-controllers.

Quoted 1 line
Quoted 5 lines
Quoted 3 lines

If these options don't work, and the choice may be board specific, then how about ti,skip-reset-on-init, and ti,skip-idle-on-init?

This looks fine too.

Or you could also have various bus specific bindings for the ocp with lists of phandles?

Quoted 8 lines
ocp {
	reg = <...>;
	interrupts = <...>;
	ti,reset-on-init = <&module1, &module2>;
	...
};

Or something similar.

The only problem I see with this is that some of these modules could be board specific ones and need to be part of the board dts files, like some boards which have PMIC power switch hooked up to some gpio etc. So there could be some SoC specific modules (like emif/gpmc on OMAPs) and some which depend on how the boards are designed.

Quoted 3 lines

Tony

maintainer
#7 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 12 lines · Rajendra Nayak
Quoted 11 lines · Tony Lindgren

Or you could also have various bus specific bindings for the ocp with lists of phandles?

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,reset-on-init = <&module1, &module2>;
	...
};
Quoted 7 lines
Quoted 1 line

Or something similar.

The only problem I see with this is that some of these modules could be board specific ones and need to be part of the board dts files, like some boards which have PMIC power switch hooked up to some gpio etc. So there could be some SoC specific modules (like emif/gpmc on OMAPs) and some which depend on how the boards are designed.

You can still override the ocp entry in the board specific .dts file. Would probably be a lot easier than to override each module separately in the board specific .dts file.

Regards,

Tony

#8 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 21 lines · Tony Lindgren
Quoted 12 lines · Rajendra Nayak
Quoted 11 lines · Tony Lindgren

Or you could also have various bus specific bindings for the ocp with lists of phandles?

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,reset-on-init = <&module1, &module2>;
	...
};
Quoted 7 lines
Quoted 1 line

Or something similar.

The only problem I see with this is that some of these modules could be board specific ones and need to be part of the board dts files, like some boards which have PMIC power switch hooked up to some gpio etc. So there could be some SoC specific modules (like emif/gpmc on OMAPs) and some which depend on how the boards are designed.

Quoted 3 lines

You can still override the ocp entry in the board specific .dts file. Would probably be a lot easier than to override each module separately in the board specific .dts file.

So, If I understand this right we would have the dt entries something like,

omap4.dtsi
------

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,no-reset-on-init = <&emif1, &emif2, &gpmc>;
	...
};

omap4-panda-es.dts
------

ocp {
	ti,no-reset-on-init = <&emif1, &emif2, &gpmc, &gpio4>;
	...
};

Is it that, or you suggesting we can _append_ the soc list of modules with board specific modules, which I am not sure if its possible.

#9 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 13 lines

So, If I understand this right we would have the dt entries something like,

omap4.dtsi
------

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,no-reset-on-init = <&emif1, &emif2, &gpmc>;
	...
};
Quoted 3 lines

omap4-panda-es.dts
------
ocp {

This would actually be
&ocp {

Quoted 13 lines
	ti,no-reset-on-init = <&emif1, &emif2, &gpmc, &gpio4>;
	...
};

Is it that, or you suggesting we can _append_ the soc list of modules with board specific modules, which I am not sure if its possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

maintainer
#10 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

In reply to Rajendra Nayak
Quoted 40 lines · Rajendra Nayak
Quoted 21 lines · Tony Lindgren
Quoted 12 lines · Rajendra Nayak
Quoted 11 lines · Tony Lindgren

Or you could also have various bus specific bindings for the ocp with lists of phandles?

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,reset-on-init = <&module1, &module2>;
	...
};
Quoted 7 lines
Quoted 1 line

Or something similar.

The only problem I see with this is that some of these modules could be board specific ones and need to be part of the board dts files, like some boards which have PMIC power switch hooked up to some gpio etc. So there could be some SoC specific modules (like emif/gpmc on OMAPs) and some which depend on how the boards are designed.

Quoted 3 lines

You can still override the ocp entry in the board specific .dts file. Would probably be a lot easier than to override each module separately in the board specific .dts file.

So, If I understand this right we would have the dt entries something like,

omap4.dtsi
------

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,no-reset-on-init = <&emif1, &emif2, &gpmc>;
	...
};

omap4-panda-es.dts
------

Quoted 8 lines
ocp {
	ti,no-reset-on-init = <&emif1, &emif2, &gpmc, &gpio4>;
	...
};

Is it that, or you suggesting we can _append_ the soc list of modules with board specific modules, which I am not sure if its possible.

Yes I think the board specific entry just overrides the .dtsi entry.

Regards,

Tony

#11 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 46 lines · Tony Lindgren
Quoted 45 lines · Rajendra Nayak
Quoted 21 lines · Tony Lindgren
Quoted 12 lines · Rajendra Nayak
Quoted 11 lines · Tony Lindgren

Or you could also have various bus specific bindings for the ocp with lists of phandles?

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,reset-on-init = <&module1, &module2>;
	...
};
Quoted 7 lines
Quoted 1 line

Or something similar.

The only problem I see with this is that some of these modules could be board specific ones and need to be part of the board dts files, like some boards which have PMIC power switch hooked up to some gpio etc. So there could be some SoC specific modules (like emif/gpmc on OMAPs) and some which depend on how the boards are designed.

Quoted 3 lines

You can still override the ocp entry in the board specific .dts file. Would probably be a lot easier than to override each module separately in the board specific .dts file.

So, If I understand this right we would have the dt entries something like,

omap4.dtsi
------

ocp {
	reg = <...>;
	interrupts = <...>;
	ti,no-reset-on-init = <&emif1, &emif2, &gpmc>;
	...
};

omap4-panda-es.dts
------

ocp {
	ti,no-reset-on-init = <&emif1, &emif2, &gpmc, &gpio4>;
	...
};
Quoted 6 lines
Quoted 3 lines

Is it that, or you suggesting we can _append_ the soc list of modules with board specific modules, which I am not sure if its possible.

Yes I think the board specific entry just overrides the .dtsi entry.

That's the issue of using a single attribute with a list. You cannot add or remove an entry. That's why I think it is better to stick to a per device attribute.

Moreover, in term of SW, this is managed at device level today. I'm not convince it will make sense to move that to the OCP level for the moment.

Even if we consider that in the future an omap-ocp bus driver will handle most of the PM/reset glue managed today by hwmod, it will still be a per device attribute. The bus driver, being the parent of the device, will anyway be able to retrieve the information at device level if needed.

Regards,
Benoit

#12 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

In reply to Tony Lindgren
Quoted 17 lines · Tony Lindgren
Quoted 16 lines · Rajendra Nayak

On OMAP we have co-processor IPs, memory controllers, GPIOs which control regulators and power switches to PMIC, and SoC internal Bus IPs, some or most of which should either not be reset or idled or both. Have a way to pass this information from DT.
(In some cases there are erratas which prevent an IPs from being reset)

Also update omap_hwmod to extract this from DT.

---
  .../devicetree/bindings/arm/omap/omap.txt          |    3 ++-
  arch/arm/mach-omap2/omap_hwmod.c                   |   22 +++++++++++++-------
  2 files changed, 17 insertions(+), 8 deletions(-)
Quoted 15 lines

Documentation/devicetree/bindings/arm/omap/omap.txt Required properties: · line 25

Quoted 11 lines
Documentation/devicetree/bindings/arm/omap/omap.txt +2 -1
index 6d498c7..a08647e 100644
@@ -21,7 +21,8 @@ Required properties:
  Optional properties:
  - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
    during suspend.
-
+- ti,no-reset: When present, the module should not be reset
+- ti,no-idle: When present, the module should not be idled

This naming is a bit confusing as people may think that the hardware has no reset support or no idle support. Let's try to make this to describe the hardware a bit more instead.

Yeah, I do agree here. That should look like a real HW property and nor a configuration.

Quoted 8 lines

Then ideally we'd not map individual bits of data to properties, but describe few basic types of hardware instead and build lists of things instead of tagging things. Or maybe we can get this data from the bus hierarchy instead?

If these options don't work, and the choice may be board specific, then how about ti,skip-reset-on-init, and ti,skip-idle-on-init?

It looks like a configuration as well :-).
I was thinking of something like "ti,do-not-support-reset-on-init", but that a little bit too long.

Regards,
Benoit

#13 lore raw

PATCH 2/3 ARM: OMAP2+: Add new bindings for OMAP

Quoted 41 lines · Benoit Cousson
Quoted 17 lines · Tony Lindgren
Quoted 16 lines · Rajendra Nayak

On OMAP we have co-processor IPs, memory controllers, GPIOs which control regulators and power switches to PMIC, and SoC internal Bus IPs, some or most of which should either not be reset or idled or both. Have a way to pass this information from DT.
(In some cases there are erratas which prevent an IPs from being reset)

Also update omap_hwmod to extract this from DT.

---
  .../devicetree/bindings/arm/omap/omap.txt          |    3 ++-
  arch/arm/mach-omap2/omap_hwmod.c                   |   22 +++++++++++++-------
  2 files changed, 17 insertions(+), 8 deletions(-)
Quoted 15 lines

Documentation/devicetree/bindings/arm/omap/omap.txt Required properties: · line 25

Quoted 11 lines
Documentation/devicetree/bindings/arm/omap/omap.txt +2 -1
index 6d498c7..a08647e 100644
@@ -21,7 +21,8 @@ Required properties:
  Optional properties:
  - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
    during suspend.
-
+- ti,no-reset: When present, the module should not be reset
+- ti,no-idle: When present, the module should not be idled

This naming is a bit confusing as people may think that the hardware has no reset support or no idle support. Let's try to make this to describe the hardware a bit more instead.

Yeah, I do agree here. That should look like a real HW property and nor a configuration.

Quoted 5 lines

Then ideally we'd not map individual bits of data to properties, but describe few basic types of hardware instead and build lists of things instead of tagging things. Or maybe we can get this data from the bus hierarchy instead?

Quoted 6 lines
Quoted 3 lines

If these options don't work, and the choice may be board specific, then how about ti,skip-reset-on-init, and ti,skip-idle-on-init?

It looks like a configuration as well :-).
I was thinking of something like "ti,do-not-support-reset-on-init", but that a little bit too long.

How about 'ti,no-reset-on-init' :) which is what I had initially which I then moved to 'ti,no-reset'

Quoted 4 lines

Regards,
Benoit

#14 lore raw

PATCH 3/3 ARM: OMAP2+: Let DT say what devices should not to idled or reset

In reply to Rajendra Nayak

Now that we have DT bindings to specify which devices on the SoC should not be reset or idled, get rid of the same information existing as part of the hwmod data files and pass this info from DT instead.

For GPMC, the HWMOD_INIT_NO_RESET flag seems to be added in hwmod not due to any errata around the GPMC IP, but rather because any timings set by the bootloader are not being correctly programmed by the kernel. This seems like something that needs to be fixed as part of GPMC driver in the kernel, and hence the flag is left as is in hwmod, which can be removed once the driver does what its expected to.

---
 arch/arm/boot/dts/am33xx.dtsi              |    2 ++
 arch/arm/boot/dts/omap4.dtsi               |    3 +++
 arch/arm/boot/dts/omap5.dtsi               |    2 ++
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c |    4 ++--
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    4 +---
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    2 --
 6 files changed, 10 insertions(+), 7 deletions(-)
arch/arm/boot/dts/am33xx.dtsi +2 -0
index 38b446b..402d373 100644
@@ -492,6 +492,7 @@
 			reg = <0x44d00000 0x4000	/* M3 UMEM */
 			       0x44d80000 0x2000>;	/* M3 DMEM */
 			ti,hwmods = "wkup_m3";
+			ti,no-reset;
 		};
 
 		elm: elm at 48080000 {
@@ -522,6 +523,7 @@
 		gpmc: gpmc at 50000000 {
 			compatible = "ti,am3352-gpmc";
 			ti,hwmods = "gpmc";
+			ti,no-idle;
 			reg = <0x50000000 0x2000>;
 			interrupts = <100>;
 			gpmc,num-cs = <7>;
arch/arm/boot/dts/omap4.dtsi +3 -0
index 22d9f2b..9e22399 100644
@@ -214,6 +214,7 @@
 			gpmc,num-cs = <8>;
 			gpmc,num-waitpins = <4>;
 			ti,hwmods = "gpmc";
+			ti,no-idle;
 		};
 
 		uart1: serial at 4806a000 {
@@ -492,6 +493,7 @@
 			reg = <0x4c000000 0x100>;
 			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "emif1";
+			ti,no-idle;
 			phy-type = <1>;
 			hw-caps-read-idle-ctrl;
 			hw-caps-ll-interface;
@@ -503,6 +505,7 @@
 			reg = <0x4d000000 0x100>;
 			interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "emif2";
+			ti,no-idle;
 			phy-type = <1>;
 			hw-caps-read-idle-ctrl;
 			hw-caps-ll-interface;
arch/arm/boot/dts/omap5.dtsi +2 -0
index e643620..85d20af 100644
@@ -607,6 +607,7 @@
 		emif1: emif at 0x4c000000 {
 			compatible	= "ti,emif-4d5";
 			ti,hwmods	= "emif1";
+			ti,no-idle;
 			phy-type	= <2>; /* DDR PHY type: Intelli PHY */
 			reg = <0x4c000000 0x400>;
 			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
@@ -618,6 +619,7 @@
 		emif2: emif at 0x4d000000 {
 			compatible	= "ti,emif-4d5";
 			ti,hwmods	= "emif2";
+			ti,no-idle;
 			phy-type	= <2>; /* DDR PHY type: Intelli PHY */
 			reg = <0x4d000000 0x400>;
 			interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>;
arch/arm/mach-omap2/omap_hwmod_33xx_data.c +2 -2
index d4f0531..f34612b 100644
@@ -198,7 +198,7 @@ static struct omap_hwmod am33xx_wkup_m3_hwmod = {
 	.class		= &am33xx_wkup_m3_hwmod_class,
 	.clkdm_name	= "l4_wkup_aon_clkdm",
 	/* Keep hardreset asserted */
-	.flags		= HWMOD_INIT_NO_RESET | HWMOD_NO_IDLEST,
+	.flags		= HWMOD_NO_IDLEST,
 	.main_clk	= "dpll_core_m4_div2_ck",
 	.prcm		= {
 		.omap4	= {
@@ -926,7 +926,7 @@ static struct omap_hwmod am33xx_gpmc_hwmod = {
 	.name		= "gpmc",
 	.class		= &am33xx_gpmc_hwmod_class,
 	.clkdm_name	= "l3s_clkdm",
-	.flags		= (HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET),
+	.flags		= HWMOD_INIT_NO_RESET,
 	.main_clk	= "l3s_gclk",
 	.prcm		= {
 		.omap4	= {
arch/arm/mach-omap2/omap_hwmod_44xx_data.c +1 -3
index 1e5b12c..a507a70 100644
@@ -914,7 +914,6 @@ static struct omap_hwmod omap44xx_emif1_hwmod = {
 	.name		= "emif1",
 	.class		= &omap44xx_emif_hwmod_class,
 	.clkdm_name	= "l3_emif_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "ddrphy_ck",
 	.prcm = {
 		.omap4 = {
@@ -930,7 +929,6 @@ static struct omap_hwmod omap44xx_emif2_hwmod = {
 	.name		= "emif2",
 	.class		= &omap44xx_emif_hwmod_class,
 	.clkdm_name	= "l3_emif_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "ddrphy_ck",
 	.prcm = {
 		.omap4 = {
@@ -1184,7 +1182,7 @@ static struct omap_hwmod omap44xx_gpmc_hwmod = {
 	 * the kernel from the board file or DT data.
 	 * HWMOD_INIT_NO_RESET should be removed ASAP.
 	 */
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
+	.flags		= HWMOD_INIT_NO_RESET,
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L3_2_GPMC_CLKCTRL_OFFSET,
arch/arm/mach-omap2/omap_hwmod_54xx_data.c +0 -2
index 5483c6d..2cdc194 100644
@@ -353,7 +353,6 @@ static struct omap_hwmod omap54xx_emif1_hwmod = {
 	.name		= "emif1",
 	.class		= &omap54xx_emif_hwmod_class,
 	.clkdm_name	= "emif_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_core_h11x2_ck",
 	.prcm = {
 		.omap4 = {
@@ -369,7 +368,6 @@ static struct omap_hwmod omap54xx_emif2_hwmod = {
 	.name		= "emif2",
 	.class		= &omap54xx_emif_hwmod_class,
 	.clkdm_name	= "emif_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE,
 	.main_clk	= "dpll_core_h11x2_ck",
 	.prcm = {
 		.omap4 = {
Signature · 1 line
1.7.9.5
maintainer
#15 lore raw

PATCH 3/3 ARM: OMAP2+: Let DT say what devices should not to idled or reset

Hi Beno?t, Rajendra,

Quoted 12 lines · Rajendra Nayak

Now that we have DT bindings to specify which devices on the SoC should not be reset or idled, get rid of the same information existing as part of the hwmod data files and pass this info from DT instead.

For GPMC, the HWMOD_INIT_NO_RESET flag seems to be added in hwmod not due to any errata around the GPMC IP, but rather because any timings set by the bootloader are not being correctly programmed by the kernel. This seems like something that needs to be fixed as part of GPMC driver in the kernel, and hence the flag is left as is in hwmod, which can be removed once the driver does what its expected to.

Quoted 8 lines
---
 arch/arm/boot/dts/am33xx.dtsi              |    2 ++
 arch/arm/boot/dts/omap4.dtsi               |    3 +++
 arch/arm/boot/dts/omap5.dtsi               |    2 ++
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c |    4 ++--
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    4 +---
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    2 --
 6 files changed, 10 insertions(+), 7 deletions(-)

Looking at this one, maybe the best thing for this patch is for Rajendra to split it into two patches. Beno?t can merge the DTS patch first, then I can merge the hwmod side as a cleanup once the first one goes in. That will avoid conflicts from other DTS and hwmod changes going into the tree.

Rajendra, when you do the split, please add in the DT documentation part from patch 2. Am going to strip that out from the second patch and just merge the hwmod changes.

Sound good?

- Paul

#16 lore raw

PATCH 3/3 ARM: OMAP2+: Let DT say what devices should not to idled or reset

Quoted 30 lines · Paul Walmsley

Hi Beno?t, Rajendra,

Quoted 12 lines · Rajendra Nayak

Now that we have DT bindings to specify which devices on the SoC should not be reset or idled, get rid of the same information existing as part of the hwmod data files and pass this info from DT instead.

For GPMC, the HWMOD_INIT_NO_RESET flag seems to be added in hwmod not due to any errata around the GPMC IP, but rather because any timings set by the bootloader are not being correctly programmed by the kernel. This seems like something that needs to be fixed as part of GPMC driver in the kernel, and hence the flag is left as is in hwmod, which can be removed once the driver does what its expected to.

Quoted 8 lines
---
 arch/arm/boot/dts/am33xx.dtsi              |    2 ++
 arch/arm/boot/dts/omap4.dtsi               |    3 +++
 arch/arm/boot/dts/omap5.dtsi               |    2 ++
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c |    4 ++--
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    4 +---
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    2 --
 6 files changed, 10 insertions(+), 7 deletions(-)

Looking at this one, maybe the best thing for this patch is for Rajendra to split it into two patches. Beno?t can merge the DTS patch first, then I can merge the hwmod side as a cleanup once the first one goes in. That will avoid conflicts from other DTS and hwmod changes going into the tree.

Quoted 5 lines

Rajendra, when you do the split, please add in the DT documentation part from patch 2. Am going to strip that out from the second patch and just merge the hwmod changes.

Sound good?

Sure Paul, I'll repost the complete series with proper splits such that its easier for you and Benoit to pick them up independently.

Quoted 4 lines

- Paul

#17 lore raw

PATCH 3/3 ARM: OMAP2+: Let DT say what devices should not to idled or reset

Hi Rajendra,

Quoted 31 lines · Rajendra Nayak
Quoted 30 lines · Paul Walmsley

Hi Beno?t, Rajendra,

Quoted 12 lines · Rajendra Nayak

Now that we have DT bindings to specify which devices on the SoC should not be reset or idled, get rid of the same information existing as part of the hwmod data files and pass this info from DT instead.

For GPMC, the HWMOD_INIT_NO_RESET flag seems to be added in hwmod not due to any errata around the GPMC IP, but rather because any timings set by the bootloader are not being correctly programmed by the kernel. This seems like something that needs to be fixed as part of GPMC driver in the kernel, and hence the flag is left as is in hwmod, which can be removed once the driver does what its expected to.

Quoted 8 lines
---
  arch/arm/boot/dts/am33xx.dtsi              |    2 ++
  arch/arm/boot/dts/omap4.dtsi               |    3 +++
  arch/arm/boot/dts/omap5.dtsi               |    2 ++
  arch/arm/mach-omap2/omap_hwmod_33xx_data.c |    4 ++--
  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    4 +---
  arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    2 --
  6 files changed, 10 insertions(+), 7 deletions(-)

Looking at this one, maybe the best thing for this patch is for Rajendra to split it into two patches. Beno?t can merge the DTS patch first, then I can merge the hwmod side as a cleanup once the first one goes in. That will avoid conflicts from other DTS and hwmod changes going into the tree.

Quoted 8 lines
Quoted 5 lines

Rajendra, when you do the split, please add in the DT documentation part from patch 2. Am going to strip that out from the second patch and just merge the hwmod changes.

Sound good?

Sure Paul, I'll repost the complete series with proper splits such that its easier for you and Benoit to pick them up independently.

If you could do it soon, I'm about to send an early pull request to Tony to avoid the trouble we had last time.

Thanks,
Benoit

#18 lore raw

PATCH 3/3 ARM: OMAP2+: Let DT say what devices should not to idled or reset

Quoted 38 lines · Benoit Cousson

Hi Rajendra,

Quoted 35 lines · Rajendra Nayak
Quoted 34 lines · Paul Walmsley

Hi Beno?t, Rajendra,

Quoted 12 lines · Rajendra Nayak

Now that we have DT bindings to specify which devices on the SoC should not be reset or idled, get rid of the same information existing as part of the hwmod data files and pass this info from DT instead.

For GPMC, the HWMOD_INIT_NO_RESET flag seems to be added in hwmod not due to any errata around the GPMC IP, but rather because any timings set by the bootloader are not being correctly programmed by the kernel. This seems like something that needs to be fixed as part of GPMC driver in the kernel, and hence the flag is left as is in hwmod, which can be removed once the driver does what its expected to.

Quoted 8 lines
---
  arch/arm/boot/dts/am33xx.dtsi              |    2 ++
  arch/arm/boot/dts/omap4.dtsi               |    3 +++
  arch/arm/boot/dts/omap5.dtsi               |    2 ++
  arch/arm/mach-omap2/omap_hwmod_33xx_data.c |    4 ++--
  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    4 +---
  arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    2 --
  6 files changed, 10 insertions(+), 7 deletions(-)

Looking at this one, maybe the best thing for this patch is for Rajendra to split it into two patches. Beno?t can merge the DTS patch first, then I can merge the hwmod side as a cleanup once the first one goes in. That will avoid conflicts from other DTS and hwmod changes going into the tree.

Rajendra, when you do the split, please add in the DT documentation part from patch 2. Am going to strip that out from the second patch and just merge the hwmod changes.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

Sound good?

Sure Paul, I'll repost the complete series with proper splits such that its easier for you and Benoit to pick them up independently.

If you could do it soon, I'm about to send an early pull request to Tony to avoid the trouble we had last time.

I am sending them right-away Benoit.

Quoted 4 lines

Thanks,
Benoit