All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jon-hunter@ti.com>
To: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Felipe Balbi <balbi@ti.com>,
	linux-omap@vger.kernel.org, Paul Walmsley <paul@pwsan.com>,
	Igor Grinberg <grinberg@compulab.co.il>,
	linux-arm-kernel@lists.infradead.org,
	Grazvydas Ignotas <notasas@gmail.com>
Subject: Re: [PATCH v2] gpio/omap: fix off-mode bug: clear debounce clock enable mask on free/reset
Date: Wed, 24 Oct 2012 17:54:15 -0500	[thread overview]
Message-ID: <50887197.8010104@ti.com> (raw)
In-Reply-To: <1351098641-23917-1-git-send-email-khilman@deeprootsystems.com>


On 10/24/2012 12:10 PM, Kevin Hilman wrote:
> From: Kevin Hilman <khilman@ti.com>
> 
> When a GPIO bank is freed or shutdown, ensure that the banks
> dbck_enable_mask is cleared also.  Otherwise, context restore on
> subsequent off-mode transition will restore previous value from the
> shadow copies (bank->context.debounce*) leading to mismatch state
> between driver state and hardware state.
> 
> This was discovered when board code was doing
> 
>   gpio_request_one()
>   gpio_set_debounce()
>   gpio_free()
> 
> which was leaving the GPIO debounce settings in a confused state.  If
> that GPIO bank is subsequently used with off-mode enabled, bogus state
> would be restored, leaving GPIO debounce enabled which then prevented
> the CORE powerdomain from transitioning.
> 
> To fix, ensure that bank->dbck_enable_mask is cleared when the bank
> is freed/shutdown so debounce state doesn't persist.
> 
> Special thanks to Grazvydas Ignotas for pointing out a bug in an
> earlier version that would've disabled debounce on any runtime PM
> transition.
> 
> Reported-by: Paul Walmsley <paul@pwsan.com>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Cc: Grazvydas Ignotas <notasas@gmail.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
> v2: only clear mask in free/shutdown, not in runtime PM paths, 
>     clarified changelog   
> Applies on v3.7-rc2.
> 
>  drivers/gpio/gpio-omap.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 94cbc84..113b167 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -539,6 +539,7 @@ static void _reset_gpio(struct gpio_bank *bank, int gpio)
>  	_set_gpio_irqenable(bank, gpio, 0);
>  	_clear_gpio_irqstatus(bank, gpio);
>  	_set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), IRQ_TYPE_NONE);
> +	bank->dbck_enable_mask = 0;
>  }

Does this need to be ...

+	bank->dbck_enable_mask &= ~(GPIO_BIT(bank, gpio));
+	_gpio_dbck_disable(bank);

There could be more than one gpio using debounce and so we should only
clear the appropriate bit. Also after clearing a bit we could see if we
can disable the debounce clock too.

Cheers
Jon

WARNING: multiple messages have this Message-ID (diff)
From: jon-hunter@ti.com (Jon Hunter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] gpio/omap: fix off-mode bug: clear debounce clock enable mask on free/reset
Date: Wed, 24 Oct 2012 17:54:15 -0500	[thread overview]
Message-ID: <50887197.8010104@ti.com> (raw)
In-Reply-To: <1351098641-23917-1-git-send-email-khilman@deeprootsystems.com>


On 10/24/2012 12:10 PM, Kevin Hilman wrote:
> From: Kevin Hilman <khilman@ti.com>
> 
> When a GPIO bank is freed or shutdown, ensure that the banks
> dbck_enable_mask is cleared also.  Otherwise, context restore on
> subsequent off-mode transition will restore previous value from the
> shadow copies (bank->context.debounce*) leading to mismatch state
> between driver state and hardware state.
> 
> This was discovered when board code was doing
> 
>   gpio_request_one()
>   gpio_set_debounce()
>   gpio_free()
> 
> which was leaving the GPIO debounce settings in a confused state.  If
> that GPIO bank is subsequently used with off-mode enabled, bogus state
> would be restored, leaving GPIO debounce enabled which then prevented
> the CORE powerdomain from transitioning.
> 
> To fix, ensure that bank->dbck_enable_mask is cleared when the bank
> is freed/shutdown so debounce state doesn't persist.
> 
> Special thanks to Grazvydas Ignotas for pointing out a bug in an
> earlier version that would've disabled debounce on any runtime PM
> transition.
> 
> Reported-by: Paul Walmsley <paul@pwsan.com>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Cc: Grazvydas Ignotas <notasas@gmail.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
> v2: only clear mask in free/shutdown, not in runtime PM paths, 
>     clarified changelog   
> Applies on v3.7-rc2.
> 
>  drivers/gpio/gpio-omap.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 94cbc84..113b167 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -539,6 +539,7 @@ static void _reset_gpio(struct gpio_bank *bank, int gpio)
>  	_set_gpio_irqenable(bank, gpio, 0);
>  	_clear_gpio_irqstatus(bank, gpio);
>  	_set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), IRQ_TYPE_NONE);
> +	bank->dbck_enable_mask = 0;
>  }

Does this need to be ...

+	bank->dbck_enable_mask &= ~(GPIO_BIT(bank, gpio));
+	_gpio_dbck_disable(bank);

There could be more than one gpio using debounce and so we should only
clear the appropriate bit. Also after clearing a bit we could see if we
can disable the debounce clock too.

Cheers
Jon

  reply	other threads:[~2012-10-24 22:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-24 17:10 [PATCH v2] gpio/omap: fix off-mode bug: clear debounce clock enable mask on free/reset Kevin Hilman
2012-10-24 17:10 ` Kevin Hilman
2012-10-24 22:54 ` Jon Hunter [this message]
2012-10-24 22:54   ` Jon Hunter
2012-10-25  7:00   ` Santosh Shilimkar
2012-10-25  7:00     ` Santosh Shilimkar
2012-10-25 13:11     ` Jon Hunter
2012-10-25 13:11       ` Jon Hunter
2012-10-25 13:19       ` Santosh Shilimkar
2012-10-25 13:19         ` Santosh Shilimkar
2012-10-25 16:30       ` Kevin Hilman
2012-10-25 16:30         ` Kevin Hilman
2012-10-25 16:49         ` Jon Hunter
2012-10-25 16:49           ` Jon Hunter
2012-10-25 16:26   ` Kevin Hilman
2012-10-25 16:26     ` Kevin Hilman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50887197.8010104@ti.com \
    --to=jon-hunter@ti.com \
    --cc=balbi@ti.com \
    --cc=grinberg@compulab.co.il \
    --cc=khilman@deeprootsystems.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=notasas@gmail.com \
    --cc=paul@pwsan.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.