Skip to contentSkip to conversation

OMAP2+: mux: Enable wakeup for wakeup enable requested pads

PATCH New Govindraj.R 5 participants 13 messages Mar 2, 2011 – Mar 9, 2011

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

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

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.

---
 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(-)
arch/arm/mach-omap2/mux.c +28 -0
index 98148b6..50edcea 100644
@@ -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;
arch/arm/mach-omap2/mux.h +13 -0
index a4ab17a..84a8fc2 100644
@@ -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;
arch/arm/mach-omap2/omap_hwmod.c +13 -0
index 9e89a58..893dae1 100644
@@ -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;
+}
arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0
index fedd829..4100be0 100644
@@ -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
Signature · 1 line
1.7.1
#1 lore raw

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

Hi Govindraj,

Quoted 24 lines · 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.

---
 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(-)
arch/arm/mach-omap2/mux.c +0 -0
index 98148b6..50edcea 100644

arch/arm/mach-omap2/mux.c err1: · line 321

Quoted 5 lines
arch/arm/mach-omap2/mux.c +2 -0
@@ -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)

Quoted 25 lines
arch/arm/mach-omap2/mux.c +21 -0
+{
+	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)
 {

arch/arm/mach-omap2/mux.c void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) · line 370

Quoted 6 lines
arch/arm/mach-omap2/mux.c +3 -0
@@ -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.

arch/arm/mach-omap2/mux.c void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) · line 388

Quoted 9 lines
arch/arm/mach-omap2/mux.c +2 -0
 			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.

Quoted 35 lines
arch/arm/mach-omap2/mux.c +0 -0
 			omap_mux_write(pad->partition, val,
 					pad->mux->reg_offset);
 			pad->flags = flags;
arch/arm/mach-omap2/mux.h +13 -0
index a4ab17a..84a8fc2 100644
@@ -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;
arch/arm/mach-omap2/omap_hwmod.c +0 -0
index 9e89a58..893dae1 100644
@@ -2239,3 +2239,16 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
 

arch/arm/mach-omap2/omap_hwmod.c u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh) · line 2249

Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod.c +8 -0
 	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)

#2 lore raw

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

Quoted 1 line · Kevin Hilman

"Govindraj.R" <[email protected]> writes:

[...]

Quoted 13 lines

arch/arm/mach-omap2/mux.c void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) · line 370

Quoted 10 lines

/* Assumes the calling function takes care of locking */ void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) {

arch/arm/mach-omap2/mux.c +3 -0
@@ -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?

Quoted 3 lines

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

#3 lore raw

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

Quoted 32 lines · Kevin Hilman
Quoted 1 line · Kevin Hilman

"Govindraj.R" <[email protected]> writes:

[...]

Quoted 13 lines

Review void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) · line 370

Quoted 10 lines

?/* Assumes the calling function takes care of locking */ ?void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) ?{

(quoted diff) +3 -0
@@ -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?

Quoted 3 lines

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.

Quoted 6 lines

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

Quoted 7 lines

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

maintainer
#4 lore raw

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

Quoted 4 lines · Govindraj

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.

Quoted 6 lines

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

#5 lore raw

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

Quoted 15 lines · Tony Lindgren
Quoted 4 lines · Govindraj

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.

Quoted 6 lines

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.

Quoted 3 lines

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

maintainer
#6 lore raw

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

Hi

Quoted 30 lines · Kevin Hilman
Quoted 15 lines · Tony Lindgren
Quoted 4 lines · Govindraj

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.

Quoted 6 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

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...

Quoted 6 lines

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

#8 lore raw

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

Quoted 5 lines · 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:

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

--
Thanks,
Govindraj.R

Quoted 5 lines

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

- Paul

#9 lore raw

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

In reply to Kevin Hilman
Quoted 36 lines · Kevin Hilman

Hi Govindraj,

Quoted 24 lines · 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.

---

?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(-)

arch/arm/mach-omap2/mux.c +0 -0
index 98148b6..50edcea 100644

arch/arm/mach-omap2/mux.c err1: · line 321

Quoted 5 lines
arch/arm/mach-omap2/mux.c +2 -0
@@ -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

Quoted 7 lines

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.

Quoted 35 lines
Quoted 26 lines
arch/arm/mach-omap2/mux.c +21 -0
+{
+ ? ? 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) ?{

arch/arm/mach-omap2/mux.c void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) · line 370

Quoted 6 lines
arch/arm/mach-omap2/mux.c +3 -0
@@ -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?

Quoted 3 lines

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.

Quoted 15 lines

arch/arm/mach-omap2/mux.c void omap_hwmod_mux(struct omap_hwmod_mux_info *hmux, u8 state) · line 388

Quoted 10 lines
arch/arm/mach-omap2/mux.c +2 -0
 ? ? ? ? ? ? ? ? ? ? ? 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.

Quoted 8 lines

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.]

