Re: [PATCH 1/4] pinctrl: Remove duplicate code in pinctrl_pm_select_state functions

2013-07-22 Thread Linus Walleij 
On Thu, Jul 18, 2013 at 5:15 PM, Tony Lindgren t...@atomide.com wrote:

 There's no need to duplicate essentially the same functions. Let's
 introduce static int pinctrl_pm_select_state() and make the other
 related functions call that.

 This allows us to add support later on for multiple active states,
 and more optimized dynamic remuxing.

 Note that we still need to export the various pinctrl_pm_select
 functions as we want to keep struct pinctrl_state private to the
 pinctrl code, and cannot replace those with inline functions.

 Cc: Felipe Balbi ba...@ti.com
 Cc: Stephen Warren swar...@wwwdotorg.org
 Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
 Signed-off-by: Tony Lindgren t...@atomide.com

Aha that is how to do it. Stephen complained about it earlier
but I couldn't come up with a good enough refactoring.

Patch applied.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] pinctrl: Remove duplicate code in pinctrl_pm_select_state functions

2013-07-18 Thread Tony Lindgren 
There's no need to duplicate essentially the same functions. Let's
introduce static int pinctrl_pm_select_state() and make the other
related functions call that.

This allows us to add support later on for multiple active states,
and more optimized dynamic remuxing.

Note that we still need to export the various pinctrl_pm_select
functions as we want to keep struct pinctrl_state private to the
pinctrl code, and cannot replace those with inline functions.

Cc: Felipe Balbi ba...@ti.com
Cc: Stephen Warren swar...@wwwdotorg.org
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
 drivers/pinctrl/core.c |   55 
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 5b272bf..a97b717 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1227,23 +1227,36 @@ EXPORT_SYMBOL_GPL(pinctrl_force_default);
 #ifdef CONFIG_PM
 
 /**
- * pinctrl_pm_select_default_state() - select default pinctrl state for PM
+ * pinctrl_pm_select_state() - select pinctrl state for PM
  * @dev: device to select default state for
+ * @state: state to set
  */
-int pinctrl_pm_select_default_state(struct device *dev)
+static int pinctrl_pm_select_state(struct device *dev,
+  struct pinctrl_state *state)
 {
struct dev_pin_info *pins = dev-pins;
int ret;
 
-   if (!pins)
-   return 0;
-   if (IS_ERR(pins-default_state))
-   return 0; /* No default state */
-   ret = pinctrl_select_state(pins-p, pins-default_state);
+   if (IS_ERR(state))
+   return 0; /* No such state */
+   ret = pinctrl_select_state(pins-p, state);
if (ret)
-   dev_err(dev, failed to activate default pinctrl state\n);
+   dev_err(dev, failed to activate pinctrl state %s\n,
+   state-name);
return ret;
 }
+
+/**
+ * pinctrl_pm_select_default_state() - select default pinctrl state for PM
+ * @dev: device to select default state for
+ */
+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);
 
 /**
@@ -1252,17 +1265,10 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
  */
 int pinctrl_pm_select_sleep_state(struct device *dev)
 {
-   struct dev_pin_info *pins = dev-pins;
-   int ret;
-
-   if (!pins)
+   if (!dev-pins)
return 0;
-   if (IS_ERR(pins-sleep_state))
-   return 0; /* No sleep state */
-   ret = pinctrl_select_state(pins-p, pins-sleep_state);
-   if (ret)
-   dev_err(dev, failed to activate pinctrl sleep state\n);
-   return ret;
+
+   return pinctrl_pm_select_state(dev, dev-pins-sleep_state);
 }
 EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
 
@@ -1272,17 +1278,10 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
  */
 int pinctrl_pm_select_idle_state(struct device *dev)
 {
-   struct dev_pin_info *pins = dev-pins;
-   int ret;
-
-   if (!pins)
+   if (!dev-pins)
return 0;
-   if (IS_ERR(pins-idle_state))
-   return 0; /* No idle state */
-   ret = pinctrl_select_state(pins-p, pins-idle_state);
-   if (ret)
-   dev_err(dev, failed to activate pinctrl idle state\n);
-   return ret;
+
+   return pinctrl_pm_select_state(dev, dev-pins-idle_state);
 }
 EXPORT_SYMBOL_GPL(pinctrl_pm_select_idle_state);
 #endif

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

Re: [PATCH 1/4] pinctrl: Remove duplicate code in pinctrl_pm_select_state functions

