On Thu, Apr 12, 2012 at 1:04 AM, Paul Walmsley <paul@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
