[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Govindraj.R]

For device pads which have OMAP_DEVICE_PAD_WAKEUP set (which means they
are wakeup capable) enable the IO-daisy wakeup capability.
During re-muxing avoid direct write with val as this can disturb if any
mux done at bootloader level so read the pad then write back.

Also add a api to fetch the wake-up status bit is set for given omap-hwmod
device using available mux info which is added during omap_hwmod_mux_init.
Wakeup status bit is needed for uart_resume_idle call from sram_idle path
based on wake-up event has occurred for given uart we can enable clocks back.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/mux.c                    |   23 +++++++++++++++++++++++
 arch/arm/mach-omap2/mux.h                    |   13 +++++++++++++
 arch/arm/mach-omap2/omap_hwmod.c             |   13 +++++++++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
 4 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index 98148b6..5338b93 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -317,6 +317,24 @@ err1:
 	return NULL;
 }
 
+/* Gets the wakeup status of given pad from omap-hwmod */
+int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
+{
+	int i;
+	unsigned int val = -EINVAL;
+
+	for (i = 0; i < hmux->nr_pads; i++) {
+		struct omap_device_pad *pad = &hmux->pads[i];
+
+		val = omap_mux_read(pad->partition, pad->mux->reg_offset);
+	}
+
+	if (val > 0 && val & OMAP_WAKEUP_EVENT)
+		return 1;
+	else
+		return 0;
+}
+
 /* Assumes the calling function takes care of locking */
 void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
 {
@@ -342,6 +360,9 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
 				break;
 			flags &= ~OMAP_DEVICE_PAD_ENABLED;
 			val = pad->idle;
+			if (flags & OMAP_DEVICE_PAD_WAKEUP)
+				val |= OMAP_WAKEUP_EN;
+
 			pr_debug("%s: Idling %s %x\n", __func__,
 					pad->name, val);
 			break;
@@ -358,6 +379,8 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
 		};
 
 		if (val >= 0) {
+			val |= omap_mux_read(pad->partition,
+						pad->mux->reg_offset);
 			omap_mux_write(pad->partition, val,
 					pad->mux->reg_offset);
 			pad->flags = flags;
diff --git a/arch/arm/mach-omap2/mux.h b/arch/arm/mach-omap2/mux.h
index a4ab17a..84a8fc2 100644
--- a/arch/arm/mach-omap2/mux.h
+++ b/arch/arm/mach-omap2/mux.h
@@ -220,8 +220,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads, int nr_pads);
  */
 void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);
 
+
+/**
+ * omap_hwmod_mux_wakeup - omap hwmod check given pad had wakeup event
+ * @hmux:		Pads for a hwmod
+ *
+ * Called only from omap_hwmod.c, do not use.
+ */
+int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux);
 #else
 
+static inline int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
+{
+	return 0;
+}
+
 static inline int omap_mux_init_gpio(int gpio, int val)
 {
 	return 0;
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 9e89a58..893dae1 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2239,3 +2239,16 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
 
 	return ret;
 }
+
+/**
+ * omap_hwmod_pad_wakeup_status - get pad wakeup status if mux is available.
+ * @oh: struct omap_hwmod *
+ *
+ * Returns the wake_up status bit of available pad mux pin.
+ */
+int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh)
+{
+	if (oh->mux)
+		return omap_hwmod_mux_wakeup(oh->mux);
+	return -EINVAL;
+}
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index fedd829..4100be0 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -588,6 +588,7 @@ int omap_hwmod_for_each_by_class(const char *classname,
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
 u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
+int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh);
 /*
  * Chip variant-specific hwmod init routines - XXX should be converted
  * to use initcalls once the initial boot ordering is straightened out
-- 
1.7.1

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Varadarajan, Charulatha]

Govind,

Couple of minor comments.

On Mon, Feb 28, 2011 at 20:09, Govindraj.R <govindraj.raja@ti.com> wrote:
> For device pads which have OMAP_DEVICE_PAD_WAKEUP set (which means they
> are wakeup capable) enable the IO-daisy wakeup capability.
> During re-muxing avoid direct write with val as this can disturb if any
> mux done at bootloader level so read the pad then write back.
>
> Also add a api to fetch the wake-up status bit is set for given omap-hwmod

%s/a api/an API/

> device using available mux info which is added during omap_hwmod_mux_init.
> Wakeup status bit is needed for uart_resume_idle call from sram_idle path
> based on wake-up event has occurred for given uart we can enable clocks back.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
> ?arch/arm/mach-omap2/mux.c ? ? ? ? ? ? ? ? ? ?| ? 23 +++++++++++++++++++++++
> ?arch/arm/mach-omap2/mux.h ? ? ? ? ? ? ? ? ? ?| ? 13 +++++++++++++
> ?arch/arm/mach-omap2/omap_hwmod.c ? ? ? ? ? ? | ? 13 +++++++++++++
> ?arch/arm/plat-omap/include/plat/omap_hwmod.h | ? ?1 +
> ?4 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
> index 98148b6..5338b93 100644
> --- a/arch/arm/mach-omap2/mux.c
> +++ b/arch/arm/mach-omap2/mux.c
> @@ -317,6 +317,24 @@ err1:
> ? ? ? ?return NULL;
> ?}
>
> +/* Gets the wakeup status of given pad from omap-hwmod */
> +int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
> +{
> + ? ? ? int i;
> + ? ? ? unsigned int val = -EINVAL;
> +
> + ? ? ? for (i = 0; i < hmux->nr_pads; i++) {
> + ? ? ? ? ? ? ? struct omap_device_pad *pad = &hmux->pads[i];
> +
> + ? ? ? ? ? ? ? val = omap_mux_read(pad->partition, pad->mux->reg_offset);
> + ? ? ? }
> +
> + ? ? ? if (val > 0 && val & OMAP_WAKEUP_EVENT)
> + ? ? ? ? ? ? ? return 1;
> + ? ? ? else

No need of else here.

> + ? ? ? ? ? ? ? return 0;
> +}
> +