2013-07-17 Thread Tony Lindgren 
* Grygorii Strashko grygorii.stras...@ti.com [130716 07:32]:
 Hi Tony,
 
 On 07/16/2013 04:41 PM, Tony Lindgren wrote:
 * Grygorii Strashko grygorii.stras...@ti.com [130716 06:22]:
 Hi Tony,
 
 This patch causes boot failure when I've applied my patch
 [RFC] ARM: OMAP2+: omap_device: add pinctrl handling
 https://lkml.org/lkml/2013/6/21/309
 
 on top of it:
 
 Hmm this patch alone removes duplicate code and if it causes
 issues I must have made a typo somewhere.
 
 No typo :) You've removed the check for !dev-pins.

Oh OK, sorry that was not intentional.

 And the failure place is:
 int pinctrl_pm_select_active_state(struct device *dev)
 {
return pinctrl_pm_select_state(dev, dev-pins-active_state);
  here
 }
 
 If I understand everything right, the pinctrl support in Device core
 assumed to be optional - so, It's valid case, when there are no
 definition for device's pinctrl in DT at all.
 
 And in this case dev-pins == NULL and  pinctrl_pm_select_*() API
 should return 0 always.

Care to post your patch as it sounds like you have it fixed
and tested?

Regards,

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

[PATCH 1/4] pinctrl: Remove duplicate code in pinctrl_pm_select_state functions

2013-07-16 Thread Tony Lindgren 
There's no need to duplicate essentially the same functions. Let's
introduce static int pinctrl_pm_select_state() and make the other
related functions call that.

This allows us to add support later on for multiple active states,
and more optimized dynamic remuxing.

Note that we still need to export the various pinctrl_pm_select
functions as we want to keep struct pinctrl_state private to the
pinctrl code, and cannot replace those with inline functions.

Cc: Stephen Warren swar...@wwwdotorg.org
Signed-off-by: Tony Lindgren t...@atomide.com
---
 drivers/pinctrl/core.c |   47 +++
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 5b272bf..bda2c61 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1227,23 +1227,34 @@ EXPORT_SYMBOL_GPL(pinctrl_force_default);
 #ifdef CONFIG_PM
 
 /**
- * pinctrl_pm_select_default_state() - select default pinctrl state for PM
+ * pinctrl_pm_select_state() - select pinctrl state for PM
  * @dev: device to select default state for
+ * @state: state to set
  */
-int pinctrl_pm_select_default_state(struct device *dev)
+static int pinctrl_pm_select_state(struct device *dev, struct pinctrl_state 
*state)
 {
struct dev_pin_info *pins = dev-pins;
int ret;
 
if (!pins)
return 0;
-   if (IS_ERR(pins-default_state))
-   return 0; /* No default state */
-   ret = pinctrl_select_state(pins-p, pins-default_state);
+   if (IS_ERR(state))
+   return 0; /* No such state */
+   ret = pinctrl_select_state(pins-p, state);
if (ret)
-   dev_err(dev, failed to activate default pinctrl state\n);
+   dev_err(dev, failed to activate pinctrl state %s\n,
+   state-name);
return ret;
 }
+
+/**
+ * pinctrl_pm_select_default_state() - select default pinctrl state for PM
+ * @dev: device to select default state for
+ */
+int pinctrl_pm_select_default_state(struct device *dev)
+{
+   return pinctrl_pm_select_state(dev, dev-pins-default_state);
+}
 EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
 
 /**
@@ -1252,17 +1263,7 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
  */
 int pinctrl_pm_select_sleep_state(struct device *dev)
 {
-   struct dev_pin_info *pins = dev-pins;
-   int ret;
-
-   if (!pins)
-   return 0;
-   if (IS_ERR(pins-sleep_state))
-   return 0; /* No sleep state */
-   ret = pinctrl_select_state(pins-p, pins-sleep_state);
-   if (ret)
-   dev_err(dev, failed to activate pinctrl sleep state\n);
-   return ret;
+   return pinctrl_pm_select_state(dev, dev-pins-sleep_state);
 }
 EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
 
@@ -1272,17 +1273,7 @@ EXPORT_SYMBOL_GPL(pinctrl_pm_select_sleep_state);
  */
 int pinctrl_pm_select_idle_state(struct device *dev)
 {
-   struct dev_pin_info *pins = dev-pins;
-   int ret;
-
-   if (!pins)
-   return 0;
-   if (IS_ERR(pins-idle_state))
-   return 0; /* No idle state */
-   ret = pinctrl_select_state(pins-p, pins-idle_state);
-   if (ret)
-   dev_err(dev, failed to activate pinctrl idle state\n);
-   return ret;
+   return pinctrl_pm_select_state(dev, dev-pins-idle_state);
 }
 EXPORT_SYMBOL_GPL(pinctrl_pm_select_idle_state);
 #endif

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

