Re: [PATCH 4/5] gpiolib: handle deferral probe error
On Fri, Oct 07, 2011 at 04:09:38PM -0600, Grant Likely wrote: On Fri, Oct 7, 2011 at 4:06 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: On Fri, 07 Oct 2011 10:33:09 +0500 G, Manjunath Kondaiah manj...@ti.com wrote: The gpio library should return -EPROBE_DEFER in gpio_request if gpio driver is not ready. Why not use the perfectly good existing error codes we have for this ? We have EAGAIN and EUNATCH both of which look sensible. I want a distinct error code for probe deferral so that a) it doesn't overlap with something a driver is already doing, and b) so that all the users can be found again at a later date. That said, I'm not in agreement with this patch. It is fine for gpio lib to have a code that means the pin doesn't exist (yet), but the device driver needs to be the one to decide whether or not it is appropriate to use probe deferral. During gpio_request, driver gpio_request is not available. How can we expect driver to request deferred probe in this case? -M -- 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/5] drivercore: add new error value for deferred probe
On Sun, Oct 09, 2011 at 06:06:56PM -0700, Greg KH wrote: On Sun, Oct 09, 2011 at 04:59:31PM -0600, Grant Likely wrote: On Fri, Oct 7, 2011 at 6:12 PM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 07:28:33PM -0400, valdis.kletni...@vt.edu wrote: On Fri, 07 Oct 2011 16:12:45 MDT, Grant Likely said: On Fri, Oct 7, 2011 at 12:43 AM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 10:33:06AM +0500, G, Manjunath Kondaiah wrote: +#define EPROBE_DEFER 517 /* restart probe again after some time */ Can we really do this? According to Arnd, yes this is okay. Isn't this some user/kernel api here? What's wrong with just overloading on top of an existing error code? Surely one of the other 516 types could be used here, right? overloading makes it really hard to find the users at a later date. Would proposing '#define EPROBE_DEFER EAGAIN' be acceptable to everybody? That would allow overloading EAGAIN, but still make it easy to tell the usages apart if we need to separate them later... Yes, please do that, it is what USB does for it's internal error code handling. Really? When we've only currently used approximately 2^9 of a 2^31 numberspace? I'm fine with making sure that the number doesn't show up in the userspace headers, but it makes no sense to overload the #defines. Particularly so in this case where it isn't feasible to audit every driver to figure out what probe might possibly return. It is well within the realm of possibility that existing drivers are already returning -EAGAIN. I doubt they are, but you are right, it's really hard to tell. Besides; linux/errno.h *already* has linux-internal error codes that do not get exported out to userspace. There is an #ifdef __KERNEL__ block around ERESTARTSYS through EIOCBRETRY which is filtered out when exporting headers. I can't see any possible reason why we wouldn't add Linux internal error codes here. As long as it stays internal, that's fine, I was worried that this would be exported to userspace. Alan, still object to this? I hope no one has objections to use EPROBE_DEFER -M -- 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 2/5] drivercore: Add driver probe deferral mechanism
On Mon, Oct 10, 2011 at 10:37:22AM -0700, Andrei Warkentin wrote: Hi, - Original Message - From: Greg KH g...@kroah.com To: Josh Triplett j...@joshtriplett.org Cc: G, Manjunath Kondaiah manj...@ti.com, linux-arm-ker...@lists.infradead.org, Grant Likely grant.lik...@secretlab.ca, linux-omap@vger.kernel.org, linux-...@vger.kernel.org, linux-ker...@vger.kernel.org, Dilan Lee di...@nvidia.com, Mark Brown broo...@opensource.wolfsonmicro.com, manjun...@jasper.es Sent: Saturday, October 8, 2011 11:55:02 AM Subject: Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism I'm a bit of a fly on the wall here, but I'm curious how this impacts suspend/resume. device_initialize-device_pm_init are called from device_register, so certainly this patch doesn't also ensure that the PM ordering matches probe ordering, which is bound to break suspend, right? Was this ever tested with the OMAP target? Shouldn't the PM change be also part of this patch set? I don't see why you would want to have this in without the PM changes. suspend/resume handling is already in TODO list: http://permalink.gmane.org/gmane.linux.ports.arm.kernel/135461 -M Maybe I have it all wrong though :-). A -- 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 7/8] OMAP3: update cpuidle latency and threshold figures
Hi Ming,
On Wed, Oct 12, 2011 at 4:48 AM, Ming Lei tom.leim...@gmail.com wrote:
Hi,
On Thu, Sep 22, 2011 at 12:14 AM, Jean Pihet jean.pi...@newoldbits.com
wrote:
Update the data from the measurements performed at HW and SW levels.
Cf.
http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement
for a detailed explanation on where are the numbers magically coming from.
ToDo:
- Measure the wake-up latencies for all power domains for OMAP3
- Correct some numbers when sys_clkreq and sys_offmode are supported
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 28 ++--
1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 1b8e0da..4b3e994 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -44,20 +44,20 @@
* pass these details from the board file.
*/
static struct cpuidle_params cpuidle_params_table[] = {
- /* C1 */
- {2 + 2, 5, 1},
- /* C2 */
- {10 + 10, 30, 1},
- /* C3 */
- {50 + 50, 300, 1},
- /* C4 */
- {1500 + 1800, 4000, 1},
- /* C5 */
- {2500 + 7500, 12000, 1},
- /* C6 */
- {3000 + 8500, 15000, 1},
- /* C7 */
- {1 + 3, 30, 1},
+ /* C1 . MPU WFI + Core active */
+ {73 + 78, 152, 1},
+ /* C2 . MPU WFI + Core inactive */
+ {165 + 88, 345, 1},
+ /* C3 . MPU CSWR + Core inactive */
+ {163 + 182, 345, 1},
+ /* C4 . MPU OFF + Core inactive */
+ {2852 + 605, 15, 1},
+ /* C5 . MPU RET + Core RET */
+ {800 + 366, 2120, 1},
C4 exit_latency is longer than C5's, not sure if it is correct?
Since MPU is OFF in C4 the impact is bigger in latency. In MPU OFF
mode the caches and MPU context are saved and later restored, which is
costly.
Thanks for the review!
Regards,
Jean
+ /* C6 . MPU OFF + Core RET */
+ {4080 + 801, 215000, 1},
+ /* C7 . MPU OFF + Core OFF */
+ {4300 + 13000, 215000, 1},
};
#define OMAP3_NUM_STATES ARRAY_SIZE(cpuidle_params_table)
--
1.7.4.1
--
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
thanks,
--
Ming Lei
--
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: [PATCHv9 03/18] TEMP: OMAP3xxx: hwmod data: add PRM hwmod
On 10/11/2011 12:35 AM, Paul Walmsley wrote: On Tue, 11 Oct 2011, Cousson, Benoit wrote: On 10/10/2011 10:42 PM, Paul Walmsley wrote: If it's the 3xxx that you're objecting to in the name, we could call it prm2 or prmxyz - the '3xxx' just seemed like the most logical approach. The name of the hwmod class in the patch is still prm, of course. Yes, but that's different, the number is supposed to represent the instance number in the IP naming convention. So prm2 != prmv2. Heh, that works as long as there's no prmv IP block ;-) Thoughts? In fact the device name does not have to match the hwmod name. So we can just create an omap2_prm omap_device for OMAP2, omap3_prm omap_device for OMAP3... That will allow the relevant PRM driver to be bound to the proper device. We can, we'd just need to add this extra mapping layer, so it doesn't become a nasty special-case hack for each IP block that this applies to. Sounds like something for 3.3 (if ever...) Yeah, since PRM and CM are critical pieces for the hwmod to DT migration, having them DT adapted for 3.3 will be very nice. 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: [PATCHv9 03/18] TEMP: OMAP3xxx: hwmod data: add PRM hwmod
On 10/11/2011 1:26 AM, Paul Walmsley wrote: On Tue, 11 Oct 2011, Cousson, Benoit wrote: In fact the device name does not have to match the hwmod name. So we can just create an omap2_prm omap_device for OMAP2, omap3_prm omap_device for OMAP3... That will allow the relevant PRM driver to be bound to the proper device. Incidentally, given that we would be using the hwmod name and the version number to determine the appropriate omap_device name, what IP version numbers should we assign to these PRM IP blocks for different SoCs? It can just be 1, 2 and 3... The idea is just to differentiate the IP for each OMAP. 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
Re: [PATCH v6 06/16] OMAP2+: UART: Remove certain feilds from omap_uart_state struct
On Tue, Oct 11, 2011 at 5:01 AM, Kevin Hilman khil...@ti.com wrote:
Govindraj.R govindraj.r...@ti.com writes:
Removing some of the uart info that is maintained in omap_uart_state struct
used for UART PM in serial.c.
Remove omap_uart_state struct dependency from omap_serial_init,
omap_serial_init_port, omap_serial_early_init and omap_uart_idle_init
functions. And populate the same info in omap_uart_port_info struct
used as pdata struct.
IMO, this change doesn't belong in this patch and leads to clutter. The
rest of the series slowly removes/replaces all the fields from this
struct, so the right place to remove it's usage all together is at the
end of the series when (if) all the fields are no longer needed (or have
been moved.)
Okay will move it to end.
Stated differently, IMO, this patch should leave the uart-num and
uart-oh and the list_head (uart-node) alone (probably uart-pdev too)
and just cleanup the fields that are no longer used. Removing num, oh,
node here causes churn because you're force to change things here that
are then removed in later patches.
okay will retain the list part.
Added omap_uart_hwmod_lookup function to look up oh by name used in
serial_port_init and omap_serial_early_init functions.
Because of the above change, you now are doing a hwmod lookup 2 times
for every UART. Leaving the uart_list and uart-num in place will avoid
the need for that change.
yes since uart_list was removed, will retain the uart_list
to avoid the look up twice.
A list of omap_uart_state was maintained one for each uart, the same
is removed. Number of uarts available is maintained in num_uarts
field, re-use the same in omap_serial_init func to register each uart.
Remove omap_info which used details from omap_uart_state and use a
pdata pointer to pass platform specific info to driver.
There is no omap_info. Did you mean omap_up_info?
yes sorry typo.
The mapbase (start_address), membase(io_remap cookie) maintained as
part of uart_state struct and pdata struct are removed as this is
handled within driver.
This part makes sense.
okay will retain this part only.
Errata field is also moved to pdata.
Why in this patch instead of the subsequent Move errata handling from
serial.c to omap-serial patch?
will move to the errata patch.
These changes are done to cleanup serial.c file and prepare for
runtime changes.
There are a lot of changes in this patch with very little description as
to why, and many appear to be unrelated. They should probably be
separate patches, or have a better description as to how all the changes
are related so they belong in the same patch.
okay fine will split them into smaller changes.
Signed-off-by: Govindraj.R govindraj.r...@ti.com
---
arch/arm/mach-omap2/serial.c | 132
+---
arch/arm/plat-omap/include/plat/omap-serial.h | 4 +-
drivers/tty/serial/omap-serial.c | 12 ++-
3 files changed, 61 insertions(+), 87 deletions(-)
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index c98b9b4..8c43d1c 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -68,14 +68,6 @@ struct omap_uart_state {
int clocked;
int regshift;
- void __iomem *membase;
- resource_size_t mapbase;
-
- struct list_head node;
- struct omap_hwmod *oh;
- struct platform_device *pdev;
-
- u32 errata;
#if defined(CONFIG_ARCH_OMAP3) defined(CONFIG_PM)
int context_valid;
@@ -90,7 +82,6 @@ struct omap_uart_state {
#endif
};
-static LIST_HEAD(uart_list);
static u8 num_uarts;
static int uart_idle_hwmod(struct omap_device *od)
@@ -143,7 +134,19 @@ static inline void serial_write_reg(struct
omap_uart_state *uart, int offset,
__raw_writeb(value, uart-membase + offset);
}
-#if defined(CONFIG_PM) defined(CONFIG_ARCH_OMAP3)
+struct omap_hwmod *omap_uart_hwmod_lookup(int num)
+{
+ struct omap_hwmod *oh;
+ char oh_name[MAX_UART_HWMOD_NAME_LEN];
+
+ snprintf(oh_name, MAX_UART_HWMOD_NAME_LEN, uart%d, num + 1);
+ oh = omap_hwmod_lookup(oh_name);
+ WARN(IS_ERR(oh), Could not lookup hmwod info for %s\n,
+ oh_name);
+ return oh;
+}
+
+#if defined(CONFIG_PM)
The CONFIG_ARCH_OMAP3 part of this #if was dropped with this change with
no mention as to why. (I understand why it was done, but it's not
releveant to $SUBJECT patch so should be a separate patch.)
/*
* Work Around for Errata i202 (3430 - 1.12, 3630 - 1.6)
@@ -357,22 +360,17 @@ int omap_uart_can_sleep(void)
return can_sleep;
}
-static void omap_uart_idle_init(struct omap_uart_state *uart)
+static void omap_uart_idle_init(struct omap_uart_port_info *uart,
+ unsigned short num)
{
- int ret;
-
- uart-can_sleep = 0;
- omap_uart_smart_idle_enable(uart, 0);
-
if (cpu_is_omap34xx()
Re: [PATCH v6 09/16] OMAP2+: UART: Add runtime pm support for omap-serial driver
On Tue, Oct 11, 2011 at 5:12 AM, Kevin Hilman khil...@ti.com wrote:
Govindraj.R govindraj.r...@ti.com writes:
Adapts omap-serial driver to use pm_runtime API's.
[...]
@@ -1065,19 +1123,18 @@ static struct uart_driver serial_omap_reg = {
.cons = OMAP_CONSOLE,
};
-static int
-serial_omap_suspend(struct platform_device *pdev, pm_message_t state)
+static int serial_omap_suspend(struct device *dev)
{
- struct uart_omap_port *up = platform_get_drvdata(pdev);
+ struct uart_omap_port *up = dev_get_drvdata(dev);
if (up)
uart_suspend_port(serial_omap_reg, up-port);
return 0;
}
-static int serial_omap_resume(struct platform_device *dev)
+static int serial_omap_resume(struct device *dev)
{
- struct uart_omap_port *up = platform_get_drvdata(dev);
+ struct uart_omap_port *up = dev_get_drvdata(dev);
if (up)
uart_resume_port(serial_omap_reg, up-port);
These functions need to be wrapped in #ifdef CONFIG_SUSPEND, otherwise,
when building with !CONFIG_SUSPEND you'll get :
/work/kernel/omap/pm/drivers/tty/serial/omap-serial.c:1134:12: warning:
'serial_omap_suspend' defined but not used
/work/kernel/omap/pm/drivers/tty/serial/omap-serial.c:1150:12: warning:
'serial_omap_resume' defined but not used
[...]
+static int serial_omap_runtime_suspend(struct device *dev)
+{
+ struct uart_omap_port *up = dev_get_drvdata(dev);
+ struct omap_uart_port_info *pdata = dev-platform_data;
+
+ if (!up)
+ return -EINVAL;
+
+ if (!pdata-enable_wakeup || !pdata-get_context_loss_count)
+ return 0;
+
+ if (pdata-get_context_loss_count)
+ up-context_loss_cnt = pdata-get_context_loss_count(dev);
+
+ if (device_may_wakeup(dev)) {
+ if (!up-wakeups_enabled) {
+ pdata-enable_wakeup(up-pdev, true);
+ up-wakeups_enabled = true;
+ }
+ } else {
+ if (up-wakeups_enabled) {
+ pdata-enable_wakeup(up-pdev, false);
+ up-wakeups_enabled = false;
+ }
+ }
+
+ return 0;
+}
+
+static int serial_omap_runtime_resume(struct device *dev)
+{
+ struct uart_omap_port *up = dev_get_drvdata(dev);
+ struct omap_uart_port_info *pdata = dev-platform_data;
+
+ if (up) {
+ if (pdata-get_context_loss_count) {
+ u32 loss_cnt = pdata-get_context_loss_count(dev);
+
+ if (up-context_loss_cnt != loss_cnt)
+ serial_omap_restore_context(up);
+ }
+ }
+
return 0;
}
Similarily, thse need to be wrapped with #ifdef CONFIG_PM_RUNTIME,
otherwise, when !CONFIG_PM_RUNTIME:
/work/kernel/omap/pm/drivers/tty/serial/omap-serial.c:1498:12: warning:
'serial_omap_runtime_suspend' defined but not used
/work/kernel/omap/pm/drivers/tty/serial/omap-serial.c:1531:12: warning:
'serial_omap_runtime_resume' defined but not used
+static const struct dev_pm_ops serial_omap_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume)
+ SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend,
+ serial_omap_runtime_resume, NULL)
+};
+
Note that you don't need #else parts to the above #ifdefs since
the SET_*_OPS() macros used here take care of that.
Yes fine, Corrected
Thanks for catching this.
--
Govindraj.R
--
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 v6 09/16] OMAP2+: UART: Add runtime pm support for omap-serial driver
On Tue, Oct 11, 2011 at 5:26 AM, Kevin Hilman khil...@ti.com wrote: Govindraj.R govindraj.r...@ti.com writes: Adapts omap-serial driver to use pm_runtime API's. Use runtime runtime API's to handle uart clocks and obtain device_usage statics. Set runtime API's usage to irq_safe so that we can use get_sync from irq context. Auto-suspend for port specific activities and put for reg access. Use device_may_wakeup to check whether uart has wakeup capabilities and then enable uart runtime usage for the uart. OK. Current patch should do only this part. The rest should be separate patches with their own descriptive changelogs. Yes, started splitting them. Removing save_context/restore_context functions from serial.c Adding context restore to .runtime_suspend and using reg values from port structure to restore the uart port context based on context_loss_count. Maintain internal state machine using wakeups_enabled field for avoiding repeated enable/disable of uart port wakeup mechanism. This part should be a separate patch that follows. okay, Remove omap_uart_disable_wakeup and modify omap_uart_enable_wakeup to accept pdev and bool value to enable/disable the uart wakeup mechanism after uart clock's are cut. omap_hwmod_enable_wakeup is used to set pad wakeup for the uarts. PM_WKEN reg values are left to default. Removed omap_uart_enable/disable_clocks in serial.c now clock handling done with runtime API's. As stated in previous reviews, this wakeup enable/disable needs more description as the functionality is changing compared to current code. Current version modifies wakeup enable/disable at both power-domain level (PM_WKEN) and at the IO ring. Updated version modifies wakeups at module-level (SYSCONFIG) and at IO ring using omap_hwmod_enable_wakeup() IMO, the updated version makes more sense, but needs a description as to why that change in functionality will have equivalent results compared to the existing one. Okay, By default uart autosuspend delay is set to -1 to avoid character loss if uart's are autoidled and woken up on rx pin. OK, good. After boot up UART's can be autoidled by setting autosuspendi delay from sysfs. echo 3000 /sys/devices/platform/omap/omap_uart.X/power/autosuspend_delay_ms X=0,1,2,3 for UART1/2/3/4. Number of uarts available may vary across omap_soc. Acked-by: Alan Cox a...@linux.intel.com Signed-off-by: Govindraj.R govindraj.r...@ti.com Kevin -- To unsubscribe from this list: send the line unsubscribe linux-serial in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 v6 11/16] OMAP2+: UART: Move errata handling from serial.c to omap-serial
On Wed, Oct 12, 2011 at 2:31 AM, Kevin Hilman khil...@ti.com wrote: Govindraj.R govindraj.r...@ti.com writes: Move the errata handling mechanism from serial.c to omap-serial file and utilise the same func in driver file. Errata i202, i291 are moved to be handled with omap-serial Moving the errata macro from serial.c file to driver header file as from on errata will be handled in driver file itself. Corrected errata id from chapter reference 2.15 to errata id i291. Acked-by: Alan Cox a...@linux.intel.com Signed-off-by: Govindraj.R govindraj.r...@ti.com The errata handling for i291 is moved from device code to driver code but it's functionality is also changed (but not described.) In the current mainline code, The workaround for i291 is done whenever UART clocks are about to be cut. When the clocks are (re)enabled, the device is put back into no-idle. This patch puts the device into force-idle just before the idle/suspend transistion, but never puts it back into no-idle. So after the first idle/suspend transition, the IP remains in force-idle forever. I don't think that's what we want. I was thinking no_idle will be done once we re-enable uart clocks from omap_device_enable but that's not the case looking into the code. Will correct it will add no_idle call from .runtime_resume as done early where we put into no_idle(sysc) while enabling the clocks. -- Thanks, Govindraj.R -- 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 v6 12/16] OMAP2+: UART: Allow UART parameters to be configured from board file.
On Wed, Oct 12, 2011 at 12:23 AM, Kevin Hilman khil...@ti.com wrote:
Govindraj.R govindraj.r...@ti.com writes:
From: Deepak K deepa...@ti.com
The following UART parameters are defined within the UART driver:
1). Whether the UART uses DMA (dma_enabled), by default set to 0
2). The size of dma buffer (set to 4096 bytes)
3). The time after which the dma should stop if no more data is received.
4). The auto suspend delay that will be passed for pm_runtime_autosuspend
where uart will be disabled after timeout
Different UARTs may be used for different purpose such as the console,
for interfacing bluetooth chip, for interfacing to a modem chip, etc.
Therefore, it is necessary to be able to customize the above settings
for a given board on a per UART basis.
This change allows these parameters to be configured from the board file
and allows the parameters to be configured for each UART independently.
If a board does not define its own custom parameters for the UARTs, then
use the default parameters in the structure omap_serial_default_info.
The default parameters are defined to be the same as the current settings
in the UART driver to avoid breaking the UART for any cuurnelty supported
boards. By default, make all boards use the default UART parameters.
Signed-off-by: Deepak K deepa...@ti.com
Signed-off-by: Jon Hunter jon-hun...@ti.com
Signed-off-by: Govindraj.R govindraj.r...@ti.com
A couple minor comments below...
---
This patch is derived and reworked from a custom 2.6.35 kernel
Available here:
http://git.omapzoom.org/?p=kernel/omap.git;
a=commitdiff;h=584ef316542f77312be7ba96a0f3013c8f64852b;
hp=7233a76cb362c0fc603f773274159adff91d3513
arch/arm/mach-omap2/board-n8x0.c | 6 +-
arch/arm/mach-omap2/serial.c | 56
arch/arm/plat-omap/include/plat/omap-serial.h | 7 ++-
arch/arm/plat-omap/include/plat/serial.h | 5 ++-
drivers/tty/serial/omap-serial.c | 8 +--
5 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-omap2/board-n8x0.c
b/arch/arm/mach-omap2/board-n8x0.c
index e11f0c5..3408726 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -656,15 +656,15 @@ static inline void board_serial_init(void)
bdata.pads_cnt = 0;
bdata.id = 0;
- omap_serial_init_port(bdata);
+ omap_serial_init_port(bdata, NULL);
bdata.id = 1;
- omap_serial_init_port(bdata);
+ omap_serial_init_port(bdata, NULL);
bdata.id = 2;
bdata.pads = serial2_pads;
bdata.pads_cnt = ARRAY_SIZE(serial2_pads);
- omap_serial_init_port(bdata);
+ omap_serial_init_port(bdata, NULL);
}
#else
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 0731575..78f7051 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -43,17 +43,29 @@
#include mux.h
/*
- * NOTE: By default the serial timeout is disabled as it causes lost
characters
- * over the serial ports. This means that the UART clocks will stay on until
- * disabled via sysfs. This also causes that any deeper omap sleep states
are
- * blocked.
+ * NOTE: By default the serial auto_suspend timeout is disabled as it causes
+ * lost characters over the serial ports. This means that the UART clocks
will
+ * stay on until power/autosuspend_delay is set for the uart from sysfs.
+ * This also causes that any deeper omap sleep states are blocked.
*/
-#define DEFAULT_TIMEOUT 0
+#define DEFAULT_AUTOSUSPEND_DELAY -1
#define MAX_UART_HWMOD_NAME_LEN 16
static u8 num_uarts;
+#define DEFAULT_RXDMA_TIMEOUT 1 /* RX DMA polling rate
(us) */
+#define DEFAULT_RXDMA_BUFSIZE 4096 /* RX DMA buffer size
*/
+
+static struct omap_uart_port_info omap_serial_default_info[] = {
This could be __initdata
yes correct, will change this.
+ {
+ .dma_enabled = 0,
This field is a bool, use 'false' instead of 0.
yes fine.
+ .dma_rx_buf_size = DEFAULT_RXDMA_BUFSIZE,
+ .dma_rx_timeout = DEFAULT_RXDMA_TIMEOUT,
+ .autosuspend_timeout = DEFAULT_AUTOSUSPEND_DELAY,
+ },
+};
+
static int uart_idle_hwmod(struct omap_device *od)
{
omap_hwmod_idle(od-hwmods[0]);
@@ -298,6 +310,7 @@ core_initcall(omap_serial_early_init);
/**
* omap_serial_init_port() - initialize single serial port
* @bdata: port specific board data pointer
+ * @info: platform specific data pointer
*
* This function initialies serial driver for given port only.
* Platforms can call this function instead of omap_serial_init()
@@ -306,7 +319,8 @@ core_initcall(omap_serial_early_init);
* Don't mix calls to omap_serial_init_port() and omap_serial_init(),
* use only one of the two.
*/
-void __init omap_serial_init_port(struct omap_board_data *bdata)
+void
Re: [PATCH v6 14/16] OMAP2+: UART: Take console_lock in suspend path if not taken
On Wed, Oct 12, 2011 at 12:31 AM, Kevin Hilman khil...@ti.com wrote:
Govindraj.R govindraj.r...@ti.com writes:
In suspend path the console_lock is taken by uart_port_suspend
however when no_console_suspend is used console_lock is not taken.
During system wide suspend omap_pwr_domain hooks cut all
clocks that are left enabled. So its unsafe to proceed printing after
clocks are cut by pwr_domain hooks.
As I've mentioned in previous reviews, when no_console_suspend is
enabled, the user has explicitly requested console output during
suspend. In order to support that, we should not be cutting clocks at
all in that mode.
One way to address this would be to just disable runtime PM in the
-prepare method of the driver if no_console_suspend is enabled.
Okay fine exploring this option, right API's would be to use
pm_runtime_forbid/allow.
SNIP
+static int serial_omap_runtime_prepare(struct device *dev)
+{
+ if (!console_suspend_enabled)
+ pm_runtime_forbid(dev);
+
+ return 0;
+}
+
+static void serial_omap_runtime_complete(struct device *dev)
+{
+ if (!console_suspend_enabled)
+ pm_runtime_allow(dev);
+}
+
static const struct dev_pm_ops serial_omap_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume)
SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend,
serial_omap_runtime_resume, NULL)
+ .prepare = serial_omap_runtime_prepare,
+ .complete = serial_omap_runtime_complete,
};
SNIP
But to either use runtime forbid or disable we have ensure that
power_domain hooks don't go ahead and disable
the clocks with omap_device_idle as *runtime forbid or disable will
not set runtime_status to RPM_SUSPENDED*
and will stay in RPM_ACTIVE if we call runtime disable or forbid from
active state.
in power_domain hooks we just check the pm_runtime_status_suspended
this will be false even if
we do runtime disable/forbid and it will cut uart clocks always.
So we may need below check also:
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 26aee5c..286a534 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -592,7 +592,8 @@ static int _od_suspend_noirq(struct device *dev)
ret = pm_generic_suspend_noirq(dev);
- if (!ret !pm_runtime_status_suspended(dev)) {
+ if (!ret pm_runtime_enabled(dev)
+ !pm_runtime_status_suspended(dev)) {
if (pm_generic_runtime_suspend(dev) == 0) {
omap_device_idle(pdev);
od-flags |= OMAP_DEVICE_SUSPENDED;
--
Thanks,
Govindraj.R
--
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: [RFT/PATCH 7/7] arm: omap: smartreflex: micro-optimization for sanity check
Hello.
On 11-10-2011 16:19, Felipe Balbi wrote:
On 10-10-2011 16:26, Felipe Balbi wrote:
val (val != 1) == val 1
Signed-off-by: Felipe Balbiba...@ti.com
---
arch/arm/mach-omap2/smartreflex.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/smartreflex.c
b/arch/arm/mach-omap2/smartreflex.c
index 7bdabfa..4b0d6a8 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -866,7 +866,7 @@ static int omap_sr_autocomp_store(void *data, u64 val)
}
/* Sanity check */
- if (val (val != 1)) {
+ if (val 1) {
pr_warning(%s: Invalid argument %lld\n, __func__, val);
The format specified should be %llu?
and what does that have to do with $SUBJECT ??
Well, nothing. Just an idea for another patch I guess.
WBR, Sergei
--
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 2/5 v13] arm: omap: usb: ehci and ohci hwmod structures for omap3
On Tue, Oct 11, 2011 at 1:29 PM, Munegowda, Keshava
keshava_mgo...@ti.com wrote:
On Tue, Oct 11, 2011 at 11:18 AM, Munegowda, Keshava
keshava_mgo...@ti.com wrote:
On Tue, Oct 11, 2011 at 6:08 AM, Paul Walmsley p...@pwsan.com wrote:
Hi
so I just noticed another problem with these hwmods:
On Thu, 6 Oct 2011, Keshava Munegowda wrote:
Following 2 hwmod structures are added
1. usb_host_hs
The hwmod of usbhs with uhh, ehci and ohci base addresses
functional clock and ehci, ohci irqs
2. usb_tll_hs
hwmod of usbhs with the TLL base address and irq.
Signed-off-by: Keshava Munegowda keshava_mgo...@ti.com
Reviewed-by: Partha Basak part...@india.ti.com
---
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 227
1 files changed, 227 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 59fdb9f..b8ca690 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
...
+static struct omap_hwmod_ocp_if omap34xx_ick_cfg__usb_tll_hs = {
This interface is missing a .master and .slave. It must have both. So,
dropping this patch until it's fixed.
Clearly we also need to modify the hwmod code needs to reject any
instances where either .master or .slave is NULL.
- Paul
Hi Paul
I will fix it today only and I will send the updated
patches in couple of hours.
please help me to make it in this merge window.
Hi Paul
I have posted v14 of the patches; please do review
if it is OK, please merge to your branch for 3.2 version
please let me know if you have any comments here.
Thanks and Regards
keshava
Hi Paul
requesting to please do review the patch set
if it is OK, please merge to your branch for 3.2 version
please let me know if you have any comments.
regards
keshava
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] ARM: OMAP: Fix build for OMAP3 only builds
When building for OMAP3 only I get the
following errors:
In function `omap2420_init_early':
undefined reference to `omap2_set_globals_242x'
undefined reference to `omap2xxx_voltagedomains_init'
undefined reference to `omap242x_powerdomains_init'
undefined reference to `omap242x_clockdomains_init'
undefined reference to `omap2420_hwmod_init'
In function `omap2430_init_early':
undefined reference to `omap2_set_globals_243x'
undefined reference to `omap2xxx_voltagedomains_init'
undefined reference to `omap243x_powerdomains_init'
undefined reference to `omap243x_clockdomains_init'
undefined reference to `omap2430_hwmod_init'
In function `omap4430_init_early':
undefined reference to `omap2_set_globals_443x'
undefined reference to `omap44xx_voltagedomains_init'
undefined reference to `omap44xx_powerdomains_init'
undefined reference to `omap44xx_clockdomains_init'
undefined reference to `omap44xx_hwmod_init'
undefined reference to `omap4xxx_clk_init'
With this patch the omap_init_early will be compiled only
when their SOC's are selected.
Signed-off-by: Thomas Weber we...@corscience.de
---
arch/arm/mach-omap2/io.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index aa96538..f8b9cfa 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -359,6 +359,7 @@ static void __init omap_hwmod_init_postsetup(void)
omap_pm_if_early_init();
}
+#ifdef CONFIG_SOC_OMAP2420
void __init omap2420_init_early(void)
{
omap2_set_globals_242x();
@@ -370,7 +371,9 @@ void __init omap2420_init_early(void)
omap_hwmod_init_postsetup();
omap2420_clk_init();
}
+#endif
+#ifdef CONFIG_SOC_OMAP2430
void __init omap2430_init_early(void)
{
omap2_set_globals_243x();
@@ -382,7 +385,9 @@ void __init omap2430_init_early(void)
omap_hwmod_init_postsetup();
omap2430_clk_init();
}
+#endif
+#ifdef CONFIG_ARCH_OMAP3
/*
* Currently only board-omap3beagle.c should call this because of the
* same machine_id for 34xx and 36xx beagle.. Will get fixed with DT.
@@ -430,7 +435,9 @@ void __init ti816x_init_early(void)
omap_hwmod_init_postsetup();
omap3xxx_clk_init();
}
+#endif
+#ifdef CONFIG_ARCH_OMAP4
void __init omap4430_init_early(void)
{
omap2_set_globals_443x();
@@ -442,6 +449,7 @@ void __init omap4430_init_early(void)
omap_hwmod_init_postsetup();
omap4xxx_clk_init();
}
+#endif
void __init omap_sdrc_init(struct omap_sdrc_params *sdrc_cs0,
struct omap_sdrc_params *sdrc_cs1)
--
1.7.7
--
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 v8 REPOST 17/24] gpio/omap: fix debounce clock handling
Currently debounce clock state is not tracked in the system. The bank-dbck
is enabled/disabled in suspend/idle paths irrespective of whether debounce
interval has been set or not. Ideally, it should be handled only for those
gpio banks where the debounce is enabled. In _set_gpio_debounce, enable
debounce clock before accessing registers.
Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com
---
During further internal testing it was found that image was crashing within
_set_gpio_debounce(). It is observed that we are trying to access registers
without enabling debounce clock. This patch incorporates the change whereby
the debounce clock is enabled before accessing registers and disabled at the
end of the function.
drivers/gpio/gpio-omap.c | 60 -
1 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index fa6c9c5..85e9c2a 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -65,6 +65,7 @@ struct gpio_bank {
struct clk *dbck;
u32 mod_usage;
u32 dbck_enable_mask;
+ bool dbck_enabled;
struct device *dev;
bool is_mpuio;
bool dbck_flag;
@@ -156,6 +157,22 @@ static inline void _gpio_rmw(void __iomem *base, u32 reg,
u32 mask, bool set)
__raw_writel(l, base + reg);
}
+static inline void _gpio_dbck_enable(struct gpio_bank *bank)
+{
+ if (bank-dbck_enable_mask !bank-dbck_enabled) {
+ clk_enable(bank-dbck);
+ bank-dbck_enabled = true;
+ }
+}
+
+static inline void _gpio_dbck_disable(struct gpio_bank *bank)
+{
+ if (bank-dbck_enable_mask bank-dbck_enabled) {
+ clk_disable(bank-dbck);
+ bank-dbck_enabled = false;
+ }
+}
+
/**
* _set_gpio_debounce - low level gpio debounce time
* @bank: the gpio bank we're acting upon
@@ -184,22 +201,22 @@ static void _set_gpio_debounce(struct gpio_bank *bank,
unsigned gpio,
l = GPIO_BIT(bank, gpio);
+ clk_enable(bank-dbck);
reg = bank-base + bank-regs-debounce;
__raw_writel(debounce, reg);
reg = bank-base + bank-regs-debounce_en;
val = __raw_readl(reg);
- if (debounce) {
+ if (debounce)
val |= l;
- clk_enable(bank-dbck);
- } else {
+ else
val = ~l;
- clk_disable(bank-dbck);
- }
+
bank-dbck_enable_mask = val;
__raw_writel(val, reg);
+ clk_disable(bank-dbck);
}
static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio,
@@ -485,8 +502,10 @@ static int omap_gpio_request(struct gpio_chip *chip,
unsigned offset)
* If this is the first gpio_request for the bank,
* enable the bank module.
*/
- if (!bank-mod_usage)
+ if (!bank-mod_usage) {
+ _gpio_dbck_enable(bank);
pm_runtime_get_sync(bank-dev);
+ }
spin_lock_irqsave(bank-lock, flags);
/* Set trigger to none. You need to enable the desired trigger with
@@ -549,8 +568,10 @@ static void omap_gpio_free(struct gpio_chip *chip,
unsigned offset)
* If this is the last gpio to be freed in the bank,
* disable the bank module.
*/
- if (!bank-mod_usage)
+ if (!bank-mod_usage) {
pm_runtime_put_sync(bank-dev);
+ _gpio_dbck_disable(bank);
+ }
}
/*
@@ -829,8 +850,10 @@ static int gpio_debounce(struct gpio_chip *chip, unsigned
offset,
if (!bank-dbck) {
bank-dbck = clk_get(bank-dev, dbclk);
- if (IS_ERR(bank-dbck))
+ if (IS_ERR(bank-dbck)) {
dev_err(bank-dev, Could not get gpio dbck\n);
+ return -EINVAL;
+ }
}
spin_lock_irqsave(bank-lock, flags);
@@ -1086,6 +1109,8 @@ static int omap_gpio_suspend(struct device *dev)
bank-saved_wakeup = __raw_readl(wake_status);
_gpio_rmw(base, bank-regs-wkup_en, bank-suspend_wakeup, 1);
spin_unlock_irqrestore(bank-lock, flags);
+
+ _gpio_dbck_disable(bank);
}
return 0;
@@ -1102,6 +1127,8 @@ static int omap_gpio_resume(struct device *dev)
if (!bank-regs-wkup_en)
return 0;
+ _gpio_dbck_enable(bank);
+
spin_lock_irqsave(bank-lock, flags);
_gpio_rmw(base, bank-regs-wkup_en, bank-saved_wakeup, 1);
spin_unlock_irqrestore(bank-lock, flags);
@@ -1120,16 +1147,14 @@ void omap2_gpio_prepare_for_idle(int off_mode)
list_for_each_entry(bank, omap_gpio_list, node) {
u32 l1 = 0, l2 = 0;
- int j;
if (!bank-loses_context)
continue;
- for (j = 0; j hweight_long(bank-dbck_enable_mask); j++)
-
[PATCH v8 REPOST 21/24] gpio/omap: save and restore debounce registers
From: Nishanth Menon n...@ti.com
GPIO debounce registers need to be saved and restored for proper functioning
of driver. To save the registers, we cannot cut the clock before the save,
hence move the clk disable after the save.
Signed-off-by: Nishanth Menon n...@ti.com
Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com
Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com
---
Rebased on top of:
[PATCH v8 REPOST 17/24] gpio/omap: fix debounce clock handling
drivers/gpio/gpio-omap.c | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index d074408..6a89560 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -42,6 +42,8 @@ struct gpio_regs {
u32 risingdetect;
u32 fallingdetect;
u32 dataout;
+ u32 debounce;
+ u32 debounce_en;
};
struct gpio_bank {
@@ -219,6 +221,9 @@ static void _set_gpio_debounce(struct gpio_bank *bank,
unsigned gpio,
__raw_writel(val, reg);
clk_disable(bank-dbck);
+
+ bank-context.debounce = debounce;
+ bank-context.debounce_en = val;
}
static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio,
@@ -1350,6 +1355,12 @@ static void omap_gpio_restore_context(struct gpio_bank
*bank)
__raw_writel(bank-context.fallingdetect,
bank-base + bank-regs-fallingdetect);
__raw_writel(bank-context.dataout, bank-base + bank-regs-dataout);
+ if (bank-dbck_enable_mask) {
+ __raw_writel(bank-context.debounce, bank-base +
+ bank-regs-debounce);
+ __raw_writel(bank-context.debounce_en,
+ bank-base + bank-regs-debounce_en);
+ }
}
#else
#define omap_gpio_suspend NULL
--
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 v6 10/16] OMAP2+: UART: Modify omap_uart_can_sleep function
On Tue, Oct 11, 2011 at 11:54 PM, Kevin Hilman khil...@ti.com wrote:
Govindraj.R govindraj.r...@ti.com writes:
Modify the omap_uart_can_sleep function to check uart is active
or not to be used by pm code to enter low power states.
Doesn't the driver now control when the UART clocks are gated (using
runtime PM autosuspend)?
IMO, this check should be completely removed and the driver should
be managing this with the autosuspend timeout.
Removing this check can cause console response little sluggish.
Sluggish in what way?
response is slower like when we type something or cat debugfs/pm_count
see things little slower on console, there is no character loss.
Happens even though we have not set the autosuspend timeout and uart
clocks are active,
which basically means allowing mpu to enter retention keeping uart active.
this delay in response or sluggishness is not there on my 3430SDP or
3630zoom board but I was able to see this behavior on a beagle board(xm rev c).
retaining this uart_can_sleep check in omap3_can_sleep ensures a better
console user experience. (not allowing mpu to enter retention
until uart clocks are cut)
If the driver is runtime suspended, it should only be sluggish for the
first character. After that, the autosuspend timeout should prevent it
from feeling sluggish.
However no characters will be lost until uart clocks are gated
and woken up using rx-pad. UART interface clocks can be auto gated
this can make response on uart slower. This behaviour was observed
only on some of OMAP3 boards(beagleboard xm rev c).
Reported-by: Tero Kristo t-kri...@ti.com
Signed-off-by: Govindraj.R govindraj.r...@ti.com
---
arch/arm/mach-omap2/serial.c | 21 +
1 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 6725caf..ccf3550 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -156,23 +156,20 @@ static void omap_uart_smart_idle_enable(struct
omap_uart_state *uart,
int omap_uart_can_sleep(void)
{
- struct omap_uart_state *uart;
- int can_sleep = 1;
-
- list_for_each_entry(uart, uart_list, node) {
- if (!uart-clocked)
- continue;
+ struct omap_hwmod *oh;
+ u8 i, ret = true;
- if (!uart-can_sleep) {
- can_sleep = 0;
+ for (i = 0; i num_uarts; i++) {
+ oh = omap_uart_hwmod_lookup(i);
This is a heavy operation to add for *every* entry into idle.
removing uart_list caused this ops, retaining uart_list
will ensure this will be replaced with list_for_each_entry
as done in existing code.
--
Thanks,
Govindraj.R
--
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 v6 15/16] OMAP2+: UART: Enable back uart clocks with runtime API for early console
On Wed, Oct 12, 2011 at 2:36 AM, Kevin Hilman khil...@ti.com wrote: Govindraj.R govindraj.r...@ti.com writes: For the early console probing we had avoided hwmod reset and idling and uart was idled using hwmod API and enabled back using omap_device API after omap_device registration. Now since we are using runtime API's to enable back uart, move hwmod idling and use runtime API to enable back UART. Signed-off-by: Govindraj.R govindraj.r...@ti.com Now that the driver is using runtime PM. Why do we still need HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET? The comment in the code says: /* * During UART early init, device need to be probed * to determine SoC specific init before omap_device * is ready. Therefore, don't allow idle here */ This was true when using the 8250 driver because it was not using runtime PM so could not know how to (re)enable the device. However, since the driver is now runtime PM adapted, any device access should be contained within a runtime PM get/put block, so there should no longer be a reason not allow the IP blocks to be reset during boot. Forgot to add, this is still needed for earlyprintk(CONFIG_EARLY_PRINTK) use case, The initial boot prints until a console driver is available is from arch/arm/kernel/early_printk.c which does a tx on uart console and relies on configuration from bootloader. during bootup earlyprink does a tx on uart console and if uart driver is not available yet uart reset or idle done by hwmod layer can cause boot failures. -- put_char from earlyprintk.c -- reset/idle from hwmod layer -- put_char from earlyprintk.c So console_uart reset or clock gating must be done only after uart driver is available or be prevented using these available hwmod_flags. omap_serial_early_init should be now be binded with CONFIG_SERIAL_OMAP macro ? -- Thanks, Govindraj.R -- 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 v3 0/8] PM QoS: implement the OMAP low level constraints management code
From: Jean Pihet j-pi...@ti.com . Convert the OMAP I2C driver to the PM QoS API for MPU latency constraints . Remove the remove the latency related functions from OMAP PM in favor of the generic per-device PM QoS API . Implement the devices wake-up latency constraints using the global device PM QoS notification handler which applies the constraints to the underlying layer . Implement the low level code which controls the power domains next power states, through the hwmod and pwrdm layers . Add cpuidle and power domains wake-up latency figures for OMAP3, cf. [1] for the details on where the numbers are coming from . Implement the relation between the cpuidle and per-device PM QoS frameworks in the OMAP3 specific idle callbacks. The chosen C-state shall satisfy the following conditions: . the 'valid' field is enabled, . it satisfies the enable_off_mode flag, . the next state for MPU and CORE power domains is not lower than the state programmed by the per-device PM QoS. ToDo: 1. validate the constraints framework on OMAP4 HW (done on OMAP3) 2. Re-visit the OMAP power domains states initialization procedure. Currently the power states that have been changed from the constraints API which were applied before the initialization of the power domains are lost 3. Further clean-up the OMAP PM layer, use the generic frameworks instead (OPP, PM QoS...) Based on the pm-qos branch of the linux-pm git tree (3.1.0-rc3), cf. [2]. Tested on OMAP3 Beagleboard (ES2.x) with constraints on MPU, CORE, PER in RETention and OFF modes. [1] http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement [2] git://github.com/rjwysocki/linux-pm.git v3: reworked the error return path and improved the kerneldoc v2: reworked the OMAP specific cpuidle code to demote the initial C-state to a valid C-state which fulfills the per-device constraints v1: initial version Jean Pihet (8): OMAP: convert I2C driver to PM QoS for latency constraints OMAP: PM: remove the latency related functions from the API OMAP2+: powerdomain: control power domains next state OMAP2+: omap_hwmod: manage the wake-up latency constraints OMAP: PM: register to the per-device PM QoS framework OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints OMAP3: update cpuidle latency and threshold figures OMAP3: powerdomain data: add wake-up latency figures Documentation/arm/OMAP/omap_pm | 55 ++-- arch/arm/mach-omap2/cpuidle34xx.c| 77 ++- arch/arm/mach-omap2/omap_hwmod.c | 27 - arch/arm/mach-omap2/pm.c | 63 arch/arm/mach-omap2/pm.h | 17 ++- arch/arm/mach-omap2/powerdomain.c| 200 ++ arch/arm/mach-omap2/powerdomain.h| 35 +- arch/arm/mach-omap2/powerdomains3xxx_data.c | 78 ++ arch/arm/plat-omap/i2c.c | 20 --- arch/arm/plat-omap/include/plat/omap-pm.h| 99 - arch/arm/plat-omap/include/plat/omap_hwmod.h |2 + arch/arm/plat-omap/omap-pm-noop.c| 88 --- drivers/i2c/busses/i2c-omap.c| 30 ++-- include/linux/i2c-omap.h |1 - 14 files changed, 484 insertions(+), 308 deletions(-) -- 1.7.4.1 -- 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/8] OMAP: convert I2C driver to PM QoS for latency constraints
From: Jean Pihet j-pi...@ti.com
Convert the driver from the outdated omap_pm_set_max_mpu_wakeup_lat
API to the new PM QoS API.
Since the constraint is on the MPU subsystem, use the PM_QOS_CPU_DMA_LATENCY
class of PM QoS. The resulting MPU constraints are used by cpuidle to
decide the next power state of the MPU subsystem.
The I2C device latency timing is derived from the FIFO size and the
clock speed and so is applicable to all OMAP SoCs.
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/plat-omap/i2c.c | 20
drivers/i2c/busses/i2c-omap.c | 30 +++---
include/linux/i2c-omap.h |1 -
3 files changed, 15 insertions(+), 36 deletions(-)
diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c
index 3341ca4..e1e2502 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -34,7 +34,6 @@
#include mach/irqs.h
#include plat/mux.h
#include plat/i2c.h
-#include plat/omap-pm.h
#include plat/omap_device.h
#define OMAP_I2C_SIZE 0x3f
@@ -113,16 +112,6 @@ static inline int omap1_i2c_add_bus(int bus_id)
#ifdef CONFIG_ARCH_OMAP2PLUS
-/*
- * XXX This function is a temporary compatibility wrapper - only
- * needed until the I2C driver can be converted to call
- * omap_pm_set_max_dev_wakeup_lat() and handle a return code.
- */
-static void omap_pm_set_max_mpu_wakeup_lat_compat(struct device *dev, long t)
-{
- omap_pm_set_max_mpu_wakeup_lat(dev, t);
-}
-
static struct omap_device_pm_latency omap_i2c_latency[] = {
[0] = {
.deactivate_func= omap_device_idle_hwmods,
@@ -151,15 +140,6 @@ static inline int omap2_i2c_add_bus(int bus_id)
}
pdata = i2c_pdata[bus_id - 1];
- /*
-* When waiting for completion of a i2c transfer, we need to
-* set a wake up latency constraint for the MPU. This is to
-* ensure quick enough wakeup from idle, when transfer
-* completes.
-* Only omap3 has support for constraints
-*/
- if (cpu_is_omap34xx())
- pdata-set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat_compat;
od = omap_device_build(name, bus_id, oh, pdata,
sizeof(struct omap_i2c_bus_platform_data),
omap_i2c_latency, ARRAY_SIZE(omap_i2c_latency), 0);
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 1a766cf..95e5205 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -40,6 +40,7 @@
#include linux/slab.h
#include linux/i2c-omap.h
#include linux/pm_runtime.h
+#include linux/pm_qos.h
/* I2C controller revisions */
#define OMAP_I2C_REV_2 0x20
@@ -179,8 +180,7 @@ struct omap_i2c_dev {
struct completion cmd_complete;
struct resource *ioarea;
u32 latency;/* maximum mpu wkup latency */
- void(*set_mpu_wkup_lat)(struct device *dev,
- long latency);
+ struct pm_qos_request pm_qos_request;
u32 speed; /* Speed of bus in Khz */
u16 cmd_err;
u8 *buf;
@@ -648,8 +648,14 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
if (r 0)
goto out;
- if (dev-set_mpu_wkup_lat != NULL)
- dev-set_mpu_wkup_lat(dev-dev, dev-latency);
+ /*
+* When waiting for completion of a i2c transfer, we need to
+* set a wake up latency constraint for the MPU. This is to
+* ensure quick enough wakeup from idle, when transfer
+* completes.
+*/
+ pm_qos_add_request(dev-pm_qos_request, PM_QOS_CPU_DMA_LATENCY,
+ dev-latency);
for (i = 0; i num; i++) {
r = omap_i2c_xfer_msg(adap, msgs[i], (i == (num - 1)));
@@ -657,8 +663,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
break;
}
- if (dev-set_mpu_wkup_lat != NULL)
- dev-set_mpu_wkup_lat(dev-dev, -1);
+ pm_qos_remove_request(dev-pm_qos_request);
if (r == 0)
r = num;
@@ -1007,13 +1012,10 @@ omap_i2c_probe(struct platform_device *pdev)
goto err_release_region;
}
- if (pdata != NULL) {
+ if (pdata != NULL)
speed = pdata-clkrate;
- dev-set_mpu_wkup_lat = pdata-set_mpu_wkup_lat;
- } else {
+ else
speed = 100;/* Default speed */
- dev-set_mpu_wkup_lat = NULL;
- }
dev-speed = speed;
dev-idle = 1;
@@ -1066,10 +1068,8 @@ omap_i2c_probe(struct platform_device *pdev)
dev-fifo_size = (dev-fifo_size / 2);
dev-b_hw = 1; /* Enable hardware fixes */
}
-
[PATCH 2/8] OMAP: PM: remove the latency related functions from the API
From: Jean Pihet j-pi...@ti.com Remove the following functions from the API: omap_pm_set_max_mpu_wakeup_lat omap_pm_set_max_dev_wakeup_lat omap_pm_set_max_sdma_lat The generic per-device PM QoS functions shall be used instead, cf. include/linux/pm_qos.h. Signed-off-by: Jean Pihet j-pi...@ti.com --- Documentation/arm/OMAP/omap_pm| 55 +++- arch/arm/plat-omap/include/plat/omap-pm.h | 99 - arch/arm/plat-omap/omap-pm-noop.c | 88 - 3 files changed, 9 insertions(+), 233 deletions(-) diff --git a/Documentation/arm/OMAP/omap_pm b/Documentation/arm/OMAP/omap_pm index 9012bb0..26f9669 100644 --- a/Documentation/arm/OMAP/omap_pm +++ b/Documentation/arm/OMAP/omap_pm @@ -29,21 +29,12 @@ Drivers need to express PM parameters which: This document proposes the OMAP PM interface, including the following -five power management functions for driver code: +power management functions for driver code: -1. Set the maximum MPU wakeup latency: - (*pdata-set_max_mpu_wakeup_lat)(struct device *dev, unsigned long t) - -2. Set the maximum device wakeup latency: - (*pdata-set_max_dev_wakeup_lat)(struct device *dev, unsigned long t) - -3. Set the maximum system DMA transfer start latency (CORE pwrdm): - (*pdata-set_max_sdma_lat)(struct device *dev, long t) - -4. Set the minimum bus throughput needed by a device: +1. Set the minimum bus throughput needed by a device: (*pdata-set_min_bus_tput)(struct device *dev, u8 agent_id, unsigned long r) -5. Return the number of times the device has lost context +2. Return the number of times the device has lost context (*pdata-get_dev_context_loss_count)(struct device *dev) @@ -55,10 +46,12 @@ The OMAP PM layer is intended to be temporary - The intention is that eventually the Linux PM QoS layer should support -the range of power management features present in OMAP3. As this +the range of power management features present in OMAP3. As this happens, existing drivers using the OMAP PM interface can be modified to use the Linux PM QoS code; and the OMAP PM interface can disappear. +The set_min_bus_tput function shall be converted to a throughput PM QoS +framework. Driver usage of the OMAP PM functions - @@ -66,39 +59,9 @@ Driver usage of the OMAP PM functions As the 'pdata' in the above examples indicates, these functions are exposed to drivers through function pointers in driver .platform_data structures. The function pointers are initialized by the board-*.c -files to point to the corresponding OMAP PM functions: -.set_max_dev_wakeup_lat will point to -omap_pm_set_max_dev_wakeup_lat(), etc. Other architectures which do -not support these functions should leave these function pointers set -to NULL. Drivers should use the following idiom: - -if (pdata-set_max_dev_wakeup_lat) -(*pdata-set_max_dev_wakeup_lat)(dev, t); - -The most common usage of these functions will probably be to specify -the maximum time from when an interrupt occurs, to when the device -becomes accessible. To accomplish this, driver writers should use the -set_max_mpu_wakeup_lat() function to to constrain the MPU wakeup -latency, and the set_max_dev_wakeup_lat() function to constrain the -device wakeup latency (from clk_enable() to accessibility). For -example, - -/* Limit MPU wakeup latency */ -if (pdata-set_max_mpu_wakeup_lat) -(*pdata-set_max_mpu_wakeup_lat)(dev, tc); - -/* Limit device powerdomain wakeup latency */ -if (pdata-set_max_dev_wakeup_lat) -(*pdata-set_max_dev_wakeup_lat)(dev, td); - -/* total wakeup latency in this example: (tc + td) */ - -The PM parameters can be overwritten by calling the function again -with the new value. The settings can be removed by calling the -function with a t argument of -1 (except in the case of -set_max_bus_tput(), which should be called with an r argument of 0). - -The fifth function above, omap_pm_get_dev_context_loss_count(), +files to point to the corresponding OMAP PM functions. + +The omap_pm_get_dev_context_loss_count() function is intended as an optimization to allow drivers to determine whether the device has lost its internal context. If context has been lost, the driver must restore its internal context before proceeding. diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h index 0840df8..c371364 100644 --- a/arch/arm/plat-omap/include/plat/omap-pm.h +++ b/arch/arm/plat-omap/include/plat/omap-pm.h @@ -62,44 +62,6 @@ void omap_pm_if_exit(void); * Device-driver-originated constraints (via board-*.c files, platform_data) */ - -/** - * omap_pm_set_max_mpu_wakeup_lat - set the maximum MPU wakeup latency - * @dev: struct device * requesting the constraint - * @t: maximum MPU wakeup latency in microseconds - * - * Request
[PATCH 3/8] OMAP2+: powerdomain: control power domains next state
From: Jean Pihet j-pi...@ti.com
When a PM QoS device latency constraint is requested or removed the
PM QoS layer notifies the underlying layer with the updated aggregated
constraint value. The constraint is stored in the powerdomain constraints
list and then applied to the corresponding power domain.
The power domains get the next power state programmed directly in the
registers via pwrdm_wakeuplat_update_pwrst.
Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using
wake-up latency constraints on MPU, CORE and PER.
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/mach-omap2/powerdomain.c | 200 +
arch/arm/mach-omap2/powerdomain.h | 35 ++-
2 files changed, 233 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index 9af0847..50f5802 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -17,8 +17,10 @@
#include linux/kernel.h
#include linux/types.h
#include linux/list.h
+#include linux/slab.h
#include linux/errno.h
#include linux/string.h
+#include linux/pm_qos.h
#include trace/events/power.h
#include cm2xxx_3xxx.h
@@ -104,6 +106,12 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
for (i = 0; i pwrdm-banks; i++)
pwrdm-ret_mem_off_counter[i] = 0;
+ /* Initialize the per-od wake-up constraints data */
+ spin_lock_init(pwrdm-wkup_lat_plist_lock);
+ plist_head_init(pwrdm-wkup_lat_plist_head);
+ pwrdm-wkup_lat_next_state = PWRDM_POWER_OFF;
+
+ /* Initialize the pwrdm state */
pwrdm_wait_transition(pwrdm);
pwrdm-state = pwrdm_read_pwrst(pwrdm);
pwrdm-state_counter[pwrdm-state] = 1;
@@ -191,6 +199,84 @@ static int _pwrdm_post_transition_cb(struct powerdomain
*pwrdm, void *unused)
return 0;
}
+/**
+ * pwrdm_wakeuplat_update_pwrst - Update power domain power state if needed
+ * @pwrdm: struct powerdomain * to which requesting device belongs to.
+ * @min_latency: the allowed wake-up latency for the given power domain. A
+ * value of PM_QOS_DEV_LAT_DEFAULT_VALUE means 'no constraint' on the pwrdm.
+ *
+ * Finds the power domain next power state that fulfills the constraint.
+ * Programs a new target state if it is different from current power state.
+ * The power domains get the next power state programmed directly in the
+ * registers.
+ *
+ * Returns 0 in case of success, -EINVAL in case of invalid parameters,
+ * or the return value from omap_set_pwrdm_state.
+ */
+static int pwrdm_wakeuplat_update_pwrst(struct powerdomain *pwrdm,
+ long min_latency)
+{
+ int ret = 0, new_state;
+
+ if (!pwrdm) {
+ WARN(1, powerdomain: %s: invalid parameter(s), __func__);
+ return -EINVAL;
+ }
+
+ /*
+* Apply constraints to power domains by programming
+* the pwrdm next power state.
+*/
+
+ /*
+* Find the next supported power state with
+* wakeup latency minimum constraint
+*/
+ for (new_state = 0x0; new_state PWRDM_MAX_PWRSTS; new_state++) {
+ if (min_latency == PM_QOS_DEV_LAT_DEFAULT_VALUE)
+ break;
+ if ((pwrdm-wakeup_lat[new_state] != UNSUP_STATE)
+ (pwrdm-wakeup_lat[new_state] = min_latency))
+ break;
+ }
+
+ switch (new_state) {
+ case PWRDM_FUNC_PWRST_OFF:
+ new_state = PWRDM_POWER_OFF;
+ break;
+ case PWRDM_FUNC_PWRST_OSWR:
+ pwrdm_set_logic_retst(pwrdm, PWRDM_POWER_OFF);
+ new_state = PWRDM_POWER_RET;
+ break;
+ case PWRDM_FUNC_PWRST_CSWR:
+ pwrdm_set_logic_retst(pwrdm, PWRDM_POWER_RET);
+ new_state = PWRDM_POWER_RET;
+ break;
+ case PWRDM_FUNC_PWRST_INACTIVE:
+ new_state = PWRDM_POWER_INACTIVE;
+ break;
+ case PWRDM_FUNC_PWRST_ON:
+ new_state = PWRDM_POWER_ON;
+ break;
+ default:
+ pr_warn(powerdomain: requested latency constraint not
+ supported %s set to ON state\n, pwrdm-name);
+ new_state = PWRDM_POWER_ON;
+ break;
+ }
+
+ pwrdm-wkup_lat_next_state = new_state;
+ if (pwrdm_read_next_pwrst(pwrdm) != new_state)
+ ret = omap_set_pwrdm_state(pwrdm, new_state);
+
+ pr_debug(powerdomain: %s pwrst: curr=%d, prev=%d next=%d
+min_latency=%ld, set_state=%d\n, pwrdm-name,
+pwrdm_read_pwrst(pwrdm), pwrdm_read_prev_pwrst(pwrdm),
+pwrdm_read_next_pwrst(pwrdm), min_latency, new_state);
+
+ return ret;
+}
+
/* Public functions */
/**
@@ -931,6 +1017,120 @@ int pwrdm_post_transition(void)
}
/**
+ * pwrdm_set_wkup_lat_constraint - Set/update/remove a
[PATCH 4/8] OMAP2+: omap_hwmod: manage the wake-up latency constraints
From: Jean Pihet j-pi...@ti.com
Hwmod is queried from the OMAP_PM layer to manage the power domains
wake-up latency constraints. Hwmod retrieves the correct power domain
and if it exists it calls the corresponding power domain function.
Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
latency constraints on MPU, CORE and PER.
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/mach-omap2/omap_hwmod.c | 27 +-
arch/arm/plat-omap/include/plat/omap_hwmod.h |2 +
2 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 84cc0bd..023f3e7 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -143,6 +143,7 @@
#include powerdomain.h
#include plat/clock.h
#include plat/omap_hwmod.h
+#include plat/omap_device.h
#include plat/prcm.h
#include cm2xxx_3xxx.h
@@ -2619,10 +2620,34 @@ ohsps_unlock:
}
/**
+ * omap_hwmod_set_wkup_constraint- set/release a wake-up latency constraint
+ *
+ * @oh: struct omap_hwmod* to which the target device belongs to.
+ * @cookie: identifier of the constraints list for @oh.
+ * @min_latency: the minimum allowed wake-up latency for @oh.
+ *
+ * Returns 0 upon success, -EINVAL in case of invalid parameters, or
+ * the return value from pwrdm_set_wkup_lat_constraint.
+ */
+int omap_hwmod_set_wkup_lat_constraint(struct omap_hwmod *oh,
+ void *cookie, long min_latency)
+{
+ struct powerdomain *pwrdm = omap_hwmod_get_pwrdm(oh);
+
+ if (!pwrdm) {
+ pr_err(%s: Error: could not find powerdomain
+ for %s\n, __func__, oh-name);
+ return -EINVAL;
+ }
+
+ return pwrdm_set_wkup_lat_constraint(pwrdm, cookie, min_latency);
+}
+
+/**
* omap_hwmod_get_context_loss_count - get lost context count
* @oh: struct omap_hwmod *
*
- * Query the powerdomain of of @oh to get the context loss
+ * Query the powerdomain of @oh to get the context loss
* count for this device.
*
* Returns the context loss count of the powerdomain assocated with @oh
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h
b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 0e329ca..75e0e7a 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -603,6 +603,8 @@ int omap_hwmod_for_each_by_class(const char *classname,
void *user);
int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
+int omap_hwmod_set_wkup_lat_constraint(struct omap_hwmod *oh, void *cookie,
+ long min_latency);
u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
--
1.7.4.1
--
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 5/8] OMAP: PM: register to the per-device PM QoS framework
From: Jean Pihet j-pi...@ti.com
Implement the devices wake-up latency constraints using the global
device PM QoS notification handler which applies the constraints to the
underlying layer by calling the corresponding function at hwmod level.
Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
latency constraints on MPU, CORE and PER.
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/mach-omap2/pm.c | 63 ++
1 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 3feb359..58b4b76 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -11,13 +11,16 @@
#include linux/kernel.h
#include linux/init.h
+#include linux/notifier.h
#include linux/io.h
#include linux/err.h
#include linux/opp.h
+#include linux/pm_qos.h
#include plat/omap-pm.h
#include plat/omap_device.h
#include plat/common.h
+#include plat/omap_hwmod.h
#include voltage.h
#include powerdomain.h
@@ -242,11 +245,71 @@ static void __init omap4_init_voltages(void)
omap2_set_init_voltage(iva, dpll_iva_m5x2_ck, iva_dev);
}
+/* Interface to the per-device PM QoS framework */
+static int omap2_dev_pm_qos_handler(struct notifier_block *nb,
+ unsigned long new_value,
+ void *req)
+{
+ struct omap_device *od;
+ struct omap_hwmod *oh;
+ struct platform_device *pdev;
+ struct dev_pm_qos_request *dev_pm_qos_req = req;
+
+ pr_debug(OMAP PM CONSTRAINTS: req@0x%p, new_value=%lu\n,
+req, new_value);
+
+ /* Look for the platform device for the constraint target device */
+ pdev = to_platform_device(dev_pm_qos_req-dev);
+
+ /* Try to catch non platform devices */
+ if (pdev-name == NULL) {
+ pr_err(%s: Error: platform device for device %s not valid\n,
+ __func__, dev_name(dev_pm_qos_req-dev));
+ return -EINVAL;
+ }
+
+ /* Find the associated omap_device for dev */
+ od = container_of(pdev, struct omap_device, pdev);
+ if (od-hwmods_cnt != 1) {
+ pr_err(%s: Error: No unique hwmod for device %s\n,
+ __func__, dev_name(dev_pm_qos_req-dev));
+ return -EINVAL;
+ }
+
+ /* Find the primary omap_hwmod for dev */
+ oh = od-hwmods[0];
+
+ pr_debug(OMAP PM CONSTRAINTS: req@0x%p, dev=0x%p, new_value=%lu\n,
+req, dev_pm_qos_req-dev, new_value);
+
+ /* Apply the constraint */
+ return omap_hwmod_set_wkup_lat_constraint(oh, dev_pm_qos_req,
+ new_value);
+}
+
+static struct notifier_block omap2_dev_pm_qos_notifier = {
+ .notifier_call = omap2_dev_pm_qos_handler,
+};
+
+static int __init omap2_dev_pm_qos_init(void)
+{
+ int ret;
+
+ ret = dev_pm_qos_add_global_notifier(omap2_dev_pm_qos_notifier);
+ if (ret)
+ WARN(1, KERN_ERR Cannot add global notifier for dev PM QoS\n);
+
+ return ret;
+}
+
static int __init omap2_common_pm_init(void)
{
omap2_init_processor_devices();
omap_pm_if_init();
+ /* Register to the per-device PM QoS framework */
+ omap2_dev_pm_qos_init();
+
return 0;
}
postcore_initcall(omap2_common_pm_init);
--
1.7.4.1
--
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 6/8] OMAP3: cpuidle: next C-state decision depends on the PM QoS MPU and CORE constraints
From: Jean Pihet j-pi...@ti.com
The MPU latency figures for cpuidle include the MPU itself and also
the peripherals needed for the MPU to execute instructions (e.g.
main memory, caches, IRQ controller, MMU etc). On OMAP3 those
peripherals belong to the MPU and CORE power domains and so the
cpuidle C-states are a combination of MPU and CORE states.
This patch implements the relation between the cpuidle and per-
device PM QoS frameworks in the OMAP3 specific idle callbacks.
The chosen C-state shall satisfy the following conditions:
. the 'valid' field is enabled,
. it satisfies the enable_off_mode flag,
. the next state for MPU and CORE power domains is not lower than the
next state calculated by the per-device PM QoS.
Tested on OMAP3 Beagleboard in RET/OFF using wake-up latency constraints
on MPU, CORE and PER.
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 49 ++---
arch/arm/mach-omap2/pm.h | 17 +++-
2 files changed, 44 insertions(+), 22 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 4bf6e6e..1b8e0da 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -37,7 +37,7 @@
#ifdef CONFIG_CPU_IDLE
/*
- * The latencies/thresholds for various C states have
+ * The MPU latencies/thresholds for various C states have
* to be configured from the respective board files.
* These are some default values (which might not provide
* the best power savings) used on boards which do not
@@ -72,14 +72,14 @@ struct omap3_idle_statedata
omap3_idle_data[OMAP3_NUM_STATES];
struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
+ struct clockdomain *clkdm)
{
clkdm_allow_idle(clkdm);
return 0;
}
static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
- struct clockdomain *clkdm)
+ struct clockdomain *clkdm)
{
clkdm_deny_idle(clkdm);
return 0;
@@ -92,9 +92,13 @@ static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
*
* Called from the CPUidle framework to program the device to the
* specified target state selected by the governor.
+ *
+ * Note: this function does not check for any pending activity or dependency
+ * between power domains states, so the caller shall check the parameters
+ * correctness.
*/
static int omap3_enter_idle(struct cpuidle_device *dev,
- struct cpuidle_state *state)
+ struct cpuidle_state *state)
{
struct omap3_idle_statedata *cx = cpuidle_get_statedata(state);
struct timespec ts_preidle, ts_postidle, ts_idle;
@@ -146,8 +150,11 @@ return_sleep_time:
* Else, this function searches for a lower c-state which is still
* valid.
*
- * A state is valid if the 'valid' field is enabled and
- * if it satisfies the enable_off_mode condition.
+ * A state is valid if:
+ * . the 'valid' field is enabled,
+ * . it satisfies the enable_off_mode flag,
+ * . the next state for MPU and CORE power domains is not lower than the
+ * state programmed by the per-device PM QoS.
*/
static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
struct cpuidle_state *curr)
@@ -156,6 +163,8 @@ static struct cpuidle_state *next_valid_state(struct
cpuidle_device *dev,
struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr);
u32 mpu_deepest_state = PWRDM_POWER_RET;
u32 core_deepest_state = PWRDM_POWER_RET;
+ u32 mpu_pm_qos_next_state = mpu_pd-wkup_lat_next_state;
+ u32 core_pm_qos_next_state = core_pd-wkup_lat_next_state;
if (enable_off_mode) {
mpu_deepest_state = PWRDM_POWER_OFF;
@@ -171,7 +180,9 @@ static struct cpuidle_state *next_valid_state(struct
cpuidle_device *dev,
/* Check if current state is valid */
if ((cx-valid)
(cx-mpu_state = mpu_deepest_state)
- (cx-core_state = core_deepest_state)) {
+ (cx-core_state = core_deepest_state)
+ (cx-mpu_state = mpu_pm_qos_next_state)
+ (cx-core_state = core_pm_qos_next_state)) {
return curr;
} else {
int idx = OMAP3_NUM_STATES - 1;
@@ -196,7 +207,9 @@ static struct cpuidle_state *next_valid_state(struct
cpuidle_device *dev,
cx = cpuidle_get_statedata(dev-states[idx]);
if ((cx-valid)
(cx-mpu_state = mpu_deepest_state)
- (cx-core_state = core_deepest_state)) {
+ (cx-core_state = core_deepest_state)
+ (cx-mpu_state = mpu_pm_qos_next_state)
+
[PATCH 7/8] OMAP3: update cpuidle latency and threshold figures
From: Jean Pihet j-pi...@ti.com
Update the data from the measurements performed at HW and SW levels.
Cf. http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement
for a detailed explanation on where are the numbers magically coming from.
ToDo:
- Measure the wake-up latencies for all power domains for OMAP3
- Correct some numbers when sys_clkreq and sys_offmode are supported
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/mach-omap2/cpuidle34xx.c | 28 ++--
1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 1b8e0da..4b3e994 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -44,20 +44,20 @@
* pass these details from the board file.
*/
static struct cpuidle_params cpuidle_params_table[] = {
- /* C1 */
- {2 + 2, 5, 1},
- /* C2 */
- {10 + 10, 30, 1},
- /* C3 */
- {50 + 50, 300, 1},
- /* C4 */
- {1500 + 1800, 4000, 1},
- /* C5 */
- {2500 + 7500, 12000, 1},
- /* C6 */
- {3000 + 8500, 15000, 1},
- /* C7 */
- {1 + 3, 30, 1},
+ /* C1 . MPU WFI + Core active */
+ {73 + 78, 152, 1},
+ /* C2 . MPU WFI + Core inactive */
+ {165 + 88, 345, 1},
+ /* C3 . MPU CSWR + Core inactive */
+ {163 + 182, 345, 1},
+ /* C4 . MPU OFF + Core inactive */
+ {2852 + 605, 15, 1},
+ /* C5 . MPU RET + Core RET */
+ {800 + 366, 2120, 1},
+ /* C6 . MPU OFF + Core RET */
+ {4080 + 801, 215000, 1},
+ /* C7 . MPU OFF + Core OFF */
+ {4300 + 13000, 215000, 1},
};
#define OMAP3_NUM_STATES ARRAY_SIZE(cpuidle_params_table)
--
1.7.4.1
--
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 8/8] OMAP3: powerdomain data: add wake-up latency figures
From: Jean Pihet j-pi...@ti.com
Figures are added to the power domains structs for RET and OFF modes.
Note: the latency figures for MPU, PER, CORE, NEON have been obtained
from actual measurements.
The latency figures for the other power domains are preliminary and
shall be added.
Cf. http://www.omappedia.org/wiki/Power_Management_Device_Latencies_Measurement
for a detailed explanation on where are the numbers magically coming from.
Tested on OMAP3 Beagleboard in RET/OFF using wake-up latency constraints
on MPU, CORE and PER.
Signed-off-by: Jean Pihet j-pi...@ti.com
---
arch/arm/mach-omap2/powerdomains3xxx_data.c | 78 +++
1 files changed, 78 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/powerdomains3xxx_data.c
b/arch/arm/mach-omap2/powerdomains3xxx_data.c
index 469a920..32922bb 100644
--- a/arch/arm/mach-omap2/powerdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/powerdomains3xxx_data.c
@@ -31,6 +31,14 @@
/*
* Powerdomains
+ *
+ * The wakeup_lat values are derived from measurements on
+ * the actual target.
+ *
+ * Note: the latency figures for MPU, PER, CORE, NEON have been obtained
+ * from actual measurements.
+ * The latency figures for the other power domains are preliminary and
+ * shall be added.
*/
static struct powerdomain iva2_pwrdm = {
@@ -52,6 +60,13 @@ static struct powerdomain iva2_pwrdm = {
[2] = PWRSTS_OFF_ON,
[3] = PWRSTS_ON,
},
+ .wakeup_lat = {
+ [PWRDM_FUNC_PWRST_OFF] = 1100,
+ [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_CSWR] = 350,
+ [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_ON] = 0,
+ },
};
static struct powerdomain mpu_3xxx_pwrdm = {
@@ -68,6 +83,13 @@ static struct powerdomain mpu_3xxx_pwrdm = {
.pwrsts_mem_on= {
[0] = PWRSTS_OFF_ON,
},
+ .wakeup_lat = {
+ [PWRDM_FUNC_PWRST_OFF] = 1830,
+ [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_CSWR] = 121,
+ [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_ON] = 0,
+ },
};
/*
@@ -98,6 +120,13 @@ static struct powerdomain core_3xxx_pre_es3_1_pwrdm = {
[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
},
+ .wakeup_lat = {
+ [PWRDM_FUNC_PWRST_OFF] = 3082,
+ [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_CSWR] = 153,
+ [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_ON] = 0,
+ },
};
static struct powerdomain core_3xxx_es3_1_pwrdm = {
@@ -121,6 +150,13 @@ static struct powerdomain core_3xxx_es3_1_pwrdm = {
[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
},
+ .wakeup_lat = {
+ [PWRDM_FUNC_PWRST_OFF] = 3082,
+ [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_CSWR] = 153,
+ [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_ON] = 0,
+ },
};
static struct powerdomain dss_pwrdm = {
@@ -136,6 +172,13 @@ static struct powerdomain dss_pwrdm = {
.pwrsts_mem_on= {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
+ .wakeup_lat = {
+ [PWRDM_FUNC_PWRST_OFF] = 70,
+ [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_CSWR] = 20,
+ [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_ON] = 0,
+ },
};
/*
@@ -157,6 +200,13 @@ static struct powerdomain sgx_pwrdm = {
.pwrsts_mem_on= {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
+ .wakeup_lat = {
+ [PWRDM_FUNC_PWRST_OFF] = 1000,
+ [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_CSWR] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_ON] = 0,
+ },
};
static struct powerdomain cam_pwrdm = {
@@ -172,6 +222,13 @@ static struct powerdomain cam_pwrdm = {
.pwrsts_mem_on= {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
+ .wakeup_lat = {
+ [PWRDM_FUNC_PWRST_OFF] = 850,
+ [PWRDM_FUNC_PWRST_OSWR] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_CSWR] = 35,
+ [PWRDM_FUNC_PWRST_INACTIVE] = UNSUP_STATE,
+ [PWRDM_FUNC_PWRST_ON] = 0,
+ },
};
static struct powerdomain per_pwrdm = {
@@ -187,6 +244,13 @@ static struct powerdomain per_pwrdm = {
.pwrsts_mem_on= {
[0] = PWRSTS_ON, /* MEMONSTATE */
},
+ .wakeup_lat = {
+ [PWRDM_FUNC_PWRST_OFF] = 671,
+
Re: [PATCH 4/8] OMAP2+: omap_hwmod: manage the wake-up latency constraints
Hi Paul, The series has been updated with the kerneldoc and error path rework, then re-submitted as v3. Can you please look at it and possibly pull it? Please let me know. Regards, Jean On Mon, Oct 10, 2011 at 10:08 AM, Jean Pihet jean.pi...@newoldbits.com wrote: Hi Paull, On Fri, Oct 7, 2011 at 4:53 AM, Paul Walmsley p...@pwsan.com wrote: Hi a comment: On Wed, 21 Sep 2011, jean.pi...@newoldbits.com wrote: From: Jean Pihet j-pi...@ti.com Hwmod is queried from the OMAP_PM layer to manage the power domains wake-up latency constraints. Hwmod retrieves the correct power domain and if it exists it calls the corresponding power domain function. Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up latency constraints on MPU, CORE and PER. Signed-off-by: Jean Pihet j-pi...@ti.com --- arch/arm/mach-omap2/omap_hwmod.c | 26 +- arch/arm/plat-omap/include/plat/omap_hwmod.h | 2 ++ 2 files changed, 27 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 84cc0bd..c6b1cc9 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -2618,11 +2619,34 @@ ohsps_unlock: return ret; } +/* + * omap_hwmod_set_wkup_constraint- set/release a wake-up latency constraint + * + * @oh: struct omap_hwmod* to which the target device belongs to. + * @cookie: identifier of the constraints list for @oh. + * @min_latency: the minimum allowed wake-up latency for @oh. + * + * Returns 0 upon success. + */ It's good that there is some documentation here, but it would be better if it were kerneldoc-style documentation. Please see Documentation/kernel-doc-nano-HOWTO.txt. Also it would be good to have a bit more documentation here beyond simply Returns 0 upon success. For example, how should a caller remove a wakeup latency constraint? Also, this function can return -EINVAL, not counting whatever pwrdm_set_wkup_lat_constraint() can return, so that should be documented above also. This applies to the function comments in the rest of the patches too. Some of them have quite good kerneldoc comments, such as pwrdm_wakeuplat_update_pwrst(); others need some work, like pwrdm_set_wkup_lat_constraint(). Not all functions have kernel doc comments, i.e. this one does not (no /** at the start of the header). The intention is to document the functions from the API and the ones that implement the core service (pwrdm_set_wkup_lat_constraint()), and not the pure pass-through functions (omap_hwmod_set_wkup_constraint). In any case I will review the comments and resubmit. Thanks, Jean - 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 v6 10/16] OMAP2+: UART: Modify omap_uart_can_sleep function
Govindraj govindraj...@gmail.com writes: On Tue, Oct 11, 2011 at 11:54 PM, Kevin Hilman khil...@ti.com wrote: Govindraj.R govindraj.r...@ti.com writes: Modify the omap_uart_can_sleep function to check uart is active or not to be used by pm code to enter low power states. Doesn't the driver now control when the UART clocks are gated (using runtime PM autosuspend)? IMO, this check should be completely removed and the driver should be managing this with the autosuspend timeout. Removing this check can cause console response little sluggish. Sluggish in what way? response is slower like when we type something or cat debugfs/pm_count see things little slower on console, there is no character loss. Happens even though we have not set the autosuspend timeout and uart clocks are active, which basically means allowing mpu to enter retention keeping uart active. OK, I see now. this delay in response or sluggishness is not there on my 3430SDP or 3630zoom board but I was able to see this behavior on a beagle board(xm rev c). Here's why: The difference is the powerdomain that the console UART is on for these boards. UART1,2 are in CORE, UART2/3 are in PER. SDP uses UART1 (CORE), Zoom3 doesn't use OMAP UARTs at all, and Beagle uses UART3 (PER). Due to a HW sleepdep between MPU and CORE, MPU will not transition until CORE does, which means MPU will not transition until UART 1 2 are idle. On Beagle, the console is ttyO2 (UART3) which is in PER, and since the MPU is free to transition independently of PER, that is what is happening, resulting in slower response time on for any boards that have PER-UART consoles. retaining this uart_can_sleep check in omap3_can_sleep ensures a better console user experience. (not allowing mpu to enter retention until uart clocks are cut) Yes, but obviously comes at the expense of power savings. IOW, This is a hard-coded power vs. performance trade off that we are trying to get away from. So, the root of the problem is that the MPU wakeup latency is causing a sluggish console. The solution? request an MPU wakeup latency constraint. This is a classic use-case for such a constraint, and the serial driver should have the option of requesting a constraint to prevent the sluggish console. The constraint only needs to be held until the auto-suspend delay expires, so should be relased in the -runtime_suspend() method of the driver. This constraint needs to be configurable, probably from the board file, so that it is optional, and so users who don't care about sluggish consoles (or non-console UART users who don't care about response time) have the option of preferring power savings over UART responsiveness. As a reference, the i2c driver is currently doing something similar in that it request an MPU constraint to prevent the MPU from going into retention/off while waiting for an i2c interrupt to arrive. 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
Re: [PATCH v6 14/16] OMAP2+: UART: Take console_lock in suspend path if not taken
Govindraj govindraj...@gmail.com writes:
On Wed, Oct 12, 2011 at 12:31 AM, Kevin Hilman khil...@ti.com wrote:
Govindraj.R govindraj.r...@ti.com writes:
In suspend path the console_lock is taken by uart_port_suspend
however when no_console_suspend is used console_lock is not taken.
During system wide suspend omap_pwr_domain hooks cut all
clocks that are left enabled. So its unsafe to proceed printing after
clocks are cut by pwr_domain hooks.
As I've mentioned in previous reviews, when no_console_suspend is
enabled, the user has explicitly requested console output during
suspend. In order to support that, we should not be cutting clocks at
all in that mode.
One way to address this would be to just disable runtime PM in the
-prepare method of the driver if no_console_suspend is enabled.
Okay fine exploring this option, right API's would be to use
pm_runtime_forbid/allow.
Yes.
SNIP
+static int serial_omap_runtime_prepare(struct device *dev)
+{
+ if (!console_suspend_enabled)
+ pm_runtime_forbid(dev);
+
+ return 0;
+}
+
+static void serial_omap_runtime_complete(struct device *dev)
+{
+ if (!console_suspend_enabled)
+ pm_runtime_allow(dev);
+}
+
static const struct dev_pm_ops serial_omap_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume)
SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend,
serial_omap_runtime_resume, NULL)
+ .prepare = serial_omap_runtime_prepare,
+ .complete = serial_omap_runtime_complete,
};
OK, but please add comments to these functions about exactly why they
are needed.
But to either use runtime forbid or disable we have ensure that
power_domain hooks don't go ahead and disable
the clocks with omap_device_idle as *runtime forbid or disable will
not set runtime_status to RPM_SUSPENDED*
and will stay in RPM_ACTIVE if we call runtime disable or forbid from
active state.
Correct.
in power_domain hooks we just check the pm_runtime_status_suspended
this will be false even if
we do runtime disable/forbid and it will cut uart clocks always.
So we may need below check also:
diff --git a/arch/arm/plat-omap/omap_device.c
b/arch/arm/plat-omap/omap_device.c
index 26aee5c..286a534 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -592,7 +592,8 @@ static int _od_suspend_noirq(struct device *dev)
ret = pm_generic_suspend_noirq(dev);
- if (!ret !pm_runtime_status_suspended(dev)) {
+ if (!ret pm_runtime_enabled(dev)
+ !pm_runtime_status_suspended(dev)) {
if (pm_generic_runtime_suspend(dev) == 0) {
omap_device_idle(pdev);
od-flags |= OMAP_DEVICE_SUSPENDED;
This isn't right either because devices that may not yet be initialized
(or loaded) may not have runtime PM enabled, so those devices may not
be properly idled.
We have an omap_device API to disable this feature at the PM domain level:
omap_device_disable_idle_on_suspend(). All you should have to do is to
use this API in the device init code on the console UART if
no_console_suspend has been enabled.
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
Re: [PATCH v6 15/16] OMAP2+: UART: Enable back uart clocks with runtime API for early console
Govindraj govindraj...@gmail.com writes: On Wed, Oct 12, 2011 at 2:36 AM, Kevin Hilman khil...@ti.com wrote: Govindraj.R govindraj.r...@ti.com writes: For the early console probing we had avoided hwmod reset and idling and uart was idled using hwmod API and enabled back using omap_device API after omap_device registration. Now since we are using runtime API's to enable back uart, move hwmod idling and use runtime API to enable back UART. Signed-off-by: Govindraj.R govindraj.r...@ti.com Now that the driver is using runtime PM. Why do we still need HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET? The comment in the code says: /* * During UART early init, device need to be probed * to determine SoC specific init before omap_device * is ready. Therefore, don't allow idle here */ This was true when using the 8250 driver because it was not using runtime PM so could not know how to (re)enable the device. However, since the driver is now runtime PM adapted, any device access should be contained within a runtime PM get/put block, so there should no longer be a reason not allow the IP blocks to be reset during boot. Forgot to add, this is still needed for earlyprintk(CONFIG_EARLY_PRINTK) use case, Ah, right. I forgot about that. Please update the changelog (and comment in the code) to reflect that. The initial boot prints until a console driver is available is from arch/arm/kernel/early_printk.c which does a tx on uart console and relies on configuration from bootloader. during bootup earlyprink does a tx on uart console and if uart driver is not available yet uart reset or idle done by hwmod layer can cause boot failures. -- put_char from earlyprintk.c -- reset/idle from hwmod layer -- put_char from earlyprintk.c So console_uart reset or clock gating must be done only after uart driver is available or be prevented using these available hwmod_flags. So why not leave the driver out of it and solve it like the current code does? The current codes use the hwmod flags, then waits until the UART driver is available (after omap_device_build) and uses omap_hwmod_idle() to do an clean idle of the device. Notably this is inside a console_lock/console_unlock block so that prints are buffered. The current code then does an omap_device_enable() to re-enable the device, but you shouldn't need that after the driver is converted to runtime PM. 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
Re: [PATCH v6 09/16] OMAP2+: UART: Add runtime pm support for omap-serial driver
Govindraj.R govindraj.r...@ti.com writes:
[...]
Use device_may_wakeup to check whether uart has wakeup capabilities
and then enable uart runtime usage for the uart.
Curious about what happens when device_may_wakeup() is not set during
device init.
[...]
@@ -1305,6 +1363,16 @@ static int serial_omap_probe(struct platform_device
*pdev)
up-uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
}
+ pm_runtime_use_autosuspend(pdev-dev);
+ pm_runtime_set_autosuspend_delay(pdev-dev,
+ OMAP_UART_AUTOSUSPEND_DELAY);
+
+ pm_runtime_irq_safe(pdev-dev);
+ if (device_may_wakeup(pdev-dev)) {
+ pm_runtime_enable(pdev-dev);
So if device_may_wakeup() is false, runtime PM is not enabled, then...
+ pm_runtime_get_sync(pdev-dev);
...this get doesn't happen, and the first register access causes a crash.
So either this get isn't needed, because there is no device access in
this path, or this path is broken and needs validation.
+ }
+
ui[pdev-id] = up;
serial_omap_add_console_port(up);
@@ -1312,6 +1380,7 @@ static int serial_omap_probe(struct platform_device
*pdev)
if (ret != 0)
goto do_release_region;
+ pm_runtime_put(pdev-dev);
...also, this put is not balanced with a corresponding get.
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
Re: [PATCH v6 10/16] OMAP2+: UART: Modify omap_uart_can_sleep function
On Thu, Oct 13, 2011 at 1:11 AM, Kevin Hilman khil...@ti.com wrote: Govindraj govindraj...@gmail.com writes: On Tue, Oct 11, 2011 at 11:54 PM, Kevin Hilman khil...@ti.com wrote: Govindraj.R govindraj.r...@ti.com writes: Modify the omap_uart_can_sleep function to check uart is active or not to be used by pm code to enter low power states. Doesn't the driver now control when the UART clocks are gated (using runtime PM autosuspend)? IMO, this check should be completely removed and the driver should be managing this with the autosuspend timeout. Removing this check can cause console response little sluggish. Sluggish in what way? response is slower like when we type something or cat debugfs/pm_count see things little slower on console, there is no character loss. Happens even though we have not set the autosuspend timeout and uart clocks are active, which basically means allowing mpu to enter retention keeping uart active. OK, I see now. this delay in response or sluggishness is not there on my 3430SDP or 3630zoom board but I was able to see this behavior on a beagle board(xm rev c). Here's why: The difference is the powerdomain that the console UART is on for these boards. UART1,2 are in CORE, UART2/3 are in PER. SDP uses UART1 (CORE), Zoom3 doesn't use OMAP UARTs at all, and Beagle uses UART3 (PER). Due to a HW sleepdep between MPU and CORE, MPU will not transition until CORE does, which means MPU will not transition until UART 1 2 are idle. On Beagle, the console is ttyO2 (UART3) which is in PER, and since the MPU is free to transition independently of PER, that is what is happening, resulting in slower response time on for any boards that have PER-UART consoles. retaining this uart_can_sleep check in omap3_can_sleep ensures a better console user experience. (not allowing mpu to enter retention until uart clocks are cut) Yes, but obviously comes at the expense of power savings. IOW, This is a hard-coded power vs. performance trade off that we are trying to get away from. So, the root of the problem is that the MPU wakeup latency is causing a sluggish console. The solution? request an MPU wakeup latency constraint. Okay, Will explore this. This is a classic use-case for such a constraint, and the serial driver should have the option of requesting a constraint to prevent the sluggish console. The constraint only needs to be held until the auto-suspend delay expires, so should be relased in the -runtime_suspend() method of the driver. This constraint needs to be configurable, probably from the board file, so that it is optional, and so users who don't care about sluggish consoles (or non-console UART users who don't care about response time) have the option of preferring power savings over UART responsiveness. As a reference, the i2c driver is currently doing something similar in that it request an MPU constraint to prevent the MPU from going into retention/off while waiting for an i2c interrupt to arrive. Thanks, will check and try to use the mpu constraints -- Govindraj.R. -- 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 v6 14/16] OMAP2+: UART: Take console_lock in suspend path if not taken
On Thu, Oct 13, 2011 at 5:17 AM, Kevin Hilman khil...@ti.com wrote:
Govindraj govindraj...@gmail.com writes:
On Wed, Oct 12, 2011 at 12:31 AM, Kevin Hilman khil...@ti.com wrote:
Govindraj.R govindraj.r...@ti.com writes:
In suspend path the console_lock is taken by uart_port_suspend
however when no_console_suspend is used console_lock is not taken.
During system wide suspend omap_pwr_domain hooks cut all
clocks that are left enabled. So its unsafe to proceed printing after
clocks are cut by pwr_domain hooks.
As I've mentioned in previous reviews, when no_console_suspend is
enabled, the user has explicitly requested console output during
suspend. In order to support that, we should not be cutting clocks at
all in that mode.
One way to address this would be to just disable runtime PM in the
-prepare method of the driver if no_console_suspend is enabled.
Okay fine exploring this option, right API's would be to use
pm_runtime_forbid/allow.
Yes.
SNIP
+static int serial_omap_runtime_prepare(struct device *dev)
+{
+ if (!console_suspend_enabled)
+ pm_runtime_forbid(dev);
+
+ return 0;
+}
+
+static void serial_omap_runtime_complete(struct device *dev)
+{
+ if (!console_suspend_enabled)
+ pm_runtime_allow(dev);
+}
+
static const struct dev_pm_ops serial_omap_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume)
SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend,
serial_omap_runtime_resume, NULL)
+ .prepare = serial_omap_runtime_prepare,
+ .complete = serial_omap_runtime_complete,
};
OK, but please add comments to these functions about exactly why they
are needed.
But to either use runtime forbid or disable we have ensure that
power_domain hooks don't go ahead and disable
the clocks with omap_device_idle as *runtime forbid or disable will
not set runtime_status to RPM_SUSPENDED*
and will stay in RPM_ACTIVE if we call runtime disable or forbid from
active state.
Correct.
in power_domain hooks we just check the pm_runtime_status_suspended
this will be false even if
we do runtime disable/forbid and it will cut uart clocks always.
So we may need below check also:
diff --git a/arch/arm/plat-omap/omap_device.c
b/arch/arm/plat-omap/omap_device.c
index 26aee5c..286a534 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -592,7 +592,8 @@ static int _od_suspend_noirq(struct device *dev)
ret = pm_generic_suspend_noirq(dev);
- if (!ret !pm_runtime_status_suspended(dev)) {
+ if (!ret pm_runtime_enabled(dev)
+ !pm_runtime_status_suspended(dev)) {
if (pm_generic_runtime_suspend(dev) == 0) {
omap_device_idle(pdev);
od-flags |= OMAP_DEVICE_SUSPENDED;
This isn't right either because devices that may not yet be initialized
(or loaded) may not have runtime PM enabled, so those devices may not
be properly idled.
We have an omap_device API to disable this feature at the PM domain level:
omap_device_disable_idle_on_suspend(). All you should have to do is to
use this API in the device init code on the console UART if
no_console_suspend has been enabled.
Yes seems okay to me,
Will check if no_console_suspend is used then set
omap_device_disable_idle_on_suspend
in serial.c.
--
Thanks,
Govindraj.R
--
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 v6 15/16] OMAP2+: UART: Enable back uart clocks with runtime API for early console
On Thu, Oct 13, 2011 at 5:30 AM, Kevin Hilman khil...@ti.com wrote: Govindraj govindraj...@gmail.com writes: On Wed, Oct 12, 2011 at 2:36 AM, Kevin Hilman khil...@ti.com wrote: Govindraj.R govindraj.r...@ti.com writes: For the early console probing we had avoided hwmod reset and idling and uart was idled using hwmod API and enabled back using omap_device API after omap_device registration. Now since we are using runtime API's to enable back uart, move hwmod idling and use runtime API to enable back UART. Signed-off-by: Govindraj.R govindraj.r...@ti.com Now that the driver is using runtime PM. Why do we still need HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET? The comment in the code says: /* * During UART early init, device need to be probed * to determine SoC specific init before omap_device * is ready. Therefore, don't allow idle here */ This was true when using the 8250 driver because it was not using runtime PM so could not know how to (re)enable the device. However, since the driver is now runtime PM adapted, any device access should be contained within a runtime PM get/put block, so there should no longer be a reason not allow the IP blocks to be reset during boot. Forgot to add, this is still needed for earlyprintk(CONFIG_EARLY_PRINTK) use case, Ah, right. I forgot about that. Please update the changelog (and comment in the code) to reflect that. The initial boot prints until a console driver is available is from arch/arm/kernel/early_printk.c which does a tx on uart console and relies on configuration from bootloader. during bootup earlyprink does a tx on uart console and if uart driver is not available yet uart reset or idle done by hwmod layer can cause boot failures. -- put_char from earlyprintk.c -- reset/idle from hwmod layer -- put_char from earlyprintk.c So console_uart reset or clock gating must be done only after uart driver is available or be prevented using these available hwmod_flags. So why not leave the driver out of it and solve it like the current code does? The current codes use the hwmod flags, then waits until the UART driver is available (after omap_device_build) and uses omap_hwmod_idle() to do an clean idle of the device. Notably this is inside a console_lock/console_unlock block so that prints are buffered. The current code then does an omap_device_enable() to re-enable the device, but you shouldn't need that after the driver is converted to runtime PM. Yes similar approach here, We are not doing hwmod idle until console driver is available, once omap-serial is available from probe doing hwmod_idle* and then get_sync. hwmod idle in serial.c will still cause problems if ealryprintk tries to print until omap-uart console driver is not available, as now with rumtime adaptation only driver can enable back clocks. So have added a function pointer to pdata which calls hwmod_idle implemented in serial.c calling omap_hwmod_idle. -- Thanks, Govindraj.R *function pointer implemented in serial.c not directly calling omap_hmwod_idle API. -- 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 v6 09/16] OMAP2+: UART: Add runtime pm support for omap-serial driver
On Thu, Oct 13, 2011 at 5:36 AM, Kevin Hilman khil...@ti.com wrote:
Govindraj.R govindraj.r...@ti.com writes:
[...]
Use device_may_wakeup to check whether uart has wakeup capabilities
and then enable uart runtime usage for the uart.
Curious about what happens when device_may_wakeup() is not set during
device init.
[...]
@@ -1305,6 +1363,16 @@ static int serial_omap_probe(struct platform_device
*pdev)
up-uart_dma.rx_dma_channel = OMAP_UART_DMA_CH_FREE;
}
+ pm_runtime_use_autosuspend(pdev-dev);
+ pm_runtime_set_autosuspend_delay(pdev-dev,
+ OMAP_UART_AUTOSUSPEND_DELAY);
+
+ pm_runtime_irq_safe(pdev-dev);
+ if (device_may_wakeup(pdev-dev)) {
+ pm_runtime_enable(pdev-dev);
So if device_may_wakeup() is false, runtime PM is not enabled, then...
+ pm_runtime_get_sync(pdev-dev);
...this get doesn't happen, and the first register access causes a crash.
Actually no crash, clocks will left enabled from boot up (hwmod_no_reset/idle)
that are idled and enabled back here.
Since hwmod_idle is binded here later ([PATCH v6 15/16]),
So either this get isn't needed, because there is no device access in
this path, or this path is broken and needs validation.
+ }
+
ui[pdev-id] = up;
serial_omap_add_console_port(up);
@@ -1312,6 +1380,7 @@ static int serial_omap_probe(struct platform_device
*pdev)
if (ret != 0)
goto do_release_region;
+ pm_runtime_put(pdev-dev);
...also, this put is not balanced with a corresponding get.
this needs to happen only when runtime is enabled.
will correct this.
--
Thanks,
Govindraj.,R
--
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 2/5] drivercore: Add driver probe deferral mechanism
On Tue, Oct 11, 2011 at 08:29:18PM +0800, Ming Lei wrote: On Tue, Oct 11, 2011 at 1:37 AM, Andrei Warkentin awarken...@vmware.com wrote: Hi, - Original Message - From: Greg KH g...@kroah.com To: Josh Triplett j...@joshtriplett.org Cc: G, Manjunath Kondaiah manj...@ti.com, linux-arm-ker...@lists.infradead.org, Grant Likely grant.lik...@secretlab.ca, linux-omap@vger.kernel.org, linux-...@vger.kernel.org, linux-ker...@vger.kernel.org, Dilan Lee di...@nvidia.com, Mark Brown broo...@opensource.wolfsonmicro.com, manjun...@jasper.es Sent: Saturday, October 8, 2011 11:55:02 AM Subject: Re: [PATCH 2/5] drivercore: Add driver probe deferral mechanism I'm a bit of a fly on the wall here, but I'm curious how this impacts suspend/resume. device_initialize-device_pm_init are called from device_register, so certainly this patch doesn't also ensure that the PM ordering matches probe ordering, which is bound to break suspend, right? Was this ever tested with the OMAP target? Shouldn't the Inside device_add(), device_pm_add is called before bus_probe_device, so the patch can't change the device order in pm list, and just change the driver probe order. That's the way it works now, but can it be reworked? It would be possible to adjust the list order after successful probe. However, I'm not clear on the ordering rules for the dpm_list. Right now it is explicitly ordered to have parents before children, but as already expressed, that doesn't accurately represent ordering constraints for multiple device dependancies. So, reordering the list would probably require maintaining the existing parent-child ordering constraint, but to also shift devices (and any possible children?) to be after drivers that are already probed. That alone will be difficult to implement and get right, but maybe the constraints can be simplified. It needs some further thought. g. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] drivercore: add new error value for deferred probe
On Wed, Oct 12, 2011 at 11:48:23AM +0530, G, Manjunath Kondaiah wrote: On Sun, Oct 09, 2011 at 06:06:56PM -0700, Greg KH wrote: On Sun, Oct 09, 2011 at 04:59:31PM -0600, Grant Likely wrote: On Fri, Oct 7, 2011 at 6:12 PM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 07:28:33PM -0400, valdis.kletni...@vt.edu wrote: On Fri, 07 Oct 2011 16:12:45 MDT, Grant Likely said: On Fri, Oct 7, 2011 at 12:43 AM, Greg KH g...@kroah.com wrote: On Fri, Oct 07, 2011 at 10:33:06AM +0500, G, Manjunath Kondaiah wrote: +#define EPROBE_DEFER 517 /* restart probe again after some time */ Can we really do this? According to Arnd, yes this is okay. Isn't this some user/kernel api here? What's wrong with just overloading on top of an existing error code? Surely one of the other 516 types could be used here, right? overloading makes it really hard to find the users at a later date. Would proposing '#define EPROBE_DEFER EAGAIN' be acceptable to everybody? That would allow overloading EAGAIN, but still make it easy to tell the usages apart if we need to separate them later... Yes, please do that, it is what USB does for it's internal error code handling. Really? When we've only currently used approximately 2^9 of a 2^31 numberspace? I'm fine with making sure that the number doesn't show up in the userspace headers, but it makes no sense to overload the #defines. Particularly so in this case where it isn't feasible to audit every driver to figure out what probe might possibly return. It is well within the realm of possibility that existing drivers are already returning -EAGAIN. I doubt they are, but you are right, it's really hard to tell. Besides; linux/errno.h *already* has linux-internal error codes that do not get exported out to userspace. There is an #ifdef __KERNEL__ block around ERESTARTSYS through EIOCBRETRY which is filtered out when exporting headers. I can't see any possible reason why we wouldn't add Linux internal error codes here. As long as it stays internal, that's fine, I was worried that this would be exported to userspace. Alan, still object to this? I hope no one has objections to use EPROBE_DEFER I say go with that value. If Alan still objects, then he will speak up. g. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] gpiolib: handle deferral probe error
On Wed, Oct 12, 2011 at 11:44:32AM +0530, G, Manjunath Kondaiah wrote: On Fri, Oct 07, 2011 at 04:09:38PM -0600, Grant Likely wrote: On Fri, Oct 7, 2011 at 4:06 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: On Fri, 07 Oct 2011 10:33:09 +0500 G, Manjunath Kondaiah manj...@ti.com wrote: The gpio library should return -EPROBE_DEFER in gpio_request if gpio driver is not ready. Why not use the perfectly good existing error codes we have for this ? We have EAGAIN and EUNATCH both of which look sensible. I want a distinct error code for probe deferral so that a) it doesn't overlap with something a driver is already doing, and b) so that all the users can be found again at a later date. That said, I'm not in agreement with this patch. It is fine for gpio lib to have a code that means the pin doesn't exist (yet), but the device driver needs to be the one to decide whether or not it is appropriate to use probe deferral. During gpio_request, driver gpio_request is not available. How can we expect driver to request deferred probe in this case? If gpio_request fails, the driver can then explicitly make the decision to return -EPROBE_DEFER. It isn't forced to pass on the error code from gpio_request(). g. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