- V Charulatha

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Govindraj]

On Wed, Mar 2, 2011 at 10:19 AM, Varadarajan, Charulatha <charu@ti.com> wrote:
> Govind,
>
> Couple of minor comments.
>
> On Mon, Feb 28, 2011 at 20:09, Govindraj.R <govindraj.raja@ti.com> wrote:
>> For device pads which have OMAP_DEVICE_PAD_WAKEUP set (which means they
>> are wakeup capable) enable the IO-daisy wakeup capability.
>> During re-muxing avoid direct write with val as this can disturb if any
>> mux done at bootloader level so read the pad then write back.
>>
>> Also add a api to fetch the wake-up status bit is set for given omap-hwmod
>
> %s/a api/an API/
>
>> device using available mux info which is added during omap_hwmod_mux_init.
>> Wakeup status bit is needed for uart_resume_idle call from sram_idle path
>> based on wake-up event has occurred for given uart we can enable clocks back.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>> ---
>> ?arch/arm/mach-omap2/mux.c ? ? ? ? ? ? ? ? ? ?| ? 23 +++++++++++++++++++++++
>> ?arch/arm/mach-omap2/mux.h ? ? ? ? ? ? ? ? ? ?| ? 13 +++++++++++++
>> ?arch/arm/mach-omap2/omap_hwmod.c ? ? ? ? ? ? | ? 13 +++++++++++++
>> ?arch/arm/plat-omap/include/plat/omap_hwmod.h | ? ?1 +
>> ?4 files changed, 50 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
>> index 98148b6..5338b93 100644
>> --- a/arch/arm/mach-omap2/mux.c
>> +++ b/arch/arm/mach-omap2/mux.c
>> @@ -317,6 +317,24 @@ err1:
>> ? ? ? ?return NULL;
>> ?}
>>
>> +/* Gets the wakeup status of given pad from omap-hwmod */
>> +int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
>> +{
>> + ? ? ? int i;
>> + ? ? ? unsigned int val = -EINVAL;
>> +
>> + ? ? ? for (i = 0; i < hmux->nr_pads; i++) {
>> + ? ? ? ? ? ? ? struct omap_device_pad *pad = &hmux->pads[i];
>> +
>> + ? ? ? ? ? ? ? val = omap_mux_read(pad->partition, pad->mux->reg_offset);
>> + ? ? ? }
>> +
>> + ? ? ? if (val > 0 && val & OMAP_WAKEUP_EVENT)
>> + ? ? ? ? ? ? ? return 1;
>> + ? ? ? else
>
> No need of else here.

Yes correct.

I am resending this patch with some more modifications
to fix one more issue in the above loop.

--
Thanks,
Govindraj.R



<<SNIP>>

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Govindraj.R]

For device pads which have OMAP_DEVICE_PAD_WAKEUP set (which means they
are wakeup capable) enable the IO-daisy wakeup capability.
During re-muxing avoid direct write with val as this can disturb if any
mux done at bootloader level so read the pad then write back.

Also add an API to fetch the wake-up status bit is set for given omap-hwmod
device using available mux info which is added during omap_hwmod_mux_init.
Wakeup status bit is needed for uart_resume_idle call from sram_idle path
based on wake-up event has occurred for given uart we can enable clocks back.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
 arch/arm/mach-omap2/mux.c                    |   28 ++++++++++++++++++++++++++
 arch/arm/mach-omap2/mux.h                    |   13 ++++++++++++
 arch/arm/mach-omap2/omap_hwmod.c             |   13 ++++++++++++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
 4 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index 98148b6..50edcea 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -317,6 +317,29 @@ err1:
 	return NULL;
 }
 
+/* Gets the wakeup status of given pad from omap-hwmod */
+int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
+{
+	int i;
+	unsigned int val;
+	u8 ret = 0;
+
+	for (i = 0; i < hmux->nr_pads; i++) {
+		struct omap_device_pad *pad = &hmux->pads[i];
+
+		if (pad->flags & OMAP_DEVICE_PAD_WAKEUP) {
+			val = omap_mux_read(pad->partition,
+						pad->mux->reg_offset);
+			if (val & OMAP_WAKEUP_EVENT) {
+				ret = 1;
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
+
 /* Assumes the calling function takes care of locking */
 void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
 {
@@ -342,6 +365,9 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
 				break;
 			flags &= ~OMAP_DEVICE_PAD_ENABLED;
 			val = pad->idle;
+			if (flags & OMAP_DEVICE_PAD_WAKEUP)
+				val |= OMAP_WAKEUP_EN;
+
 			pr_debug("%s: Idling %s %x\n", __func__,
 					pad->name, val);
 			break;
@@ -358,6 +384,8 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
 		};
 
 		if (val >= 0) {
+			val |= omap_mux_read(pad->partition,
+						pad->mux->reg_offset);
 			omap_mux_write(pad->partition, val,
 					pad->mux->reg_offset);
 			pad->flags = flags;
diff --git a/arch/arm/mach-omap2/mux.h b/arch/arm/mach-omap2/mux.h
index a4ab17a..84a8fc2 100644
--- a/arch/arm/mach-omap2/mux.h
+++ b/arch/arm/mach-omap2/mux.h
@@ -220,8 +220,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads, int nr_pads);
  */
 void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);
 
+
+/**
+ * omap_hwmod_mux_wakeup - omap hwmod check given pad had wakeup event
+ * @hmux:		Pads for a hwmod
+ *
+ * Called only from omap_hwmod.c, do not use.
+ */
+int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux);
 #else
 
+static inline int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
+{
+	return 0;
+}
+
 static inline int omap_mux_init_gpio(int gpio, int val)
 {
 	return 0;
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 9e89a58..893dae1 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2239,3 +2239,16 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
 
 	return ret;
 }
+
+/**
+ * omap_hwmod_pad_wakeup_status - get pad wakeup status if mux is available.
+ * @oh: struct omap_hwmod *
+ *
+ * Returns the wake_up status bit of available pad mux pin.
+ */
+int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh)
+{
+	if (oh->mux)
+		return omap_hwmod_mux_wakeup(oh->mux);
+	return -EINVAL;
+}
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index fedd829..4100be0 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -588,6 +588,7 @@ int omap_hwmod_for_each_by_class(const char *classname,
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
 u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
+int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh);
 /*
  * Chip variant-specific hwmod init routines - XXX should be converted
  * to use initcalls once the initial boot ordering is straightened out
-- 
1.7.1

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Kevin Hilman]

Hi Govindraj,

"Govindraj.R" <govindraj.raja@ti.com> writes:

> For device pads which have OMAP_DEVICE_PAD_WAKEUP set (which means they
> are wakeup capable) enable the IO-daisy wakeup capability.
> During re-muxing avoid direct write with val as this can disturb if any
> mux done at bootloader level so read the pad then write back.
>
> Also add an API to fetch the wake-up status bit is set for given omap-hwmod
> device using available mux info which is added during omap_hwmod_mux_init.
> Wakeup status bit is needed for uart_resume_idle call from sram_idle path
> based on wake-up event has occurred for given uart we can enable clocks back.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
>  arch/arm/mach-omap2/mux.c                    |   28 ++++++++++++++++++++++++++
>  arch/arm/mach-omap2/mux.h                    |   13 ++++++++++++
>  arch/arm/mach-omap2/omap_hwmod.c             |   13 ++++++++++++
>  arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
>  4 files changed, 55 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
> index 98148b6..50edcea 100644
> --- a/arch/arm/mach-omap2/mux.c
> +++ b/arch/arm/mach-omap2/mux.c
> @@ -317,6 +317,29 @@ err1:
>  	return NULL;
>  }
>  
> +/* Gets the wakeup status of given pad from omap-hwmod */
> +int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)

