diff mbox

OMAP: hwmod: add kernel cmdline flag to avoid resetting IP blocks during init

Message ID 20110703010206.19046.85293.stgit@dusk
State New
Headers show

Commit Message

Paul Walmsley July 3, 2011, 1:02 a.m. UTC
On some boards, it's not possible to reset certain IP blocks during
kernel initialization.  Some boards connect GPIO lines to external
voltage controllers, so resetting them may cause voltage to drop on a
critical supply[1].  Other boards connect GPIO lines to reset pins on
external devices without open documentation, so if the GPIO blocks are
reset, the external devices cannot currently be reconfigured[2].

The correct way to fix these issues on OMAP is to add a
hwmod_no_setup_reset(hwmod) call to the board file, that references
the IP block that must preserve its state.  This allows the OMAP core
code to reset the remaining IP blocks.

However, during initial bring-up of a new board, it may be desirable
to not reset any IP block, for ease of debugging.  This patch adds a
kernel command line parameter, 'hwmod_no_setup_reset', that prevents
the OMAP hwmod code from resetting any IP block during init.

1. Nayak, Rajendra.  _[PATCH 7/7] 4460sdp/blaze/panda: hwmod: Prevent
   gpio1 reset during hwmod init_.  1 July 2011.  E-mail to the
   linux-omap@vger.kernel.org mailing list.  Retrieved from
   http://www.mail-archive.com/linux-omap@vger.kernel.org/msg51992.html

2. Buesch, Michael.  _Nokia n810 LCD (MIPID/blizzard) on 2.6.38_.  28
   Feb 2011.  E-mail to the linux-omap@vger.kernel.org mailing list.
   Retrieved from http://www.spinics.net/lists/linux-omap/msg47277.html

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: BenoƮt Cousson <b-cousson@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Michael Buesch <mb@bu3sch.de>
Cc: Tony Lindgren <tony@atomide.com>
---
 Documentation/kernel-parameters.txt |    9 +++++++++
 arch/arm/mach-omap2/omap_hwmod.c    |   19 +++++++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

Comments

Tony Lindgren July 4, 2011, 8:55 a.m. UTC | #1
* Paul Walmsley <paul@pwsan.com> [110702 17:58]:
> On some boards, it's not possible to reset certain IP blocks during
> kernel initialization.  Some boards connect GPIO lines to external
> voltage controllers, so resetting them may cause voltage to drop on a
> critical supply[1].  Other boards connect GPIO lines to reset pins on
> external devices without open documentation, so if the GPIO blocks are
> reset, the external devices cannot currently be reconfigured[2].
> 
> The correct way to fix these issues on OMAP is to add a
> hwmod_no_setup_reset(hwmod) call to the board file, that references
> the IP block that must preserve its state.  This allows the OMAP core
> code to reset the remaining IP blocks.
> 
> However, during initial bring-up of a new board, it may be desirable
> to not reset any IP block, for ease of debugging.  This patch adds a
> kernel command line parameter, 'hwmod_no_setup_reset', that prevents
> the OMAP hwmod code from resetting any IP block during init.
> 
> 1. Nayak, Rajendra.  _[PATCH 7/7] 4460sdp/blaze/panda: hwmod: Prevent
>    gpio1 reset during hwmod init_.  1 July 2011.  E-mail to the
>    linux-omap@vger.kernel.org mailing list.  Retrieved from
>    http://www.mail-archive.com/linux-omap@vger.kernel.org/msg51992.html
> 
> 2. Buesch, Michael.  _Nokia n810 LCD (MIPID/blizzard) on 2.6.38_.  28
>    Feb 2011.  E-mail to the linux-omap@vger.kernel.org mailing list.
>    Retrieved from http://www.spinics.net/lists/linux-omap/msg47277.html

Also related is the gptimer12 on secure omaps as reported by Aaro.
 
> +	hwmod_no_setup_reset
> +			[OMAP] During kernel initialization,
> +			prevent the subarchitecture code from resetting
> +			IP blocks before the driver loads.  Useful for
> +			initial bring-up of boards which require some
> +			bootloader configuration to be retained on GPIO or
> +			other IP blocks.
> +

