Re: [PATCH 2/5] OMAP: GPIO: use pwrdmn name to find wkup dmn GPIO

2011-03-07 Thread Kevin Hilman 
Varadarajan, Charulatha ch...@ti.com writes:

 On Sat, Mar 5, 2011 at 02:21, Kevin Hilman khil...@ti.com wrote:
 Charulatha V ch...@ti.com writes:

 In omap3, save/restore context is implemented for GPIO
 banks 2-6 as GPIO bank1 is in wakeup domain. Instead
 of identifying bank's power domain by bank id, make use
 of powerdomain name itself.

 For this, omap_hwmod_get_pwrdm() is used. omap_device_get_pwrdm()
 could not be used as the pwrdm information needs to be filled
 in pdata. But omap_device_get_pwrdm() can be used only after
 omap_device_build() call.

 Signed-off-by: Charulatha V ch...@ti.com

 Tested-by: Tarun Kanti DebBarma tarun.ka...@ti.com
 (2430-SDP testing)

 I like the idea of this change, but not the implementation...

 [...]

 @@ -1865,16 +1867,15 @@ static int workaround_enabled;
  void omap2_gpio_prepare_for_idle(int off_mode)
  {
       int i, c = 0;
 -     int min = 0;

 -     if (cpu_is_omap34xx())
 -             min = 1;
 -
 -     for (i = min; i  gpio_bank_count; i++) {
 +     for (i = 0; i  gpio_bank_count; i++) {
               struct gpio_bank *bank = gpio_bank[i];
               u32 l1 = 0, l2 = 0;
               int j;

 +             if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
 +                     continue;
 +

 This adds a string compare for every bank during every idle
 transistion (and every resume.)  That's a lot of unneeded overhead.

 I'd rather see a per-bank flag 'looses_context' or something that can be
 checked more efficiently in this fast path.  This flag can be set based
 on the powerdomain name in the device init code.

 This looks better. Will do the needful.
 One question, can looses_context be made as part of dev_attr?

I guess that's up to Benoît.

But, I don't think that's necessary. It should be easy to set at runtime
just doing a strcmp on the powerdomain during the device init,
omap_device_build phase.

Kevin

--
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 2/5] OMAP: GPIO: use pwrdmn name to find wkup dmn GPIO

2011-03-07 Thread Paul Walmsley 
Hi

On Mon, 7 Mar 2011, Kevin Hilman wrote:

 Varadarajan, Charulatha ch...@ti.com writes:
 
  On Sat, Mar 5, 2011 at 02:21, Kevin Hilman khil...@ti.com wrote:
  Charulatha V ch...@ti.com writes:
 
  +             if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
  +                     continue;
  +
 
  This adds a string compare for every bank during every idle
  transistion (and every resume.)  That's a lot of unneeded overhead.
 
  I'd rather see a per-bank flag 'looses_context' or something that can be
  checked more efficiently in this fast path.  This flag can be set based
  on the powerdomain name in the device init code.
 
  This looks better. Will do the needful.
  One question, can looses_context be made as part of dev_attr?
 
 I guess that's up to Benoît.
 
 But, I don't think that's necessary. It should be easy to set at runtime
 just doing a strcmp on the powerdomain during the device init,
 omap_device_build phase.

It shouldn't be part of .dev_attr, since it's not a IP block-specific 
attribute, it's a powerdomain-specific attribute.  The same hwmod 
structure might be used on another OMAP chip that places the device in a 
different powerdomain.

It would also be good to avoid doing strcmp()s here.  The powerdomain name 
string, like any name string, should basically be opaque to code.

In this case, the best approach is probably for the subarch integration 
code to ask the powerdomain code whether the hwmod's powerdomain can ever 
lose context.  I just posted a patch series to do this[1], so I'd suggest 
you use the function that it exports.


- Paul

1. http://marc.info/?l=linux-omapm=129955064112024w=2

Re: [PATCH 2/5] OMAP: GPIO: use pwrdmn name to find wkup dmn GPIO

2011-03-07 Thread Varadarajan, Charulatha 
Paul, Kevin,

Thanks.

On Tue, Mar 8, 2011 at 07:55, Paul Walmsley p...@pwsan.com wrote:
 Hi

 On Mon, 7 Mar 2011, Kevin Hilman wrote:

 Varadarajan, Charulatha ch...@ti.com writes:

  On Sat, Mar 5, 2011 at 02:21, Kevin Hilman khil...@ti.com wrote:
  Charulatha V ch...@ti.com writes:
 
  +             if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
  +                     continue;
  +
 
  This adds a string compare for every bank during every idle
  transistion (and every resume.)  That's a lot of unneeded overhead.
 
  I'd rather see a per-bank flag 'looses_context' or something that can be
  checked more efficiently in this fast path.  This flag can be set based
  on the powerdomain name in the device init code.
 
  This looks better. Will do the needful.
  One question, can looses_context be made as part of dev_attr?

 I guess that's up to Benoît.

 But, I don't think that's necessary. It should be easy to set at runtime
 just doing a strcmp on the powerdomain during the device init,
 omap_device_build phase.

 It shouldn't be part of .dev_attr, since it's not a IP block-specific
 attribute, it's a powerdomain-specific attribute.  The same hwmod
 structure might be used on another OMAP chip that places the device in a
 different powerdomain.

 It would also be good to avoid doing strcmp()s here.  The powerdomain name
 string, like any name string, should basically be opaque to code.

 In this case, the best approach is probably for the subarch integration
 code to ask the powerdomain code whether the hwmod's powerdomain can ever
 lose context.  I just posted a patch series to do this[1], so I'd suggest
 you use the function that it exports.

Will do the needful.
- V Charulatha



 - Paul

 1. http://marc.info/?l=linux-omapm=129955064112024w=2
--
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 2/5] OMAP: GPIO: use pwrdmn name to find wkup dmn GPIO

2011-03-04 Thread Kevin Hilman 
Charulatha V ch...@ti.com writes:

 In omap3, save/restore context is implemented for GPIO
 banks 2-6 as GPIO bank1 is in wakeup domain. Instead
 of identifying bank's power domain by bank id, make use
 of powerdomain name itself.

 For this, omap_hwmod_get_pwrdm() is used. omap_device_get_pwrdm()
 could not be used as the pwrdm information needs to be filled
 in pdata. But omap_device_get_pwrdm() can be used only after
 omap_device_build() call.

 Signed-off-by: Charulatha V ch...@ti.com

 Tested-by: Tarun Kanti DebBarma tarun.ka...@ti.com
 (2430-SDP testing)

I like the idea of this change, but not the implementation...

[...]

 @@ -1865,16 +1867,15 @@ static int workaround_enabled;
  void omap2_gpio_prepare_for_idle(int off_mode)
  {
   int i, c = 0;
 - int min = 0;
  
 - if (cpu_is_omap34xx())
 - min = 1;
 -
 - for (i = min; i  gpio_bank_count; i++) {
 + for (i = 0; i  gpio_bank_count; i++) {
   struct gpio_bank *bank = gpio_bank[i];
   u32 l1 = 0, l2 = 0;
   int j;
  
 + if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
 + continue;
 +

This adds a string compare for every bank during every idle
transistion (and every resume.)  That's a lot of unneeded overhead.

I'd rather see a per-bank flag 'looses_context' or something that can be
checked more efficiently in this fast path.  This flag can be set based
on the powerdomain name in the device init code.

Kevin
--
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 2/5] OMAP: GPIO: use pwrdmn name to find wkup dmn GPIO

2011-03-04 Thread Varadarajan, Charulatha 
On Sat, Mar 5, 2011 at 02:21, Kevin Hilman khil...@ti.com wrote:
 Charulatha V ch...@ti.com writes:

 In omap3, save/restore context is implemented for GPIO
 banks 2-6 as GPIO bank1 is in wakeup domain. Instead
 of identifying bank's power domain by bank id, make use
 of powerdomain name itself.

 For this, omap_hwmod_get_pwrdm() is used. omap_device_get_pwrdm()
 could not be used as the pwrdm information needs to be filled
 in pdata. But omap_device_get_pwrdm() can be used only after
 omap_device_build() call.

 Signed-off-by: Charulatha V ch...@ti.com

 Tested-by: Tarun Kanti DebBarma tarun.ka...@ti.com
 (2430-SDP testing)

 I like the idea of this change, but not the implementation...

 [...]

 @@ -1865,16 +1867,15 @@ static int workaround_enabled;
  void omap2_gpio_prepare_for_idle(int off_mode)
  {
       int i, c = 0;
 -     int min = 0;

 -     if (cpu_is_omap34xx())
 -             min = 1;
 -
 -     for (i = min; i  gpio_bank_count; i++) {
 +     for (i = 0; i  gpio_bank_count; i++) {
               struct gpio_bank *bank = gpio_bank[i];
               u32 l1 = 0, l2 = 0;
               int j;

 +             if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
 +                     continue;
 +

 This adds a string compare for every bank during every idle
 transistion (and every resume.)  That's a lot of unneeded overhead.

 I'd rather see a per-bank flag 'looses_context' or something that can be
 checked more efficiently in this fast path.  This flag can be set based
 on the powerdomain name in the device init code.

This looks better. Will do the needful.
One question, can looses_context be made as part of dev_attr?


 Kevin

--
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 2/5] OMAP: GPIO: use pwrdmn name to find wkup dmn GPIO

2011-02-28 Thread Charulatha V 
In omap3, save/restore context is implemented for GPIO
banks 2-6 as GPIO bank1 is in wakeup domain. Instead
of identifying bank's power domain by bank id, make use
of powerdomain name itself.

For this, omap_hwmod_get_pwrdm() is used. omap_device_get_pwrdm()
could not be used as the pwrdm information needs to be filled
in pdata. But omap_device_get_pwrdm() can be used only after
omap_device_build() call.

Signed-off-by: Charulatha V ch...@ti.com

Tested-by: Tarun Kanti DebBarma tarun.ka...@ti.com
(2430-SDP testing)
---
 arch/arm/mach-omap2/gpio.c |6 ++
 arch/arm/plat-omap/gpio.c  |   31 ---
 arch/arm/plat-omap/include/plat/gpio.h |1 +
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
index 413de18..39b0d96 100644
--- a/arch/arm/mach-omap2/gpio.c
+++ b/arch/arm/mach-omap2/gpio.c
@@ -24,6 +24,8 @@
 #include plat/omap_hwmod.h
 #include plat/omap_device.h
 
+#include powerdomain.h
+
 static struct omap_device_pm_latency omap_gpio_latency[] = {
[0] = {
.deactivate_func = omap_device_idle_hwmods,
@@ -39,6 +41,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void 
*unused)
struct omap_gpio_dev_attr *dev_attr;
char *name = omap_gpio;
int id;
+   struct powerdomain *pwrdm;
 
/*
 * extract the device id from name field available in the
@@ -75,6 +78,9 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void 
*unused)
return -EINVAL;
}
 
+   pwrdm = omap_hwmod_get_pwrdm(oh);
+   strcpy(pdata-pwrdm_name, pwrdm-name);
+
od = omap_device_build(name, id - 1, oh, pdata,
sizeof(*pdata), omap_gpio_latency,
ARRAY_SIZE(omap_gpio_latency),
diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index c6ab0ff..ff341ec 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -172,6 +172,7 @@ struct gpio_bank {
struct device *dev;
bool dbck_flag;
int stride;
+   const char *pwrdm_name;
 };
 
 /*
@@ -1720,6 +1721,7 @@ static int __devinit omap_gpio_probe(struct 
platform_device *pdev)
bank-dbck_flag = pdata-dbck_flag;
bank-stride = pdata-bank_stride;
bank_width = pdata-bank_width;
+   bank-pwrdm_name = pdata-pwrdm_name;
 
spin_lock_init(bank-lock);
 
@@ -1865,16 +1867,15 @@ static int workaround_enabled;
 void omap2_gpio_prepare_for_idle(int off_mode)
 {
int i, c = 0;
-   int min = 0;
 
-   if (cpu_is_omap34xx())
-   min = 1;
-
-   for (i = min; i  gpio_bank_count; i++) {
+   for (i = 0; i  gpio_bank_count; i++) {
struct gpio_bank *bank = gpio_bank[i];
u32 l1 = 0, l2 = 0;
int j;
 
+   if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
+   continue;
+
for (j = 0; j  hweight_long(bank-dbck_enable_mask); j++)
clk_disable(bank-dbck);
 
@@ -1934,15 +1935,15 @@ void omap2_gpio_prepare_for_idle(int off_mode)
 void omap2_gpio_resume_after_idle(void)
 {
int i;
-   int min = 0;
 
-   if (cpu_is_omap34xx())
-   min = 1;
-   for (i = min; i  gpio_bank_count; i++) {
+   for (i = 0; i  gpio_bank_count; i++) {
struct gpio_bank *bank = gpio_bank[i];
u32 l = 0, gen, gen0, gen1;
int j;
 
+   if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
+   continue;
+
for (j = 0; j  hweight_long(bank-dbck_enable_mask); j++)
clk_enable(bank-dbck);
 
@@ -2037,8 +2038,12 @@ void omap_gpio_save_context(void)
int i;
 
/* saving banks from 2-6 only since GPIO1 is in WKUP */
-   for (i = 1; i  gpio_bank_count; i++) {
+   for (i = 0; i  gpio_bank_count; i++) {
struct gpio_bank *bank = gpio_bank[i];
+
+   if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
+   continue;
+
bank-context.irqenable1 =
__raw_readl(bank-base + OMAP24XX_GPIO_IRQENABLE1);
bank-context.irqenable2 =
@@ -2067,8 +2072,12 @@ void omap_gpio_restore_context(void)
 {
int i;
 
-   for (i = 1; i  gpio_bank_count; i++) {
+   for (i = 0; i  gpio_bank_count; i++) {
struct gpio_bank *bank = gpio_bank[i];
+
+   if (!strcmp(bank-pwrdm_name, wkup_pwrdm))
+   continue;
+
__raw_writel(bank-context.irqenable1,
bank-base + OMAP24XX_GPIO_IRQENABLE1);
__raw_writel(bank-context.irqenable2,
diff --git a/arch/arm/plat-omap/include/plat/gpio.h 
b/arch/arm/plat-omap/include/plat/gpio.h
index d6f9fa0..2c46caa 100644
--- a/arch/arm/plat-omap/include/plat/gpio.h
+++