should return a bool

Also, documentation should be clear that it returns true if *any* pad
associated with this device has a wakeup event.

Also a name reflecting the get/check/status nature of this function
would help readability, maybe:

bool omap_hwmod_mux_wakeup_event(struct omap_hwmod_mux_info *hmux)

> +{
> +	int i;
> +	unsigned int val;
> +	u8 ret = 0;
> +
> +	for (i = 0; i < hmux->nr_pads; i++) {
> +		struct omap_device_pad *pad = &hmux->pads[i];
> +
> +		if (pad->flags & OMAP_DEVICE_PAD_WAKEUP) {
> +			val = omap_mux_read(pad->partition,
> +						pad->mux->reg_offset);
> +			if (val & OMAP_WAKEUP_EVENT) {
> +				ret = 1;
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  /* Assumes the calling function takes care of locking */
>  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>  {
> @@ -342,6 +365,9 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>  				break;
>  			flags &= ~OMAP_DEVICE_PAD_ENABLED;
>  			val = pad->idle;
> +			if (flags & OMAP_DEVICE_PAD_WAKEUP)
> +				val |= OMAP_WAKEUP_EN;
> +

Is this needed on every idle transition?  

You're currently setting it on every idle transition, but never clearing
it.  If it is to be a one-time thing, then move it to the early init of
the mux.

>  			pr_debug("%s: Idling %s %x\n", __func__,
>  					pad->name, val);
>  			break;
> @@ -358,6 +384,8 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>  		};
>  
>  		if (val >= 0) {
> +			val |= omap_mux_read(pad->partition,
> +						pad->mux->reg_offset);

I don't think this is doing what you expect, and will lead to very
difficult to find bugs.  Since your OR'ing in bits from the current
value, you will never be able to clear any bits in this register.

As just a dumb example, consider the enable case where
pad->enable=MUX_MODE0 (0x0).  If the bootloader has initialized this pad
to MUX_MODE7 (0x7), this code will never set it to MUX_MODE0, since this
read and OR of the existing value will always set it back to MUX_MODE7.

At a minimum, this change should be a separate patch with a more
detailed description.


>  			omap_mux_write(pad->partition, val,
>  					pad->mux->reg_offset);
>  			pad->flags = flags;
> diff --git a/arch/arm/mach-omap2/mux.h b/arch/arm/mach-omap2/mux.h
> index a4ab17a..84a8fc2 100644
> --- a/arch/arm/mach-omap2/mux.h
> +++ b/arch/arm/mach-omap2/mux.h
> @@ -220,8 +220,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads, int nr_pads);
>   */
>  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);
>  
> +
> +/**
> + * omap_hwmod_mux_wakeup - omap hwmod check given pad had wakeup event
> + * @hmux:		Pads for a hwmod
> + *
> + * Called only from omap_hwmod.c, do not use.
> + */
> +int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux);
>  #else
>  
> +static inline int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
> +{
> +	return 0;
> +}
> +
>  static inline int omap_mux_init_gpio(int gpio, int val)
>  {
>  	return 0;
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 9e89a58..893dae1 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2239,3 +2239,16 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
>  
>  	return ret;
>  }
> +
> +/**
> + * omap_hwmod_pad_wakeup_status - get pad wakeup status if mux is available.
> + * @oh: struct omap_hwmod *
> + *
> + * Returns the wake_up status bit of available pad mux pin.
> + */
> +int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh)

Again, naming here could improve readability.  Maybe:

bool omap_hmwod_get_io_wakeup_status(struct omap_hwmod *oh)

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Kevin Hilman]

"Govindraj.R" <govindraj.raja@ti.com> writes:

> For device pads which have OMAP_DEVICE_PAD_WAKEUP set (which means they
> are wakeup capable) enable the IO-daisy wakeup capability.
> During re-muxing avoid direct write with val as this can disturb if any
> mux done at bootloader level so read the pad then write back.
>
> Also add an API to fetch the wake-up status bit is set for given omap-hwmod
> device using available mux info which is added during omap_hwmod_mux_init.
> Wakeup status bit is needed for uart_resume_idle call from sram_idle path
> based on wake-up event has occurred for given uart we can enable clocks back.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>

Another issue with this patch...

