diff mbox

[1/2] gpio/omap: maintain GPIO and IRQ usage separately

Message ID 1380069414-2922-1-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas Sept. 25, 2013, 12:36 a.m. UTC
The GPIO OMAP controller pins can be used as IRQ and GPIO
independently so is necessary to keep track GPIO pins and
IRQ lines usage separately to make sure that the bank will
always be enabled while being used.

Also move gpio_is_input() definition in preparation for the
next patch that setups the controller's irq_chip driver when
a caller requests an interrupt line.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/gpio/gpio-omap.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

Comments

Lars Poeschel Sept. 27, 2013, 2:31 p.m. UTC | #1
On Friday 27 September 2013 02:36:52, Javier Martinez Canillas wrote:
> The GPIO OMAP controller pins can be used as IRQ and GPIO
> independently so is necessary to keep track GPIO pins and
> IRQ lines usage separately to make sure that the bank will
> always be enabled while being used.
> 
> Also move gpio_is_input() definition in preparation for the
> next patch that setups the controller's irq_chip driver when
> a caller requests an interrupt line.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

For both patches:

Tested-by: Lars Poeschel <poeschel@lemonage.de>

on an am3359 (which is DT booting).
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Sept. 27, 2013, 6:08 p.m. UTC | #2
Javier Martinez Canillas <javier.martinez@collabora.co.uk> writes:

> The GPIO OMAP controller pins can be used as IRQ and GPIO
> independently so is necessary to keep track GPIO pins and
> IRQ lines usage separately to make sure that the bank will
> always be enabled while being used.
>
> Also move gpio_is_input() definition in preparation for the
> next patch that setups the controller's irq_chip driver when
> a caller requests an interrupt line.
>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

I'm fine with this appproach.  For both patches:

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

Also, I gave it a spin across a handful of OMAP boards using v3.12-rc2 +
these 2 patches.

Boot tested successfully with DT boot:

omap3530/beagle
omap3730/beagle-xm
omap3530/overo (Tobi w/GPIO IRQ networking)
omap3730/overo STORM (w/GPIO IRQ for networking)
am335x/beaglebone
am335x/beaglebone black
omap4430/panda
omap4460/panda-es
omap5912/OSK (omap1)

I also verified non-DT boot on the OMAP3 platforms that still support
legacy boot.

So feel free to also add

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

Thanks for your persistence in getting a fix for this upstream.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar Sept. 27, 2013, 7:31 p.m. UTC | #3
On Tuesday 24 September 2013 08:36 PM, Javier Martinez Canillas wrote:
> The GPIO OMAP controller pins can be used as IRQ and GPIO
> independently so is necessary to keep track GPIO pins and
> IRQ lines usage separately to make sure that the bank will
> always be enabled while being used.
> 
> Also move gpio_is_input() definition in preparation for the
> next patch that setups the controller's irq_chip driver when
> a caller requests an interrupt line.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
Considering GPIO core maintainer is fine with this approach,
am fine with both of your patches. Again thanks a lot for
fixing the long nagging issue.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Sept. 27, 2013, 9:01 p.m. UTC | #4
* Javier Martinez Canillas <javier.martinez@collabora.co.uk> [130924 17:45]:
> The GPIO OMAP controller pins can be used as IRQ and GPIO
> independently so is necessary to keep track GPIO pins and
> IRQ lines usage separately to make sure that the bank will
> always be enabled while being used.
> 
> Also move gpio_is_input() definition in preparation for the
> next patch that setups the controller's irq_chip driver when
> a caller requests an interrupt line.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Thanks both of these look good to me for fixing
the regression:

Acked-by: Tony Lindgren <tony@atomide.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas Oct. 1, 2013, 7:34 a.m. UTC | #5
On 09/27/2013 08:08 PM, Kevin Hilman wrote:
> Javier Martinez Canillas <javier.martinez@collabora.co.uk> writes:
> 
>> The GPIO OMAP controller pins can be used as IRQ and GPIO
>> independently so is necessary to keep track GPIO pins and
>> IRQ lines usage separately to make sure that the bank will
>> always be enabled while being used.
>>
>> Also move gpio_is_input() definition in preparation for the
>> next patch that setups the controller's irq_chip driver when
>> a caller requests an interrupt line.
>>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> 
> I'm fine with this appproach.  For both patches:
> 
> Reviewed-by: Kevin Hilman <khilman@linaro.org>
> 
> Also, I gave it a spin across a handful of OMAP boards using v3.12-rc2 +
> these 2 patches.
> 
> Boot tested successfully with DT boot:
> 
> omap3530/beagle
> omap3730/beagle-xm
> omap3530/overo (Tobi w/GPIO IRQ networking)
> omap3730/overo STORM (w/GPIO IRQ for networking)
> am335x/beaglebone
> am335x/beaglebone black
> omap4430/panda
> omap4460/panda-es
> omap5912/OSK (omap1)
> 
> I also verified non-DT boot on the OMAP3 platforms that still support
> legacy boot.
> 
> So feel free to also add
> 
> Tested-by: Kevin Hilman <khilman@linaro.org>
> 
> Thanks for your persistence in getting a fix for this upstream.
> 
> Kevin
> 

Thanks a lot Kevin for testing on so many boards.

Linus,

Since this patch-set doesn't cause any regression and fix a long standing issue
on OMAP, do you think that it would be possible to include on the -rc series as
a bugfix or do you prefer to wait until 3.13?

