Re: Suspend broken on 3.3?

["Raja, Govindraj" <govindraj.raja@ti.com>]

On Thu, Apr 12, 2012 at 1:04 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hi
>
> a few brief comments based on a quick scan:
>
> On Wed, 11 Apr 2012, Raja, Govindraj wrote:
>
>> Here is the patch [1] to do the same.
>
> - I don't see where it affects I/O wakeups for the UART.  If I/O wakeups
> are still set on the UART pads, won't that still wake the chip up from
> suspend, even if module-level wakeups are disabled?

pdata->enable_wakeup => omap_uart_enable_wakeup
was disabling both module level and pad wakeup.

omap_uart_enable_wakeup => has enabling/disabling both
module level and pad wakeup together.

>
> - The UART driver and integration code should not be reading from or
> writing to registers outside the UART IP block.  PRM register reads and
> writes belong in the PRM code, which should then export a higher-level
> interface to the calling code.  This is because, aside from making the
> code easier to read and debug, we're trying to move the PRM and CM code to
> drivers/.

okay.

>
> - The code to change the PM_WKEN* and test the PM_WKST* bits should
> probably be called from omap_hwmod_{enable,disable}_wakeup(), not the UART
> code directly.  The UART code shouldn't need to care about the hardware
> details; it should just ask the integration layer to enable or disable
> module-level wakeups.

To implement this I plan to extend the omap_hwmod_omap2_prcm structure
like this:

(unaligned tmp code snippet)

diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h
b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 8070145..5c7711b 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -343,6 +343,8 @@ struct omap_hwmod_class_sysconfig {
  * @idlest_reg_id: IDLEST register ID (e.g., 3 for CM_IDLEST3)
  * @idlest_idle_bit: register bit shift for CM_IDLEST slave idle bit
  * @idlest_stdby_bit: register bit shift for CM_IDLEST master standby bit
+ * @module_wakeup_offs: PRCM register offset for PM_WKEN
+ * @module_wakeup_bit: regiter bit mask for PM_WKEN
  *
  * @prcm_reg_id and @module_bit are specific to the AUTOIDLE, WKST,
  * WKEN, GRPSEL registers.  In an ideal world, no extra information
@@ -357,6 +359,8 @@ struct omap_hwmod_omap2_prcm {
        u8 idlest_reg_id;
        u8 idlest_idle_bit;
        u8 idlest_stdby_bit;
+       s16 module_wakeup_offs;
+       u32 module_wakeup_bit;
 };

>
> As you work on these changes, please split them up into several different
> topic series - one for the PRM changes, one for hwmod code/data changes,
> and one for the UART driver/integration changes.  Just note the
> dependencies in the series description E-mails.  That way, we can avoid
> merge conflicts.
>

Yes fine. Since most changes will be on /mach-omap2/omap_hwmod*.c
Do you prefer the patches to be on any particular tree/branch where hwmod fixes
are already queued.

--
Thanks,
Govindraj.R
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html