This actually is a change in behavior from the previous code.  The
current code (albiet hacky) only enables IO-ring wakeups when
device_may_wakeup() == true.

This code should probably also do the same.

Kevin

> ---
>  arch/arm/mach-omap2/mux.c                    |   28 ++++++++++++++++++++++++++
>  arch/arm/mach-omap2/mux.h                    |   13 ++++++++++++
>  arch/arm/mach-omap2/omap_hwmod.c             |   13 ++++++++++++
>  arch/arm/plat-omap/include/plat/omap_hwmod.h |    1 +
>  4 files changed, 55 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
> index 98148b6..50edcea 100644
> --- a/arch/arm/mach-omap2/mux.c
> +++ b/arch/arm/mach-omap2/mux.c
> @@ -317,6 +317,29 @@ err1:
>  	return NULL;
>  }
>  
> +/* Gets the wakeup status of given pad from omap-hwmod */
> +int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
> +{
> +	int i;
> +	unsigned int val;
> +	u8 ret = 0;
> +
> +	for (i = 0; i < hmux->nr_pads; i++) {
> +		struct omap_device_pad *pad = &hmux->pads[i];
> +
> +		if (pad->flags & OMAP_DEVICE_PAD_WAKEUP) {
> +			val = omap_mux_read(pad->partition,
> +						pad->mux->reg_offset);
> +			if (val & OMAP_WAKEUP_EVENT) {
> +				ret = 1;
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  /* Assumes the calling function takes care of locking */
>  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>  {
> @@ -342,6 +365,9 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>  				break;
>  			flags &= ~OMAP_DEVICE_PAD_ENABLED;
>  			val = pad->idle;
> +			if (flags & OMAP_DEVICE_PAD_WAKEUP)
> +				val |= OMAP_WAKEUP_EN;
> +
>  			pr_debug("%s: Idling %s %x\n", __func__,
>  					pad->name, val);
>  			break;
> @@ -358,6 +384,8 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>  		};
>  
>  		if (val >= 0) {
> +			val |= omap_mux_read(pad->partition,
> +						pad->mux->reg_offset);
>  			omap_mux_write(pad->partition, val,
>  					pad->mux->reg_offset);
>  			pad->flags = flags;
> diff --git a/arch/arm/mach-omap2/mux.h b/arch/arm/mach-omap2/mux.h
> index a4ab17a..84a8fc2 100644
> --- a/arch/arm/mach-omap2/mux.h
> +++ b/arch/arm/mach-omap2/mux.h
> @@ -220,8 +220,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads, int nr_pads);
>   */
>  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);
>  
> +
> +/**
> + * omap_hwmod_mux_wakeup - omap hwmod check given pad had wakeup event
> + * @hmux:		Pads for a hwmod
> + *
> + * Called only from omap_hwmod.c, do not use.
> + */
> +int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux);
>  #else
>  
> +static inline int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
> +{
> +	return 0;
> +}
> +
>  static inline int omap_mux_init_gpio(int gpio, int val)
>  {
>  	return 0;
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 9e89a58..893dae1 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2239,3 +2239,16 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
>  
>  	return ret;
>  }
> +
> +/**
> + * omap_hwmod_pad_wakeup_status - get pad wakeup status if mux is available.
> + * @oh: struct omap_hwmod *
> + *
> + * Returns the wake_up status bit of available pad mux pin.
> + */
> +int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh)
> +{
> +	if (oh->mux)
> +		return omap_hwmod_mux_wakeup(oh->mux);
> +	return -EINVAL;
> +}
> diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> index fedd829..4100be0 100644
> --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
> +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> @@ -588,6 +588,7 @@ int omap_hwmod_for_each_by_class(const char *classname,
>  int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
>  u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
>  
> +int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh);
>  /*
>   * Chip variant-specific hwmod init routines - XXX should be converted
>   * to use initcalls once the initial boot ordering is straightened out

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Kevin Hilman]

Kevin Hilman <khilman@ti.com> writes:

> "Govindraj.R" <govindraj.raja@ti.com> writes:

[...]

>>  /* Assumes the calling function takes care of locking */
>>  void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>>  {
>> @@ -342,6 +365,9 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>>  				break;
>>  			flags &= ~OMAP_DEVICE_PAD_ENABLED;
>>  			val = pad->idle;
>> +			if (flags & OMAP_DEVICE_PAD_WAKEUP)
>> +				val |= OMAP_WAKEUP_EN;
>> +
>
> Is this needed on every idle transition?  
>
> You're currently setting it on every idle transition, but never clearing
> it.  If it is to be a one-time thing, then move it to the early init of
> the mux.

Just to clarify...

So as soon as the hwmod is first idled, IO-ring wakeups are enabled for
any pads that have OMAP_DEVICE_PAD_WAKEUP.

The problem here (compared with behavior of existing code) is that there 
is no provision for disabling IO-ring wakeups.

For example, with current code, you can disable wakeups from userspace
using the sysfs control file /sys/devices/.../power/wakeup, whose value
can be checked using device_may_wakeup(), like current
mach-omap2/serial.c code does.  

With this patch, IO-ring wakeups are always enabled, and never disabled.

Kevin

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Govindraj]

> On Sat, Mar 5, 2011 at 7:27 AM, Kevin Hilman <khilman@ti.com> wrote:
> Kevin Hilman <khilman@ti.com> writes:
>
>> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
> [...]
>
>>> ?/* Assumes the calling function takes care of locking */
>>> ?void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>>> ?{
>>> @@ -342,6 +365,9 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?break;
>>> ? ? ? ? ? ? ? ? ? ? ?flags &= ~OMAP_DEVICE_PAD_ENABLED;
>>> ? ? ? ? ? ? ? ? ? ? ?val = pad->idle;
>>> + ? ? ? ? ? ? ? ? ? ?if (flags & OMAP_DEVICE_PAD_WAKEUP)
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?val |= OMAP_WAKEUP_EN;
>>> +
>>
>> Is this needed on every idle transition?
>>
>> You're currently setting it on every idle transition, but never clearing
>> it. ?If it is to be a one-time thing, then move it to the early init of
>> the mux.
>
> Just to clarify...
>
> So as soon as the hwmod is first idled, IO-ring wakeups are enabled for
> any pads that have OMAP_DEVICE_PAD_WAKEUP.
>
> The problem here (compared with behavior of existing code) is that there
> is no provision for disabling IO-ring wakeups.
>
> For example, with current code, you can disable wakeups from userspace
> using the sysfs control file /sys/devices/.../power/wakeup, whose value
> can be checked using device_may_wakeup(), like current
> mach-omap2/serial.c code does.
>
> With this patch, IO-ring wakeups are always enabled, and never disabled.

I am reworking on this patch with new patch this will not have below two snip
changes and will be removed.

+                       if (flags & OMAP_DEVICE_PAD_WAKEUP)
+                               val |= OMAP_WAKEUP_EN;
+

and

+                       val |= omap_mux_read(pad->partition,
+                                               pad->mux->reg_offset);

some more changes I have added for default rx serial pad as below
[this will be done in patch 6/7]

+       {
+               .name   = "uart1_rx.uart1_rx",
+               .flags  = OMAP_DEVICE_PAD_REMUX | OMAP_DEVICE_PAD_WAKEUP,
+               .enable = OMAP_PIN_INPUT_PULLUP | OMAP_MUX_MODE0,
+               .idle   = OMAP_WAKEUP_EN,
+       },
+};


So with this change it will enable io-pad wakeup once we disable
clocks based on
.idle values and remux back to .enable values once we enable clocks back.
so io-pad wakeup for rx will be enabled only when I do put_sync.
[I verified this with lauterbach also]

I am not sure whether now we can control read/writes to pad_mux from any driver
interface and I think we have to go through mux framework for any
read/writes to mux.
with this I cant control pad wake up capabilities with sysfs as done
earlier in serial.c
now control to read/write to pad is outside scope of the driver
interface as omap_hwmod_mux
is the one that takes decision for driver with mux values provided
during omap_hwmod_mux_init.

--
Thanks,
Govindraj.R


>
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Govindraj]

