* [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
@ 2013-07-17 11:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tony, Kevin
This patch series introduces dynamic pinctrl handling in OMAP device framework
in the same way as it was before switching to DT.
This allow OMAP devices driver's developers to simply add dynamic pinctrl
handling for "default", "active", "idle", "sleep" PIN states in their drivers
by modifying DT definitions only - no modifications in drivers code are not needed.
Tested on OMAP4 SDP(Tablet2) board using omap4-sdp.dts
- UART3/4 autoidle
- suspend, by adding some fake "sleep" pin state
- wake up (PRCM) IRQ generation by ading call to omap44xx_prm_reconfigure_io_chain()
right after each call to pinctrl_pm_select_xx() API.
UART3(console) reg dumps:
- active
0x4A100140(RTS/CTS) - 0x00000118
0x4A100140(TX/RX) - 0x00000100
- idle
0x4A100140(RTS/CTS) - 0x00000118
0x4A100144(TX/RX) - 0x00004100
UART4 reg dumps:
- active
0x4A10015C(TX/RX) - 0x00000100
- idle
0x4A10015C(TX/RX) - 0x00004100
Debugfs:
# cat sys/kernel/debug/pinctrl/4a100040.pinmux/pinmux-pins
[snip]
pin 128 (4a100140.0): 48020000.serial (GPIO UNCLAIMED) function pinmux_uart3_pins group pinmux_uart3_pins
pin 129 (4a100142.0): 48020000.serial (GPIO UNCLAIMED) function pinmux_uart3_pins group pinmux_uart3_pins
pin 130 (4a100144.0): 48020000.serial (GPIO UNCLAIMED) function pinmux_uart3_pins_active group pinmux_uart3_pins_active
pin 131 (4a100146.0): 48020000.serial (GPIO UNCLAIMED) function pinmux_uart3_pins group pinmux_uart3_pins
[snip]
pin 142 (4a10015c.0): 4806e000.serial (GPIO UNCLAIMED) function pinmux_uart4_pins_idle group pinmux_uart4_pins_idle
pin 143 (4a10015e.0): 4806e000.serial (GPIO UNCLAIMED) function pinmux_uart4_pins group pinmux_uart4_pins
Changes since RFC:
- _od_resume_noirq() fixed - "idle" state was not selected on resume
- description updated
RFC: https://lkml.org/lkml/2013/6/21/309
Based on v3.11-rc1 plus patch series from Tony Lindgren
"[PATCH 0/4] improved support for runtime muxing for pinctrl"
http://www.spinics.net/lists/arm-kernel/msg258827.html:
fd7937b drivers: Add pinctrl handling for dynamic pin states
479246b pinctrl: Add support for additional dynamic states
c9f37e8 pinctrl: Allow pinctrl to have multiple active states
756f10b pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
ad81f05 Linux 3.11-rc1
Related discussions:
- drivers: pinctrl sleep and idle states in the core
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html
- [PATCH] drivers: pinctrl: add active state to core
http://marc.info/?l=linux-kernel&m=137094012703340&w=2
- [PATCH] pinctrl: document the pinctrl PM states
https://lkml.org/lkml/2013/6/11/501
- [3/3] i2c: nomadik: use pinctrl PM helpers
https://patchwork.kernel.org/patch/2670291/
- mmc: omap_hsmmc: Remux pins to support SDIO interrupt and PM runtime
https://patchwork.kernel.org/patch/2690191/
- [PATCH 00/11] drivers: Add Pinctrl PM support
https://lkml.org/lkml/2013/5/31/210
Grygorii Strashko (3):
pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
ARM: OMAP2+: omap_device: add pinctrl handling
ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
arch/arm/boot/dts/omap4-sdp.dts | 34 +++++++++++++++++++++++++++----
arch/arm/mach-omap2/omap_device.c | 40 +++++++++++++++++++++++++++++++------
drivers/pinctrl/core.c | 12 +++++++++++
3 files changed, 76 insertions(+), 10 deletions(-)
Regards,
-grygorii
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Stephen Warren <swarren@wwwdotorg.org>
CC: Roger Quadros <rogerq@ti.com>
--
1.7.9.5
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
@ 2013-07-17 11:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: Tony Lindgren, Kevin Hilman
Cc: Grygorii Strashko, Stephen Warren, devicetree-discuss,
linux-kernel, linux-omap, Linus Walleij, linux-arm-kernel,
Roger Quadros
Hi Tony, Kevin
This patch series introduces dynamic pinctrl handling in OMAP device framework
in the same way as it was before switching to DT.
This allow OMAP devices driver's developers to simply add dynamic pinctrl
handling for "default", "active", "idle", "sleep" PIN states in their drivers
by modifying DT definitions only - no modifications in drivers code are not needed.
Tested on OMAP4 SDP(Tablet2) board using omap4-sdp.dts
- UART3/4 autoidle
- suspend, by adding some fake "sleep" pin state
- wake up (PRCM) IRQ generation by ading call to omap44xx_prm_reconfigure_io_chain()
right after each call to pinctrl_pm_select_xx() API.
UART3(console) reg dumps:
- active
0x4A100140(RTS/CTS) - 0x00000118
0x4A100140(TX/RX) - 0x00000100
- idle
0x4A100140(RTS/CTS) - 0x00000118
0x4A100144(TX/RX) - 0x00004100
UART4 reg dumps:
- active
0x4A10015C(TX/RX) - 0x00000100
- idle
0x4A10015C(TX/RX) - 0x00004100
Debugfs:
# cat sys/kernel/debug/pinctrl/4a100040.pinmux/pinmux-pins
[snip]
pin 128 (4a100140.0): 48020000.serial (GPIO UNCLAIMED) function pinmux_uart3_pins group pinmux_uart3_pins
pin 129 (4a100142.0): 48020000.serial (GPIO UNCLAIMED) function pinmux_uart3_pins group pinmux_uart3_pins
pin 130 (4a100144.0): 48020000.serial (GPIO UNCLAIMED) function pinmux_uart3_pins_active group pinmux_uart3_pins_active
pin 131 (4a100146.0): 48020000.serial (GPIO UNCLAIMED) function pinmux_uart3_pins group pinmux_uart3_pins
[snip]
pin 142 (4a10015c.0): 4806e000.serial (GPIO UNCLAIMED) function pinmux_uart4_pins_idle group pinmux_uart4_pins_idle
pin 143 (4a10015e.0): 4806e000.serial (GPIO UNCLAIMED) function pinmux_uart4_pins group pinmux_uart4_pins
Changes since RFC:
- _od_resume_noirq() fixed - "idle" state was not selected on resume
- description updated
RFC: https://lkml.org/lkml/2013/6/21/309
Based on v3.11-rc1 plus patch series from Tony Lindgren
"[PATCH 0/4] improved support for runtime muxing for pinctrl"
http://www.spinics.net/lists/arm-kernel/msg258827.html:
fd7937b drivers: Add pinctrl handling for dynamic pin states
479246b pinctrl: Add support for additional dynamic states
c9f37e8 pinctrl: Allow pinctrl to have multiple active states
756f10b pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
ad81f05 Linux 3.11-rc1
Related discussions:
- drivers: pinctrl sleep and idle states in the core
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html
- [PATCH] drivers: pinctrl: add active state to core
http://marc.info/?l=linux-kernel&m=137094012703340&w=2
- [PATCH] pinctrl: document the pinctrl PM states
https://lkml.org/lkml/2013/6/11/501
- [3/3] i2c: nomadik: use pinctrl PM helpers
https://patchwork.kernel.org/patch/2670291/
- mmc: omap_hsmmc: Remux pins to support SDIO interrupt and PM runtime
https://patchwork.kernel.org/patch/2690191/
- [PATCH 00/11] drivers: Add Pinctrl PM support
https://lkml.org/lkml/2013/5/31/210
Grygorii Strashko (3):
pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
ARM: OMAP2+: omap_device: add pinctrl handling
ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
arch/arm/boot/dts/omap4-sdp.dts | 34 +++++++++++++++++++++++++++----
arch/arm/mach-omap2/omap_device.c | 40 +++++++++++++++++++++++++++++++------
drivers/pinctrl/core.c | 12 +++++++++++
3 files changed, 76 insertions(+), 10 deletions(-)
Regards,
-grygorii
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Stephen Warren <swarren@wwwdotorg.org>
CC: Roger Quadros <rogerq@ti.com>
--
1.7.9.5
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
2013-07-17 11:41 ` Grygorii Strashko
(?)
@ 2013-07-17 11:41 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: Tony Lindgren, Kevin Hilman
Cc: linux-arm-kernel, linux-omap, linux-kernel, devicetree-discuss,
Grygorii Strashko, Linus Walleij, Stephen Warren
The pinctrl support in Device core assumed to be optional - so, It's
valid case, when there are no definition for default device's pinctrl
states in DT at all ("default", "active", "idle", "sleep").
And in this case dev->pins == NULL and pinctrl_pm_select*() API
should return 0 always.
Now the checks for !dev->pins have been removed from
pinctrl_pm_select*() API mistakenly by the patch
pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
http://www.spinics.net/lists/arm-kernel/msg258829.html
Hence, rollback these checks in in pinctrl_pm_select*() APIs.
CC: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
Seems this one can be squashed in series
[PATCH 0/4] improved support for runtime muxing for pinctrl"
http://www.spinics.net/lists/arm-kernel/msg258827.html
drivers/pinctrl/core.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 4f58a97..ace4eb8 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1390,6 +1390,9 @@ static int pinctrl_pm_select_state(struct device *dev, struct pinctrl_state *sta
*/
int pinctrl_pm_select_default_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->default_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
@@ -1400,6 +1403,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
*/
int pinctrl_pm_select_active_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->active_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_active_state);
@@ -1410,6 +1416,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_active_state);
*/
int pinctrl_pm_select_sleep_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->sleep_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
@@ -1420,6 +1429,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
*/
int pinctrl_pm_select_idle_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->idle_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_idle_state);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
@ 2013-07-17 11:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: linux-arm-kernel
The pinctrl support in Device core assumed to be optional - so, It's
valid case, when there are no definition for default device's pinctrl
states in DT at all ("default", "active", "idle", "sleep").
And in this case dev->pins == NULL and pinctrl_pm_select*() API
should return 0 always.
Now the checks for !dev->pins have been removed from
pinctrl_pm_select*() API mistakenly by the patch
pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
http://www.spinics.net/lists/arm-kernel/msg258829.html
Hence, rollback these checks in in pinctrl_pm_select*() APIs.
CC: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
Seems this one can be squashed in series
[PATCH 0/4] improved support for runtime muxing for pinctrl"
http://www.spinics.net/lists/arm-kernel/msg258827.html
drivers/pinctrl/core.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 4f58a97..ace4eb8 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1390,6 +1390,9 @@ static int pinctrl_pm_select_state(struct device *dev, struct pinctrl_state *sta
*/
int pinctrl_pm_select_default_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->default_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
@@ -1400,6 +1403,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
*/
int pinctrl_pm_select_active_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->active_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_active_state);
@@ -1410,6 +1416,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_active_state);
*/
int pinctrl_pm_select_sleep_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->sleep_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
@@ -1420,6 +1429,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
*/
int pinctrl_pm_select_idle_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->idle_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_idle_state);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
@ 2013-07-17 11:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: Tony Lindgren, Kevin Hilman
Cc: linux-arm-kernel, linux-omap, linux-kernel, devicetree-discuss,
Grygorii Strashko, Linus Walleij, Stephen Warren
The pinctrl support in Device core assumed to be optional - so, It's
valid case, when there are no definition for default device's pinctrl
states in DT at all ("default", "active", "idle", "sleep").
And in this case dev->pins == NULL and pinctrl_pm_select*() API
should return 0 always.
Now the checks for !dev->pins have been removed from
pinctrl_pm_select*() API mistakenly by the patch
pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
http://www.spinics.net/lists/arm-kernel/msg258829.html
Hence, rollback these checks in in pinctrl_pm_select*() APIs.
CC: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
Seems this one can be squashed in series
[PATCH 0/4] improved support for runtime muxing for pinctrl"
http://www.spinics.net/lists/arm-kernel/msg258827.html
drivers/pinctrl/core.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 4f58a97..ace4eb8 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1390,6 +1390,9 @@ static int pinctrl_pm_select_state(struct device *dev, struct pinctrl_state *sta
*/
int pinctrl_pm_select_default_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->default_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
@@ -1400,6 +1403,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
*/
int pinctrl_pm_select_active_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->active_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_active_state);
@@ -1410,6 +1416,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_active_state);
*/
int pinctrl_pm_select_sleep_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->sleep_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
@@ -1420,6 +1429,9 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
*/
int pinctrl_pm_select_idle_state(struct device *dev)
{
+ if (!dev->pins)
+ return 0;
+
return pinctrl_pm_select_state(dev, dev->pins->idle_state);
}
EXPORT_SYMBOL_GPL(pinctrl_pm_select_idle_state);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
2013-07-17 11:41 ` Grygorii Strashko
@ 2013-07-17 15:40 ` Tony Lindgren
-1 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-17 15:40 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Linus Walleij, Stephen Warren
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> The pinctrl support in Device core assumed to be optional - so, It's
> valid case, when there are no definition for default device's pinctrl
> states in DT at all ("default", "active", "idle", "sleep").
> And in this case dev->pins == NULL and pinctrl_pm_select*() API
> should return 0 always.
>
> Now the checks for !dev->pins have been removed from
> pinctrl_pm_select*() API mistakenly by the patch
> pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
> http://www.spinics.net/lists/arm-kernel/msg258829.html
>
> Hence, rollback these checks in in pinctrl_pm_select*() APIs.
Thanks, it's best that I fold this fix into my patch as it has not
been committed yet.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread* [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
@ 2013-07-17 15:40 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-17 15:40 UTC (permalink / raw)
To: linux-arm-kernel
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> The pinctrl support in Device core assumed to be optional - so, It's
> valid case, when there are no definition for default device's pinctrl
> states in DT at all ("default", "active", "idle", "sleep").
> And in this case dev->pins == NULL and pinctrl_pm_select*() API
> should return 0 always.
>
> Now the checks for !dev->pins have been removed from
> pinctrl_pm_select*() API mistakenly by the patch
> pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
> http://www.spinics.net/lists/arm-kernel/msg258829.html
>
> Hence, rollback these checks in in pinctrl_pm_select*() APIs.
Thanks, it's best that I fold this fix into my patch as it has not
been committed yet.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
2013-07-17 15:40 ` Tony Lindgren
(?)
@ 2013-07-26 23:22 ` Linus Walleij
-1 siblings, 0 replies; 53+ messages in thread
From: Linus Walleij @ 2013-07-26 23:22 UTC (permalink / raw)
To: Tony Lindgren
Cc: Grygorii Strashko, Kevin Hilman, linux-arm-kernel, Linux-OMAP,
linux-kernel, devicetree-discuss, Stephen Warren
On Wed, Jul 17, 2013 at 5:40 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> The pinctrl support in Device core assumed to be optional - so, It's
>> valid case, when there are no definition for default device's pinctrl
>> states in DT at all ("default", "active", "idle", "sleep").
>> And in this case dev->pins == NULL and pinctrl_pm_select*() API
>> should return 0 always.
>>
>> Now the checks for !dev->pins have been removed from
>> pinctrl_pm_select*() API mistakenly by the patch
>> pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
>> http://www.spinics.net/lists/arm-kernel/msg258829.html
>>
>> Hence, rollback these checks in in pinctrl_pm_select*() APIs.
>
> Thanks, it's best that I fold this fix into my patch as it has not
> been committed yet.
I think I've applied the correct v2 version,
please check that linux-next is in good shape...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 53+ messages in thread* [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
@ 2013-07-26 23:22 ` Linus Walleij
0 siblings, 0 replies; 53+ messages in thread
From: Linus Walleij @ 2013-07-26 23:22 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 17, 2013 at 5:40 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> The pinctrl support in Device core assumed to be optional - so, It's
>> valid case, when there are no definition for default device's pinctrl
>> states in DT at all ("default", "active", "idle", "sleep").
>> And in this case dev->pins == NULL and pinctrl_pm_select*() API
>> should return 0 always.
>>
>> Now the checks for !dev->pins have been removed from
>> pinctrl_pm_select*() API mistakenly by the patch
>> pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
>> http://www.spinics.net/lists/arm-kernel/msg258829.html
>>
>> Hence, rollback these checks in in pinctrl_pm_select*() APIs.
>
> Thanks, it's best that I fold this fix into my patch as it has not
> been committed yet.
I think I've applied the correct v2 version,
please check that linux-next is in good shape...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs
@ 2013-07-26 23:22 ` Linus Walleij
0 siblings, 0 replies; 53+ messages in thread
From: Linus Walleij @ 2013-07-26 23:22 UTC (permalink / raw)
To: Tony Lindgren
Cc: Grygorii Strashko, Kevin Hilman, linux-arm-kernel, Linux-OMAP,
linux-kernel, devicetree-discuss, Stephen Warren
On Wed, Jul 17, 2013 at 5:40 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> The pinctrl support in Device core assumed to be optional - so, It's
>> valid case, when there are no definition for default device's pinctrl
>> states in DT at all ("default", "active", "idle", "sleep").
>> And in this case dev->pins == NULL and pinctrl_pm_select*() API
>> should return 0 always.
>>
>> Now the checks for !dev->pins have been removed from
>> pinctrl_pm_select*() API mistakenly by the patch
>> pinctrl: Remove duplicate code in pinctrl_pm_select_state functions
>> http://www.spinics.net/lists/arm-kernel/msg258829.html
>>
>> Hence, rollback these checks in in pinctrl_pm_select*() APIs.
>
> Thanks, it's best that I fold this fix into my patch as it has not
> been committed yet.
I think I've applied the correct v2 version,
please check that linux-next is in good shape...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
2013-07-17 11:41 ` Grygorii Strashko
(?)
@ 2013-07-17 11:41 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: Tony Lindgren, Kevin Hilman
Cc: linux-arm-kernel, linux-omap, linux-kernel, devicetree-discuss,
Grygorii Strashko, Linus Walleij, Stephen Warren
Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
framework. After switching to DT-boot the pinctrl handling was dropped from
hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
(see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
But this is not right for OMAP2+ SoC where real IPs state is controlled
by omap_device core which enables/disables modules & clocks actually.
For example, if OMAP I2C driver will handle pinctrl state during system wide
suspend the following issue may occure:
- suspend_noirq - I2C device can be still active because of PM auto-suspend
|-_od_suspend_noirq
|- omap_i2c_suspend_noirq
|- PINs state set to SLEEP
|- pm_generic_runtime_suspend
|- omap_i2c_runtime_suspend()
|- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
|- omap_device_idle()
|- omap_hwmod_idle()
|- _idle()
|- disbale module (sysc&clocks)
- resume_noirq - I2C was active before suspend
|-_od_resume_noirq
|- omap_hwmod_enable()
|- _enable()
|- enable module (sysc&clocks)
|- pm_generic_runtime_resume
|- omap_i2c_runtime_resume()
|- PINs state set to DEFAULT <--- !!!!
|- omap_i2c_resume_noirq
|- PINs state set to DEFAULT
|- PINs state set to IDLE <--- *big oops* we have active module and its
PINs state is IDLE
(see https://patchwork.kernel.org/patch/2642101/)
Of course, everything can be handled by adding a tons of code in ecah driver to
check PM state of device and override default behavior of omap_device core, but
this not good.
Hence, add pinctrl handling in omap_device core as shown below:
+
|
| .probe()
|
+-----v--------+
| |
| default |
| |
+----+--+------+
| |
pm_runtime_get()| | pm_runtime_put()
+----------------+ +------------+
| |
+------v------+ pm_runtime_put()+-------v-----+
| +-----------------> |
+-----------> Active |pm_runtime_get() | Idle <----------+
| | <-----------------+ | |
| +-------+-----+ +-------+-----+ |
| |.suspend_noirq() |.suspend_noirq()|
| | | |
| | | |
| | | |
| | | |
| +-------v-----+ +-------v-----+ |
| | | | | |
+-----------+ Sleep ------+ | Sleep +----------+
.resume_noirq() | | | | |.resume_noirq()
+-------|-----+ | +-------------+
| Idle |
+-----------+
1) on PM runtime resume
- switch pinctrl state to "active"
2) on PM runtime suspend
- switch pinctrl state to "idle"
3) during system wide suspend
- switch pinctrl state to "sleep" or "idle" if omap_device core disables device
- switch pinctrl state to "sleep" if device has been disabled already
4) during system wide resume
- switch pinctrl state to "active" if omap_device core has
disabled device during suspend
- switch pinctrl state to "idle" if device was already disabled before suspend
This will enable pinctrl for all OMAP2+ IP's drivers by default -
no changes in code is needed and only DT data will need to be updated
(add "default", "active", "idle", "sleep" states).
Related discussions:
- [3/3] i2c: nomadik: use pinctrl PM helpers
https://patchwork.kernel.org/patch/2670291/
- mmc: omap_hsmmc: Remux pins to support SDIO interrupt and PM runtime
https://patchwork.kernel.org/patch/2690191/
- [PATCH 00/11] drivers: Add Pinctrl PM support
https://lkml.org/lkml/2013/5/31/210
CC: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/mach-omap2/omap_device.c | 40 +++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 5cc9287..4ebdbd7 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -39,6 +39,7 @@
#include "soc.h"
#include "omap_device.h"
#include "omap_hwmod.h"
+#include "iomap.h"
/* Private functions */
@@ -582,8 +583,10 @@ static int _od_runtime_suspend(struct device *dev)
ret = pm_generic_runtime_suspend(dev);
- if (!ret)
+ if (!ret) {
omap_device_idle(pdev);
+ pinctrl_pm_select_idle_state(dev);
+ }
return ret;
}
@@ -592,12 +595,25 @@ static int _od_runtime_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
+ pinctrl_pm_select_active_state(dev);
+
omap_device_enable(pdev);
return pm_generic_runtime_resume(dev);
}
#endif
+void _od_suspend_sel_pinctrl_state(struct device *dev)
+{
+ if (!dev->pins)
+ return;
+ /* try to select *deepest* pinctrl state */
+ if (IS_ERR(dev->pins->sleep_state))
+ pinctrl_pm_select_idle_state(dev);
+ else
+ pinctrl_pm_select_sleep_state(dev);
+}
+
#ifdef CONFIG_SUSPEND
static int _od_suspend_noirq(struct device *dev)
{
@@ -610,11 +626,19 @@ static int _od_suspend_noirq(struct device *dev)
return 0;
ret = pm_generic_suspend_noirq(dev);
-
- if (!ret && !pm_runtime_status_suspended(dev)) {
- if (pm_generic_runtime_suspend(dev) == 0) {
- omap_device_idle(pdev);
- od->flags |= OMAP_DEVICE_SUSPENDED;
+ if (!ret) {
+ if (!pm_runtime_status_suspended(dev)) {
+ if (pm_generic_runtime_suspend(dev) == 0) {
+ omap_device_idle(pdev);
+ od->flags |= OMAP_DEVICE_SUSPENDED;
+ _od_suspend_sel_pinctrl_state(dev);
+ }
+ } else {
+ /*
+ * "idle" pinctrl state already applied -
+ * try to set "sleep" state
+ */
+ pinctrl_pm_select_sleep_state(dev);
}
}
@@ -630,7 +654,11 @@ static int _od_resume_noirq(struct device *dev)
!pm_runtime_status_suspended(dev)) {
od->flags &= ~OMAP_DEVICE_SUSPENDED;
omap_device_enable(pdev);
+ pinctrl_pm_select_active_state(dev);
pm_generic_runtime_resume(dev);
+ } else if (pm_runtime_status_suspended(dev)) {
+ /* switch back to "idle" pinctrl state */
+ pinctrl_pm_select_idle_state(dev);
}
return pm_generic_resume_noirq(dev);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
@ 2013-07-17 11:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: linux-arm-kernel
Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
framework. After switching to DT-boot the pinctrl handling was dropped from
hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
(see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
But this is not right for OMAP2+ SoC where real IPs state is controlled
by omap_device core which enables/disables modules & clocks actually.
For example, if OMAP I2C driver will handle pinctrl state during system wide
suspend the following issue may occure:
- suspend_noirq - I2C device can be still active because of PM auto-suspend
|-_od_suspend_noirq
|- omap_i2c_suspend_noirq
|- PINs state set to SLEEP
|- pm_generic_runtime_suspend
|- omap_i2c_runtime_suspend()
|- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
|- omap_device_idle()
|- omap_hwmod_idle()
|- _idle()
|- disbale module (sysc&clocks)
- resume_noirq - I2C was active before suspend
|-_od_resume_noirq
|- omap_hwmod_enable()
|- _enable()
|- enable module (sysc&clocks)
|- pm_generic_runtime_resume
|- omap_i2c_runtime_resume()
|- PINs state set to DEFAULT <--- !!!!
|- omap_i2c_resume_noirq
|- PINs state set to DEFAULT
|- PINs state set to IDLE <--- *big oops* we have active module and its
PINs state is IDLE
(see https://patchwork.kernel.org/patch/2642101/)
Of course, everything can be handled by adding a tons of code in ecah driver to
check PM state of device and override default behavior of omap_device core, but
this not good.
Hence, add pinctrl handling in omap_device core as shown below:
+
|
| .probe()
|
+-----v--------+
| |
| default |
| |
+----+--+------+
| |
pm_runtime_get()| | pm_runtime_put()
+----------------+ +------------+
| |
+------v------+ pm_runtime_put()+-------v-----+
| +-----------------> |
+-----------> Active |pm_runtime_get() | Idle <----------+
| | <-----------------+ | |
| +-------+-----+ +-------+-----+ |
| |.suspend_noirq() |.suspend_noirq()|
| | | |
| | | |
| | | |
| | | |
| +-------v-----+ +-------v-----+ |
| | | | | |
+-----------+ Sleep ------+ | Sleep +----------+
.resume_noirq() | | | | |.resume_noirq()
+-------|-----+ | +-------------+
| Idle |
+-----------+
1) on PM runtime resume
- switch pinctrl state to "active"
2) on PM runtime suspend
- switch pinctrl state to "idle"
3) during system wide suspend
- switch pinctrl state to "sleep" or "idle" if omap_device core disables device
- switch pinctrl state to "sleep" if device has been disabled already
4) during system wide resume
- switch pinctrl state to "active" if omap_device core has
disabled device during suspend
- switch pinctrl state to "idle" if device was already disabled before suspend
This will enable pinctrl for all OMAP2+ IP's drivers by default -
no changes in code is needed and only DT data will need to be updated
(add "default", "active", "idle", "sleep" states).
Related discussions:
- [3/3] i2c: nomadik: use pinctrl PM helpers
https://patchwork.kernel.org/patch/2670291/
- mmc: omap_hsmmc: Remux pins to support SDIO interrupt and PM runtime
https://patchwork.kernel.org/patch/2690191/
- [PATCH 00/11] drivers: Add Pinctrl PM support
https://lkml.org/lkml/2013/5/31/210
CC: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/mach-omap2/omap_device.c | 40 +++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 5cc9287..4ebdbd7 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -39,6 +39,7 @@
#include "soc.h"
#include "omap_device.h"
#include "omap_hwmod.h"
+#include "iomap.h"
/* Private functions */
@@ -582,8 +583,10 @@ static int _od_runtime_suspend(struct device *dev)
ret = pm_generic_runtime_suspend(dev);
- if (!ret)
+ if (!ret) {
omap_device_idle(pdev);
+ pinctrl_pm_select_idle_state(dev);
+ }
return ret;
}
@@ -592,12 +595,25 @@ static int _od_runtime_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
+ pinctrl_pm_select_active_state(dev);
+
omap_device_enable(pdev);
return pm_generic_runtime_resume(dev);
}
#endif
+void _od_suspend_sel_pinctrl_state(struct device *dev)
+{
+ if (!dev->pins)
+ return;
+ /* try to select *deepest* pinctrl state */
+ if (IS_ERR(dev->pins->sleep_state))
+ pinctrl_pm_select_idle_state(dev);
+ else
+ pinctrl_pm_select_sleep_state(dev);
+}
+
#ifdef CONFIG_SUSPEND
static int _od_suspend_noirq(struct device *dev)
{
@@ -610,11 +626,19 @@ static int _od_suspend_noirq(struct device *dev)
return 0;
ret = pm_generic_suspend_noirq(dev);
-
- if (!ret && !pm_runtime_status_suspended(dev)) {
- if (pm_generic_runtime_suspend(dev) == 0) {
- omap_device_idle(pdev);
- od->flags |= OMAP_DEVICE_SUSPENDED;
+ if (!ret) {
+ if (!pm_runtime_status_suspended(dev)) {
+ if (pm_generic_runtime_suspend(dev) == 0) {
+ omap_device_idle(pdev);
+ od->flags |= OMAP_DEVICE_SUSPENDED;
+ _od_suspend_sel_pinctrl_state(dev);
+ }
+ } else {
+ /*
+ * "idle" pinctrl state already applied -
+ * try to set "sleep" state
+ */
+ pinctrl_pm_select_sleep_state(dev);
}
}
@@ -630,7 +654,11 @@ static int _od_resume_noirq(struct device *dev)
!pm_runtime_status_suspended(dev)) {
od->flags &= ~OMAP_DEVICE_SUSPENDED;
omap_device_enable(pdev);
+ pinctrl_pm_select_active_state(dev);
pm_generic_runtime_resume(dev);
+ } else if (pm_runtime_status_suspended(dev)) {
+ /* switch back to "idle" pinctrl state */
+ pinctrl_pm_select_idle_state(dev);
}
return pm_generic_resume_noirq(dev);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
@ 2013-07-17 11:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: Tony Lindgren, Kevin Hilman
Cc: linux-arm-kernel, linux-omap, linux-kernel, devicetree-discuss,
Grygorii Strashko, Linus Walleij, Stephen Warren
Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
framework. After switching to DT-boot the pinctrl handling was dropped from
hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
(see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
But this is not right for OMAP2+ SoC where real IPs state is controlled
by omap_device core which enables/disables modules & clocks actually.
For example, if OMAP I2C driver will handle pinctrl state during system wide
suspend the following issue may occure:
- suspend_noirq - I2C device can be still active because of PM auto-suspend
|-_od_suspend_noirq
|- omap_i2c_suspend_noirq
|- PINs state set to SLEEP
|- pm_generic_runtime_suspend
|- omap_i2c_runtime_suspend()
|- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
|- omap_device_idle()
|- omap_hwmod_idle()
|- _idle()
|- disbale module (sysc&clocks)
- resume_noirq - I2C was active before suspend
|-_od_resume_noirq
|- omap_hwmod_enable()
|- _enable()
|- enable module (sysc&clocks)
|- pm_generic_runtime_resume
|- omap_i2c_runtime_resume()
|- PINs state set to DEFAULT <--- !!!!
|- omap_i2c_resume_noirq
|- PINs state set to DEFAULT
|- PINs state set to IDLE <--- *big oops* we have active module and its
PINs state is IDLE
(see https://patchwork.kernel.org/patch/2642101/)
Of course, everything can be handled by adding a tons of code in ecah driver to
check PM state of device and override default behavior of omap_device core, but
this not good.
Hence, add pinctrl handling in omap_device core as shown below:
+
|
| .probe()
|
+-----v--------+
| |
| default |
| |
+----+--+------+
| |
pm_runtime_get()| | pm_runtime_put()
+----------------+ +------------+
| |
+------v------+ pm_runtime_put()+-------v-----+
| +-----------------> |
+-----------> Active |pm_runtime_get() | Idle <----------+
| | <-----------------+ | |
| +-------+-----+ +-------+-----+ |
| |.suspend_noirq() |.suspend_noirq()|
| | | |
| | | |
| | | |
| | | |
| +-------v-----+ +-------v-----+ |
| | | | | |
+-----------+ Sleep ------+ | Sleep +----------+
.resume_noirq() | | | | |.resume_noirq()
+-------|-----+ | +-------------+
| Idle |
+-----------+
1) on PM runtime resume
- switch pinctrl state to "active"
2) on PM runtime suspend
- switch pinctrl state to "idle"
3) during system wide suspend
- switch pinctrl state to "sleep" or "idle" if omap_device core disables device
- switch pinctrl state to "sleep" if device has been disabled already
4) during system wide resume
- switch pinctrl state to "active" if omap_device core has
disabled device during suspend
- switch pinctrl state to "idle" if device was already disabled before suspend
This will enable pinctrl for all OMAP2+ IP's drivers by default -
no changes in code is needed and only DT data will need to be updated
(add "default", "active", "idle", "sleep" states).
Related discussions:
- [3/3] i2c: nomadik: use pinctrl PM helpers
https://patchwork.kernel.org/patch/2670291/
- mmc: omap_hsmmc: Remux pins to support SDIO interrupt and PM runtime
https://patchwork.kernel.org/patch/2690191/
- [PATCH 00/11] drivers: Add Pinctrl PM support
https://lkml.org/lkml/2013/5/31/210
CC: Linus Walleij <linus.walleij@linaro.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/mach-omap2/omap_device.c | 40 +++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 5cc9287..4ebdbd7 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -39,6 +39,7 @@
#include "soc.h"
#include "omap_device.h"
#include "omap_hwmod.h"
+#include "iomap.h"
/* Private functions */
@@ -582,8 +583,10 @@ static int _od_runtime_suspend(struct device *dev)
ret = pm_generic_runtime_suspend(dev);
- if (!ret)
+ if (!ret) {
omap_device_idle(pdev);
+ pinctrl_pm_select_idle_state(dev);
+ }
return ret;
}
@@ -592,12 +595,25 @@ static int _od_runtime_resume(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
+ pinctrl_pm_select_active_state(dev);
+
omap_device_enable(pdev);
return pm_generic_runtime_resume(dev);
}
#endif
+void _od_suspend_sel_pinctrl_state(struct device *dev)
+{
+ if (!dev->pins)
+ return;
+ /* try to select *deepest* pinctrl state */
+ if (IS_ERR(dev->pins->sleep_state))
+ pinctrl_pm_select_idle_state(dev);
+ else
+ pinctrl_pm_select_sleep_state(dev);
+}
+
#ifdef CONFIG_SUSPEND
static int _od_suspend_noirq(struct device *dev)
{
@@ -610,11 +626,19 @@ static int _od_suspend_noirq(struct device *dev)
return 0;
ret = pm_generic_suspend_noirq(dev);
-
- if (!ret && !pm_runtime_status_suspended(dev)) {
- if (pm_generic_runtime_suspend(dev) == 0) {
- omap_device_idle(pdev);
- od->flags |= OMAP_DEVICE_SUSPENDED;
+ if (!ret) {
+ if (!pm_runtime_status_suspended(dev)) {
+ if (pm_generic_runtime_suspend(dev) == 0) {
+ omap_device_idle(pdev);
+ od->flags |= OMAP_DEVICE_SUSPENDED;
+ _od_suspend_sel_pinctrl_state(dev);
+ }
+ } else {
+ /*
+ * "idle" pinctrl state already applied -
+ * try to set "sleep" state
+ */
+ pinctrl_pm_select_sleep_state(dev);
}
}
@@ -630,7 +654,11 @@ static int _od_resume_noirq(struct device *dev)
!pm_runtime_status_suspended(dev)) {
od->flags &= ~OMAP_DEVICE_SUSPENDED;
omap_device_enable(pdev);
+ pinctrl_pm_select_active_state(dev);
pm_generic_runtime_resume(dev);
+ } else if (pm_runtime_status_suspended(dev)) {
+ /* switch back to "idle" pinctrl state */
+ pinctrl_pm_select_idle_state(dev);
}
return pm_generic_resume_noirq(dev);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
2013-07-17 11:41 ` Grygorii Strashko
@ 2013-07-17 15:38 ` Tony Lindgren
-1 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-17 15:38 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Linus Walleij, Stephen Warren
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
> framework. After switching to DT-boot the pinctrl handling was dropped from
> hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
> to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
>
> But this is not right for OMAP2+ SoC where real IPs state is controlled
> by omap_device core which enables/disables modules & clocks actually.
I'm not convinced we should try to handle this in a generic way
as only some devices need dynamic remuxing of some pins.
> For example, if OMAP I2C driver will handle pinctrl state during system wide
> suspend the following issue may occure:
> - suspend_noirq - I2C device can be still active because of PM auto-suspend
> |-_od_suspend_noirq
> |- omap_i2c_suspend_noirq
> |- PINs state set to SLEEP
> |- pm_generic_runtime_suspend
> |- omap_i2c_runtime_suspend()
> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
> |- omap_device_idle()
> |- omap_hwmod_idle()
> |- _idle()
> |- disbale module (sysc&clocks)
And in this example you are assuming that you need separate idle and
sleep states, which is not true at least for most cases I've seen.
It is possible that am33xx needs separate idle and sleep states, but
most likely only for some pins. For omap[345] we can get away with
just the default state for most cases.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
@ 2013-07-17 15:38 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-17 15:38 UTC (permalink / raw)
To: linux-arm-kernel
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
> framework. After switching to DT-boot the pinctrl handling was dropped from
> hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
> to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
>
> But this is not right for OMAP2+ SoC where real IPs state is controlled
> by omap_device core which enables/disables modules & clocks actually.
I'm not convinced we should try to handle this in a generic way
as only some devices need dynamic remuxing of some pins.
> For example, if OMAP I2C driver will handle pinctrl state during system wide
> suspend the following issue may occure:
> - suspend_noirq - I2C device can be still active because of PM auto-suspend
> |-_od_suspend_noirq
> |- omap_i2c_suspend_noirq
> |- PINs state set to SLEEP
> |- pm_generic_runtime_suspend
> |- omap_i2c_runtime_suspend()
> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
> |- omap_device_idle()
> |- omap_hwmod_idle()
> |- _idle()
> |- disbale module (sysc&clocks)
And in this example you are assuming that you need separate idle and
sleep states, which is not true at least for most cases I've seen.
It is possible that am33xx needs separate idle and sleep states, but
most likely only for some pins. For omap[345] we can get away with
just the default state for most cases.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
2013-07-17 15:38 ` Tony Lindgren
(?)
@ 2013-07-17 16:56 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 16:56 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Linus Walleij, Stephen Warren
On 07/17/2013 06:38 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
>> framework. After switching to DT-boot the pinctrl handling was dropped from
>> hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
>> to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
>> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
>>
>> But this is not right for OMAP2+ SoC where real IPs state is controlled
>> by omap_device core which enables/disables modules & clocks actually.
>
> I'm not convinced we should try to handle this in a generic way
> as only some devices need dynamic remuxing of some pins.
>
>> For example, if OMAP I2C driver will handle pinctrl state during system wide
>> suspend the following issue may occure:
>> - suspend_noirq - I2C device can be still active because of PM auto-suspend
>> |-_od_suspend_noirq
>> |- omap_i2c_suspend_noirq
>> |- PINs state set to SLEEP
>> |- pm_generic_runtime_suspend
>> |- omap_i2c_runtime_suspend()
>> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
>> |- omap_device_idle()
>> |- omap_hwmod_idle()
>> |- _idle()
>> |- disbale module (sysc&clocks)
>
> And in this example you are assuming that you need separate idle and
> sleep states, which is not true at least for most cases I've seen.
I don't need both states (at least right now) :), but
- if any OMAP2+ driver will have two states defined: "idle" and "sleep"
- and if it will try to manage them from drivers callbacks only using
pure calls to pinctrl_pm_select_xx() helpers
the "idle" state will be selected during suspend and *not* "sleep".
>
> It is possible that am33xx needs separate idle and sleep states, but
> most likely only for some pins. For omap[345] we can get away with
> just the default state for most cases.
>
In case, if only "default" state is defined for device - nothing will
be done by OMAP device framework for it (I mean any call to
pinctrl_pm_select_xx() will do nothing - it just checks that there is
no state and returns 0).
> Regards,
>
> Tony
>
Regards,
- grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
@ 2013-07-17 16:56 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 16:56 UTC (permalink / raw)
To: linux-arm-kernel
On 07/17/2013 06:38 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
>> framework. After switching to DT-boot the pinctrl handling was dropped from
>> hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
>> to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
>> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
>>
>> But this is not right for OMAP2+ SoC where real IPs state is controlled
>> by omap_device core which enables/disables modules & clocks actually.
>
> I'm not convinced we should try to handle this in a generic way
> as only some devices need dynamic remuxing of some pins.
>
>> For example, if OMAP I2C driver will handle pinctrl state during system wide
>> suspend the following issue may occure:
>> - suspend_noirq - I2C device can be still active because of PM auto-suspend
>> |-_od_suspend_noirq
>> |- omap_i2c_suspend_noirq
>> |- PINs state set to SLEEP
>> |- pm_generic_runtime_suspend
>> |- omap_i2c_runtime_suspend()
>> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
>> |- omap_device_idle()
>> |- omap_hwmod_idle()
>> |- _idle()
>> |- disbale module (sysc&clocks)
>
> And in this example you are assuming that you need separate idle and
> sleep states, which is not true at least for most cases I've seen.
I don't need both states (at least right now) :), but
- if any OMAP2+ driver will have two states defined: "idle" and "sleep"
- and if it will try to manage them from drivers callbacks only using
pure calls to pinctrl_pm_select_xx() helpers
the "idle" state will be selected during suspend and *not* "sleep".
>
> It is possible that am33xx needs separate idle and sleep states, but
> most likely only for some pins. For omap[345] we can get away with
> just the default state for most cases.
>
In case, if only "default" state is defined for device - nothing will
be done by OMAP device framework for it (I mean any call to
pinctrl_pm_select_xx() will do nothing - it just checks that there is
no state and returns 0).
> Regards,
>
> Tony
>
Regards,
- grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
@ 2013-07-17 16:56 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 16:56 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Linus Walleij, Stephen Warren
On 07/17/2013 06:38 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
>> framework. After switching to DT-boot the pinctrl handling was dropped from
>> hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
>> to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
>> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
>>
>> But this is not right for OMAP2+ SoC where real IPs state is controlled
>> by omap_device core which enables/disables modules & clocks actually.
>
> I'm not convinced we should try to handle this in a generic way
> as only some devices need dynamic remuxing of some pins.
>
>> For example, if OMAP I2C driver will handle pinctrl state during system wide
>> suspend the following issue may occure:
>> - suspend_noirq - I2C device can be still active because of PM auto-suspend
>> |-_od_suspend_noirq
>> |- omap_i2c_suspend_noirq
>> |- PINs state set to SLEEP
>> |- pm_generic_runtime_suspend
>> |- omap_i2c_runtime_suspend()
>> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
>> |- omap_device_idle()
>> |- omap_hwmod_idle()
>> |- _idle()
>> |- disbale module (sysc&clocks)
>
> And in this example you are assuming that you need separate idle and
> sleep states, which is not true at least for most cases I've seen.
I don't need both states (at least right now) :), but
- if any OMAP2+ driver will have two states defined: "idle" and "sleep"
- and if it will try to manage them from drivers callbacks only using
pure calls to pinctrl_pm_select_xx() helpers
the "idle" state will be selected during suspend and *not* "sleep".
>
> It is possible that am33xx needs separate idle and sleep states, but
> most likely only for some pins. For omap[345] we can get away with
> just the default state for most cases.
>
In case, if only "default" state is defined for device - nothing will
be done by OMAP device framework for it (I mean any call to
pinctrl_pm_select_xx() will do nothing - it just checks that there is
no state and returns 0).
> Regards,
>
> Tony
>
Regards,
- grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
2013-07-17 16:56 ` Grygorii Strashko
@ 2013-07-18 8:14 ` Tony Lindgren
-1 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-18 8:14 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Linus Walleij, Stephen Warren
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 10:11]:
> On 07/17/2013 06:38 PM, Tony Lindgren wrote:
> >* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> >>Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
> >>framework. After switching to DT-boot the pinctrl handling was dropped from
> >>hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
> >>to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
> >>(see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
> >>
> >>But this is not right for OMAP2+ SoC where real IPs state is controlled
> >>by omap_device core which enables/disables modules & clocks actually.
> >
> >I'm not convinced we should try to handle this in a generic way
> >as only some devices need dynamic remuxing of some pins.
> >
> >>For example, if OMAP I2C driver will handle pinctrl state during system wide
> >>suspend the following issue may occure:
> >>- suspend_noirq - I2C device can be still active because of PM auto-suspend
> >> |-_od_suspend_noirq
> >> |- omap_i2c_suspend_noirq
> >> |- PINs state set to SLEEP
> >> |- pm_generic_runtime_suspend
> >> |- omap_i2c_runtime_suspend()
> >> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
> >> |- omap_device_idle()
> >> |- omap_hwmod_idle()
> >> |- _idle()
> >> |- disbale module (sysc&clocks)
> >
> >And in this example you are assuming that you need separate idle and
> >sleep states, which is not true at least for most cases I've seen.
>
> I don't need both states (at least right now) :), but
> - if any OMAP2+ driver will have two states defined: "idle" and "sleep"
> - and if it will try to manage them from drivers callbacks only using
> pure calls to pinctrl_pm_select_xx() helpers
>
> the "idle" state will be selected during suspend and *not* "sleep".
But the drivers have separate calls for runtime PM and suspend/resume?
> >It is possible that am33xx needs separate idle and sleep states, but
> >most likely only for some pins. For omap[345] we can get away with
> >just the default state for most cases.
> >
>
>
> In case, if only "default" state is defined for device - nothing will
> be done by OMAP device framework for it (I mean any call to
> pinctrl_pm_select_xx() will do nothing - it just checks that there is
> no state and returns 0).
If we want to automate something, it should be done at Linux generic
level rather than at omap "bus" level. That sort of relates to drivers
needing to know when they've lost context too, which should be
implemented in Linux generic way.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
@ 2013-07-18 8:14 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-18 8:14 UTC (permalink / raw)
To: linux-arm-kernel
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 10:11]:
> On 07/17/2013 06:38 PM, Tony Lindgren wrote:
> >* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> >>Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
> >>framework. After switching to DT-boot the pinctrl handling was dropped from
> >>hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
> >>to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
> >>(see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
> >>
> >>But this is not right for OMAP2+ SoC where real IPs state is controlled
> >>by omap_device core which enables/disables modules & clocks actually.
> >
> >I'm not convinced we should try to handle this in a generic way
> >as only some devices need dynamic remuxing of some pins.
> >
> >>For example, if OMAP I2C driver will handle pinctrl state during system wide
> >>suspend the following issue may occure:
> >>- suspend_noirq - I2C device can be still active because of PM auto-suspend
> >> |-_od_suspend_noirq
> >> |- omap_i2c_suspend_noirq
> >> |- PINs state set to SLEEP
> >> |- pm_generic_runtime_suspend
> >> |- omap_i2c_runtime_suspend()
> >> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
> >> |- omap_device_idle()
> >> |- omap_hwmod_idle()
> >> |- _idle()
> >> |- disbale module (sysc&clocks)
> >
> >And in this example you are assuming that you need separate idle and
> >sleep states, which is not true at least for most cases I've seen.
>
> I don't need both states (at least right now) :), but
> - if any OMAP2+ driver will have two states defined: "idle" and "sleep"
> - and if it will try to manage them from drivers callbacks only using
> pure calls to pinctrl_pm_select_xx() helpers
>
> the "idle" state will be selected during suspend and *not* "sleep".
But the drivers have separate calls for runtime PM and suspend/resume?
> >It is possible that am33xx needs separate idle and sleep states, but
> >most likely only for some pins. For omap[345] we can get away with
> >just the default state for most cases.
> >
>
>
> In case, if only "default" state is defined for device - nothing will
> be done by OMAP device framework for it (I mean any call to
> pinctrl_pm_select_xx() will do nothing - it just checks that there is
> no state and returns 0).
If we want to automate something, it should be done at Linux generic
level rather than at omap "bus" level. That sort of relates to drivers
needing to know when they've lost context too, which should be
implemented in Linux generic way.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
2013-07-18 8:14 ` Tony Lindgren
(?)
@ 2013-07-18 11:22 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 11:22 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Linus Walleij, Stephen Warren
On 07/18/2013 11:14 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 10:11]:
>> On 07/17/2013 06:38 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
>>>> framework. After switching to DT-boot the pinctrl handling was dropped from
>>>> hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
>>>> to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
>>>> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
>>>>
>>>> But this is not right for OMAP2+ SoC where real IPs state is controlled
>>>> by omap_device core which enables/disables modules & clocks actually.
>>>
>>> I'm not convinced we should try to handle this in a generic way
>>> as only some devices need dynamic remuxing of some pins.
>>>
>>>> For example, if OMAP I2C driver will handle pinctrl state during system wide
>>>> suspend the following issue may occure:
>>>> - suspend_noirq - I2C device can be still active because of PM auto-suspend
>>>> |-_od_suspend_noirq
>>>> |- omap_i2c_suspend_noirq
>>>> |- PINs state set to SLEEP
>>>> |- pm_generic_runtime_suspend
>>>> |- omap_i2c_runtime_suspend()
>>>> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
>>>> |- omap_device_idle()
>>>> |- omap_hwmod_idle()
>>>> |- _idle()
>>>> |- disbale module (sysc&clocks)
Above call sequence explains what's going on when .suspend_noirq() is
called - .suspend_noirq() handler ==> _od_suspend_noirq() and it's set
for all OMAP2+ devices.
>>>
>>> And in this example you are assuming that you need separate idle and
>>> sleep states, which is not true at least for most cases I've seen.
>>
>> I don't need both states (at least right now) :), but
>> - if any OMAP2+ driver will have two states defined: "idle" and "sleep"
>> - and if it will try to manage them from drivers callbacks only using
>> pure calls to pinctrl_pm_select_xx() helpers
>>
>> the "idle" state will be selected during suspend and *not* "sleep".
>
> But the drivers have separate calls for runtime PM and suspend/resume?
Yes, but for OMAP2+ everything is handled by OMAP device framework :)
>
>>> It is possible that am33xx needs separate idle and sleep states, but
>>> most likely only for some pins. For omap[345] we can get away with
>>> just the default state for most cases.
>>>
>>
>>
>> In case, if only "default" state is defined for device - nothing will
>> be done by OMAP device framework for it (I mean any call to
>> pinctrl_pm_select_xx() will do nothing - it just checks that there is
>> no state and returns 0).
>
> If we want to automate something, it should be done at Linux generic
> level rather than at omap "bus" level. That sort of relates to drivers
> needing to know when they've lost context too, which should be
> implemented in Linux generic way.
>
> Regards,
>
> Tony
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
@ 2013-07-18 11:22 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 11:22 UTC (permalink / raw)
To: linux-arm-kernel
On 07/18/2013 11:14 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 10:11]:
>> On 07/17/2013 06:38 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
>>>> framework. After switching to DT-boot the pinctrl handling was dropped from
>>>> hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
>>>> to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
>>>> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
>>>>
>>>> But this is not right for OMAP2+ SoC where real IPs state is controlled
>>>> by omap_device core which enables/disables modules & clocks actually.
>>>
>>> I'm not convinced we should try to handle this in a generic way
>>> as only some devices need dynamic remuxing of some pins.
>>>
>>>> For example, if OMAP I2C driver will handle pinctrl state during system wide
>>>> suspend the following issue may occure:
>>>> - suspend_noirq - I2C device can be still active because of PM auto-suspend
>>>> |-_od_suspend_noirq
>>>> |- omap_i2c_suspend_noirq
>>>> |- PINs state set to SLEEP
>>>> |- pm_generic_runtime_suspend
>>>> |- omap_i2c_runtime_suspend()
>>>> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
>>>> |- omap_device_idle()
>>>> |- omap_hwmod_idle()
>>>> |- _idle()
>>>> |- disbale module (sysc&clocks)
Above call sequence explains what's going on when .suspend_noirq() is
called - .suspend_noirq() handler ==> _od_suspend_noirq() and it's set
for all OMAP2+ devices.
>>>
>>> And in this example you are assuming that you need separate idle and
>>> sleep states, which is not true at least for most cases I've seen.
>>
>> I don't need both states (at least right now) :), but
>> - if any OMAP2+ driver will have two states defined: "idle" and "sleep"
>> - and if it will try to manage them from drivers callbacks only using
>> pure calls to pinctrl_pm_select_xx() helpers
>>
>> the "idle" state will be selected during suspend and *not* "sleep".
>
> But the drivers have separate calls for runtime PM and suspend/resume?
Yes, but for OMAP2+ everything is handled by OMAP device framework :)
>
>>> It is possible that am33xx needs separate idle and sleep states, but
>>> most likely only for some pins. For omap[345] we can get away with
>>> just the default state for most cases.
>>>
>>
>>
>> In case, if only "default" state is defined for device - nothing will
>> be done by OMAP device framework for it (I mean any call to
>> pinctrl_pm_select_xx() will do nothing - it just checks that there is
>> no state and returns 0).
>
> If we want to automate something, it should be done at Linux generic
> level rather than at omap "bus" level. That sort of relates to drivers
> needing to know when they've lost context too, which should be
> implemented in Linux generic way.
>
> Regards,
>
> Tony
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling
@ 2013-07-18 11:22 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 11:22 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, Stephen Warren, devicetree-discuss, linux-kernel,
linux-omap, Linus Walleij, linux-arm-kernel
On 07/18/2013 11:14 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 10:11]:
>> On 07/17/2013 06:38 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Before switching to DT pinctrl states of OMAP IPs have been handled by hwmod
>>>> framework. After switching to DT-boot the pinctrl handling was dropped from
>>>> hwmod framework and, as it was recommended, OMAP IP's drivers have to be updated
>>>> to handle pinctrl states by itself using pinctrl_pm_select_xx() helpers
>>>> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/173514.html)
>>>>
>>>> But this is not right for OMAP2+ SoC where real IPs state is controlled
>>>> by omap_device core which enables/disables modules & clocks actually.
>>>
>>> I'm not convinced we should try to handle this in a generic way
>>> as only some devices need dynamic remuxing of some pins.
>>>
>>>> For example, if OMAP I2C driver will handle pinctrl state during system wide
>>>> suspend the following issue may occure:
>>>> - suspend_noirq - I2C device can be still active because of PM auto-suspend
>>>> |-_od_suspend_noirq
>>>> |- omap_i2c_suspend_noirq
>>>> |- PINs state set to SLEEP
>>>> |- pm_generic_runtime_suspend
>>>> |- omap_i2c_runtime_suspend()
>>>> |- PINs state set to IDLE <--- *oops* PINs state is IDLE and not SLEEP
>>>> |- omap_device_idle()
>>>> |- omap_hwmod_idle()
>>>> |- _idle()
>>>> |- disbale module (sysc&clocks)
Above call sequence explains what's going on when .suspend_noirq() is
called - .suspend_noirq() handler ==> _od_suspend_noirq() and it's set
for all OMAP2+ devices.
>>>
>>> And in this example you are assuming that you need separate idle and
>>> sleep states, which is not true at least for most cases I've seen.
>>
>> I don't need both states (at least right now) :), but
>> - if any OMAP2+ driver will have two states defined: "idle" and "sleep"
>> - and if it will try to manage them from drivers callbacks only using
>> pure calls to pinctrl_pm_select_xx() helpers
>>
>> the "idle" state will be selected during suspend and *not* "sleep".
>
> But the drivers have separate calls for runtime PM and suspend/resume?
Yes, but for OMAP2+ everything is handled by OMAP device framework :)
>
>>> It is possible that am33xx needs separate idle and sleep states, but
>>> most likely only for some pins. For omap[345] we can get away with
>>> just the default state for most cases.
>>>
>>
>>
>> In case, if only "default" state is defined for device - nothing will
>> be done by OMAP device framework for it (I mean any call to
>> pinctrl_pm_select_xx() will do nothing - it just checks that there is
>> no state and returns 0).
>
> If we want to automate something, it should be done at Linux generic
> level rather than at omap "bus" level. That sort of relates to drivers
> needing to know when they've lost context too, which should be
> implemented in Linux generic way.
>
> Regards,
>
> Tony
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
2013-07-17 11:41 ` Grygorii Strashko
(?)
@ 2013-07-17 11:41 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: Tony Lindgren, Kevin Hilman
Cc: linux-arm-kernel, linux-omap, linux-kernel, devicetree-discuss,
Grygorii Strashko, Benoit Cousson, Linus Walleij, Stephen Warren
Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
when uart3/4 state is switched from active to idle and back by Runtime
PM or during system suspend.
CC: Benoit Cousson <benoit.cousson@linaro.org>
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/boot/dts/omap4-sdp.dts | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 7951b4e..2624504 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -181,18 +181,40 @@
pinctrl-single,pins = <
0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
- 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
>;
};
+ uart3_pins_active: pinmux_uart3_pins_active {
+ pinctrl-single,pins = <
+ 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
+ uart3_pins_idle: pinmux_uart3_pins_idle {
+ pinctrl-single,pins = <
+ 0x104 (WAKEUP_EN | PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
uart4_pins: pinmux_uart4_pins {
pinctrl-single,pins = <
- 0x11c (PIN_INPUT | MUX_MODE0) /* uart4_rx.uart4_rx */
0x11e (PIN_OUTPUT | MUX_MODE0) /* uart4_tx.uart4_tx */
>;
};
+ uart4_pins_active: pinmux_uart4_pins_active {
+ pinctrl-single,pins = <
+ 0x11c (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
+ uart4_pins_idle: pinmux_uart4_pins_idle {
+ pinctrl-single,pins = <
+ 0x11c (WAKEUP_EN | PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
twl6030_pins: pinmux_twl6030_pins {
pinctrl-single,pins = <
0x15e (WAKEUP_EN | PIN_INPUT_PULLUP | MUX_MODE0) /* sys_nirq1.sys_nirq1 */
@@ -510,13 +532,17 @@
};
&uart3 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "active", "idle";
pinctrl-0 = <&uart3_pins>;
+ pinctrl-1 = <&uart3_pins_active>;
+ pinctrl-2 = <&uart3_pins_idle>;
};
&uart4 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "active", "idle";
pinctrl-0 = <&uart4_pins>;
+ pinctrl-1 = <&uart4_pins_active>;
+ pinctrl-2 = <&uart4_pins_idle>;
};
&mcbsp3 {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-17 11:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: linux-arm-kernel
Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
when uart3/4 state is switched from active to idle and back by Runtime
PM or during system suspend.
CC: Benoit Cousson <benoit.cousson@linaro.org>
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/boot/dts/omap4-sdp.dts | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 7951b4e..2624504 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -181,18 +181,40 @@
pinctrl-single,pins = <
0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
- 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
>;
};
+ uart3_pins_active: pinmux_uart3_pins_active {
+ pinctrl-single,pins = <
+ 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
+ uart3_pins_idle: pinmux_uart3_pins_idle {
+ pinctrl-single,pins = <
+ 0x104 (WAKEUP_EN | PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
uart4_pins: pinmux_uart4_pins {
pinctrl-single,pins = <
- 0x11c (PIN_INPUT | MUX_MODE0) /* uart4_rx.uart4_rx */
0x11e (PIN_OUTPUT | MUX_MODE0) /* uart4_tx.uart4_tx */
>;
};
+ uart4_pins_active: pinmux_uart4_pins_active {
+ pinctrl-single,pins = <
+ 0x11c (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
+ uart4_pins_idle: pinmux_uart4_pins_idle {
+ pinctrl-single,pins = <
+ 0x11c (WAKEUP_EN | PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
twl6030_pins: pinmux_twl6030_pins {
pinctrl-single,pins = <
0x15e (WAKEUP_EN | PIN_INPUT_PULLUP | MUX_MODE0) /* sys_nirq1.sys_nirq1 */
@@ -510,13 +532,17 @@
};
&uart3 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "active", "idle";
pinctrl-0 = <&uart3_pins>;
+ pinctrl-1 = <&uart3_pins_active>;
+ pinctrl-2 = <&uart3_pins_idle>;
};
&uart4 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "active", "idle";
pinctrl-0 = <&uart4_pins>;
+ pinctrl-1 = <&uart4_pins_active>;
+ pinctrl-2 = <&uart4_pins_idle>;
};
&mcbsp3 {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-17 11:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 11:41 UTC (permalink / raw)
To: Tony Lindgren, Kevin Hilman
Cc: linux-arm-kernel, linux-omap, linux-kernel, devicetree-discuss,
Grygorii Strashko, Benoit Cousson, Linus Walleij, Stephen Warren
Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
when uart3/4 state is switched from active to idle and back by Runtime
PM or during system suspend.
CC: Benoit Cousson <benoit.cousson@linaro.org>
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/boot/dts/omap4-sdp.dts | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 7951b4e..2624504 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -181,18 +181,40 @@
pinctrl-single,pins = <
0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
- 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
>;
};
+ uart3_pins_active: pinmux_uart3_pins_active {
+ pinctrl-single,pins = <
+ 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
+ uart3_pins_idle: pinmux_uart3_pins_idle {
+ pinctrl-single,pins = <
+ 0x104 (WAKEUP_EN | PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
uart4_pins: pinmux_uart4_pins {
pinctrl-single,pins = <
- 0x11c (PIN_INPUT | MUX_MODE0) /* uart4_rx.uart4_rx */
0x11e (PIN_OUTPUT | MUX_MODE0) /* uart4_tx.uart4_tx */
>;
};
+ uart4_pins_active: pinmux_uart4_pins_active {
+ pinctrl-single,pins = <
+ 0x11c (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
+ uart4_pins_idle: pinmux_uart4_pins_idle {
+ pinctrl-single,pins = <
+ 0x11c (WAKEUP_EN | PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
+ >;
+ };
+
twl6030_pins: pinmux_twl6030_pins {
pinctrl-single,pins = <
0x15e (WAKEUP_EN | PIN_INPUT_PULLUP | MUX_MODE0) /* sys_nirq1.sys_nirq1 */
@@ -510,13 +532,17 @@
};
&uart3 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "active", "idle";
pinctrl-0 = <&uart3_pins>;
+ pinctrl-1 = <&uart3_pins_active>;
+ pinctrl-2 = <&uart3_pins_idle>;
};
&uart4 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "active", "idle";
pinctrl-0 = <&uart4_pins>;
+ pinctrl-1 = <&uart4_pins_active>;
+ pinctrl-2 = <&uart4_pins_idle>;
};
&mcbsp3 {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-17 15:32 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-17 15:32 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
> when uart3/4 state is switched from active to idle and back by Runtime
> PM or during system suspend.
This is good for testing code, but should not be merged because
omap4 has the iopad wake-ups available for uarts. So those can
be always enabled.
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -181,18 +181,40 @@
> pinctrl-single,pins = <
> 0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
> 0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
> - 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
> 0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
> >;
> };
This just need to become:
pinctrl-single,pins = <
...
0x104 (WAKEUP_EN | PIN_INPUT_MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
...
>;
interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
};
And so on. For am33xx you need to remux things to GPIO for the wake-up
events, so you should maybe test on that instead.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-17 15:32 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-17 15:32 UTC (permalink / raw)
To: linux-arm-kernel
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
> when uart3/4 state is switched from active to idle and back by Runtime
> PM or during system suspend.
This is good for testing code, but should not be merged because
omap4 has the iopad wake-ups available for uarts. So those can
be always enabled.
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -181,18 +181,40 @@
> pinctrl-single,pins = <
> 0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
> 0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
> - 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
> 0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
> >;
> };
This just need to become:
pinctrl-single,pins = <
...
0x104 (WAKEUP_EN | PIN_INPUT_MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
...
>;
interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
};
And so on. For am33xx you need to remux things to GPIO for the wake-up
events, so you should maybe test on that instead.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-17 15:32 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-17 15:32 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benoit Cousson,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
* Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org> [130717 04:49]:
> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
> when uart3/4 state is switched from active to idle and back by Runtime
> PM or during system suspend.
This is good for testing code, but should not be merged because
omap4 has the iopad wake-ups available for uarts. So those can
be always enabled.
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -181,18 +181,40 @@
> pinctrl-single,pins = <
> 0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
> 0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
> - 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
> 0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
> >;
> };
This just need to become:
pinctrl-single,pins = <
...
0x104 (WAKEUP_EN | PIN_INPUT_MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
...
>;
interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
};
And so on. For am33xx you need to remux things to GPIO for the wake-up
events, so you should maybe test on that instead.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
2013-07-17 15:32 ` Tony Lindgren
(?)
@ 2013-07-17 16:41 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 16:41 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
Hi,
On 07/17/2013 06:32 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>> when uart3/4 state is switched from active to idle and back by Runtime
>> PM or during system suspend.
>
> This is good for testing code, but should not be merged because
> omap4 has the iopad wake-ups available for uarts. So those can
> be always enabled.
In this case, 2 IRQ will be received per each UART RX event - one from
PRCM and from UART - and that's not good from PM perspective (It will
affect on CPUIdle and CPUFreq at least).
-- log when wake-up enabled always
106: 729 0 GIC 106 OMAP UART2
115: 13 0 GIC 115 mmc0
118: 68 0 GIC 118 mmc1
151: 0 0 GIC 151 twl6040
361: 322 0 PRCM hwmod_io
^^^^
-- log when wake-up enabled only when UART3 (console) is active
106: 1104 0 GIC 106 OMAP UART2
115: 13 0 GIC 115 mmc0
118: 68 0 GIC 118 mmc1
151: 0 0 GIC 151 twl6040
361: 7 0 PRCM hwmod_io
The same is valid for all devices which will leave WAKEUP_EN set
all the time (
>
>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>> @@ -181,18 +181,40 @@
>> pinctrl-single,pins = <
>> 0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
>> 0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
>> - 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
>> 0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
>> >;
>> };
>
> This just need to become:
> pinctrl-single,pins = <
> ...
> 0x104 (WAKEUP_EN | PIN_INPUT_MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
> ...
> >;
> interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> And so on. For am33xx you need to remux things to GPIO for the wake-up
> events, so you should maybe test on that instead.
>
> Regards,
>
> Tony
>
Regards,
- grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-17 16:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 16:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 07/17/2013 06:32 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>> when uart3/4 state is switched from active to idle and back by Runtime
>> PM or during system suspend.
>
> This is good for testing code, but should not be merged because
> omap4 has the iopad wake-ups available for uarts. So those can
> be always enabled.
In this case, 2 IRQ will be received per each UART RX event - one from
PRCM and from UART - and that's not good from PM perspective (It will
affect on CPUIdle and CPUFreq at least).
-- log when wake-up enabled always
106: 729 0 GIC 106 OMAP UART2
115: 13 0 GIC 115 mmc0
118: 68 0 GIC 118 mmc1
151: 0 0 GIC 151 twl6040
361: 322 0 PRCM hwmod_io
^^^^
-- log when wake-up enabled only when UART3 (console) is active
106: 1104 0 GIC 106 OMAP UART2
115: 13 0 GIC 115 mmc0
118: 68 0 GIC 118 mmc1
151: 0 0 GIC 151 twl6040
361: 7 0 PRCM hwmod_io
The same is valid for all devices which will leave WAKEUP_EN set
all the time (
>
>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>> @@ -181,18 +181,40 @@
>> pinctrl-single,pins = <
>> 0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
>> 0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
>> - 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
>> 0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
>> >;
>> };
>
> This just need to become:
> pinctrl-single,pins = <
> ...
> 0x104 (WAKEUP_EN | PIN_INPUT_MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
> ...
> >;
> interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> And so on. For am33xx you need to remux things to GPIO for the wake-up
> events, so you should maybe test on that instead.
>
> Regards,
>
> Tony
>
Regards,
- grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-17 16:41 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 16:41 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
Hi,
On 07/17/2013 06:32 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>> when uart3/4 state is switched from active to idle and back by Runtime
>> PM or during system suspend.
>
> This is good for testing code, but should not be merged because
> omap4 has the iopad wake-ups available for uarts. So those can
> be always enabled.
In this case, 2 IRQ will be received per each UART RX event - one from
PRCM and from UART - and that's not good from PM perspective (It will
affect on CPUIdle and CPUFreq at least).
-- log when wake-up enabled always
106: 729 0 GIC 106 OMAP UART2
115: 13 0 GIC 115 mmc0
118: 68 0 GIC 118 mmc1
151: 0 0 GIC 151 twl6040
361: 322 0 PRCM hwmod_io
^^^^
-- log when wake-up enabled only when UART3 (console) is active
106: 1104 0 GIC 106 OMAP UART2
115: 13 0 GIC 115 mmc0
118: 68 0 GIC 118 mmc1
151: 0 0 GIC 151 twl6040
361: 7 0 PRCM hwmod_io
The same is valid for all devices which will leave WAKEUP_EN set
all the time (
>
>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>> @@ -181,18 +181,40 @@
>> pinctrl-single,pins = <
>> 0x100 (PIN_INPUT_PULLUP | MUX_MODE0) /* uart3_cts_rctx.uart3_cts_rctx */
>> 0x102 (PIN_OUTPUT | MUX_MODE0) /* uart3_rts_sd.uart3_rts_sd */
>> - 0x104 (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
>> 0x106 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
>> >;
>> };
>
> This just need to become:
> pinctrl-single,pins = <
> ...
> 0x104 (WAKEUP_EN | PIN_INPUT_MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
> ...
> >;
> interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> And so on. For am33xx you need to remux things to GPIO for the wake-up
> events, so you should maybe test on that instead.
>
> Regards,
>
> Tony
>
Regards,
- grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-18 8:09 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-18 8:09 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 09:48]:
> Hi,
>
> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
> >* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> >>Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
> >>when uart3/4 state is switched from active to idle and back by Runtime
> >>PM or during system suspend.
> >
> >This is good for testing code, but should not be merged because
> >omap4 has the iopad wake-ups available for uarts. So those can
> >be always enabled.
>
> In this case, 2 IRQ will be received per each UART RX event - one from
> PRCM and from UART - and that's not good from PM perspective (It will
> affect on CPUIdle and CPUFreq at least).
Oh I see, that's because I accidentally left the debug code enabled
to make it easier to test the wake-up events without having to
have working off-idle. The wake flags can be kept on always for
sure.
The patch below should sort out the issue of getting wake-up interrupts
during runtime as long as you don't have DEBUG defined.
Regards,
Tony
--- a/drivers/pinctrl/pinctrl-single-omap.c
+++ b/drivers/pinctrl/pinctrl-single-omap.c
@@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
generic_handle_irq(wakeirq);
}
-
+#ifdef DEBUG
+ /*
+ * This enables wake-up interrupts during runtime also
+ * causing duplicate interrupts. But it also makes debugging
+ * the wake-up events easy as deeper idle states often are
+ * not working for new devices while the drivers are being
+ * developed.
+ */
if (pcso->reconfigure_io_chain)
pcso->reconfigure_io_chain();
+#endif
return IRQ_HANDLED;
}
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-18 8:09 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-18 8:09 UTC (permalink / raw)
To: linux-arm-kernel
* Grygorii Strashko <grygorii.strashko@ti.com> [130717 09:48]:
> Hi,
>
> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
> >* Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
> >>Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
> >>when uart3/4 state is switched from active to idle and back by Runtime
> >>PM or during system suspend.
> >
> >This is good for testing code, but should not be merged because
> >omap4 has the iopad wake-ups available for uarts. So those can
> >be always enabled.
>
> In this case, 2 IRQ will be received per each UART RX event - one from
> PRCM and from UART - and that's not good from PM perspective (It will
> affect on CPUIdle and CPUFreq at least).
Oh I see, that's because I accidentally left the debug code enabled
to make it easier to test the wake-up events without having to
have working off-idle. The wake flags can be kept on always for
sure.
The patch below should sort out the issue of getting wake-up interrupts
during runtime as long as you don't have DEBUG defined.
Regards,
Tony
--- a/drivers/pinctrl/pinctrl-single-omap.c
+++ b/drivers/pinctrl/pinctrl-single-omap.c
@@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
generic_handle_irq(wakeirq);
}
-
+#ifdef DEBUG
+ /*
+ * This enables wake-up interrupts during runtime also
+ * causing duplicate interrupts. But it also makes debugging
+ * the wake-up events easy as deeper idle states often are
+ * not working for new devices while the drivers are being
+ * developed.
+ */
if (pcso->reconfigure_io_chain)
pcso->reconfigure_io_chain();
+#endif
return IRQ_HANDLED;
}
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-18 8:09 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-18 8:09 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benoit Cousson,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
* Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org> [130717 09:48]:
> Hi,
>
> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
> >* Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org> [130717 04:49]:
> >>Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
> >>when uart3/4 state is switched from active to idle and back by Runtime
> >>PM or during system suspend.
> >
> >This is good for testing code, but should not be merged because
> >omap4 has the iopad wake-ups available for uarts. So those can
> >be always enabled.
>
> In this case, 2 IRQ will be received per each UART RX event - one from
> PRCM and from UART - and that's not good from PM perspective (It will
> affect on CPUIdle and CPUFreq at least).
Oh I see, that's because I accidentally left the debug code enabled
to make it easier to test the wake-up events without having to
have working off-idle. The wake flags can be kept on always for
sure.
The patch below should sort out the issue of getting wake-up interrupts
during runtime as long as you don't have DEBUG defined.
Regards,
Tony
--- a/drivers/pinctrl/pinctrl-single-omap.c
+++ b/drivers/pinctrl/pinctrl-single-omap.c
@@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
generic_handle_irq(wakeirq);
}
-
+#ifdef DEBUG
+ /*
+ * This enables wake-up interrupts during runtime also
+ * causing duplicate interrupts. But it also makes debugging
+ * the wake-up events easy as deeper idle states often are
+ * not working for new devices while the drivers are being
+ * developed.
+ */
if (pcso->reconfigure_io_chain)
pcso->reconfigure_io_chain();
+#endif
return IRQ_HANDLED;
}
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
2013-07-18 8:09 ` Tony Lindgren
(?)
@ 2013-07-18 8:54 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 8:54 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
On 07/18/2013 11:09 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 09:48]:
>> Hi,
>>
>> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>>>> when uart3/4 state is switched from active to idle and back by Runtime
>>>> PM or during system suspend.
>>>
>>> This is good for testing code, but should not be merged because
>>> omap4 has the iopad wake-ups available for uarts. So those can
>>> be always enabled.
>>
>> In this case, 2 IRQ will be received per each UART RX event - one from
>> PRCM and from UART - and that's not good from PM perspective (It will
>> affect on CPUIdle and CPUFreq at least).
>
> Oh I see, that's because I accidentally left the debug code enabled
> to make it easier to test the wake-up events without having to
> have working off-idle. The wake flags can be kept on always for
> sure.
>
> The patch below should sort out the issue of getting wake-up interrupts
> during runtime as long as you don't have DEBUG defined.
>
> Regards,
>
> Tony
>
> --- a/drivers/pinctrl/pinctrl-single-omap.c
> +++ b/drivers/pinctrl/pinctrl-single-omap.c
> @@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
> if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
> generic_handle_irq(wakeirq);
> }
> -
> +#ifdef DEBUG
Don't think it's debug code - IO chain need to be rearmed after each
PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
OMAP4, OMAP5 requires more complex handling(( ).
> + /*
> + * This enables wake-up interrupts during runtime also
> + * causing duplicate interrupts. But it also makes debugging
> + * the wake-up events easy as deeper idle states often are
> + * not working for new devices while the drivers are being
> + * developed.
> + */
> if (pcso->reconfigure_io_chain)
> pcso->reconfigure_io_chain();
> +#endif
>
> return IRQ_HANDLED;
> }
>
I didn't pick up your padconf patches yet -seems i need to be in sync :)
Below the diff I used to verify IO wake up (It follows old IO daisy
chain hanlding models in hwmod before DT):
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -574,7 +574,7 @@ odbs_exit:
return ERR_PTR(ret);
}
-
+#include "prm44xx.h"
#ifdef CONFIG_PM_RUNTIME
static int _od_runtime_suspend(struct device *dev)
{
@@ -586,6 +586,7 @@ static int _od_runtime_suspend(struct device *dev)
if (!ret) {
omap_device_idle(pdev);
pinctrl_pm_select_idle_state(dev);
+ omap44xx_prm_reconfigure_io_chain();
}
return ret;
@@ -596,6 +597,7 @@ static int _od_runtime_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
pinctrl_pm_select_active_state(dev);
+ omap44xx_prm_reconfigure_io_chain();
omap_device_enable(pdev);
diff --git a/arch/arm/mach-omap2/prm_common.c
b/arch/arm/mach-omap2/prm_common.c
index 228b850..5db073a 100644
--- a/arch/arm/mach-omap2/prm_common.c
+++ b/arch/arm/mach-omap2/prm_common.c
@@ -79,7 +79,7 @@ static void omap_prcm_events_filter_priority(unsigned
long *events,
events[i] ^= priority_events[i];
}
}
-
+#include "prm44xx.h"
/*
* PRCM Interrupt Handler
*
@@ -144,6 +144,7 @@ static void omap_prcm_irq_handler(unsigned int irq,
struct irq_desc *desc)
chip->irq_unmask(&desc->irq_data);
prcm_irq_setup->ocp_barrier(); /* avoid spurious IRQs */
+ omap44xx_prm_reconfigure_io_chain();
}
^ permalink raw reply related [flat|nested] 53+ messages in thread* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-18 8:54 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 8:54 UTC (permalink / raw)
To: linux-arm-kernel
On 07/18/2013 11:09 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 09:48]:
>> Hi,
>>
>> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>>>> when uart3/4 state is switched from active to idle and back by Runtime
>>>> PM or during system suspend.
>>>
>>> This is good for testing code, but should not be merged because
>>> omap4 has the iopad wake-ups available for uarts. So those can
>>> be always enabled.
>>
>> In this case, 2 IRQ will be received per each UART RX event - one from
>> PRCM and from UART - and that's not good from PM perspective (It will
>> affect on CPUIdle and CPUFreq at least).
>
> Oh I see, that's because I accidentally left the debug code enabled
> to make it easier to test the wake-up events without having to
> have working off-idle. The wake flags can be kept on always for
> sure.
>
> The patch below should sort out the issue of getting wake-up interrupts
> during runtime as long as you don't have DEBUG defined.
>
> Regards,
>
> Tony
>
> --- a/drivers/pinctrl/pinctrl-single-omap.c
> +++ b/drivers/pinctrl/pinctrl-single-omap.c
> @@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
> if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
> generic_handle_irq(wakeirq);
> }
> -
> +#ifdef DEBUG
Don't think it's debug code - IO chain need to be rearmed after each
PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
OMAP4, OMAP5 requires more complex handling(( ).
> + /*
> + * This enables wake-up interrupts during runtime also
> + * causing duplicate interrupts. But it also makes debugging
> + * the wake-up events easy as deeper idle states often are
> + * not working for new devices while the drivers are being
> + * developed.
> + */
> if (pcso->reconfigure_io_chain)
> pcso->reconfigure_io_chain();
> +#endif
>
> return IRQ_HANDLED;
> }
>
I didn't pick up your padconf patches yet -seems i need to be in sync :)
Below the diff I used to verify IO wake up (It follows old IO daisy
chain hanlding models in hwmod before DT):
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -574,7 +574,7 @@ odbs_exit:
return ERR_PTR(ret);
}
-
+#include "prm44xx.h"
#ifdef CONFIG_PM_RUNTIME
static int _od_runtime_suspend(struct device *dev)
{
@@ -586,6 +586,7 @@ static int _od_runtime_suspend(struct device *dev)
if (!ret) {
omap_device_idle(pdev);
pinctrl_pm_select_idle_state(dev);
+ omap44xx_prm_reconfigure_io_chain();
}
return ret;
@@ -596,6 +597,7 @@ static int _od_runtime_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
pinctrl_pm_select_active_state(dev);
+ omap44xx_prm_reconfigure_io_chain();
omap_device_enable(pdev);
diff --git a/arch/arm/mach-omap2/prm_common.c
b/arch/arm/mach-omap2/prm_common.c
index 228b850..5db073a 100644
--- a/arch/arm/mach-omap2/prm_common.c
+++ b/arch/arm/mach-omap2/prm_common.c
@@ -79,7 +79,7 @@ static void omap_prcm_events_filter_priority(unsigned
long *events,
events[i] ^= priority_events[i];
}
}
-
+#include "prm44xx.h"
/*
* PRCM Interrupt Handler
*
@@ -144,6 +144,7 @@ static void omap_prcm_irq_handler(unsigned int irq,
struct irq_desc *desc)
chip->irq_unmask(&desc->irq_data);
prcm_irq_setup->ocp_barrier(); /* avoid spurious IRQs */
+ omap44xx_prm_reconfigure_io_chain();
}
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-18 8:54 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 8:54 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
On 07/18/2013 11:09 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 09:48]:
>> Hi,
>>
>> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>>>> when uart3/4 state is switched from active to idle and back by Runtime
>>>> PM or during system suspend.
>>>
>>> This is good for testing code, but should not be merged because
>>> omap4 has the iopad wake-ups available for uarts. So those can
>>> be always enabled.
>>
>> In this case, 2 IRQ will be received per each UART RX event - one from
>> PRCM and from UART - and that's not good from PM perspective (It will
>> affect on CPUIdle and CPUFreq at least).
>
> Oh I see, that's because I accidentally left the debug code enabled
> to make it easier to test the wake-up events without having to
> have working off-idle. The wake flags can be kept on always for
> sure.
>
> The patch below should sort out the issue of getting wake-up interrupts
> during runtime as long as you don't have DEBUG defined.
>
> Regards,
>
> Tony
>
> --- a/drivers/pinctrl/pinctrl-single-omap.c
> +++ b/drivers/pinctrl/pinctrl-single-omap.c
> @@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
> if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
> generic_handle_irq(wakeirq);
> }
> -
> +#ifdef DEBUG
Don't think it's debug code - IO chain need to be rearmed after each
PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
OMAP4, OMAP5 requires more complex handling(( ).
> + /*
> + * This enables wake-up interrupts during runtime also
> + * causing duplicate interrupts. But it also makes debugging
> + * the wake-up events easy as deeper idle states often are
> + * not working for new devices while the drivers are being
> + * developed.
> + */
> if (pcso->reconfigure_io_chain)
> pcso->reconfigure_io_chain();
> +#endif
>
> return IRQ_HANDLED;
> }
>
I didn't pick up your padconf patches yet -seems i need to be in sync :)
Below the diff I used to verify IO wake up (It follows old IO daisy
chain hanlding models in hwmod before DT):
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -574,7 +574,7 @@ odbs_exit:
return ERR_PTR(ret);
}
-
+#include "prm44xx.h"
#ifdef CONFIG_PM_RUNTIME
static int _od_runtime_suspend(struct device *dev)
{
@@ -586,6 +586,7 @@ static int _od_runtime_suspend(struct device *dev)
if (!ret) {
omap_device_idle(pdev);
pinctrl_pm_select_idle_state(dev);
+ omap44xx_prm_reconfigure_io_chain();
}
return ret;
@@ -596,6 +597,7 @@ static int _od_runtime_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
pinctrl_pm_select_active_state(dev);
+ omap44xx_prm_reconfigure_io_chain();
omap_device_enable(pdev);
diff --git a/arch/arm/mach-omap2/prm_common.c
b/arch/arm/mach-omap2/prm_common.c
index 228b850..5db073a 100644
--- a/arch/arm/mach-omap2/prm_common.c
+++ b/arch/arm/mach-omap2/prm_common.c
@@ -79,7 +79,7 @@ static void omap_prcm_events_filter_priority(unsigned
long *events,
events[i] ^= priority_events[i];
}
}
-
+#include "prm44xx.h"
/*
* PRCM Interrupt Handler
*
@@ -144,6 +144,7 @@ static void omap_prcm_irq_handler(unsigned int irq,
struct irq_desc *desc)
chip->irq_unmask(&desc->irq_data);
prcm_irq_setup->ocp_barrier(); /* avoid spurious IRQs */
+ omap44xx_prm_reconfigure_io_chain();
}
^ permalink raw reply related [flat|nested] 53+ messages in thread* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
2013-07-18 8:54 ` Grygorii Strashko
@ 2013-07-18 9:04 ` Tony Lindgren
-1 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-18 9:04 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
* Grygorii Strashko <grygorii.strashko@ti.com> [130718 02:01]:
> On 07/18/2013 11:09 AM, Tony Lindgren wrote:
>
> Don't think it's debug code - IO chain need to be rearmed after each
> PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
> OMAP4, OMAP5 requires more complex handling(( ).
Nope, only after the mux register changes. I've verified it on
am3730 with off-idle for both serial and wl12xx.
> I didn't pick up your padconf patches yet -seems i need to be in sync :)
Well you need those for proper wake-up event support..
> Below the diff I used to verify IO wake up (It follows old IO daisy
> chain hanlding models in hwmod before DT):
..then these changes not needed with the pinctrl-single changes.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-18 9:04 ` Tony Lindgren
0 siblings, 0 replies; 53+ messages in thread
From: Tony Lindgren @ 2013-07-18 9:04 UTC (permalink / raw)
To: linux-arm-kernel
* Grygorii Strashko <grygorii.strashko@ti.com> [130718 02:01]:
> On 07/18/2013 11:09 AM, Tony Lindgren wrote:
>
> Don't think it's debug code - IO chain need to be rearmed after each
> PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
> OMAP4, OMAP5 requires more complex handling(( ).
Nope, only after the mux register changes. I've verified it on
am3730 with off-idle for both serial and wl12xx.
> I didn't pick up your padconf patches yet -seems i need to be in sync :)
Well you need those for proper wake-up event support..
> Below the diff I used to verify IO wake up (It follows old IO daisy
> chain hanlding models in hwmod before DT):
..then these changes not needed with the pinctrl-single changes.
Regards,
Tony
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
2013-07-18 9:04 ` Tony Lindgren
(?)
@ 2013-07-18 12:01 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 12:01 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
Hi Tony,
On 07/18/2013 12:04 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130718 02:01]:
>> On 07/18/2013 11:09 AM, Tony Lindgren wrote:
>>
>> Don't think it's debug code - IO chain need to be rearmed after each
>> PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
>> OMAP4, OMAP5 requires more complex handling(( ).
>
> Nope, only after the mux register changes. I've verified it on
> am3730 with off-idle for both serial and wl12xx.
Unfortunately, there is a possibility to lose wake up events in case if
IO daisy chain will be rearmed (this will clean up all WAKEUPEVENT bits)
while there is some pending WAKEUP event present. In this case,
pcs_omap_handle_irq() will not call generic_handle_irq(wakeirq).
The below patch contains explanation of such kind of issue we've solved
in K3.4
http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5ff316db224a2c3c23bfe44261275d520b4f78bb
Currently, in Mainline the same is possible on OMAP4 (which is SMP) if
some Device will be switched to idle and rearm IO chain while there is
pending WAKEUP event form USB for example. So, IO rearming need to be
delayed until PRCM irq will be served served and PRCM irq handler should
finally rearm IO daisy chain.
>
>> I didn't pick up your padconf patches yet -seems i need to be in sync :)
>
> Well you need those for proper wake-up event support..
>
>> Below the diff I used to verify IO wake up (It follows old IO daisy
>> chain hanlding models in hwmod before DT):
>
> ..then these changes not needed with the pinctrl-single changes.
>
> Regards,
>
> Tony
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-18 12:01 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 12:01 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tony,
On 07/18/2013 12:04 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130718 02:01]:
>> On 07/18/2013 11:09 AM, Tony Lindgren wrote:
>>
>> Don't think it's debug code - IO chain need to be rearmed after each
>> PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
>> OMAP4, OMAP5 requires more complex handling(( ).
>
> Nope, only after the mux register changes. I've verified it on
> am3730 with off-idle for both serial and wl12xx.
Unfortunately, there is a possibility to lose wake up events in case if
IO daisy chain will be rearmed (this will clean up all WAKEUPEVENT bits)
while there is some pending WAKEUP event present. In this case,
pcs_omap_handle_irq() will not call generic_handle_irq(wakeirq).
The below patch contains explanation of such kind of issue we've solved
in K3.4
http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5ff316db224a2c3c23bfe44261275d520b4f78bb
Currently, in Mainline the same is possible on OMAP4 (which is SMP) if
some Device will be switched to idle and rearm IO chain while there is
pending WAKEUP event form USB for example. So, IO rearming need to be
delayed until PRCM irq will be served served and PRCM irq handler should
finally rearm IO daisy chain.
>
>> I didn't pick up your padconf patches yet -seems i need to be in sync :)
>
> Well you need those for proper wake-up event support..
>
>> Below the diff I used to verify IO wake up (It follows old IO daisy
>> chain hanlding models in hwmod before DT):
>
> ..then these changes not needed with the pinctrl-single changes.
>
> Regards,
>
> Tony
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
@ 2013-07-18 12:01 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-18 12:01 UTC (permalink / raw)
To: Tony Lindgren
Cc: Kevin Hilman, linux-arm-kernel, linux-omap, linux-kernel,
devicetree-discuss, Benoit Cousson, Linus Walleij,
Stephen Warren
Hi Tony,
On 07/18/2013 12:04 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130718 02:01]:
>> On 07/18/2013 11:09 AM, Tony Lindgren wrote:
>>
>> Don't think it's debug code - IO chain need to be rearmed after each
>> PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
>> OMAP4, OMAP5 requires more complex handling(( ).
>
> Nope, only after the mux register changes. I've verified it on
> am3730 with off-idle for both serial and wl12xx.
Unfortunately, there is a possibility to lose wake up events in case if
IO daisy chain will be rearmed (this will clean up all WAKEUPEVENT bits)
while there is some pending WAKEUP event present. In this case,
pcs_omap_handle_irq() will not call generic_handle_irq(wakeirq).
The below patch contains explanation of such kind of issue we've solved
in K3.4
http://git.omapzoom.org/?p=kernel/omap.git;a=commit;h=5ff316db224a2c3c23bfe44261275d520b4f78bb
Currently, in Mainline the same is possible on OMAP4 (which is SMP) if
some Device will be switched to idle and rearm IO chain while there is
pending WAKEUP event form USB for example. So, IO rearming need to be
delayed until PRCM irq will be served served and PRCM irq handler should
finally rearm IO daisy chain.
>
>> I didn't pick up your padconf patches yet -seems i need to be in sync :)
>
> Well you need those for proper wake-up event support..
>
>> Below the diff I used to verify IO wake up (It follows old IO daisy
>> chain hanlding models in hwmod before DT):
>
> ..then these changes not needed with the pinctrl-single changes.
>
> Regards,
>
> Tony
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
2013-07-17 11:41 ` Grygorii Strashko
(?)
@ 2013-07-17 11:57 ` Roger Quadros
-1 siblings, 0 replies; 53+ messages in thread
From: Roger Quadros @ 2013-07-17 11:57 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Tony Lindgren, Kevin Hilman, linux-arm-kernel, linux-omap,
linux-kernel, devicetree-discuss, Linus Walleij, Stephen Warren
Hi Grygorii,
On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
> Hi Tony, Kevin
>
> This patch series introduces dynamic pinctrl handling in OMAP device framework
> in the same way as it was before switching to DT.
> This allow OMAP devices driver's developers to simply add dynamic pinctrl
> handling for "default", "active", "idle", "sleep" PIN states in their drivers
> by modifying DT definitions only - no modifications in drivers code are not needed.
>
Overall I like the idea but can we make a provision for device drivers to override
this default pin state handling?
The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
as it uses IO daisy chaining to implement wakeup.
So depending on whether wakeup needs to be enabled or not I must be able to chose whether
I put the pin in just "sleep" state or "sleep with wakeup" state.
cheers,
-roger
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
@ 2013-07-17 11:57 ` Roger Quadros
0 siblings, 0 replies; 53+ messages in thread
From: Roger Quadros @ 2013-07-17 11:57 UTC (permalink / raw)
To: linux-arm-kernel
Hi Grygorii,
On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
> Hi Tony, Kevin
>
> This patch series introduces dynamic pinctrl handling in OMAP device framework
> in the same way as it was before switching to DT.
> This allow OMAP devices driver's developers to simply add dynamic pinctrl
> handling for "default", "active", "idle", "sleep" PIN states in their drivers
> by modifying DT definitions only - no modifications in drivers code are not needed.
>
Overall I like the idea but can we make a provision for device drivers to override
this default pin state handling?
The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
as it uses IO daisy chaining to implement wakeup.
So depending on whether wakeup needs to be enabled or not I must be able to chose whether
I put the pin in just "sleep" state or "sleep with wakeup" state.
cheers,
-roger
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
@ 2013-07-17 11:57 ` Roger Quadros
0 siblings, 0 replies; 53+ messages in thread
From: Roger Quadros @ 2013-07-17 11:57 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, Stephen Warren, Tony Lindgren, devicetree-discuss,
linux-kernel, linux-omap, Linus Walleij, linux-arm-kernel
Hi Grygorii,
On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
> Hi Tony, Kevin
>
> This patch series introduces dynamic pinctrl handling in OMAP device framework
> in the same way as it was before switching to DT.
> This allow OMAP devices driver's developers to simply add dynamic pinctrl
> handling for "default", "active", "idle", "sleep" PIN states in their drivers
> by modifying DT definitions only - no modifications in drivers code are not needed.
>
Overall I like the idea but can we make a provision for device drivers to override
this default pin state handling?
The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
as it uses IO daisy chaining to implement wakeup.
So depending on whether wakeup needs to be enabled or not I must be able to chose whether
I put the pin in just "sleep" state or "sleep with wakeup" state.
cheers,
-roger
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
2013-07-17 11:57 ` Roger Quadros
(?)
@ 2013-07-17 12:30 ` Grygorii Strashko
-1 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 12:30 UTC (permalink / raw)
To: Roger Quadros
Cc: Tony Lindgren, Kevin Hilman, linux-arm-kernel, linux-omap,
linux-kernel, devicetree-discuss, Linus Walleij, Stephen Warren
On 07/17/2013 02:57 PM, Roger Quadros wrote:
> Hi Grygorii,
>
> On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
>> Hi Tony, Kevin
>>
>> This patch series introduces dynamic pinctrl handling in OMAP device framework
>> in the same way as it was before switching to DT.
>> This allow OMAP devices driver's developers to simply add dynamic pinctrl
>> handling for "default", "active", "idle", "sleep" PIN states in their drivers
>> by modifying DT definitions only - no modifications in drivers code are not needed.
>>
>
> Overall I like the idea but can we make a provision for device drivers to override
> this default pin state handling?
>
> The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
> as it uses IO daisy chaining to implement wakeup.
> So depending on whether wakeup needs to be enabled or not I must be able to chose whether
> I put the pin in just "sleep" state or "sleep with wakeup" state.
I think, in this case you can't use default behavior and need to define
custom pins states like "sleep_wakeup"/"sleep_no_wakeup" and do not
define pins state with name "sleep', so Device core and OMAP device
framework will not touch your pins.
>
> cheers,
> -roger
>
Regards,
-grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
@ 2013-07-17 12:30 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 12:30 UTC (permalink / raw)
To: linux-arm-kernel
On 07/17/2013 02:57 PM, Roger Quadros wrote:
> Hi Grygorii,
>
> On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
>> Hi Tony, Kevin
>>
>> This patch series introduces dynamic pinctrl handling in OMAP device framework
>> in the same way as it was before switching to DT.
>> This allow OMAP devices driver's developers to simply add dynamic pinctrl
>> handling for "default", "active", "idle", "sleep" PIN states in their drivers
>> by modifying DT definitions only - no modifications in drivers code are not needed.
>>
>
> Overall I like the idea but can we make a provision for device drivers to override
> this default pin state handling?
>
> The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
> as it uses IO daisy chaining to implement wakeup.
> So depending on whether wakeup needs to be enabled or not I must be able to chose whether
> I put the pin in just "sleep" state or "sleep with wakeup" state.
I think, in this case you can't use default behavior and need to define
custom pins states like "sleep_wakeup"/"sleep_no_wakeup" and do not
define pins state with name "sleep', so Device core and OMAP device
framework will not touch your pins.
>
> cheers,
> -roger
>
Regards,
-grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
@ 2013-07-17 12:30 ` Grygorii Strashko
0 siblings, 0 replies; 53+ messages in thread
From: Grygorii Strashko @ 2013-07-17 12:30 UTC (permalink / raw)
To: Roger Quadros
Cc: Kevin Hilman, Stephen Warren, Tony Lindgren, devicetree-discuss,
linux-kernel, linux-omap, Linus Walleij, linux-arm-kernel
On 07/17/2013 02:57 PM, Roger Quadros wrote:
> Hi Grygorii,
>
> On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
>> Hi Tony, Kevin
>>
>> This patch series introduces dynamic pinctrl handling in OMAP device framework
>> in the same way as it was before switching to DT.
>> This allow OMAP devices driver's developers to simply add dynamic pinctrl
>> handling for "default", "active", "idle", "sleep" PIN states in their drivers
>> by modifying DT definitions only - no modifications in drivers code are not needed.
>>
>
> Overall I like the idea but can we make a provision for device drivers to override
> this default pin state handling?
>
> The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
> as it uses IO daisy chaining to implement wakeup.
> So depending on whether wakeup needs to be enabled or not I must be able to chose whether
> I put the pin in just "sleep" state or "sleep with wakeup" state.
I think, in this case you can't use default behavior and need to define
custom pins states like "sleep_wakeup"/"sleep_no_wakeup" and do not
define pins state with name "sleep', so Device core and OMAP device
framework will not touch your pins.
>
> cheers,
> -roger
>
Regards,
-grygorii
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
2013-07-17 12:30 ` Grygorii Strashko
(?)
@ 2013-07-18 6:44 ` Roger Quadros
-1 siblings, 0 replies; 53+ messages in thread
From: Roger Quadros @ 2013-07-18 6:44 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Tony Lindgren, Kevin Hilman, linux-arm-kernel, linux-omap,
linux-kernel, devicetree-discuss, Linus Walleij, Stephen Warren
On 07/17/2013 03:30 PM, Grygorii Strashko wrote:
> On 07/17/2013 02:57 PM, Roger Quadros wrote:
>> Hi Grygorii,
>>
>> On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
>>> Hi Tony, Kevin
>>>
>>> This patch series introduces dynamic pinctrl handling in OMAP device framework
>>> in the same way as it was before switching to DT.
>>> This allow OMAP devices driver's developers to simply add dynamic pinctrl
>>> handling for "default", "active", "idle", "sleep" PIN states in their drivers
>>> by modifying DT definitions only - no modifications in drivers code are not needed.
>>>
>>
>> Overall I like the idea but can we make a provision for device drivers to override
>> this default pin state handling?
>>
>> The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
>> as it uses IO daisy chaining to implement wakeup.
>> So depending on whether wakeup needs to be enabled or not I must be able to chose whether
>> I put the pin in just "sleep" state or "sleep with wakeup" state.
>
> I think, in this case you can't use default behavior and need to define
> custom pins states like "sleep_wakeup"/"sleep_no_wakeup" and do not
> define pins state with name "sleep', so Device core and OMAP device
> framework will not touch your pins.
Yes, I think this should be fine. Thanks.
cheers,
-roger
^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
@ 2013-07-18 6:44 ` Roger Quadros
0 siblings, 0 replies; 53+ messages in thread
From: Roger Quadros @ 2013-07-18 6:44 UTC (permalink / raw)
To: linux-arm-kernel
On 07/17/2013 03:30 PM, Grygorii Strashko wrote:
> On 07/17/2013 02:57 PM, Roger Quadros wrote:
>> Hi Grygorii,
>>
>> On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
>>> Hi Tony, Kevin
>>>
>>> This patch series introduces dynamic pinctrl handling in OMAP device framework
>>> in the same way as it was before switching to DT.
>>> This allow OMAP devices driver's developers to simply add dynamic pinctrl
>>> handling for "default", "active", "idle", "sleep" PIN states in their drivers
>>> by modifying DT definitions only - no modifications in drivers code are not needed.
>>>
>>
>> Overall I like the idea but can we make a provision for device drivers to override
>> this default pin state handling?
>>
>> The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
>> as it uses IO daisy chaining to implement wakeup.
>> So depending on whether wakeup needs to be enabled or not I must be able to chose whether
>> I put the pin in just "sleep" state or "sleep with wakeup" state.
>
> I think, in this case you can't use default behavior and need to define
> custom pins states like "sleep_wakeup"/"sleep_no_wakeup" and do not
> define pins state with name "sleep', so Device core and OMAP device
> framework will not touch your pins.
Yes, I think this should be fine. Thanks.
cheers,
-roger
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling
@ 2013-07-18 6:44 ` Roger Quadros
0 siblings, 0 replies; 53+ messages in thread
From: Roger Quadros @ 2013-07-18 6:44 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Kevin Hilman, Stephen Warren, Tony Lindgren, devicetree-discuss,
linux-kernel, linux-omap, Linus Walleij, linux-arm-kernel
On 07/17/2013 03:30 PM, Grygorii Strashko wrote:
> On 07/17/2013 02:57 PM, Roger Quadros wrote:
>> Hi Grygorii,
>>
>> On 07/17/2013 02:41 PM, Grygorii Strashko wrote:
>>> Hi Tony, Kevin
>>>
>>> This patch series introduces dynamic pinctrl handling in OMAP device framework
>>> in the same way as it was before switching to DT.
>>> This allow OMAP devices driver's developers to simply add dynamic pinctrl
>>> handling for "default", "active", "idle", "sleep" PIN states in their drivers
>>> by modifying DT definitions only - no modifications in drivers code are not needed.
>>>
>>
>> Overall I like the idea but can we make a provision for device drivers to override
>> this default pin state handling?
>>
>> The OMAP EHCI driver is one such special case where the wakeup mechanism is tied to pinctrl states
>> as it uses IO daisy chaining to implement wakeup.
>> So depending on whether wakeup needs to be enabled or not I must be able to chose whether
>> I put the pin in just "sleep" state or "sleep with wakeup" state.
>
> I think, in this case you can't use default behavior and need to define
> custom pins states like "sleep_wakeup"/"sleep_no_wakeup" and do not
> define pins state with name "sleep', so Device core and OMAP device
> framework will not touch your pins.
Yes, I think this should be fine. Thanks.
cheers,
-roger
^ permalink raw reply [flat|nested] 53+ messages in thread