How about just make it hwmod_reset=1 with 0 being the default value
for now?

Regards,

Tony
Paul Walmsley July 6, 2011, 1:42 a.m. UTC | #2
cc'ing Aaro

On Mon, 4 Jul 2011, Tony Lindgren wrote:

> Also related is the gptimer12 on secure omaps as reported by Aaro.

This problem is actually worse.  In this case it's not just preventing 
reset; we must prevent the kernel from using the device at all. My 
understanding is also that it's the X-Loader or bootloader -- or more 
broadly, secure mode code -- that actually chooses which devices are 
protected in this way.  In other words, we can't make any automatic 
decisions that are based on knowing that a chip is a high-security chip -- 
it will need to be defined by the board file :-(

For this case, we probably need some board file function to tell the hwmod 
code to disregard a device completely, to tell the kernel to pretend that 
the device does not exist.


- Paul
Paul Walmsley July 6, 2011, 2 a.m. UTC | #3
On Tue, 5 Jul 2011, Paul Walmsley wrote:

> For this case, we probably need some board file function to tell the 
> hwmod code to disregard a device completely, to tell the kernel to 
> pretend that the device does not exist.

... and the other problem here is that we currently probe devices via an 
arch_initcall(), and the board file init_machine is also an arch_initcall.
Either we'll need to be very careful about Makefile ordering, or we should 
just call omap2_init_devices() from the board file init_machine directly.
The latter makes the most sense to me.


- Paul
Tony Lindgren July 6, 2011, 6:41 a.m. UTC | #4
* Paul Walmsley <paul@pwsan.com> [110705 18:37]:
> cc'ing Aaro
> 
> On Mon, 4 Jul 2011, Tony Lindgren wrote:
> 
> > Also related is the gptimer12 on secure omaps as reported by Aaro.
> 
> This problem is actually worse.  In this case it's not just preventing 
> reset; we must prevent the kernel from using the device at all. My 
> understanding is also that it's the X-Loader or bootloader -- or more 
> broadly, secure mode code -- that actually chooses which devices are 
> protected in this way.  In other words, we can't make any automatic 
> decisions that are based on knowing that a chip is a high-security chip -- 
> it will need to be defined by the board file :-(
> 
> For this case, we probably need some board file function to tell the hwmod 
> code to disregard a device completely, to tell the kernel to pretend that 
> the device does not exist.

Yeah that should be done in a generic way. Some shared devices can also
be used by a coprocessor like the DSP instead.

So basically we want to tell the following from the board file or board
specific .dts file:

- Device is being used on the board and can be reset and configured.
  This is the usual case.

- Device is being used on the board but can't be reset. This is the
  case for booting Linux from other operating systems initially where
  you want to keep the LCD on for debug console.

- Device is not being used on the board but can be reset for decent PM.
  This is often needed in cases where the bootloader just enables all the
  clocks.

- Device is reserved by secure mode or a coprocessor. In this case
  the device can't be reset.

So I guess that makes the flags noreset, disabled and unavailable?

For the devices with no special flags we would then reset the device.

Regards,

Tony
Tony Lindgren July 6, 2011, 6:43 a.m. UTC | #5
* Paul Walmsley <paul@pwsan.com> [110705 18:55]:
> On Tue, 5 Jul 2011, Paul Walmsley wrote:
> 
> > For this case, we probably need some board file function to tell the 
> > hwmod code to disregard a device completely, to tell the kernel to 
> > pretend that the device does not exist.
> 
> ... and the other problem here is that we currently probe devices via an 
> arch_initcall(), and the board file init_machine is also an arch_initcall.
> Either we'll need to be very careful about Makefile ordering, or we should 
> just call omap2_init_devices() from the board file init_machine directly.
> The latter makes the most sense to me.

And that can then be passed the list of devices with the special flags
for noreset, disabled and reserved. Then the late_initcall can optionally
reset the rest of the devices.

Regards,

Tony
Paul Walmsley July 6, 2011, 8:57 p.m. UTC | #6
Hi Tony

On Tue, 5 Jul 2011, Tony Lindgren wrote:

> So basically we want to tell the following from the board file or board
> specific .dts file:
> 
> - Device is being used on the board and can be reset and configured.
>   This is the usual case.

By 'used', you mean, have a Linux driver associated with them, right?

> - Device is being used on the board but can't be reset. This is the
>   case for booting Linux from other operating systems initially where
>   you want to keep the LCD on for debug console.

Or, to use examples that would be needed in a production device, the 
4460/N810 GPIO examples discussed earlier.  

Based on our current experience, there are a very small number of these 
cases, and they are board-specific.  So it seems to make sense to 
explicitly state these exceptions in the board files, via 
omap_hwmod_no_setup_reset() or something similar.

> - Device is not being used on the board but can be reset for decent PM.
>   This is often needed in cases where the bootloader just enables all the
>   clocks.

Yeah.  And it's not just bootloaders, it's also the previous kernel in the 
case of something like kexec.  Since we don't know what devices the 
bootloader or previous kernel touched, it seems to make sense to reset all 
of these devices by default, once the drivers are converted to use runtime 
PM.

The current issue here is that some drivers aren't yet converted, so the 
hwmod code might think that some devices are 'unused' when they are 
actually in use by a non-runtime-PM-based driver.
 
> - Device is reserved by secure mode or a coprocessor. In this case
>   the device can't be reset.

Indeed - or even accessed.

> So I guess that makes the flags noreset, disabled and unavailable?

I think we only need 'no reset' and 'unavailable'?  Once the runtime PM 
driver conversion is complete, I don't think there's any reason to keep 
the 'hwmod_unused_reset' command line flag around, since it can be the 
default.  That's because the hwmod code would then be able to track which 
devices were actually used.


- Paul
Paul Walmsley July 6, 2011, 9:01 p.m. UTC | #7
On Tue, 5 Jul 2011, Tony Lindgren wrote:

> * Paul Walmsley <paul@pwsan.com> [110705 18:55]:
> > On Tue, 5 Jul 2011, Paul Walmsley wrote:
> > 
> > > For this case, we probably need some board file function to tell the 
> > > hwmod code to disregard a device completely, to tell the kernel to 
> > > pretend that the device does not exist.
> > 
> > ... and the other problem here is that we currently probe devices via an 
> > arch_initcall(), and the board file init_machine is also an arch_initcall.
> > Either we'll need to be very careful about Makefile ordering, or we should 
> > just call omap2_init_devices() from the board file init_machine directly.
> > The latter makes the most sense to me.
> 
> And that can then be passed the list of devices with the special flags
> for noreset, disabled and reserved. Then the late_initcall can optionally
> reset the rest of the devices.

'disabled' can probably be the default for a device that doesn't have a 
driver associated with it, once the runtime PM conversion is complete.  
That's what the late_initcall would do.  Then if a driver is dynamically 
loaded for one of those devices, the hwmod code would just re-enable it.


- Paul
Tony Lindgren July 7, 2011, 5:27 a.m. UTC | #8
* Paul Walmsley <paul@pwsan.com> [110706 23:52]:
> Hi Tony
> 
> On Tue, 5 Jul 2011, Tony Lindgren wrote:
> 
> > So basically we want to tell the following from the board file or board
> > specific .dts file:
> > 
> > - Device is being used on the board and can be reset and configured.
> >   This is the usual case.
> 
> By 'used', you mean, have a Linux driver associated with them, right?

Right.
 
> > - Device is being used on the board but can't be reset. This is the
> >   case for booting Linux from other operating systems initially where
> >   you want to keep the LCD on for debug console.
> 
> Or, to use examples that would be needed in a production device, the 
> 4460/N810 GPIO examples discussed earlier.  
> 
> Based on our current experience, there are a very small number of these 
> cases, and they are board-specific.  So it seems to make sense to 
> explicitly state these exceptions in the board files, via 
> omap_hwmod_no_setup_reset() or something similar.

Yeah they should be all board specific. One more example comes to mind
was to keep the bootup logo around from bootloader on the LCD that the
DSS code was doing earlier.
 
> > - Device is not being used on the board but can be reset for decent PM.
> >   This is often needed in cases where the bootloader just enables all the
> >   clocks.
> 
> Yeah.  And it's not just bootloaders, it's also the previous kernel in the 
> case of something like kexec.  Since we don't know what devices the 
> bootloader or previous kernel touched, it seems to make sense to reset all 
> of these devices by default, once the drivers are converted to use runtime 
> PM.
> 
> The current issue here is that some drivers aren't yet converted, so the 
> hwmod code might think that some devices are 'unused' when they are 
> actually in use by a non-runtime-PM-based driver.
>  
> > - Device is reserved by secure mode or a coprocessor. In this case
> >   the device can't be reset.
> 
> Indeed - or even accessed.
> 
> > So I guess that makes the flags noreset, disabled and unavailable?
> 
> I think we only need 'no reset' and 'unavailable'?  Once the runtime PM 
> driver conversion is complete, I don't think there's any reason to keep 
> the 'hwmod_unused_reset' command line flag around, since it can be the 
> default.  That's because the hwmod code would then be able to track which 
> devices were actually used.

Sounds doable to me.

Regards,

Tony
diff mbox

Patch

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index fd248a31..4b1c03f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -75,6 +75,7 @@  parameter is applicable:
 	NET	Appropriate network support is enabled.
 	NUMA	NUMA support is enabled.
 	NFS	Appropriate NFS support is enabled.
+	OMAP	OMAP sub-architecture is enabled.
 	OSS	OSS sound support is enabled.
 	PV_OPS	A paravirtualized kernel is enabled.
 	PARIDE	The ParIDE (parallel port IDE) subsystem is enabled.
@@ -872,6 +873,14 @@  bytes respectively. Such letter suffixes can also be entirely omitted.
 			       If specified, z/VM IUCV HVC accepts connections
 			       from listed z/VM user IDs only.
 
+	hwmod_no_setup_reset
+			[OMAP] During kernel initialization,
+			prevent the subarchitecture code from resetting
+			IP blocks before the driver loads.  Useful for
+			initial bring-up of boards which require some
+			bootloader configuration to be retained on GPIO or
+			other IP blocks.
+
 	keep_bootcon	[KNL]
 			Do not unregister boot console at start. This is only
 			useful for debugging when something happens in the window
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 293fa6c..025bf2f 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -86,7 +86,9 @@ 
  * completely self-reliant and independent from bootloaders.  This is
  * to ensure a repeatable configuration, both to ensure consistent
  * runtime behavior, and to make it easier for others to reproduce
- * bugs.
+ * bugs.  (This behavior can be disabled while bringing up a new board
+ * by passing the kernel command line parameter
+ * 'hwmod_no_setup_reset'.)
  *
  * OMAP module activity states
  * ---------------------------
@@ -162,6 +164,12 @@  static LIST_HEAD(omap_hwmod_list);
 /* mpu_oh: used to add/remove MPU initiator from sleepdep list */
 static struct omap_hwmod *mpu_oh;
 
+/*
+ * no_setup_reset: if true, then don't reset any hwmods on boot.  This
+ * can be changed by the 'hwmod_no_setup_reset' kernel command line
+ * parameter
+ */
+static bool no_setup_reset;
 
 /* Private functions */
 
@@ -1453,7 +1461,7 @@  static int _setup(struct omap_hwmod *oh, void *data)
 		return 0;
 	}
 
-	if (!(oh->flags & HWMOD_INIT_NO_RESET)) {
+	if (!no_setup_reset && !(oh->flags & HWMOD_INIT_NO_RESET)) {
 		_reset(oh);
 
 		/*
@@ -1542,6 +1550,13 @@  static int __init _register(struct omap_hwmod *oh)
 	return 0;
 }
 
+static int __init _cmdline_no_setup_reset(char *s)
+{
+	no_setup_reset = true;
+	return 1;
+}
+
+__setup("hwmod_no_setup_reset", _cmdline_no_setup_reset);
 
 /* Public functions */