Re: [PATCH 1/4] pinctrl: Remove duplicate code in pinctrl_pm_select_state functions

2013-07-16 Thread Grygorii Strashko 

Hi Tony,

This patch causes boot failure when I've applied my patch
[RFC] ARM: OMAP2+: omap_device: add pinctrl handling
https://lkml.org/lkml/2013/6/21/309

on top of it:

[0.264007] L310 cache controller enabled
[0.268310] l2x0: 16 ways, CACHE_ID 0x41c4, AUX_CTRL 0x7e47, 
Cache size: 1048576 B

[0.282440] CPU1: Booted secondary processor
[0.366760] CPU1: thread -1, cpu 1, socket 0, mpidr 8001
[0.367401] Brought up 2 CPUs
[0.380920] SMP: Total of 2 processors activated (2387.96 BogoMIPS).
[0.387573] CPU: All CPU(s) started in SVC mode.
[0.395324] devtmpfs: initialized
[0.468658] pinctrl core: initialized pinctrl subsystem
[0.477996] regulator-dummy: no parameters
[0.485412] NET: Registered protocol family 16
[0.499145] DMA: preallocated 256 KiB pool for atomic coherent 
allocations
[0.573181] Unable to handle kernel NULL pointer dereference at 
virtual address 0008

[0.581573] pgd = c0004000
[0.584472] [0008] *pgd=
[0.588226] Internal error: Oops: 5 [#1] SMP ARM
[0.593078] Modules linked in:
[0.596313] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
3.11.0-rc1-5-g37e15e6-dirty #100

[0.604888] task: de07f3c0 ti: de08 task.ti: de08
[0.610565] PC is at pinctrl_pm_select_active_state+0x4/0xc
[0.616394] LR is at _od_runtime_resume+0xc/0x20
[0.621215] pc : [c02d4e2c]lr : [c002e930]psr: 6193
[0.621215] sp : de081cc0  ip : 6193  fp : 
[0.633209] r10: de08  r9 : c0067e90  r8 : 0004
[0.638671] r7 : c07800c0  r6 : c002e924  r5 : de0cf4a0  r4 : de0cf410
[0.645477] r3 :   r2 : 0004  r1 : 0470  r0 : de0cf410
[0.652282] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM 
Segment kernel

[0.660003] Control: 10c53c7d  Table: 8000404a  DAC: 0017
[0.665985] Process swapper/0 (pid: 1, stack limit = 0xde080240)
[0.672241] Stack: (0xde081cc0 to 0xde082000)
[0.676818] 1cc0: de0cf410 c0335310 de0cf410 de0cb410 0001 
c0335374 de0cf410 de0cb410
[0.685333] 1ce0: 0001 c033664c c0336b14 0004 c1bbdedc 
  c0507c5c
[0.693847] 1d00: de0cf4a0 de0cf410 6113 0004 c1bbdedc 
  c0336b24
[0.702362] 1d20: de07f3c0 de11ea10 de0cf410 de0cf400 de0cd780 
c02df144 de0cd740 
[0.710845] 1d40: de0cf410 c0d6a634 de0cf410   
c07efe7c  
[0.719360] 1d60:  c032d794 c032d77c c032c554 de0cf410 
c032c784  
[0.727874] 1d80: c0d6a5f0 c032ac98 de07bed8 de0fd714 de0cf410 
de0cf444 c07f65e8 c032c48c
[0.736389] 1da0: de0cf410 de0cf410 c07f65e8 c032b[RFC] ARM: OMAP2+: 
omap_device: add pinctrl handlinga94 de0cf410 de0cf418 de0cb410 c0329ef8
[0.744873] 1dc0: 4a3101ff  0200   
 c076f630 
[0.753387] 1de0: de0cf400  de0cb410 c076f630 de0cb410 
  c042c2dc
[0.761901] 1e00: de0cb410 c1bbdedc   0001 
c042c3dc 0001 c076f630
[0.770385] 1e20:  de0cb410  c0099068 6113 
c080b22c c1bbdd98 0001
[0.778900] 1e40: c076f630 c1bbcaec  c1bbdedc 0001 
c076f630  de0cb410
[0.787414] 1e60:  c042c440 0001 c076f630  
0001  c0099068
[0.795928] 1e80: 6113 c080b22c   c076f630 
c1bbcaec c1bbc09c 
[0.804412] 1ea0:  c076f630  0001  
c042c4e4 0001 c072c37c
[0.812927] 1ec0: c07221e0 de08 c0762648  00a4 
c077979c c071f1c8 c0730a6c
[0.821441] 1ee0: c0760aec c07221fc  c0008978 0083 
de07f3c0 6193 
[0.829925] 1f00: 0006 c07dbcd4 c07dbcd4 6113 c02bf100 
 c07dbcd0 c07dbcd0
[0.838439] 1f20:  c0507c5c 0002 de08 c06f1920 
c1bc531d 00a4 c00655ec
[0.846954] 1f40: c06b2bd8 c06f0ee0 0003 0003 6113 
c0762668 0003 c0762648
[0.855468] 1f60: c0817500 00a4 c077979c c071f1c8  
c071f91c 0003 0003
[0.863952] 1f80: c071f1c8   c04fd9ac  
  
[0.872467] 1fa0:  c04fd9b4  c0013ac8  
  
[0.880981] 1fc0:      
  
[0.889465] 1fe0:     0013 
 ce5331ac 3153ceac
[0.898010] [c02d4e2c] (pinctrl_pm_select_active_state+0x4/0xc) 
from [c002e930] (_od_runtime_resume+0xc/0x20)
[0.908660] [c002e930] (_od_runtime_resume+0xc/0x20) from 
[c0335310] (__rpm_callback+0x34/0x70)
[0.918060] [c0335310] (__rpm_callback+0x34/0x70) from [c0335374] 
(rpm_callback+0x28/0x88)
[0.927032] [c0335374] (rpm_callback+0x28/0x88) from [c033664c] 
(rpm_resume+0x3c8/0x624)
[0.935821] [c033664c] (rpm_resume+0x3c8/0x624) from [c0336b24] 
(__pm_runtime_resume+0x48/0x60)
[0.945220]

Re: [PATCH 1/4] pinctrl: Remove duplicate code in pinctrl_pm_select_state functions

2013-07-16 Thread Tony Lindgren 
* Grygorii Strashko grygorii.stras...@ti.com [130716 06:22]:
 Hi Tony,
 
 This patch causes boot failure when I've applied my patch
 [RFC] ARM: OMAP2+: omap_device: add pinctrl handling
 https://lkml.org/lkml/2013/6/21/309

 on top of it:

Hmm this patch alone removes duplicate code and if it causes
issues I must have made a typo somewhere.

I cannot see a typo though, but in your dmesg I see something..

 [0.610565] PC is at pinctrl_pm_select_active_state+0x4/0xc

..looks like you have something applied for the active_state
that only gets introduced later on in this series.

Maybe you have the earlier version of drivers/base/pinctrl.c
active_state patch that was in linux next for a while but
got reverted as we noticed we had to rework some things?

So maybe try with v3.11-rc1 + these four patches + your
omap_device patch?

Regards,

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

Re: [PATCH 1/4] pinctrl: Remove duplicate code in pinctrl_pm_select_state functions

2013-07-16 Thread Grygorii Strashko 

Hi Tony,

On 07/16/2013 04:41 PM, Tony Lindgren wrote:

* Grygorii Strashko grygorii.stras...@ti.com [130716 06:22]:

Hi Tony,

This patch causes boot failure when I've applied my patch
[RFC] ARM: OMAP2+: omap_device: add pinctrl handling
https://lkml.org/lkml/2013/6/21/309

on top of it:


Hmm this patch alone removes duplicate code and if it causes
issues I must have made a typo somewhere.


No typo :) You've removed the check for !dev-pins.
And the failure place is:
int pinctrl_pm_select_active_state(struct device *dev)
{
   return pinctrl_pm_select_state(dev, dev-pins-active_state);
 here
}

If I understand everything right, the pinctrl support in Device core
assumed to be optional - so, It's valid case, when there are no
definition for device's pinctrl in DT at all.

And in this case dev-pins == NULL and  pinctrl_pm_select_*() API
should return 0 always.



I cannot see a typo though, but in your dmesg I see something..


[0.610565] PC is at pinctrl_pm_select_active_state+0x4/0xc

Yep. This will happen if there are no pinctrl states defined in DT
- currently it's crashed when GPIO driver probed.


..looks like you have something applied for the active_state
that only gets introduced later on in this series.

Maybe you have the earlier version of drivers/base/pinctrl.c
active_state patch that was in linux next for a while but
got reverted as we noticed we had to rework some things?

So maybe try with v3.11-rc1 + these four patches + your
omap_device patch?

I'm on v3.11-rc1



Regards,

Tony



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