Quoted 55 lines
Quoted 36 lines
arch/arm/mach-omap2/mux.c +0 -0
 ? ? ? ? ? ? ? ? ? ? ? omap_mux_write(pad->partition, val,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pad->mux->reg_offset);
 ? ? ? ? ? ? ? ? ? ? ? pad->flags = flags;
arch/arm/mach-omap2/mux.h +0 -0
index a4ab17a..84a8fc2 100644
@@ -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);

arch/arm/mach-omap2/mux.h +8 -0
+
+/**
+ * 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

arch/arm/mach-omap2/mux.h +5 -0
+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;

arch/arm/mach-omap2/omap_hwmod.c +0 -0
index 9e89a58..893dae1 100644
@@ -2239,3 +2239,16 @@ u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
 

arch/arm/mach-omap2/omap_hwmod.c u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh) · line 2249

Quoted 10 lines
arch/arm/mach-omap2/omap_hwmod.c +8 -0
 ? ? ? 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.

Quoted 6 lines

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.

Quoted 6 lines

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

arch/arm/mach-omap2/serial.c +0 -0
index 47eef48..6343773 100644
@@ -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.

Quoted 24 lines
Quoted 14 lines
arch/arm/mach-omap2/omap_hwmod.c +5 -0
+{
+ ? ? if (oh->mux)
+ ? ? ? ? ? ? return omap_hwmod_mux_wakeup(oh->mux);
+ ? ? return -EINVAL;
+}
arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0
index fedd829..4100be0 100644
@@ -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);
Quoted 3 lines

?/*
? * 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

#10 lore raw

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

Quoted 3 lines · Govindraj

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.)

Quoted 22 lines

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

arch/arm/mach-omap2/serial.c +1 -0
index 47eef48..6343773 100644
@@ -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);
 }
 
Quoted 2 lines

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]

arch/arm/mach-omap2/pm34xx.c +2 -0
index 6e2bd38..677a235 100644
@@ -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);
 			/*
#11 lore raw

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

Quoted 9 lines · Kevin Hilman
Quoted 3 lines · Govindraj

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.

Quoted 12 lines

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

Quoted 7 lines

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.

Quoted 42 lines

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.)

Quoted 22 lines

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

arch/arm/mach-omap2/serial.c +1 -0
index 47eef48..6343773 100644
@@ -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);
 ?}
 
Quoted 2 lines

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.

Quoted 4 lines

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

Quoted 20 lines

Kevin

[1]

arch/arm/mach-omap2/pm34xx.c +2 -0
index 6e2bd38..677a235 100644
@@ -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);

? ? ? ? ? ? ? ? ? ? ? ?/*

#12 lore raw

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

In reply to Govindraj.R
Quoted 5 lines · 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.

Quoted 7 lines

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.

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

Quoted 119 lines
---
 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(-)
arch/arm/mach-omap2/mux.c +28 -0
index 98148b6..50edcea 100644
@@ -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;
arch/arm/mach-omap2/mux.h +13 -0
index a4ab17a..84a8fc2 100644
@@ -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;
arch/arm/mach-omap2/omap_hwmod.c +13 -0
index 9e89a58..893dae1 100644
@@ -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;
+}
arch/arm/plat-omap/include/plat/omap_hwmod.h +1 -0
index fedd829..4100be0 100644
@@ -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