On Sat, Mar 5, 2011 at 5:24 AM, Kevin Hilman <khilman@ti.com> wrote:
> Hi Govindraj,
>
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
>> For device pads which have OMAP_DEVICE_PAD_WAKEUP set (which means they
>> are wakeup capable) enable the IO-daisy wakeup capability.
>> During re-muxing avoid direct write with val as this can disturb if any
>> mux done at bootloader level so read the pad then write back.
>>
>> Also add an API to fetch the wake-up status bit is set for given omap-hwmod
>> device using available mux info which is added during omap_hwmod_mux_init.
>> Wakeup status bit is needed for uart_resume_idle call from sram_idle path
>> based on wake-up event has occurred for given uart we can enable clocks back.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
>> ---
>> ?arch/arm/mach-omap2/mux.c ? ? ? ? ? ? ? ? ? ?| ? 28 ++++++++++++++++++++++++++
>> ?arch/arm/mach-omap2/mux.h ? ? ? ? ? ? ? ? ? ?| ? 13 ++++++++++++
>> ?arch/arm/mach-omap2/omap_hwmod.c ? ? ? ? ? ? | ? 13 ++++++++++++
>> ?arch/arm/plat-omap/include/plat/omap_hwmod.h | ? ?1 +
>> ?4 files changed, 55 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
>> index 98148b6..50edcea 100644
>> --- a/arch/arm/mach-omap2/mux.c
>> +++ b/arch/arm/mach-omap2/mux.c
>> @@ -317,6 +317,29 @@ err1:
>> ? ? ? return NULL;
>> ?}
>>
>> +/* Gets the wakeup status of given pad from omap-hwmod */
>> +int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
>
> should return a bool
>
> Also, documentation should be clear that it returns true if *any* pad
> associated with this device has a wakeup event.
>
> Also a name reflecting the get/check/status nature of this function
> would help readability, maybe:
>
> bool omap_hwmod_mux_wakeup_event(struct omap_hwmod_mux_info *hmux)

Yes sure will modify.