Thanks a lot and best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Oct. 1, 2013, 11:14 a.m. UTC | #6
On Tue, Oct 1, 2013 at 9:34 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:

> Linus,
>
> Since this patch-set doesn't cause any regression and fix a long standing issue
> on OMAP, do you think that it would be possible to include on the -rc series as
> a bugfix or do you prefer to wait until 3.13?

I've added it to my fixes branch for v3.12. Since linux-next is not available we
need some extensive build testing first to make sure that if something explodes
it explodes on OMAP only.

So will let this boil a few days and then send a pull request.

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

Patch

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 0ff4355..a4fe038 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -63,6 +63,7 @@  struct gpio_bank {
 	struct gpio_chip chip;
 	struct clk *dbck;
 	u32 mod_usage;
+	u32 irq_usage;
 	u32 dbck_enable_mask;
 	bool dbck_enabled;
 	struct device *dev;
@@ -86,6 +87,9 @@  struct gpio_bank {
 #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
 #define GPIO_MOD_CTRL_BIT	BIT(0)
 
+#define BANK_USED(bank) (bank->mod_usage || bank->irq_usage)
+#define LINE_USED(line, offset) (line & (1 << offset))
+
 static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
 {
 	return bank->chip.base + gpio_irq;
@@ -420,6 +424,13 @@  static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
 	return 0;
 }
 
+static int gpio_is_input(struct gpio_bank *bank, int mask)
+{
+	void __iomem *reg = bank->base + bank->regs->direction;
+
+	return __raw_readl(reg) & mask;
+}
+
 static int gpio_irq_type(struct irq_data *d, unsigned type)
 {
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
@@ -427,7 +438,7 @@  static int gpio_irq_type(struct irq_data *d, unsigned type)
 	int retval;
 	unsigned long flags;
 
-	if (WARN_ON(!bank->mod_usage))
+	if (WARN_ON(!BANK_USED(bank)))
 		return -EINVAL;
 
 #ifdef CONFIG_ARCH_OMAP1
@@ -447,6 +458,7 @@  static int gpio_irq_type(struct irq_data *d, unsigned type)
 
 	spin_lock_irqsave(&bank->lock, flags);
 	retval = _set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), type);
+	bank->irq_usage |= 1 << GPIO_INDEX(bank, gpio);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))
@@ -603,7 +615,7 @@  static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 	 * If this is the first gpio_request for the bank,
 	 * enable the bank module.
 	 */
-	if (!bank->mod_usage)
+	if (!BANK_USED(bank))
 		pm_runtime_get_sync(bank->dev);
 
 	spin_lock_irqsave(&bank->lock, flags);
@@ -619,7 +631,7 @@  static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 		__raw_writel(__raw_readl(reg) | (1 << offset), reg);
 	}
 
-	if (bank->regs->ctrl && !bank->mod_usage) {
+	if (bank->regs->ctrl && !BANK_USED(bank)) {
 		void __iomem *reg = bank->base + bank->regs->ctrl;
 		u32 ctrl;
 
@@ -654,7 +666,7 @@  static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
 
 	bank->mod_usage &= ~(1 << offset);
 
-	if (bank->regs->ctrl && !bank->mod_usage) {
+	if (bank->regs->ctrl && !BANK_USED(bank)) {
 		void __iomem *reg = bank->base + bank->regs->ctrl;
 		u32 ctrl;
 
@@ -672,7 +684,7 @@  static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
 	 * If this is the last gpio to be freed in the bank,
 	 * disable the bank module.
 	 */
-	if (!bank->mod_usage)
+	if (!BANK_USED(bank))
 		pm_runtime_put(bank->dev);
 }
 
@@ -762,8 +774,10 @@  static void gpio_irq_shutdown(struct irq_data *d)
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
 	unsigned int gpio = irq_to_gpio(bank, d->hwirq);
 	unsigned long flags;
+	unsigned offset = GPIO_INDEX(bank, gpio);
 
 	spin_lock_irqsave(&bank->lock, flags);
+	bank->irq_usage &= ~(1 << offset);
 	_reset_gpio(bank, gpio);
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
@@ -897,13 +911,6 @@  static int gpio_input(struct gpio_chip *chip, unsigned offset)
 	return 0;
 }
 
-static int gpio_is_input(struct gpio_bank *bank, int mask)
-{
-	void __iomem *reg = bank->base + bank->regs->direction;
-
-	return __raw_readl(reg) & mask;
-}
-
 static int gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct gpio_bank *bank;
@@ -1400,7 +1407,7 @@  void omap2_gpio_prepare_for_idle(int pwr_mode)
 	struct gpio_bank *bank;
 
 	list_for_each_entry(bank, &omap_gpio_list, node) {
-		if (!bank->mod_usage || !bank->loses_context)
+		if (!BANK_USED(bank) || !bank->loses_context)
 			continue;
 
 		bank->power_mode = pwr_mode;
@@ -1414,7 +1421,7 @@  void omap2_gpio_resume_after_idle(void)
 	struct gpio_bank *bank;
 
 	list_for_each_entry(bank, &omap_gpio_list, node) {
-		if (!bank->mod_usage || !bank->loses_context)
+		if (!BANK_USED(bank) || !bank->loses_context)
 			continue;
 
 		pm_runtime_get_sync(bank->dev);