[PATCH] gpio/omap: ensure gpio context is initialised

[PATCH] gpio/omap: ensure gpio context is initialised

[Jon Hunter]

Commit a2797be (gpio/omap: force restore if context loss is not
detectable) broke gpio support for OMAP when booting with device-tree
because a restore of the gpio context being performed without ever
initialising the gpio context. In other words, the context restored was
bad.

This problem could also occur in the non device-tree case, however, it
is much less likely because when booting without device-tree we can
detect context loss via a platform specific API and so context restore
is performed less often.

Nevertheless we should ensure that the gpio context is initialised
during the probe for gpio banks that could lose their state regardless
of whether we are booting with device-tree or not.

Reported-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
Tested-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpio/gpio-omap.c |   53 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 0557529..0ba5cb9 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -70,6 +70,7 @@ struct gpio_bank {
 	bool is_mpuio;
 	bool dbck_flag;
 	bool loses_context;
+	bool context_valid;
 	int stride;
 	u32 width;
 	int context_loss_count;
@@ -1085,6 +1086,7 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
 }
 
 static const struct of_device_id omap_gpio_match[];
+static void omap_gpio_init_context(struct gpio_bank *p);
 
 static int omap_gpio_probe(struct platform_device *pdev)
 {
@@ -1179,8 +1181,10 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	omap_gpio_chip_init(bank);
 	omap_gpio_show_rev(bank);
 
-	if (bank->loses_context)
+	if (bank->loses_context) {
 		bank->get_context_loss_count = pdata->get_context_loss_count;
+		omap_gpio_init_context(bank);
+	}
 
 	pm_runtime_put(bank->dev);
 
@@ -1269,6 +1273,14 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	int c;
 
 	spin_lock_irqsave(&bank->lock, flags);
+
+	/*
+	 * On the first resume during the probe, the context has not
+	 * been initialised and so if the context is not valid return.
+	 */
+	if (!bank->context_valid)
+		goto done;
+
 	_gpio_dbck_enable(bank);
 
 	/*
@@ -1287,19 +1299,15 @@ static int omap_gpio_runtime_resume(struct device *dev)
 			omap_gpio_restore_context(bank);
 		} else {
 			c = bank->get_context_loss_count(bank->dev);
-			if (c != bank->context_loss_count) {
+			if (c != bank->context_loss_count)
 				omap_gpio_restore_context(bank);
-			} else {
-				spin_unlock_irqrestore(&bank->lock, flags);
-				return 0;
-			}
+			else
+				goto done;
 		}
 	}
 
-	if (!bank->workaround_enabled) {
-		spin_unlock_irqrestore(&bank->lock, flags);
-		return 0;
-	}
+	if (!bank->workaround_enabled)
+		goto done;
 
 	l = __raw_readl(bank->base + bank->regs->datain);
 
@@ -1352,6 +1360,7 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	}
 
 	bank->workaround_enabled = false;
+done:
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -1385,6 +1394,29 @@ void omap2_gpio_resume_after_idle(void)
 }
 
 #if defined(CONFIG_PM_RUNTIME)
+static void omap_gpio_init_context(struct gpio_bank *p)
+{
+	struct omap_gpio_reg_offs *regs = p->regs;
+	void __iomem *base = p->base;
+
+	p->context.ctrl		= __raw_readl(base + regs->ctrl);
+	p->context.oe		= __raw_readl(base + regs->direction);
+	p->context.wake_en	= __raw_readl(base + regs->wkup_en);
+	p->context.leveldetect0	= __raw_readl(base + regs->leveldetect0);
+	p->context.leveldetect1	= __raw_readl(base + regs->leveldetect1);
+	p->context.risingdetect	= __raw_readl(base + regs->risingdetect);
+	p->context.fallingdetect = __raw_readl(base + regs->fallingdetect);
+	p->context.irqenable1	= __raw_readl(base + regs->irqenable);
+	p->context.irqenable2	= __raw_readl(base + regs->irqenable2);
+
+	if (regs->set_dataout && p->regs->clr_dataout)
+		p->context.dataout = __raw_readl(base + regs->set_dataout);
+	else
+		p->context.dataout = __raw_readl(base + regs->dataout);
+
+	p->context_valid = true;
+}
+
 static void omap_gpio_restore_context(struct gpio_bank *bank)
 {
 	__raw_writel(bank->context.wake_en,
@@ -1422,6 +1454,7 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 #else
 #define omap_gpio_runtime_suspend NULL
 #define omap_gpio_runtime_resume NULL
+static void omap_gpio_init_context(struct gpio_bank *p) {}
 #endif
 
 static const struct dev_pm_ops gpio_pm_ops = {
-- 
1.7.10.4

[PATCH] gpio/omap: ensure gpio context is initialised

[Santosh Shilimkar]

On Thursday 18 April 2013 02:01 AM, Jon Hunter wrote:
> Commit a2797be (gpio/omap: force restore if context loss is not
> detectable) broke gpio support for OMAP when booting with device-tree
> because a restore of the gpio context being performed without ever
> initialising the gpio context. In other words, the context restored was
> bad.
> 
> This problem could also occur in the non device-tree case, however, it
> is much less likely because when booting without device-tree we can
> detect context loss via a platform specific API and so context restore
> is performed less often.
> 
> Nevertheless we should ensure that the gpio context is initialised
> during the probe for gpio banks that could lose their state regardless
> of whether we are booting with device-tree or not.
> 
> Reported-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> Tested-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/gpio/gpio-omap.c |   53 +++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 0557529..0ba5cb9 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -70,6 +70,7 @@ struct gpio_bank {
>  	bool is_mpuio;
>  	bool dbck_flag;
>  	bool loses_context;
> +	bool context_valid;
>  	int stride;
>  	u32 width;
>  	int context_loss_count;
> @@ -1085,6 +1086,7 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
>  }
>  
>  static const struct of_device_id omap_gpio_match[];
> +static void omap_gpio_init_context(struct gpio_bank *p);
>  
>  static int omap_gpio_probe(struct platform_device *pdev)
>  {
> @@ -1179,8 +1181,10 @@ static int omap_gpio_probe(struct platform_device *pdev)
>  	omap_gpio_chip_init(bank);
>  	omap_gpio_show_rev(bank);
>  
> -	if (bank->loses_context)
> +	if (bank->loses_context) {
>  		bank->get_context_loss_count = pdata->get_context_loss_count;
> +		omap_gpio_init_context(bank);
> +	}
>  
>  	pm_runtime_put(bank->dev);
>  
> @@ -1269,6 +1273,14 @@ static int omap_gpio_runtime_resume(struct device *dev)
>  	int c;
>  
>  	spin_lock_irqsave(&bank->lock, flags);
> +
> +	/*
> +	 * On the first resume during the probe, the context has not
> +	 * been initialised and so if the context is not valid return.
> +	 */
> +	if (!bank->context_valid)
> +		goto done;
> +
The use of 'context_valid' is just for the probe resume case and hence
adding a state variable for GPIO bank structures looks bit too much.
Ideally gpio_init_context() should make the first callback successful
as well if ordered correctly but then you need clocks for that
to happen which are enabled in runtime pm calls. 

Do you really need that "context_valid" flag per bank ? I mean
every gpio bank which can "loses_context", probe time context
validity is an issue, right ? In that case, won't just an
__init static variable do for all banks which can loose
context?

Regards,
Santosh

[PATCH] gpio/omap: ensure gpio context is initialised

[Jon Hunter]

On 04/18/2013 03:22 AM, Santosh Shilimkar wrote:
> On Thursday 18 April 2013 02:01 AM, Jon Hunter wrote:
>> Commit a2797be (gpio/omap: force restore if context loss is not
>> detectable) broke gpio support for OMAP when booting with device-tree
>> because a restore of the gpio context being performed without ever
>> initialising the gpio context. In other words, the context restored was
>> bad.
>>
>> This problem could also occur in the non device-tree case, however, it
>> is much less likely because when booting without device-tree we can
>> detect context loss via a platform specific API and so context restore
>> is performed less often.
>>
>> Nevertheless we should ensure that the gpio context is initialised
>> during the probe for gpio banks that could lose their state regardless
>> of whether we are booting with device-tree or not.
>>
>> Reported-by: Tony Lindgren <tony@atomide.com>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> Tested-by: Tony Lindgren <tony@atomide.com>
>> ---
>>  drivers/gpio/gpio-omap.c |   53 +++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 43 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 0557529..0ba5cb9 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -70,6 +70,7 @@ struct gpio_bank {
>>  	bool is_mpuio;
>>  	bool dbck_flag;
>>  	bool loses_context;
>> +	bool context_valid;
>>  	int stride;
>>  	u32 width;
>>  	int context_loss_count;
>> @@ -1085,6 +1086,7 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
>>  }
>>  
>>  static const struct of_device_id omap_gpio_match[];
>> +static void omap_gpio_init_context(struct gpio_bank *p);
>>  
>>  static int omap_gpio_probe(struct platform_device *pdev)
>>  {
>> @@ -1179,8 +1181,10 @@ static int omap_gpio_probe(struct platform_device *pdev)
>>  	omap_gpio_chip_init(bank);
>>  	omap_gpio_show_rev(bank);
>>  
>> -	if (bank->loses_context)
>> +	if (bank->loses_context) {
>>  		bank->get_context_loss_count = pdata->get_context_loss_count;
>> +		omap_gpio_init_context(bank);
>> +	}
>>  
>>  	pm_runtime_put(bank->dev);
>>  
>> @@ -1269,6 +1273,14 @@ static int omap_gpio_runtime_resume(struct device *dev)
>>  	int c;
>>  
>>  	spin_lock_irqsave(&bank->lock, flags);
>> +
>> +	/*
>> +	 * On the first resume during the probe, the context has not
>> +	 * been initialised and so if the context is not valid return.
>> +	 */
>> +	if (!bank->context_valid)
>> +		goto done;
>> +
> The use of 'context_valid' is just for the probe resume case and hence
> adding a state variable for GPIO bank structures looks bit too much.

Yes it is not ideal. However, it is safe.

> Ideally gpio_init_context() should make the first callback successful
> as well if ordered correctly but then you need clocks for that
> to happen which are enabled in runtime pm calls. 

Right.

> Do you really need that "context_valid" flag per bank ? I mean
> every gpio bank which can "loses_context", probe time context
> validity is an issue, right ? In that case, won't just an
> __init static variable do for all banks which can loose
> context?

I think that it would be fragile to have a global for all banks. What
happens if one bank failed during the probe? Probably unlikely but still.

The best solution would be to be able to plug the pm-runtime
resume/suspend handlers at the end of the probe or be able to enable the
module without the callback handlers being calling during the probe. I
am not sure if there is a way to do this. May be Kevin knows.

Another option is to move the initialisation of the bank->loses_context
to after the call to pm_runtime_get_sync() in the probe. This would
prevent the context restore occurring during the initial runtime resume.
However, if you look at the runtime resume function there are still some
register writes that do occur (which is basically what happens today).
This works but it is a little fragile, however, we can do that if
preferred. May be I could add a comment to the probe to say why we
initialise bank->loses_context after the pm_runtime_get_sync() so no one
moves this in the future (and more importantly I don't forget!).

Cheers
Jon

[PATCH] gpio/omap: ensure gpio context is initialised

[Kevin Hilman]

Hi Jon,

Jon Hunter <jon-hunter@ti.com> writes:

> Commit a2797be (gpio/omap: force restore if context loss is not
> detectable) broke gpio support for OMAP when booting with device-tree
> because a restore of the gpio context being performed without ever
> initialising the gpio context. In other words, the context restored was
> bad.
>
> This problem could also occur in the non device-tree case, however, it
> is much less likely because when booting without device-tree we can
> detect context loss via a platform specific API and so context restore
> is performed less often.
>
> Nevertheless we should ensure that the gpio context is initialised
> during the probe for gpio banks that could lose their state regardless
> of whether we are booting with device-tree or not.
>
> Reported-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> Tested-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/gpio/gpio-omap.c |   53 +++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 0557529..0ba5cb9 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -70,6 +70,7 @@ struct gpio_bank {
>  	bool is_mpuio;
>  	bool dbck_flag;
>  	bool loses_context;
> +	bool context_valid;
>  	int stride;
>  	u32 width;
>  	int context_loss_count;
> @@ -1085,6 +1086,7 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
>  }
>  
>  static const struct of_device_id omap_gpio_match[];
> +static void omap_gpio_init_context(struct gpio_bank *p);
>  
>  static int omap_gpio_probe(struct platform_device *pdev)
>  {
> @@ -1179,8 +1181,10 @@ static int omap_gpio_probe(struct platform_device *pdev)
>  	omap_gpio_chip_init(bank);
>  	omap_gpio_show_rev(bank);
>  
> -	if (bank->loses_context)
> +	if (bank->loses_context) {
>  		bank->get_context_loss_count = pdata->get_context_loss_count;
> +		omap_gpio_init_context(bank);
> +	}
>  
>  	pm_runtime_put(bank->dev);
>  
> @@ -1269,6 +1273,14 @@ static int omap_gpio_runtime_resume(struct device *dev)
>  	int c;
>  
>  	spin_lock_irqsave(&bank->lock, flags);
> +
> +	/*
> +	 * On the first resume during the probe, the context has not
> +	 * been initialised and so if the context is not valid return.
> +	 */
> +	if (!bank->context_valid)
> +		goto done;

Not sure I follow the reason to separate it here and in probe.  

Also, this makes the first runtime_resume a special case and leaves
things in a strange semi-initialized state that is confusing IMO.

Why not just init context right here if bank->loses_context &&
!bank->context_valid?

Then the first resume can continue as expected, and everything is fully
initialized as expected also.  IMO, this is much more readable (and
maintainable, but that's your job now, so you can decide ;)

Kevin

[PATCH] gpio/omap: ensure gpio context is initialised

[Jon Hunter]

Hi Kevin,

On 04/18/2013 04:34 PM, Kevin Hilman wrote:
> Hi Jon,
> 
> Jon Hunter <jon-hunter@ti.com> writes:
> 
>> Commit a2797be (gpio/omap: force restore if context loss is not
>> detectable) broke gpio support for OMAP when booting with device-tree
>> because a restore of the gpio context being performed without ever
>> initialising the gpio context. In other words, the context restored was
>> bad.
>>
>> This problem could also occur in the non device-tree case, however, it
>> is much less likely because when booting without device-tree we can
>> detect context loss via a platform specific API and so context restore
>> is performed less often.
>>
>> Nevertheless we should ensure that the gpio context is initialised
>> during the probe for gpio banks that could lose their state regardless
>> of whether we are booting with device-tree or not.
>>
>> Reported-by: Tony Lindgren <tony@atomide.com>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> Tested-by: Tony Lindgren <tony@atomide.com>
>> ---
>>  drivers/gpio/gpio-omap.c |   53 +++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 43 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 0557529..0ba5cb9 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -70,6 +70,7 @@ struct gpio_bank {
>>  	bool is_mpuio;
>>  	bool dbck_flag;
>>  	bool loses_context;
>> +	bool context_valid;
>>  	int stride;
>>  	u32 width;
>>  	int context_loss_count;
>> @@ -1085,6 +1086,7 @@ static void omap_gpio_chip_init(struct gpio_bank *bank)
>>  }
>>  
>>  static const struct of_device_id omap_gpio_match[];
>> +static void omap_gpio_init_context(struct gpio_bank *p);
>>  
>>  static int omap_gpio_probe(struct platform_device *pdev)
>>  {
>> @@ -1179,8 +1181,10 @@ static int omap_gpio_probe(struct platform_device *pdev)
>>  	omap_gpio_chip_init(bank);
>>  	omap_gpio_show_rev(bank);
>>  
>> -	if (bank->loses_context)
>> +	if (bank->loses_context) {
>>  		bank->get_context_loss_count = pdata->get_context_loss_count;
>> +		omap_gpio_init_context(bank);
>> +	}
>>  
>>  	pm_runtime_put(bank->dev);
>>  
>> @@ -1269,6 +1273,14 @@ static int omap_gpio_runtime_resume(struct device *dev)
>>  	int c;
>>  
>>  	spin_lock_irqsave(&bank->lock, flags);
>> +
>> +	/*
>> +	 * On the first resume during the probe, the context has not
>> +	 * been initialised and so if the context is not valid return.
>> +	 */
>> +	if (!bank->context_valid)
>> +		goto done;
> 
> Not sure I follow the reason to separate it here and in probe.  
> 
> Also, this makes the first runtime_resume a special case and leaves
> things in a strange semi-initialized state that is confusing IMO.

The first resume has always been a special case. The
"bank->get_context_loss_count" is not initialised until after the first
resume (due to another issue we had found - 7b86cef gpio/omap: fix
invalid context restore of gpio bank-0). This should not leave things in
a strange semi-init'ed state, as on the first resume nothing is really
done anyway because there is no context loss.

> Why not just init context right here if bank->loses_context &&
> !bank->context_valid?

Thanks for the suggestion.

> Then the first resume can continue as expected, and everything is fully
> initialized as expected also.  IMO, this is much more readable (and
> maintainable, but that's your job now, so you can decide ;)

If the context has not been lost, which it has not on the first resume,
then resume really does nothing. That's why I had just returned.
However, I would agree that is not completely readable.

Cheers
Jon

[PATCH] gpio/omap: ensure gpio context is initialised

[Jon Hunter]

On 04/18/2013 06:10 PM, Jon Hunter wrote:
> On 04/18/2013 04:34 PM, Kevin Hilman wrote:

...

>> Why not just init context right here if bank->loses_context &&
>> !bank->context_valid?

I really like this idea a lot. It can really clean-up the code
and really make it much more readable. Before we were playing 
some tricks with when we init'ed the get_context_loss_count()
function pointer. How about the below?

Tony, care to re-test?

Cheers
Jon

>From d7a940531d354e6be5e16ee50fa8344041df963a Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Mon, 15 Apr 2013 13:06:54 -0500
Subject: [PATCH] gpio/omap: ensure gpio context is initialised

Commit a2797be (gpio/omap: force restore if context loss is not
detectable) broke gpio support for OMAP when booting with device-tree
because a restore of the gpio context being performed without ever
initialising the gpio context. In other words, the context restored was
bad.

This problem could also occur in the non device-tree case, however, it
is much less likely because when booting without device-tree we can
detect context loss via a platform specific API and so context restore
is performed less often.

Nevertheless we should ensure that the gpio context is initialised
on the first pm-runtime resume for gpio banks that could lose their
state regardless of whether we are booting with device-tree or not.

The context loss count was being initialised on the first pm-runtime
suspend following a resume, by populating the get_count_loss_count()
function pointer after the first pm-runtime resume. To make the code
more readable and logical, initialise the context loss count on the
first pm-runtime resume if the context is not yet valid.

Reported-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 drivers/gpio/gpio-omap.c |   45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 0557529..db3c732 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -70,6 +70,7 @@ struct gpio_bank {
 	bool is_mpuio;
 	bool dbck_flag;
 	bool loses_context;
+	bool context_valid;
 	int stride;
 	u32 width;
 	int context_loss_count;
@@ -1129,6 +1130,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
 			bank->loses_context = true;
 	} else {
 		bank->loses_context = pdata->loses_context;
+		bank->get_context_loss_count = pdata->get_context_loss_count;
 	}
 
 
@@ -1179,9 +1181,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	omap_gpio_chip_init(bank);
 	omap_gpio_show_rev(bank);
 
-	if (bank->loses_context)
-		bank->get_context_loss_count = pdata->get_context_loss_count;
-
 	pm_runtime_put(bank->dev);
 
 	list_add_tail(&bank->node, &omap_gpio_list);
@@ -1260,6 +1259,8 @@ update_gpio_context_count:
 	return 0;
 }
 
+static void omap_gpio_init_context(struct gpio_bank *p);
+
 static int omap_gpio_runtime_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -1269,6 +1270,20 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	int c;
 
 	spin_lock_irqsave(&bank->lock, flags);
+
+	/*
+	 * On the first resume during the probe, the context has not
+	 * been initialised and so initialise it now. Also initialise
+	 * the context loss count.
+	 */
+	if (bank->loses_context && !bank->context_valid) {
+		omap_gpio_init_context(bank);
+
+		if (bank->get_context_loss_count)
+			bank->context_loss_count =
+				bank->get_context_loss_count(bank->dev);
+	}
+
 	_gpio_dbck_enable(bank);
 
 	/*
@@ -1385,6 +1400,29 @@ void omap2_gpio_resume_after_idle(void)
 }
 
 #if defined(CONFIG_PM_RUNTIME)
+static void omap_gpio_init_context(struct gpio_bank *p)
+{
+	struct omap_gpio_reg_offs *regs = p->regs;
+	void __iomem *base = p->base;
+
+	p->context.ctrl		= __raw_readl(base + regs->ctrl);
+	p->context.oe		= __raw_readl(base + regs->direction);
+	p->context.wake_en	= __raw_readl(base + regs->wkup_en);
+	p->context.leveldetect0	= __raw_readl(base + regs->leveldetect0);
+	p->context.leveldetect1	= __raw_readl(base + regs->leveldetect1);
+	p->context.risingdetect	= __raw_readl(base + regs->risingdetect);
+	p->context.fallingdetect = __raw_readl(base + regs->fallingdetect);
+	p->context.irqenable1	= __raw_readl(base + regs->irqenable);
+	p->context.irqenable2	= __raw_readl(base + regs->irqenable2);
+
+	if (regs->set_dataout && p->regs->clr_dataout)
+		p->context.dataout = __raw_readl(base + regs->set_dataout);
+	else
+		p->context.dataout = __raw_readl(base + regs->dataout);
+
+	p->context_valid = true;
+}
+
 static void omap_gpio_restore_context(struct gpio_bank *bank)
 {
 	__raw_writel(bank->context.wake_en,
@@ -1422,6 +1460,7 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 #else
 #define omap_gpio_runtime_suspend NULL
 #define omap_gpio_runtime_resume NULL
+static void omap_gpio_init_context(struct gpio_bank *p) {}
 #endif
 
 static const struct dev_pm_ops gpio_pm_ops = {
-- 
1.7.10.4

[PATCH] gpio/omap: ensure gpio context is initialised

[Jon Hunter]

On 04/18/2013 07:34 PM, Jon Hunter wrote:
> 
> On 04/18/2013 06:10 PM, Jon Hunter wrote:
>> On 04/18/2013 04:34 PM, Kevin Hilman wrote:
> 
> ...
> 
>>> Why not just init context right here if bank->loses_context &&
>>> !bank->context_valid?
> 
> I really like this idea a lot. It can really clean-up the code
> and really make it much more readable. Before we were playing 
> some tricks with when we init'ed the get_context_loss_count()
> function pointer. How about the below?
> 
> Tony, care to re-test?
> 
> Cheers
> Jon
> 
> From d7a940531d354e6be5e16ee50fa8344041df963a Mon Sep 17 00:00:00 2001
> From: Jon Hunter <jon-hunter@ti.com>
> Date: Mon, 15 Apr 2013 13:06:54 -0500
> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
> 
> Commit a2797be (gpio/omap: force restore if context loss is not
> detectable) broke gpio support for OMAP when booting with device-tree
> because a restore of the gpio context being performed without ever
> initialising the gpio context. In other words, the context restored was
> bad.
> 
> This problem could also occur in the non device-tree case, however, it
> is much less likely because when booting without device-tree we can
> detect context loss via a platform specific API and so context restore
> is performed less often.
> 
> Nevertheless we should ensure that the gpio context is initialised
> on the first pm-runtime resume for gpio banks that could lose their
> state regardless of whether we are booting with device-tree or not.
> 
> The context loss count was being initialised on the first pm-runtime
> suspend following a resume, by populating the get_count_loss_count()
> function pointer after the first pm-runtime resume. To make the code
> more readable and logical, initialise the context loss count on the
> first pm-runtime resume if the context is not yet valid.
> 
> Reported-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
>  drivers/gpio/gpio-omap.c |   45 ++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 0557529..db3c732 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -70,6 +70,7 @@ struct gpio_bank {
>  	bool is_mpuio;
>  	bool dbck_flag;
>  	bool loses_context;
> +	bool context_valid;
>  	int stride;
>  	u32 width;
>  	int context_loss_count;
> @@ -1129,6 +1130,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
>  			bank->loses_context = true;
>  	} else {
>  		bank->loses_context = pdata->loses_context;
> +		bank->get_context_loss_count = pdata->get_context_loss_count;

Still need to check loses_context for populating
get_context_loss_count here. Updated patch below.

Jon

>From d02ef7b7dfcf8e13bf019aedfdecb38ca3c6749f Mon Sep 17 00:00:00 2001
From: Jon Hunter <jon-hunter@ti.com>
Date: Mon, 15 Apr 2013 13:06:54 -0500
Subject: [PATCH] gpio/omap: ensure gpio context is initialised

Commit a2797be (gpio/omap: force restore if context loss is not
detectable) broke gpio support for OMAP when booting with device-tree
because a restore of the gpio context being performed without ever
initialising the gpio context. In other words, the context restored was
bad.

This problem could also occur in the non device-tree case, however, it
is much less likely because when booting without device-tree we can
detect context loss via a platform specific API and so context restore
is performed less often.

Nevertheless we should ensure that the gpio context is initialised
on the first pm-runtime resume for gpio banks that could lose their
state regardless of whether we are booting with device-tree or not.

The context loss count was being initialised on the first pm-runtime
suspend following a resume, by populating the get_count_loss_count()
function pointer after the first pm-runtime resume. To make the code
more readable and logical, initialise the context loss count on the
first pm-runtime resume if the context is not yet valid.

Reported-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Jon Hunter <jon-hunter@ti.com>
---
 drivers/gpio/gpio-omap.c |   48 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 0557529..c3c3ffe 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -70,6 +70,7 @@ struct gpio_bank {
 	bool is_mpuio;
 	bool dbck_flag;
 	bool loses_context;
+	bool context_valid;
 	int stride;
 	u32 width;
 	int context_loss_count;
@@ -1129,6 +1130,10 @@ static int omap_gpio_probe(struct platform_device *pdev)
 			bank->loses_context = true;
 	} else {
 		bank->loses_context = pdata->loses_context;
+
+		if (bank->loses_context)
+			bank->get_context_loss_count =
+				pdata->get_context_loss_count;
 	}
 
 
@@ -1179,9 +1184,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
 	omap_gpio_chip_init(bank);
 	omap_gpio_show_rev(bank);
 
-	if (bank->loses_context)
-		bank->get_context_loss_count = pdata->get_context_loss_count;
-
 	pm_runtime_put(bank->dev);
 
 	list_add_tail(&bank->node, &omap_gpio_list);
@@ -1260,6 +1262,8 @@ update_gpio_context_count:
 	return 0;
 }
 
+static void omap_gpio_init_context(struct gpio_bank *p);
+
 static int omap_gpio_runtime_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -1269,6 +1273,20 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	int c;
 
 	spin_lock_irqsave(&bank->lock, flags);
+
+	/*
+	 * On the first resume during the probe, the context has not
+	 * been initialised and so initialise it now. Also initialise
+	 * the context loss count.
+	 */
+	if (bank->loses_context && !bank->context_valid) {
+		omap_gpio_init_context(bank);
+
+		if (bank->get_context_loss_count)
+			bank->context_loss_count =
+				bank->get_context_loss_count(bank->dev);
+	}
+
 	_gpio_dbck_enable(bank);
 
 	/*
@@ -1385,6 +1403,29 @@ void omap2_gpio_resume_after_idle(void)
 }
 
 #if defined(CONFIG_PM_RUNTIME)
+static void omap_gpio_init_context(struct gpio_bank *p)
+{
+	struct omap_gpio_reg_offs *regs = p->regs;
+	void __iomem *base = p->base;
+
+	p->context.ctrl		= __raw_readl(base + regs->ctrl);
+	p->context.oe		= __raw_readl(base + regs->direction);
+	p->context.wake_en	= __raw_readl(base + regs->wkup_en);
+	p->context.leveldetect0	= __raw_readl(base + regs->leveldetect0);
+	p->context.leveldetect1	= __raw_readl(base + regs->leveldetect1);
+	p->context.risingdetect	= __raw_readl(base + regs->risingdetect);
+	p->context.fallingdetect = __raw_readl(base + regs->fallingdetect);
+	p->context.irqenable1	= __raw_readl(base + regs->irqenable);
+	p->context.irqenable2	= __raw_readl(base + regs->irqenable2);
+
+	if (regs->set_dataout && p->regs->clr_dataout)
+		p->context.dataout = __raw_readl(base + regs->set_dataout);
+	else
+		p->context.dataout = __raw_readl(base + regs->dataout);
+
+	p->context_valid = true;
+}
+
 static void omap_gpio_restore_context(struct gpio_bank *bank)
 {
 	__raw_writel(bank->context.wake_en,
@@ -1422,6 +1463,7 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 #else
 #define omap_gpio_runtime_suspend NULL
 #define omap_gpio_runtime_resume NULL
+static void omap_gpio_init_context(struct gpio_bank *p) {}
 #endif
 
 static const struct dev_pm_ops gpio_pm_ops = {
-- 
1.7.10.4

[PATCH] gpio/omap: ensure gpio context is initialised

[Santosh Shilimkar]

On Friday 19 April 2013 06:19 AM, Jon Hunter wrote:
> 
> On 04/18/2013 07:34 PM, Jon Hunter wrote:
>>
>> On 04/18/2013 06:10 PM, Jon Hunter wrote:
>>> On 04/18/2013 04:34 PM, Kevin Hilman wrote:
>>
>> ...
>>
>>>> Why not just init context right here if bank->loses_context &&
>>>> !bank->context_valid?
>>
>> I really like this idea a lot. It can really clean-up the code
>> and really make it much more readable. Before we were playing 
>> some tricks with when we init'ed the get_context_loss_count()
>> function pointer. How about the below?
>>
>> Tony, care to re-test?
>>
>> Cheers
>> Jon
>>
>> From d7a940531d354e6be5e16ee50fa8344041df963a Mon Sep 17 00:00:00 2001
>> From: Jon Hunter <jon-hunter@ti.com>
>> Date: Mon, 15 Apr 2013 13:06:54 -0500
>> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
>>
>> Commit a2797be (gpio/omap: force restore if context loss is not
>> detectable) broke gpio support for OMAP when booting with device-tree
>> because a restore of the gpio context being performed without ever
>> initialising the gpio context. In other words, the context restored was
>> bad.
>>
>> This problem could also occur in the non device-tree case, however, it
>> is much less likely because when booting without device-tree we can
>> detect context loss via a platform specific API and so context restore
>> is performed less often.
>>
>> Nevertheless we should ensure that the gpio context is initialised
>> on the first pm-runtime resume for gpio banks that could lose their
>> state regardless of whether we are booting with device-tree or not.
>>
>> The context loss count was being initialised on the first pm-runtime
>> suspend following a resume, by populating the get_count_loss_count()
>> function pointer after the first pm-runtime resume. To make the code
>> more readable and logical, initialise the context loss count on the
>> first pm-runtime resume if the context is not yet valid.
>>
>> Reported-by: Tony Lindgren <tony@atomide.com>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
>>  drivers/gpio/gpio-omap.c |   45 ++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 42 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 0557529..db3c732 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -70,6 +70,7 @@ struct gpio_bank {
>>  	bool is_mpuio;
>>  	bool dbck_flag;
>>  	bool loses_context;
>> +	bool context_valid;
>>  	int stride;
>>  	u32 width;
>>  	int context_loss_count;
>> @@ -1129,6 +1130,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
>>  			bank->loses_context = true;
>>  	} else {
>>  		bank->loses_context = pdata->loses_context;
>> +		bank->get_context_loss_count = pdata->get_context_loss_count;
> 
> Still need to check loses_context for populating
> get_context_loss_count here. Updated patch below.
> 
> Jon
> 
> From d02ef7b7dfcf8e13bf019aedfdecb38ca3c6749f Mon Sep 17 00:00:00 2001
> From: Jon Hunter <jon-hunter@ti.com>
> Date: Mon, 15 Apr 2013 13:06:54 -0500
> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
> 
> Commit a2797be (gpio/omap: force restore if context loss is not
> detectable) broke gpio support for OMAP when booting with device-tree
> because a restore of the gpio context being performed without ever
> initialising the gpio context. In other words, the context restored was
> bad.
> 
> This problem could also occur in the non device-tree case, however, it
> is much less likely because when booting without device-tree we can
> detect context loss via a platform specific API and so context restore
> is performed less often.
> 
> Nevertheless we should ensure that the gpio context is initialised
> on the first pm-runtime resume for gpio banks that could lose their
> state regardless of whether we are booting with device-tree or not.
> 
> The context loss count was being initialised on the first pm-runtime
> suspend following a resume, by populating the get_count_loss_count()
> function pointer after the first pm-runtime resume. To make the code
> more readable and logical, initialise the context loss count on the
> first pm-runtime resume if the context is not yet valid.
> 
> Reported-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
This version looks better than the first one for sure. I am still not
happy with per bank "context_valid" flag whose job just ends after
the probe. But then I do agree with you about the global flag might
be fragile and less maintainable.

So, FWIW,
Acked-by: Santosh Shilimkar<santosh.shilimkar@ti.com>

[PATCH] gpio/omap: ensure gpio context is initialised

[Kevin Hilman]

Santosh Shilimkar <santosh.shilimkar@ti.com> writes:

> On Friday 19 April 2013 06:19 AM, Jon Hunter wrote:
>> 
>> On 04/18/2013 07:34 PM, Jon Hunter wrote:
>>>
>>> On 04/18/2013 06:10 PM, Jon Hunter wrote:
>>>> On 04/18/2013 04:34 PM, Kevin Hilman wrote:
>>>
>>> ...
>>>
>>>>> Why not just init context right here if bank->loses_context &&
>>>>> !bank->context_valid?
>>>
>>> I really like this idea a lot. It can really clean-up the code
>>> and really make it much more readable. Before we were playing 
>>> some tricks with when we init'ed the get_context_loss_count()
>>> function pointer. How about the below?
>>>
>>> Tony, care to re-test?
>>>
>>> Cheers
>>> Jon
>>>
>>> From d7a940531d354e6be5e16ee50fa8344041df963a Mon Sep 17 00:00:00 2001
>>> From: Jon Hunter <jon-hunter@ti.com>
>>> Date: Mon, 15 Apr 2013 13:06:54 -0500
>>> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
>>>
>>> Commit a2797be (gpio/omap: force restore if context loss is not
>>> detectable) broke gpio support for OMAP when booting with device-tree
>>> because a restore of the gpio context being performed without ever
>>> initialising the gpio context. In other words, the context restored was
>>> bad.
>>>
>>> This problem could also occur in the non device-tree case, however, it
>>> is much less likely because when booting without device-tree we can
>>> detect context loss via a platform specific API and so context restore
>>> is performed less often.
>>>
>>> Nevertheless we should ensure that the gpio context is initialised
>>> on the first pm-runtime resume for gpio banks that could lose their
>>> state regardless of whether we are booting with device-tree or not.
>>>
>>> The context loss count was being initialised on the first pm-runtime
>>> suspend following a resume, by populating the get_count_loss_count()
>>> function pointer after the first pm-runtime resume. To make the code
>>> more readable and logical, initialise the context loss count on the
>>> first pm-runtime resume if the context is not yet valid.
>>>
>>> Reported-by: Tony Lindgren <tony@atomide.com>
>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>> ---
>>>  drivers/gpio/gpio-omap.c |   45 ++++++++++++++++++++++++++++++++++++++++++---
>>>  1 file changed, 42 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>>> index 0557529..db3c732 100644
>>> --- a/drivers/gpio/gpio-omap.c
>>> +++ b/drivers/gpio/gpio-omap.c
>>> @@ -70,6 +70,7 @@ struct gpio_bank {
>>>  	bool is_mpuio;
>>>  	bool dbck_flag;
>>>  	bool loses_context;
>>> +	bool context_valid;
>>>  	int stride;
>>>  	u32 width;
>>>  	int context_loss_count;
>>> @@ -1129,6 +1130,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
>>>  			bank->loses_context = true;
>>>  	} else {
>>>  		bank->loses_context = pdata->loses_context;
>>> +		bank->get_context_loss_count = pdata->get_context_loss_count;
>> 
>> Still need to check loses_context for populating
>> get_context_loss_count here. Updated patch below.
>> 
>> Jon
>> 
>> From d02ef7b7dfcf8e13bf019aedfdecb38ca3c6749f Mon Sep 17 00:00:00 2001
>> From: Jon Hunter <jon-hunter@ti.com>
>> Date: Mon, 15 Apr 2013 13:06:54 -0500
>> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
>> 
>> Commit a2797be (gpio/omap: force restore if context loss is not
>> detectable) broke gpio support for OMAP when booting with device-tree
>> because a restore of the gpio context being performed without ever
>> initialising the gpio context. In other words, the context restored was
>> bad.
>> 
>> This problem could also occur in the non device-tree case, however, it
>> is much less likely because when booting without device-tree we can
>> detect context loss via a platform specific API and so context restore
>> is performed less often.
>> 
>> Nevertheless we should ensure that the gpio context is initialised
>> on the first pm-runtime resume for gpio banks that could lose their
>> state regardless of whether we are booting with device-tree or not.
>> 
>> The context loss count was being initialised on the first pm-runtime
>> suspend following a resume, by populating the get_count_loss_count()
>> function pointer after the first pm-runtime resume. To make the code
>> more readable and logical, initialise the context loss count on the
>> first pm-runtime resume if the context is not yet valid.
>> 
>> Reported-by: Tony Lindgren <tony@atomide.com>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
> This version looks better than the first one for sure. I am still not
> happy with per bank "context_valid" flag whose job just ends after
> the probe. 

Assuming this driver could become a module someday (not terribly likely,
I know), but context_valid would have meaning for each module reload.

> But then I do agree with you about the global flag might
> be fragile and less maintainable.
>
> So, FWIW,
> Acked-by: Santosh Shilimkar<santosh.shilimkar@ti.com>

Yes, this version is more readable.  Thanks for the update.

Reviewed-by: Kevin Hilman <khilman@linaro.org>

[PATCH] gpio/omap: ensure gpio context is initialised

[Santosh Shilimkar]

On Friday 19 April 2013 07:35 PM, Kevin Hilman wrote:
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
> 
>> On Friday 19 April 2013 06:19 AM, Jon Hunter wrote:
>>>
>>> On 04/18/2013 07:34 PM, Jon Hunter wrote:
>>>>
>>>> On 04/18/2013 06:10 PM, Jon Hunter wrote:
>>>>> On 04/18/2013 04:34 PM, Kevin Hilman wrote:
>>>>
>>>> ...
>>>>
>>>>>> Why not just init context right here if bank->loses_context &&
>>>>>> !bank->context_valid?
>>>>
>>>> I really like this idea a lot. It can really clean-up the code
>>>> and really make it much more readable. Before we were playing 
>>>> some tricks with when we init'ed the get_context_loss_count()
>>>> function pointer. How about the below?
>>>>
>>>> Tony, care to re-test?
>>>>
>>>> Cheers
>>>> Jon
>>>>
>>>> From d7a940531d354e6be5e16ee50fa8344041df963a Mon Sep 17 00:00:00 2001
>>>> From: Jon Hunter <jon-hunter@ti.com>
>>>> Date: Mon, 15 Apr 2013 13:06:54 -0500
>>>> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
>>>>
>>>> Commit a2797be (gpio/omap: force restore if context loss is not
>>>> detectable) broke gpio support for OMAP when booting with device-tree
>>>> because a restore of the gpio context being performed without ever
>>>> initialising the gpio context. In other words, the context restored was
>>>> bad.
>>>>
>>>> This problem could also occur in the non device-tree case, however, it
>>>> is much less likely because when booting without device-tree we can
>>>> detect context loss via a platform specific API and so context restore
>>>> is performed less often.
>>>>
>>>> Nevertheless we should ensure that the gpio context is initialised
>>>> on the first pm-runtime resume for gpio banks that could lose their
>>>> state regardless of whether we are booting with device-tree or not.
>>>>
>>>> The context loss count was being initialised on the first pm-runtime
>>>> suspend following a resume, by populating the get_count_loss_count()
>>>> function pointer after the first pm-runtime resume. To make the code
>>>> more readable and logical, initialise the context loss count on the
>>>> first pm-runtime resume if the context is not yet valid.
>>>>
>>>> Reported-by: Tony Lindgren <tony@atomide.com>
>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>> ---
>>>>  drivers/gpio/gpio-omap.c |   45 ++++++++++++++++++++++++++++++++++++++++++---
>>>>  1 file changed, 42 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>>>> index 0557529..db3c732 100644
>>>> --- a/drivers/gpio/gpio-omap.c
>>>> +++ b/drivers/gpio/gpio-omap.c
>>>> @@ -70,6 +70,7 @@ struct gpio_bank {
>>>>  	bool is_mpuio;
>>>>  	bool dbck_flag;
>>>>  	bool loses_context;
>>>> +	bool context_valid;
>>>>  	int stride;
>>>>  	u32 width;
>>>>  	int context_loss_count;
>>>> @@ -1129,6 +1130,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
>>>>  			bank->loses_context = true;
>>>>  	} else {
>>>>  		bank->loses_context = pdata->loses_context;
>>>> +		bank->get_context_loss_count = pdata->get_context_loss_count;
>>>
>>> Still need to check loses_context for populating
>>> get_context_loss_count here. Updated patch below.
>>>
>>> Jon
>>>
>>> From d02ef7b7dfcf8e13bf019aedfdecb38ca3c6749f Mon Sep 17 00:00:00 2001
>>> From: Jon Hunter <jon-hunter@ti.com>
>>> Date: Mon, 15 Apr 2013 13:06:54 -0500
>>> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
>>>
>>> Commit a2797be (gpio/omap: force restore if context loss is not
>>> detectable) broke gpio support for OMAP when booting with device-tree
>>> because a restore of the gpio context being performed without ever
>>> initialising the gpio context. In other words, the context restored was
>>> bad.
>>>
>>> This problem could also occur in the non device-tree case, however, it
>>> is much less likely because when booting without device-tree we can
>>> detect context loss via a platform specific API and so context restore
>>> is performed less often.
>>>
>>> Nevertheless we should ensure that the gpio context is initialised
>>> on the first pm-runtime resume for gpio banks that could lose their
>>> state regardless of whether we are booting with device-tree or not.
>>>
>>> The context loss count was being initialised on the first pm-runtime
>>> suspend following a resume, by populating the get_count_loss_count()
>>> function pointer after the first pm-runtime resume. To make the code
>>> more readable and logical, initialise the context loss count on the
>>> first pm-runtime resume if the context is not yet valid.
>>>
>>> Reported-by: Tony Lindgren <tony@atomide.com>
>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>> ---
>> This version looks better than the first one for sure. I am still not
>> happy with per bank "context_valid" flag whose job just ends after
>> the probe. 
> 
> Assuming this driver could become a module someday (not terribly likely,
> I know), but context_valid would have meaning for each module reload.
> 
I don't think GPIO can ever be a module(at least on OMAP) considering the way
it is used in many SOCs and hence the objection on that flag. GPIO is really
an infrastructure driver which is needed to be always available for the
client drivers to work. 

Regards,
Santosh

[PATCH] gpio/omap: ensure gpio context is initialised

[Tony Lindgren]

* Santosh Shilimkar <santosh.shilimkar@ti.com> [130419 07:43]:
> On Friday 19 April 2013 07:35 PM, Kevin Hilman wrote:
> > Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
> > 
> >> On Friday 19 April 2013 06:19 AM, Jon Hunter wrote:
> >>>
> >>> On 04/18/2013 07:34 PM, Jon Hunter wrote:
> >>>>
> >>>> On 04/18/2013 06:10 PM, Jon Hunter wrote:
> >>>>> On 04/18/2013 04:34 PM, Kevin Hilman wrote:
> >>>>
> >>>> ...
> >>>>
> >>>>>> Why not just init context right here if bank->loses_context &&
> >>>>>> !bank->context_valid?
> >>>>
> >>>> I really like this idea a lot. It can really clean-up the code
> >>>> and really make it much more readable. Before we were playing 
> >>>> some tricks with when we init'ed the get_context_loss_count()
> >>>> function pointer. How about the below?
> >>>>
> >>>> Tony, care to re-test?

Still works thanks:

Tested-by: Tony Lindgren <tony@atomide.com>

> > Assuming this driver could become a module someday (not terribly likely,
> > I know), but context_valid would have meaning for each module reload.
> > 
> I don't think GPIO can ever be a module(at least on OMAP) considering the way
> it is used in many SOCs and hence the objection on that flag. GPIO is really
> an infrastructure driver which is needed to be always available for the
> client drivers to work. 

Not really related to this fix.. But when booting with initramfs, we should
be able to make everything into loadable modules except things that are related
to enabling timers and interrupts. This is important for distros to be able
do generic kernels. Surely for mounting non-initramfs root GPIO is most
likely needed though :)

Regards,

Tony

[PATCH] gpio/omap: ensure gpio context is initialised

[Linus Walleij]

On Fri, Apr 19, 2013 at 2:49 AM, Jon Hunter <jon-hunter@ti.com> wrote:

> From d02ef7b7dfcf8e13bf019aedfdecb38ca3c6749f Mon Sep 17 00:00:00 2001
> From: Jon Hunter <jon-hunter@ti.com>
> Date: Mon, 15 Apr 2013 13:06:54 -0500
> Subject: [PATCH] gpio/omap: ensure gpio context is initialised
>
> Commit a2797be (gpio/omap: force restore if context loss is not
> detectable) broke gpio support for OMAP when booting with device-tree
> because a restore of the gpio context being performed without ever
> initialising the gpio context. In other words, the context restored was
> bad.
>
> This problem could also occur in the non device-tree case, however, it
> is much less likely because when booting without device-tree we can
> detect context loss via a platform specific API and so context restore
> is performed less often.
>
> Nevertheless we should ensure that the gpio context is initialised
> on the first pm-runtime resume for gpio banks that could lose their
> state regardless of whether we are booting with device-tree or not.
>
> The context loss count was being initialised on the first pm-runtime
> suspend following a resume, by populating the get_count_loss_count()
> function pointer after the first pm-runtime resume. To make the code
> more readable and logical, initialise the context loss count on the
> first pm-runtime resume if the context is not yet valid.
>
> Reported-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>

Applied this version with Santosh's and Kevin's ACKs and
Tony's Tested-by, thanks!

Yours,
Linus Walleij