>
>> +{
>> + ? ? int i;
>> + ? ? unsigned int val;
>> + ? ? u8 ret = 0;
>> +
>> + ? ? for (i = 0; i < hmux->nr_pads; i++) {
>> + ? ? ? ? ? ? struct omap_device_pad *pad = &hmux->pads[i];
>> +
>> + ? ? ? ? ? ? if (pad->flags & OMAP_DEVICE_PAD_WAKEUP) {
>> + ? ? ? ? ? ? ? ? ? ? val = omap_mux_read(pad->partition,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pad->mux->reg_offset);
>> + ? ? ? ? ? ? ? ? ? ? if (val & OMAP_WAKEUP_EVENT) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ret = 1;
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? }
>> + ? ? }
>> +
>> + ? ? return ret;
>> +}
>> +
>> ?/* Assumes the calling function takes care of locking */
>> ?void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>> ?{
>> @@ -342,6 +365,9 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? break;
>> ? ? ? ? ? ? ? ? ? ? ? flags &= ~OMAP_DEVICE_PAD_ENABLED;
>> ? ? ? ? ? ? ? ? ? ? ? val = pad->idle;
>> + ? ? ? ? ? ? ? ? ? ? if (flags & OMAP_DEVICE_PAD_WAKEUP)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? val |= OMAP_WAKEUP_EN;
>> +
>
> Is this needed on every idle transition?
>
> You're currently setting it on every idle transition, but never clearing
> it. ?If it is to be a one-time thing, then move it to the early init of
> the mux.

I will be removing this as said in earlier mail.


>
>> ? ? ? ? ? ? ? ? ? ? ? pr_debug("%s: Idling %s %x\n", __func__,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pad->name, val);
>> ? ? ? ? ? ? ? ? ? ? ? break;
>> @@ -358,6 +384,8 @@ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state)
>> ? ? ? ? ? ? ? };
>>
>> ? ? ? ? ? ? ? if (val >= 0) {
>> + ? ? ? ? ? ? ? ? ? ? val |= omap_mux_read(pad->partition,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pad->mux->reg_offset);
>
> I don't think this is doing what you expect, and will lead to very
> difficult to find bugs. ?Since your OR'ing in bits from the current
> value, you will never be able to clear any bits in this register.
>
> As just a dumb example, consider the enable case where
> pad->enable=MUX_MODE0 (0x0). ?If the bootloader has initialized this pad
> to MUX_MODE7 (0x7), this code will never set it to MUX_MODE0, since this
> read and OR of the existing value will always set it back to MUX_MODE7.
>
> At a minimum, this change should be a separate patch with a more
> detailed description.
>

Also this is change will be removed and default serial_pad
reg will have proper values for .enable and .idle fields using
which it will muxed accordingly
[snip of default rx serial reg given in previous mail.]

>
>> ? ? ? ? ? ? ? ? ? ? ? omap_mux_write(pad->partition, val,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pad->mux->reg_offset);
>> ? ? ? ? ? ? ? ? ? ? ? pad->flags = flags;
>> diff --git a/arch/arm/mach-omap2/mux.h b/arch/arm/mach-omap2/mux.h
>> index a4ab17a..84a8fc2 100644
>> --- a/arch/arm/mach-omap2/mux.h
>> +++ b/arch/arm/mach-omap2/mux.h
>> @@ -220,8 +220,21 @@ omap_hwmod_mux_init(struct omap_device_pad *bpads, int nr_pads);
>> ? */
>> ?void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state);
>>
>> +
>> +/**
>> + * omap_hwmod_mux_wakeup - omap hwmod check given pad had wakeup event
>> + * @hmux: ? ? ? ? ? ?Pads for a hwmod
>> + *
>> + * Called only from omap_hwmod.c, do not use.
>> + */
>> +int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux);
>> ?#else
>>
>> +static inline int omap_hwmod_mux_wakeup(struct omap_hwmod_mux_info *hmux)
>> +{
>> + ? ? return 0;
>> +}
>> +
>> ?static inline int omap_mux_init_gpio(int gpio, int val)
>> ?{
>> ? ? ? return 0;
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index 9e89a58..893dae1 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -2239,3 +2239,16 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
>>
>> ? ? ? return ret;
>> ?}
>> +
>> +/**
>> + * omap_hwmod_pad_wakeup_status - get pad wakeup status if mux is available.
>> + * @oh: struct omap_hwmod *
>> + *
>> + * Returns the wake_up status bit of available pad mux pin.
>> + */
>> +int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh)
>
> Again, naming here could improve readability. ?Maybe:
>
> bool omap_hmwod_get_io_wakeup_status(struct omap_hwmod *oh)
>
> From the name and the bool return, it should be clear that this is just
> checking for the wakeup event. ?I also think it should be clear that
> this is checking for IO-ring wakeups, not module wakeups.
>
> Also, in PATCH 5/7, you're calling this hwmod API directly from a
> driver. ?Drivers should not know anything about hwmods. ?Any
> OMAP-specific hooks must be called through pdata function pointers.
> Also, they should go through omap_device, which would then call
> omap_hwmod.
>

Agree omap-device level api to pad wakeup check is better.


> Thinking more about this (and how you use it in PATCH 5/7), what is
> needed is an omap_device level API to check if a wakeup occured for that
> device. ?That function would check for either module-level wakeup or IO
> pad wakeup. ?The driver should not need to care about how the wakeup
> occurred.
>

Module level wakeup doesn't seem to work and only
way it seem to wakeup is through using resume_idle
from sram_idle.

One simple way to reproduce the problem with
existing code base also is adding this patch which
will enable module level wakeup always.

https://patchwork.kernel.org/patch/501211/

and with below change.

govindraj at Linux-BSP-Server:~/clones/linux-omap-2.6$ gd
diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 47eef48..6343773 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -380,6 +380,7 @@ static void omap_uart_allow_sleep(struct
omap_uart_state *uart)
        omap_uart_smart_idle_enable(uart, 1);
        uart->can_sleep = 1;
        del_timer(&uart->timer);
+       omap_uart_disable_clocks(uart);
 }

We can see module level wakeup doesn't wakeup the uart.
once we set sleep_timeout for uart and timer starts.


>> +{
>> + ? ? if (oh->mux)
>> + ? ? ? ? ? ? return omap_hwmod_mux_wakeup(oh->mux);
>> + ? ? return -EINVAL;
>> +}
>> diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
>> index fedd829..4100be0 100644
>> --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
>> +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
>> @@ -588,6 +588,7 @@ int omap_hwmod_for_each_by_class(const char *classname,
>> ?int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
>> ?u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
>>
>> +int omap_hmwod_pad_wakeup_status(struct omap_hwmod *oh);
>> ?/*
>> ? * Chip variant-specific hwmod init routines - XXX should be converted
>> ? * to use initcalls once the initial boot ordering is straightened out
>
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Tony Lindgren]

* Govindraj <govindraj.ti@gmail.com> [110308 03:43]:
> 
> I am not sure whether now we can control read/writes to pad_mux from any driver
> interface and I think we have to go through mux framework for any
> read/writes to mux.

Sure, the drivers should not mess with those registers directly.

> with this I cant control pad wake up capabilities with sysfs as done
> earlier in serial.c
> now control to read/write to pad is outside scope of the driver
> interface as omap_hwmod_mux
> is the one that takes decision for driver with mux values provided
> during omap_hwmod_mux_init.

The sysfs interface to control this should still be from the drivers.
Sounds like we need some way to set the wakeup flags before pm_runtime_put
is called.

Regards,

Tony

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Govindraj <govindraj.ti@gmail.com> [110308 03:43]:
>> 
>> I am not sure whether now we can control read/writes to pad_mux from any driver
>> interface and I think we have to go through mux framework for any
>> read/writes to mux.
>
> Sure, the drivers should not mess with those registers directly.
>
>> with this I cant control pad wake up capabilities with sysfs as done
>> earlier in serial.c
>> now control to read/write to pad is outside scope of the driver
>> interface as omap_hwmod_mux
>> is the one that takes decision for driver with mux values provided
>> during omap_hwmod_mux_init.
>
> The sysfs interface to control this should still be from the drivers.
> Sounds like we need some way to set the wakeup flags before pm_runtime_put
> is called.

Exactly.  Driver's can currently check device_may_wakeup(dev) to
determine if they *should* set wakeup flags, but currently, we don't
have a clean way to pass the info down to core code.

One possible option (albeit hacky) is for the omap_hwmod_mux code to
check device_may_wakeup() itself, and set the bits accordingly.  This
means drivers don't have to do anything, but drivers also loose control
of enabling/disabling wakeups.

I think what we need are two omap_device-level APIs for wakeups.

One for enable/disable of wakeups (both module-level and IO-ring, the 
driver should not care about the difference between the two):

int omap_device_[enable|disable](struct platform_device *pdev);

bool omap_device_wakeup_occured(struct platform_device *pdev);

As OMAP-specific APIs, both of these should of course be called through
pdata function pointers.

Kevin

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Paul Walmsley]

Hi

On Tue, 8 Mar 2011, Kevin Hilman wrote:

> Tony Lindgren <tony@atomide.com> writes:
> 
> > * Govindraj <govindraj.ti@gmail.com> [110308 03:43]:
> >> 
> >> I am not sure whether now we can control read/writes to pad_mux from any driver
> >> interface and I think we have to go through mux framework for any
> >> read/writes to mux.
> >
> > Sure, the drivers should not mess with those registers directly.
> >
> >> with this I cant control pad wake up capabilities with sysfs as done
> >> earlier in serial.c
> >> now control to read/write to pad is outside scope of the driver
> >> interface as omap_hwmod_mux
> >> is the one that takes decision for driver with mux values provided
> >> during omap_hwmod_mux_init.
> >
> > The sysfs interface to control this should still be from the drivers.
> > Sounds like we need some way to set the wakeup flags before pm_runtime_put
> > is called.
> 
> Exactly.  Driver's can currently check device_may_wakeup(dev) to
> determine if they *should* set wakeup flags, but currently, we don't
> have a clean way to pass the info down to core code.
> 
> One possible option (albeit hacky) is for the omap_hwmod_mux code to
> check device_may_wakeup() itself, and set the bits accordingly.  This
> means drivers don't have to do anything, but drivers also loose control
> of enabling/disabling wakeups.
> 
> I think what we need are two omap_device-level APIs for wakeups.
> 
> One for enable/disable of wakeups (both module-level and IO-ring, the 
> driver should not care about the difference between the two):

Probably this should only control IO-ring/chain wakeups.  Enabling 
asynchronous wakeups will need to be handled differently...

> int omap_device_[enable|disable](struct platform_device *pdev);
> 
> bool omap_device_wakeup_occured(struct platform_device *pdev);
> 
> As OMAP-specific APIs, both of these should of course be called through
> pdata function pointers.

Looks good to me, modulo the obvious corrections, although the names of 
these functions should probably say "ioring_wakeup" rather than just 
"wakeup" to make it clear what exactly they are intended to do.

As we discussed in chat, we can drop the existing 
omap_hwmod_{enable,disable}_wakeup() functions, since these were 
effectively obsoleted by commit 9980ce53 ("OMAP: hwmod: Enable module 
wakeup if in smartidle").


- Paul

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Paul Walmsley]

By the way, if your patch relies on OMAP_DEVICE_PAD_IDLE, you should 
probably sync up with Sricharan, who is apparently planning to remove that 
flag:

   http://www.mail-archive.com/linux-omap at vger.kernel.org/msg46296.html


- Paul

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Kevin Hilman]

Govindraj <govindraj.ti@gmail.com> writes:

> Module level wakeup doesn't seem to work and only
> way it seem to wakeup is through using resume_idle
> from sram_idle.

Have you verified if the module wakeups themselves are not working, or
just that the wakeup is not propagating to a device IRQ?

As a quick test, I did a suspend-only test shows that module-level
wakeups (at least from PER UART) are working.

On 3530/Beagle, which has console on UART3 (in PER), I successfully
tested module level wakeups from suspend.

First, I added a simple debug printk[1] to show the wakeup source.
Then, I ensure CORE stays 'ON' during suspend.  If CORE stays on, the IO
ring is never armed, so only module wakeups can happen:

   # echo 3 > /debug/pm_debug/core_pwrdm/suspend 

then I suspend

   # echo mem > /sys/power/state 

and type a key on the console to wakeup, and I see:

   PRCM IRQ: mod 0x800: WKST=0x00000800

Which shows PER wakeup from UART3.  If you do the same without setting
CORE to ON, you'll see that the first wakeup is in the WKUP powerdomain
from the IO ring.

So module-level wakeups are indeed working.

The problem is actually not that module wakeups are working, but rather
that the module does not generate an interrupt upon wakeup if it is
idle.  The resume_from_idle hack works because as soon as clocks are
enabled, the module generates the interrupt (special thanks to Paul
Walmsley for helping me understand this.)

>
> One simple way to reproduce the problem with
> existing code base also is adding this patch which
> will enable module level wakeup always.
>
> https://patchwork.kernel.org/patch/501211/
>
> and with below change.
>
> govindraj at Linux-BSP-Server:~/clones/linux-omap-2.6$ gd
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 47eef48..6343773 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -380,6 +380,7 @@ static void omap_uart_allow_sleep(struct
> omap_uart_state *uart)
>         omap_uart_smart_idle_enable(uart, 1);
>         uart->can_sleep = 1;
>         del_timer(&uart->timer);
> +       omap_uart_disable_clocks(uart);
>  }
>
> We can see module level wakeup doesn't wakeup the uart.
> once we set sleep_timeout for uart and timer starts.

The above change alone can never work.  I'm pretty sure you wouldn't see
IO-ring wakeups will not happen with this change either because the
kernel will probably hang.

If you disable the clocks here, then any console usage between the
timeout and the idle path will hang the kernel.  You can't do this
without taking the console lock.  Doing that will be much easier in your
new driver.

Kevin


[1]
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 6e2bd38..677a235 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -219,6 +219,8 @@ static int prcm_clear_mod_irqs(s16 module, u8 regs)
 		iclk = omap2_cm_read_mod_reg(module, iclk_off);
 		fclk = omap2_cm_read_mod_reg(module, fclk_off);
 		while (wkst) {
+			printk("PRCM IRQ: mod 0x%x: WKST=0x%08x\n",
+			       module, wkst);
 			clken = wkst;
 			omap2_cm_set_mod_reg_bits(clken, module, iclk_off);
 			/*

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Govindraj]

On Wed, Mar 9, 2011 at 12:51 AM, Paul Walmsley <paul@pwsan.com> wrote:
>
> By the way, if your patch relies on OMAP_DEVICE_PAD_IDLE, you should
> probably sync up with Sricharan, who is apparently planning to remove that
> flag:
>

currently I am dependent on OMAP_DEVICE_PAD_WAKEUP flag.
I am not using OMAP_DEVICE_PAD_IDLE as of now.

--
Thanks,
Govindraj.R


> ? http://www.mail-archive.com/linux-omap at vger.kernel.org/msg46296.html
>
>
> - Paul
>

[PATCH 2/7] OMAP2+: mux: Enable wakeup for wakeup enable requested pads

[Govindraj]

On Wed, Mar 9, 2011 at 6:44 AM, Kevin Hilman <khilman@ti.com> wrote:
> Govindraj <govindraj.ti@gmail.com> writes:
>
>> Module level wakeup doesn't seem to work and only
>> way it seem to wakeup is through using resume_idle
>> from sram_idle.
>
> Have you verified if the module wakeups themselves are not working, or
> just that the wakeup is not propagating to a device IRQ?
>

Actually the wakeup happens but control never moves to uart isr
Gets looped for ever in clearing wakeup status in prcm_irq_handler
path.

--> omap2_cm_rmw_mod_reg_bits
     ---> prcm_clear_mod_irqs
          ---> prcm_interrupt_handler

it gets looped here forever.


> As a quick test, I did a suspend-only test shows that module-level
> wakeups (at least from PER UART) are working.
>
> On 3530/Beagle, which has console on UART3 (in PER), I successfully
> tested module level wakeups from suspend.
>
> First, I added a simple debug printk[1] to show the wakeup source.
> Then, I ensure CORE stays 'ON' during suspend. ?If CORE stays on, the IO
> ring is never armed, so only module wakeups can happen:
>
> ? # echo 3 > /debug/pm_debug/core_pwrdm/suspend
>
> then I suspend
>
> ? # echo mem > /sys/power/state
>
> and type a key on the console to wakeup, and I see:
>
> ? PRCM IRQ: mod 0x800: WKST=0x00000800

Yes Correct, here seems to work.


>
> Which shows PER wakeup from UART3. ?If you do the same without setting
> CORE to ON, you'll see that the first wakeup is in the WKUP powerdomain
> from the IO ring.
>
> So module-level wakeups are indeed working.
>
> The problem is actually not that module wakeups are working, but rather
> that the module does not generate an interrupt upon wakeup if it is
> idle. ?The resume_from_idle hack works because as soon as clocks are
> enabled, the module generates the interrupt (special thanks to Paul
> Walmsley for helping me understand this.)
>
>>
>> One simple way to reproduce the problem with
>> existing code base also is adding this patch which
>> will enable module level wakeup always.
>>
>> https://patchwork.kernel.org/patch/501211/
>>
>> and with below change.
>>
>> govindraj at Linux-BSP-Server:~/clones/linux-omap-2.6$ gd
>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>> index 47eef48..6343773 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -380,6 +380,7 @@ static void omap_uart_allow_sleep(struct
>> omap_uart_state *uart)
>> ? ? ? ? omap_uart_smart_idle_enable(uart, 1);
>> ? ? ? ? uart->can_sleep = 1;
>> ? ? ? ? del_timer(&uart->timer);
>> + ? ? ? omap_uart_disable_clocks(uart);
>> ?}
>>
>> We can see module level wakeup doesn't wakeup the uart.
>> once we set sleep_timeout for uart and timer starts.
>
> The above change alone can never work. ?I'm pretty sure you wouldn't see
> IO-ring wakeups will not happen with this change either because the
> kernel will probably hang.
>
> If you disable the clocks here, then any console usage between the
> timeout and the idle path will hang the kernel. ?You can't do this
> without taking the console lock. ?Doing that will be much easier in your
> new driver.

sorry forgot to specify here, to avoid above scenario I am suppressing
complete printk's using sysfs interface.
echo 0 > /proc/sys/kernel/printk
so by applying this patch and booting up and then suppressing printks
then setting uart timeout, after timeout hitting any key.
Similar behavior of always in loop to clear wakeup status.

--
Thanks,
Govindraj.R


>
> Kevin
>
>
> [1]
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 6e2bd38..677a235 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -219,6 +219,8 @@ static int prcm_clear_mod_irqs(s16 module, u8 regs)
> ? ? ? ? ? ? ? ?iclk = omap2_cm_read_mod_reg(module, iclk_off);
> ? ? ? ? ? ? ? ?fclk = omap2_cm_read_mod_reg(module, fclk_off);
> ? ? ? ? ? ? ? ?while (wkst) {
> + ? ? ? ? ? ? ? ? ? ? ? printk("PRCM IRQ: mod 0x%x: WKST=0x%08x\n",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?module, wkst);
> ? ? ? ? ? ? ? ? ? ? ? ?clken = wkst;
> ? ? ? ? ? ? ? ? ? ? ? ?omap2_cm_set_mod_reg_bits(clken, module, iclk_off);
> ? ? ? ? ? ? ? ? ? ? ? ?/*
>
>