[PATCH v3 1/3] gpio/omap: replace open coded read-modify-write with _gpio_rmw function.

[PATCH v3 1/3] gpio/omap: replace open coded read-modify-write with _gpio_rmw function.

[Andreas Fenkart]

By also making it return the modified value, we save the readl
needed to update the context.

Signed-off-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
---
 drivers/gpio/gpio-omap.c |  162 ++++++++++++++--------------------------------
 1 file changed, 50 insertions(+), 112 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 159f5c5..082919e 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -87,6 +87,19 @@ struct gpio_bank {
 #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
 #define GPIO_MOD_CTRL_BIT	BIT(0)
 
+static inline u32 _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)
+{
+	u32 l = __raw_readl(base + reg);
+
+	if (set)
+		l |= mask;
+	else
+		l &= ~mask;
+
+	__raw_writel(l, base + reg);
+	return l;
+}
+
 static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
 {
 	return gpio_irq - bank->irq_base + bank->chip.base;
@@ -94,20 +107,10 @@ static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
 
 static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
 {
-	void __iomem *reg = bank->base;
-	u32 l;
-
-	reg += bank->regs->direction;
-	l = __raw_readl(reg);
-	if (is_input)
-		l |= 1 << gpio;
-	else
-		l &= ~(1 << gpio);
-	__raw_writel(l, reg);
-	bank->context.oe = l;
+	bank->context.oe = _gpio_rmw(bank->base, bank->regs->direction, 1 <<
+				     gpio, is_input);
 }
 
-
 /* set data out value using dedicate set/clear register */
 static void _set_gpio_dataout_reg(struct gpio_bank *bank, int gpio, int enable)
 {
@@ -128,17 +131,8 @@ static void _set_gpio_dataout_reg(struct gpio_bank *bank, int gpio, int enable)
 /* set data out value using mask register */
 static void _set_gpio_dataout_mask(struct gpio_bank *bank, int gpio, int enable)
 {
-	void __iomem *reg = bank->base + bank->regs->dataout;
-	u32 gpio_bit = GPIO_BIT(bank, gpio);
-	u32 l;
-
-	l = __raw_readl(reg);
-	if (enable)
-		l |= gpio_bit;
-	else
-		l &= ~gpio_bit;
-	__raw_writel(l, reg);
-	bank->context.dataout = l;
+	bank->context.dataout = _gpio_rmw(bank->base, bank->regs->dataout,
+					  GPIO_BIT(bank, gpio), enable);
 }
 
 static int _get_gpio_datain(struct gpio_bank *bank, int offset)
@@ -155,18 +149,6 @@ static int _get_gpio_dataout(struct gpio_bank *bank, int offset)
 	return (__raw_readl(reg) & (1 << offset)) != 0;
 }
 
-static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)
-{
-	int l = __raw_readl(base + reg);
-
-	if (set)
-		l |= mask;
-	else
-		l &= ~mask;
-
-	__raw_writel(l, base + reg);
-}
-
 static inline void _gpio_dbck_enable(struct gpio_bank *bank)
 {
 	if (bank->dbck_enable_mask && !bank->dbck_enabled) {
@@ -291,28 +273,24 @@ static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio,
 	void __iomem *base = bank->base;
 	u32 gpio_bit = 1 << gpio;
 
-	_gpio_rmw(base, bank->regs->leveldetect0, gpio_bit,
-		  trigger & IRQ_TYPE_LEVEL_LOW);
-	_gpio_rmw(base, bank->regs->leveldetect1, gpio_bit,
-		  trigger & IRQ_TYPE_LEVEL_HIGH);
-	_gpio_rmw(base, bank->regs->risingdetect, gpio_bit,
-		  trigger & IRQ_TYPE_EDGE_RISING);
-	_gpio_rmw(base, bank->regs->fallingdetect, gpio_bit,
-		  trigger & IRQ_TYPE_EDGE_FALLING);
 
 	bank->context.leveldetect0 =
-			__raw_readl(bank->base + bank->regs->leveldetect0);
+		_gpio_rmw(base, bank->regs->leveldetect0,
+			  gpio_bit, trigger & IRQ_TYPE_LEVEL_LOW);
 	bank->context.leveldetect1 =
-			__raw_readl(bank->base + bank->regs->leveldetect1);
+		_gpio_rmw(base, bank->regs->leveldetect1,
+			  gpio_bit, trigger & IRQ_TYPE_LEVEL_HIGH);
 	bank->context.risingdetect =
-			__raw_readl(bank->base + bank->regs->risingdetect);
+		_gpio_rmw(base, bank->regs->risingdetect,
+			  gpio_bit, trigger & IRQ_TYPE_EDGE_RISING);
 	bank->context.fallingdetect =
-			__raw_readl(bank->base + bank->regs->fallingdetect);
+		_gpio_rmw(base, bank->regs->fallingdetect,
+			  gpio_bit, trigger & IRQ_TYPE_EDGE_FALLING);
 
 	if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
-		_gpio_rmw(base, bank->regs->wkup_en, gpio_bit, trigger != 0);
 		bank->context.wake_en =
-			__raw_readl(bank->base + bank->regs->wkup_en);
+			_gpio_rmw(base, bank->regs->wkup_en, gpio_bit,
+				  trigger != 0);
 	}
 
 	/* This part needs to be executed always for OMAP{34xx, 44xx} */
@@ -406,9 +384,9 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
 			l |= 1 << (gpio << 1);
 
 		/* Enable wake-up during idle for dynamic tick */
-		_gpio_rmw(base, bank->regs->wkup_en, 1 << gpio, trigger);
 		bank->context.wake_en =
-			__raw_readl(bank->base + bank->regs->wkup_en);
+			_gpio_rmw(base, bank->regs->wkup_en, 1 << gpio,
+				  trigger);
 		__raw_writel(l, reg);
 	}
 	return 0;
@@ -450,19 +428,16 @@ static int gpio_irq_type(struct irq_data *d, unsigned type)
 
 static void _clear_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 {
-	void __iomem *reg = bank->base;
+	void __iomem *base = bank->base;
 
-	reg += bank->regs->irqstatus;
-	__raw_writel(gpio_mask, reg);
+	__raw_writel(gpio_mask, base + bank->regs->irqstatus);
 
 	/* Workaround for clearing DSP GPIO interrupts to allow retention */
-	if (bank->regs->irqstatus2) {
-		reg = bank->base + bank->regs->irqstatus2;
-		__raw_writel(gpio_mask, reg);
-	}
+	if (bank->regs->irqstatus2)
+		__raw_writel(gpio_mask, base + bank->regs->irqstatus2);
 
 	/* Flush posted write for the irq status to avoid spurious interrupts */
-	__raw_readl(reg);
+	__raw_readl(base + bank->regs->irqstatus);
 }
 
 static inline void _clear_gpio_irqstatus(struct gpio_bank *bank, int gpio)
@@ -486,46 +461,26 @@ static u32 _get_gpio_irqbank_mask(struct gpio_bank *bank)
 
 static void _enable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 {
-	void __iomem *reg = bank->base;
-	u32 l;
-
 	if (bank->regs->set_irqenable) {
-		reg += bank->regs->set_irqenable;
-		l = gpio_mask;
 		bank->context.irqenable1 |= gpio_mask;
+		__raw_writel(gpio_mask, bank->base + bank->regs->set_irqenable);
 	} else {
-		reg += bank->regs->irqenable;
-		l = __raw_readl(reg);
-		if (bank->regs->irqenable_inv)
-			l &= ~gpio_mask;
-		else
-			l |= gpio_mask;
-		bank->context.irqenable1 = l;
+		bank->context.irqenable1 =
+			_gpio_rmw(bank->base, bank->regs->irqenable, gpio_mask,
+				  bank->regs->irqenable_inv ? false : true);
 	}
-
-	__raw_writel(l, reg);
 }
 
 static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 {
-	void __iomem *reg = bank->base;
-	u32 l;
-
 	if (bank->regs->clr_irqenable) {
-		reg += bank->regs->clr_irqenable;
-		l = gpio_mask;
 		bank->context.irqenable1 &= ~gpio_mask;
+		__raw_writel(gpio_mask, bank->base + bank->regs->clr_irqenable);
 	} else {
-		reg += bank->regs->irqenable;
-		l = __raw_readl(reg);
-		if (bank->regs->irqenable_inv)
-			l |= gpio_mask;
-		else
-			l &= ~gpio_mask;
-		bank->context.irqenable1 = l;
+		bank->context.irqenable1 =
+			_gpio_rmw(bank->base, bank->regs->irqenable, gpio_mask,
+				  bank->regs->irqenable_inv ?: false);
 	}
-
-	__raw_writel(l, reg);
 }
 
 static inline void _set_gpio_irqenable(struct gpio_bank *bank, int gpio, int enable)
@@ -556,12 +511,8 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
 	}
 
 	spin_lock_irqsave(&bank->lock, flags);
-	if (enable)
-		bank->context.wake_en |= gpio_bit;
-	else
-		bank->context.wake_en &= ~gpio_bit;
-
-	__raw_writel(bank->context.wake_en, bank->base + bank->regs->wkup_en);
+	bank->context.wake_en = _gpio_rmw(bank->base, bank->regs->wkup_en,
+					  gpio_bit, enable);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -604,21 +555,14 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 	_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
 
 	if (bank->regs->pinctrl) {
-		void __iomem *reg = bank->base + bank->regs->pinctrl;
-
 		/* Claim the pin for MPU */
-		__raw_writel(__raw_readl(reg) | (1 << offset), reg);
+		_gpio_rmw(bank->base, bank->regs->pinctrl, 1 << offset, 1);
 	}
 
 	if (bank->regs->ctrl && !bank->mod_usage) {
-		void __iomem *reg = bank->base + bank->regs->ctrl;
-		u32 ctrl;
-
-		ctrl = __raw_readl(reg);
 		/* Module is enabled, clocks are not gated */
-		ctrl &= ~GPIO_MOD_CTRL_BIT;
-		__raw_writel(ctrl, reg);
-		bank->context.ctrl = ctrl;
+		bank->context.ctrl = _gpio_rmw(bank->base, bank->regs->ctrl,
+					       GPIO_MOD_CTRL_BIT, 0);
 	}
 
 	bank->mod_usage |= 1 << offset;
@@ -638,22 +582,16 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
 
 	if (bank->regs->wkup_en) {
 		/* Disable wake-up during idle for dynamic tick */
-		_gpio_rmw(base, bank->regs->wkup_en, 1 << offset, 0);
 		bank->context.wake_en =
-			__raw_readl(bank->base + bank->regs->wkup_en);
+			_gpio_rmw(base, bank->regs->wkup_en, 1 << offset, 0);
 	}
 
 	bank->mod_usage &= ~(1 << offset);
 
 	if (bank->regs->ctrl && !bank->mod_usage) {
-		void __iomem *reg = bank->base + bank->regs->ctrl;
-		u32 ctrl;
-
-		ctrl = __raw_readl(reg);
 		/* Module is disabled, clocks are gated */
-		ctrl |= GPIO_MOD_CTRL_BIT;
-		__raw_writel(ctrl, reg);
-		bank->context.ctrl = ctrl;
+		bank->context.ctrl = _gpio_rmw(base, bank->regs->ctrl,
+					       GPIO_MOD_CTRL_BIT, 1);
 	}
 
 	_reset_gpio(bank, bank->chip.base + offset);
-- 
1.7.10.4

[PATCH v3 2/3] gpio/omap: modify wake-up register with interrupt enable.

[Andreas Fenkart]

OMAP4430 TRM chap. 25.4.5.2
To reduce dynamic consumption, an efficient idle scheme is based on the
following:
• An efficient local autoclock gating for each module
• The implementation of control sideband signals between the PRCM module
  and each module
This enhanced idle control allows clocks to be activated and deactivated
safely without requiring complex software management. The idle mode
request, idle acknowledge, and wake-up request are sideband signals
between the PRCM module and the general-purpose interface

OMAP4430 TRM chap. 25.4.6.2
There must be a correlation between the wake-up enable and interrupt
enable register. If a GPIO pin has a wake-up configured on it, it must
also have the corresponding interrupt enabled. Otherwise, it is possible
there is a wake-up event, but after exiting the IDLE state, no interrupt
is generated; the corresponding bit from the interrupt status register is
not cleared, and the module does not acknowledge a future idle request.

Up to now _set_gpio_triggering() is also handling the wake-up enable
register. According the TRM this should be in sync with the interrupt
enable. Wakeup is still enabled by default, since the module would not
wake from idle otherwise.
The enabled_wakeup_gpios was introduced to remember an explicit
_set_gpio_wakeup beyond a mask/unmask cycle. Calling the flag flag
disabled_wakeup_gpios would spare the problem of initializing it, but
feels very unnatural to read.

Wakeup functionality is completely untested, since the AM335x
lacks a IRQWAKEN register.

Signed-off-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
---
 drivers/gpio/gpio-omap.c |   68 ++++++++++++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 23 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 082919e..44a93be 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -57,6 +57,7 @@ struct gpio_bank {
 	struct irq_domain *domain;
 	u32 non_wakeup_gpios;
 	u32 enabled_non_wakeup_gpios;
+	u32 enabled_wakeup_gpios;
 	struct gpio_regs context;
 	u32 saved_datain;
 	u32 level_mask;
@@ -287,12 +288,6 @@ static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio,
 		_gpio_rmw(base, bank->regs->fallingdetect,
 			  gpio_bit, trigger & IRQ_TYPE_EDGE_FALLING);
 
-	if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
-		bank->context.wake_en =
-			_gpio_rmw(base, bank->regs->wkup_en, gpio_bit,
-				  trigger != 0);
-	}
-
 	/* This part needs to be executed always for OMAP{34xx, 44xx} */
 	if (!bank->regs->irqctrl) {
 		/* On omap24xx proceed only when valid GPIO bit is set */
@@ -350,12 +345,13 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
 							unsigned trigger)
 {
 	void __iomem *reg = bank->base;
-	void __iomem *base = bank->base;
 	u32 l = 0;
 
-	if (bank->regs->leveldetect0 && bank->regs->wkup_en) {
+	if (bank->regs->leveldetect0) {
+		/* edge both flanks simultaneously / plus level */
 		set_gpio_trigger(bank, gpio, trigger);
 	} else if (bank->regs->irqctrl) {
+		/* edge single flank */
 		reg += bank->regs->irqctrl;
 
 		l = __raw_readl(reg);
@@ -370,6 +366,7 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
 
 		__raw_writel(l, reg);
 	} else if (bank->regs->edgectrl1) {
+		/* edge both flanks simultaneously */
 		if (gpio & 0x08)
 			reg += bank->regs->edgectrl2;
 		else
@@ -382,11 +379,6 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
 			l |= 2 << (gpio << 1);
 		if (trigger & IRQ_TYPE_EDGE_FALLING)
 			l |= 1 << (gpio << 1);
-
-		/* Enable wake-up during idle for dynamic tick */
-		bank->context.wake_en =
-			_gpio_rmw(base, bank->regs->wkup_en, 1 << gpio,
-				  trigger);
 		__raw_writel(l, reg);
 	}
 	return 0;
@@ -485,10 +477,19 @@ static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 
 static inline void _set_gpio_irqenable(struct gpio_bank *bank, int gpio, int enable)
 {
+	u32 gpio_bit = GPIO_BIT(bank, gpio);
+	void __iomem *base = bank->base;
+
 	if (enable)
-		_enable_gpio_irqbank(bank, GPIO_BIT(bank, gpio));
+		_enable_gpio_irqbank(bank, gpio_bit);
 	else
-		_disable_gpio_irqbank(bank, GPIO_BIT(bank, gpio));
+		_disable_gpio_irqbank(bank, gpio_bit);
+
+	if (bank->enabled_wakeup_gpios & gpio_bit) {
+		/* Enable wake-up during idle for dynamic tick */
+		bank->context.wake_en =
+			_gpio_rmw(base, bank->regs->wkup_en, gpio_bit, enable);
+	}
 }
 
 /*
@@ -511,8 +512,27 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
 	}
 
 	spin_lock_irqsave(&bank->lock, flags);
-	bank->context.wake_en = _gpio_rmw(bank->base, bank->regs->wkup_en,
-					  gpio_bit, enable);
+
+	if (enable)
+		bank->enabled_wakeup_gpios |= gpio_bit;
+	else
+		bank->enabled_wakeup_gpios &= ~gpio_bit;
+
+	/*
+	 * OMAP4430 TRM, 25.4.6.2
+	 * If there is a wake-up event, but after exiting the IDLE
+	 * state, no interrupt is generated; the corresponding bit from
+	 * the interrupt status register is not cleared, and the
+	 * module does not acknowledge a future idle request.
+	 */
+	if (enable && !(bank->context.irqenable1 & gpio_bit))
+		dev_warn(bank->dev, "Wake-up/IRQ enable mismatch for GPIO%d\n",
+			 gpio);
+	else
+		bank->context.wake_en = _gpio_rmw(bank->base,
+						  bank->regs->wkup_en,
+						  gpio_bit, enable);
+
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -525,6 +545,10 @@ static void _reset_gpio(struct gpio_bank *bank, int gpio)
 	_clear_gpio_irqstatus(bank, gpio);
 	_set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), IRQ_TYPE_NONE);
 	_clear_gpio_debounce(bank, gpio);
+
+	/* wakeup enabled by default */
+	if (!(bank->non_wakeup_gpios & GPIO_BIT(bank, gpio)))
+		bank->enabled_wakeup_gpios |= GPIO_BIT(bank, gpio);
 }
 
 /* Use disable_irq_wake() and enable_irq_wake() functions from drivers */
@@ -554,6 +578,10 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 	 */
 	_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
 
+	/* wakeup enabled by default */
+	if (!(bank->non_wakeup_gpios & (1 << offset)))
+		bank->enabled_wakeup_gpios |= 1 << offset;
+
 	if (bank->regs->pinctrl) {
 		/* Claim the pin for MPU */
 		_gpio_rmw(bank->base, bank->regs->pinctrl, 1 << offset, 1);
@@ -580,12 +608,6 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	if (bank->regs->wkup_en) {
-		/* Disable wake-up during idle for dynamic tick */
-		bank->context.wake_en =
-			_gpio_rmw(base, bank->regs->wkup_en, 1 << offset, 0);
-	}
-
 	bank->mod_usage &= ~(1 << offset);
 
 	if (bank->regs->ctrl && !bank->mod_usage) {
-- 
1.7.10.4

--
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

[PATCH v3 3/3] gpio/omap: split irq_mask callback fucntion into irq_disable/irq_mask.

[Andreas Fenkart]

Disabling an IRQ is turning off interrupt generating mechanisms in the
hardware. Masking means, the interrupt doesn't get delivered to the OS,
but the interrupt status register is still getting updated. The
difference is apparent when unmasking or re-enabling the interrupt. In
case of masking you might get an interrupt straight away, but never if
the interrupt was disabled.

This is the definition Jon Hunter formulated as a question in a earlier
thread of this patchset, the exact formulation is from ppwaskie on
#kernelnewbies. It sounds reasonable to me so I will stick to it from now
on. Neither Documentation/, source code, LDD or Google search gave a me
better definition.

The omap has separate trigger and interrupt enable registers, so it can
express the the subtle difference between masking and disabling an
interrupt. But the current implementation does not make use of it.
The public disable_irq function, uses the genirq 'lazy disable'
functionality as fallback when irq_disable is not implemented. In short
lazy disable marks the interrupt as disabled, but leaves the hardware
unmasked. If an interrupt happens, the interrupt flow handler masks the
line at the hardware level and marks the interrupt as pending.

void irq_disable(struct irq_desc *desc)
{
        irq_state_set_disabled(desc);
        if (desc->irq_data.chip->irq_disable) {
                desc->irq_data.chip->irq_disable(&desc->irq_data);
                irq_state_set_masked(desc);
        }
}

This is a contradiction with above definition of disable, since in
disabled state the interrupt should not be marked pending. So the
behavior -- at least on ARM, see HARDIRQS_SW_RESEND -- is more
similar to masking than disable.
The advantage of lazy interrupt is, that it allows to latch a pending
interrupt on a hardware that has no separate registers for signalling and
triggering. Depending on the use case, it might also be an optimization,
since it avoids a hardware access, for the common case where no interrupt
happens between marking it disabled and reenabling it again.

Getting to the point, this patch implements a feature, supported
by the HW and foreseen by the struct irq_chip. But unfortunately
no real benefit seems to emerge from that.

The basic advantage of this patch is a) while being masked, a pending
interrupt could now be latched in HW and not SW. b) with the current
implementation, there is no latching of pending interrupts while masked,
because currently triggering is disabled when masked.

Unfortunately no no code path makes use of this;
1) while there is global disable_irq, there is not global mask_irq.
2) being a chained irq, the irq of the gpio bank is masked while the
chain handler is running, hence masking individual edge type interrupts
is unnecessary, because the aggregated bank irq is already masked
3) level type interrupts are cleared after the handler has run,
since there is no earlier time it can be cleared/re-enabled.

Major drawback of the patch:
- no more pending when enable_irq, which is in contradiction with
  above definition anyway, but maybe some drivers have relied on
  that behavior
- wake-up path might be affected which depends on the interplay of
  several register, so we might introduce a nasty regression here

The only other irq chip driver having distinctive functions for
enable/disable and mask/unmask is drivers/gpio/gpio-ml-ioh.c

Implementation itself is straightfoward: mask/unmaks modifies
only the interrupt enable register, so it keeps latching new
interrupts into the status register. Enable/disable goes one step
further, also disabling latching of new interrupts.

Testing was done on AM335x, by using gpio-keys. While IRQ was
masked/disabled key was pressed. So upon unmask there had to be
an IRQ, while for enable no IRQ must be generated.
Masking/enabling the first gpio was controlled using a second
gpio-key. Selecting masking or disable the IRQ used was hardcoded
at compile time.

Wakeup functionality is completely untested, since the AM335x
lacks a IRQWAKEN register.

Signed-off-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
---
 drivers/gpio/gpio-omap.c |   71 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 44a93be..83e77a1 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -732,6 +732,13 @@ static void gpio_ack_irq(struct irq_data *d)
 	_clear_gpio_irqstatus(bank, gpio);
 }
 
+/**
+ * gpio_mask_irq - mask IRQ signaling
+ * @d : the gpio data we're acting upon
+ *
+ * Only signaling is masked. IRQs are still latched to status
+ * register.
+ */
 static void gpio_mask_irq(struct irq_data *d)
 {
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
@@ -740,33 +747,77 @@ static void gpio_mask_irq(struct irq_data *d)
 
 	spin_lock_irqsave(&bank->lock, flags);
 	_set_gpio_irqenable(bank, gpio, 0);
-	_set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), IRQ_TYPE_NONE);
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
+/**
+ * gpio_unmask_irq - unmask IRQ signaling
+ * @d : the gpio data we're acting upon
+ *
+ * If an IRQ occured while masked, there will be an IRQ straight
+ * away.
+ */
 static void gpio_unmask_irq(struct irq_data *d)
 {
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
 	unsigned int gpio = irq_to_gpio(bank, d->irq);
 	unsigned int irq_mask = GPIO_BIT(bank, gpio);
-	u32 trigger = irqd_get_trigger_type(d);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
-	if (trigger)
-		_set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), trigger);
 
-	/* For level-triggered GPIOs, the clearing must be done after
-	 * the HW source is cleared, thus after the handler has run */
-	if (bank->level_mask & irq_mask) {
-		_set_gpio_irqenable(bank, gpio, 0);
+	/*
+	 * For level-triggered GPIOs, the clearing must be done after
+	 * the HW source is cleared, thus after the handler has run
+	 */
+	if (bank->level_mask & irq_mask)
 		_clear_gpio_irqstatus(bank, gpio);
-	}
 
 	_set_gpio_irqenable(bank, gpio, 1);
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
+/**
+ * gpio_disable_irq - disable the interrupt
+ * @d : the gpio data we're acting upon
+ *
+ * While disabled all IRQ events are ignored. IRQs are not latched
+ * into status register.
+ */
+static void gpio_disable_irq(struct irq_data *d)
+{
+	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned int gpio = irq_to_gpio(bank, d->irq);
+	unsigned long flags;
+
+	spin_lock_irqsave(&bank->lock, flags);
+	_set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), IRQ_TYPE_NONE);
+	_set_gpio_irqenable(bank, gpio, 0);
+	spin_unlock_irqrestore(&bank->lock, flags);
+}
+
+/**
+ * gpio_enable_irq - enable the interrupt
+ * @d : the gpio data we're acting upon
+ *
+ * Enables latching and signaling of new IRQ. Pending IRQs
+ * are cleared.
+ */
+static void gpio_enable_irq(struct irq_data *d)
+{
+	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned int gpio = irq_to_gpio(bank, d->irq);
+	u32 trigger = irqd_get_trigger_type(d);
+	unsigned long flags;
+
+	spin_lock_irqsave(&bank->lock, flags);
+	if (trigger)
+		_set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), trigger);
+	_clear_gpio_irqstatus(bank, gpio);
+	_set_gpio_irqenable(bank, gpio, 1);
+	spin_unlock_irqrestore(&bank->lock, flags);
+}
+
 static struct irq_chip gpio_irq_chip = {
 	.name		= "GPIO",
 	.irq_shutdown	= gpio_irq_shutdown,
@@ -775,6 +826,8 @@ static struct irq_chip gpio_irq_chip = {
 	.irq_unmask	= gpio_unmask_irq,
 	.irq_set_type	= gpio_irq_type,
 	.irq_set_wake	= gpio_wake_enable,
+	.irq_disable    = gpio_disable_irq,
+	.irq_enable     = gpio_enable_irq,
 };
 
 /*---------------------------------------------------------------------*/
-- 
1.7.10.4

Re: [PATCH v3 2/3] gpio/omap: modify wake-up register with interrupt enable.

[Santosh Shilimkar]

On Wednesday 15 May 2013 02:24 AM, Andreas Fenkart wrote:
> OMAP4430 TRM chap. 25.4.5.2
> To reduce dynamic consumption, an efficient idle scheme is based on the
> following:
> • An efficient local autoclock gating for each module
> • The implementation of control sideband signals between the PRCM module
>   and each module
> This enhanced idle control allows clocks to be activated and deactivated
> safely without requiring complex software management. The idle mode
> request, idle acknowledge, and wake-up request are sideband signals
> between the PRCM module and the general-purpose interface
> 
> OMAP4430 TRM chap. 25.4.6.2
> There must be a correlation between the wake-up enable and interrupt
> enable register. If a GPIO pin has a wake-up configured on it, it must
> also have the corresponding interrupt enabled. Otherwise, it is possible
> there is a wake-up event, but after exiting the IDLE state, no interrupt
> is generated; the corresponding bit from the interrupt status register is
> not cleared, and the module does not acknowledge a future idle request.
> 
> Up to now _set_gpio_triggering() is also handling the wake-up enable
> register. According the TRM this should be in sync with the interrupt
> enable. Wakeup is still enabled by default, since the module would not
> wake from idle otherwise.
> The enabled_wakeup_gpios was introduced to remember an explicit
> _set_gpio_wakeup beyond a mask/unmask cycle. Calling the flag flag
> disabled_wakeup_gpios would spare the problem of initializing it, but
> feels very unnatural to read.
> 
> Wakeup functionality is completely untested, since the AM335x
> lacks a IRQWAKEN register.
> 
> Signed-off-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
> ---
I am ok with the subject patch and other two patches.
Would be good to get a tested by at least for though for OMAP4
and OMAP3. I have no access to boards for few weeks so
can't test it myself.

Other than that, for all three patches,
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

Re: [PATCH v3 1/3] gpio/omap: replace open coded read-modify-write with _gpio_rmw function.

[Kevin Hilman]

Andreas Fenkart <andreas.fenkart@streamunlimited.com> writes:

> By also making it return the modified value, we save the readl
> needed to update the context.
>
> Signed-off-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>

Great cleanups,   Thanks!

Some minor comments below...

Also, it would be nice to see a cover letter for this series describing
how it was tested, on what platforms, did it include PM testing
(off-mode, etc.).

[...]

> @@ -94,20 +107,10 @@ static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>  
>  static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
>  {
> -	void __iomem *reg = bank->base;
> -	u32 l;
> -
> -	reg += bank->regs->direction;
> -	l = __raw_readl(reg);
> -	if (is_input)
> -		l |= 1 << gpio;
> -	else
> -		l &= ~(1 << gpio);
> -	__raw_writel(l, reg);
> -	bank->context.oe = l;
> +	bank->context.oe = _gpio_rmw(bank->base, bank->regs->direction, 1 <<
> +				     gpio, is_input);

wrapping is funny here, IMO the "1 <<" should be on the next line along
with "gpio".

[...]

> @@ -450,19 +428,16 @@ static int gpio_irq_type(struct irq_data *d, unsigned type)
>  
>  static void _clear_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
>  {
> -	void __iomem *reg = bank->base;
> +	void __iomem *base = bank->base;
>  
> -	reg += bank->regs->irqstatus;
> -	__raw_writel(gpio_mask, reg);
> +	__raw_writel(gpio_mask, base + bank->regs->irqstatus);
>  
>  	/* Workaround for clearing DSP GPIO interrupts to allow retention */
> -	if (bank->regs->irqstatus2) {
> -		reg = bank->base + bank->regs->irqstatus2;
> -		__raw_writel(gpio_mask, reg);
> -	}
> +	if (bank->regs->irqstatus2)
> +		__raw_writel(gpio_mask, base + bank->regs->irqstatus2);
>  
>  	/* Flush posted write for the irq status to avoid spurious interrupts */
> -	__raw_readl(reg);
> +	__raw_readl(base + bank->regs->irqstatus);

All of the changes in this function are not related to the change
described in the changelog.  Either make a separate patch, or add a
description of this cleanup to the changelog too.

Thanks,

Kevin

Re: [PATCH v3 1/3] gpio/omap: replace open coded read-modify-write with _gpio_rmw function.

[Andreas Fenkart]

FYI,

this email adress will soon expire,
this is my alternate adress: afenkart at gmail.com

On Tue, May 14, 2013 at 10:54:25PM +0200, Andreas Fenkart wrote:
> By also making it return the modified value, we save the readl
> needed to update the context.
> 
> Signed-off-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
> ---
>  drivers/gpio/gpio-omap.c |  162 ++++++++++++++--------------------------------
>  1 file changed, 50 insertions(+), 112 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 159f5c5..082919e 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -87,6 +87,19 @@ struct gpio_bank {
>  #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
>  #define GPIO_MOD_CTRL_BIT	BIT(0)
>  
> +static inline u32 _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)
> +{
> +	u32 l = __raw_readl(base + reg);
> +
> +	if (set)
> +		l |= mask;
> +	else
> +		l &= ~mask;
> +
> +	__raw_writel(l, base + reg);
> +	return l;
> +}
> +
>  static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>  {
>  	return gpio_irq - bank->irq_base + bank->chip.base;
> @@ -94,20 +107,10 @@ static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>  
>  static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
>  {
> -	void __iomem *reg = bank->base;
> -	u32 l;
> -
> -	reg += bank->regs->direction;
> -	l = __raw_readl(reg);
> -	if (is_input)
> -		l |= 1 << gpio;
> -	else
> -		l &= ~(1 << gpio);
> -	__raw_writel(l, reg);
> -	bank->context.oe = l;
> +	bank->context.oe = _gpio_rmw(bank->base, bank->regs->direction, 1 <<
> +				     gpio, is_input);
>  }
>  
> -
>  /* set data out value using dedicate set/clear register */
>  static void _set_gpio_dataout_reg(struct gpio_bank *bank, int gpio, int enable)
>  {
> @@ -128,17 +131,8 @@ static void _set_gpio_dataout_reg(struct gpio_bank *bank, int gpio, int enable)
>  /* set data out value using mask register */
>  static void _set_gpio_dataout_mask(struct gpio_bank *bank, int gpio, int enable)
>  {
> -	void __iomem *reg = bank->base + bank->regs->dataout;
> -	u32 gpio_bit = GPIO_BIT(bank, gpio);
> -	u32 l;
> -
> -	l = __raw_readl(reg);
> -	if (enable)
> -		l |= gpio_bit;
> -	else
> -		l &= ~gpio_bit;
> -	__raw_writel(l, reg);
> -	bank->context.dataout = l;
> +	bank->context.dataout = _gpio_rmw(bank->base, bank->regs->dataout,
> +					  GPIO_BIT(bank, gpio), enable);
>  }
>  
>  static int _get_gpio_datain(struct gpio_bank *bank, int offset)
> @@ -155,18 +149,6 @@ static int _get_gpio_dataout(struct gpio_bank *bank, int offset)
>  	return (__raw_readl(reg) & (1 << offset)) != 0;
>  }
>  
> -static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)
> -{
> -	int l = __raw_readl(base + reg);
> -
> -	if (set)
> -		l |= mask;
> -	else
> -		l &= ~mask;
> -
> -	__raw_writel(l, base + reg);
> -}
> -
>  static inline void _gpio_dbck_enable(struct gpio_bank *bank)
>  {
>  	if (bank->dbck_enable_mask && !bank->dbck_enabled) {
> @@ -291,28 +273,24 @@ static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio,
>  	void __iomem *base = bank->base;
>  	u32 gpio_bit = 1 << gpio;
>  
> -	_gpio_rmw(base, bank->regs->leveldetect0, gpio_bit,
> -		  trigger & IRQ_TYPE_LEVEL_LOW);
> -	_gpio_rmw(base, bank->regs->leveldetect1, gpio_bit,
> -		  trigger & IRQ_TYPE_LEVEL_HIGH);
> -	_gpio_rmw(base, bank->regs->risingdetect, gpio_bit,
> -		  trigger & IRQ_TYPE_EDGE_RISING);
> -	_gpio_rmw(base, bank->regs->fallingdetect, gpio_bit,
> -		  trigger & IRQ_TYPE_EDGE_FALLING);
>  
>  	bank->context.leveldetect0 =
> -			__raw_readl(bank->base + bank->regs->leveldetect0);
> +		_gpio_rmw(base, bank->regs->leveldetect0,
> +			  gpio_bit, trigger & IRQ_TYPE_LEVEL_LOW);
>  	bank->context.leveldetect1 =
> -			__raw_readl(bank->base + bank->regs->leveldetect1);
> +		_gpio_rmw(base, bank->regs->leveldetect1,
> +			  gpio_bit, trigger & IRQ_TYPE_LEVEL_HIGH);
>  	bank->context.risingdetect =
> -			__raw_readl(bank->base + bank->regs->risingdetect);
> +		_gpio_rmw(base, bank->regs->risingdetect,
> +			  gpio_bit, trigger & IRQ_TYPE_EDGE_RISING);
>  	bank->context.fallingdetect =
> -			__raw_readl(bank->base + bank->regs->fallingdetect);
> +		_gpio_rmw(base, bank->regs->fallingdetect,
> +			  gpio_bit, trigger & IRQ_TYPE_EDGE_FALLING);
>  
>  	if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
> -		_gpio_rmw(base, bank->regs->wkup_en, gpio_bit, trigger != 0);
>  		bank->context.wake_en =
> -			__raw_readl(bank->base + bank->regs->wkup_en);
> +			_gpio_rmw(base, bank->regs->wkup_en, gpio_bit,
> +				  trigger != 0);
>  	}
>  
>  	/* This part needs to be executed always for OMAP{34xx, 44xx} */
> @@ -406,9 +384,9 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
>  			l |= 1 << (gpio << 1);
>  
>  		/* Enable wake-up during idle for dynamic tick */
> -		_gpio_rmw(base, bank->regs->wkup_en, 1 << gpio, trigger);
>  		bank->context.wake_en =
> -			__raw_readl(bank->base + bank->regs->wkup_en);
> +			_gpio_rmw(base, bank->regs->wkup_en, 1 << gpio,
> +				  trigger);
>  		__raw_writel(l, reg);
>  	}
>  	return 0;
> @@ -450,19 +428,16 @@ static int gpio_irq_type(struct irq_data *d, unsigned type)
>  
>  static void _clear_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
>  {
> -	void __iomem *reg = bank->base;
> +	void __iomem *base = bank->base;
>  
> -	reg += bank->regs->irqstatus;
> -	__raw_writel(gpio_mask, reg);
> +	__raw_writel(gpio_mask, base + bank->regs->irqstatus);
>  
>  	/* Workaround for clearing DSP GPIO interrupts to allow retention */
> -	if (bank->regs->irqstatus2) {
> -		reg = bank->base + bank->regs->irqstatus2;
> -		__raw_writel(gpio_mask, reg);
> -	}
> +	if (bank->regs->irqstatus2)
> +		__raw_writel(gpio_mask, base + bank->regs->irqstatus2);
>  
>  	/* Flush posted write for the irq status to avoid spurious interrupts */
> -	__raw_readl(reg);
> +	__raw_readl(base + bank->regs->irqstatus);
>  }
>  
>  static inline void _clear_gpio_irqstatus(struct gpio_bank *bank, int gpio)
> @@ -486,46 +461,26 @@ static u32 _get_gpio_irqbank_mask(struct gpio_bank *bank)
>  
>  static void _enable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
>  {
> -	void __iomem *reg = bank->base;
> -	u32 l;
> -
>  	if (bank->regs->set_irqenable) {
> -		reg += bank->regs->set_irqenable;
> -		l = gpio_mask;
>  		bank->context.irqenable1 |= gpio_mask;
> +		__raw_writel(gpio_mask, bank->base + bank->regs->set_irqenable);
>  	} else {
> -		reg += bank->regs->irqenable;
> -		l = __raw_readl(reg);
> -		if (bank->regs->irqenable_inv)
> -			l &= ~gpio_mask;
> -		else
> -			l |= gpio_mask;
> -		bank->context.irqenable1 = l;
> +		bank->context.irqenable1 =
> +			_gpio_rmw(bank->base, bank->regs->irqenable, gpio_mask,
> +				  bank->regs->irqenable_inv ? false : true);
>  	}
> -
> -	__raw_writel(l, reg);
>  }
>  
>  static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
>  {
> -	void __iomem *reg = bank->base;
> -	u32 l;
> -
>  	if (bank->regs->clr_irqenable) {
> -		reg += bank->regs->clr_irqenable;
> -		l = gpio_mask;
>  		bank->context.irqenable1 &= ~gpio_mask;
> +		__raw_writel(gpio_mask, bank->base + bank->regs->clr_irqenable);
>  	} else {
> -		reg += bank->regs->irqenable;
> -		l = __raw_readl(reg);
> -		if (bank->regs->irqenable_inv)
> -			l |= gpio_mask;
> -		else
> -			l &= ~gpio_mask;
> -		bank->context.irqenable1 = l;
> +		bank->context.irqenable1 =
> +			_gpio_rmw(bank->base, bank->regs->irqenable, gpio_mask,
> +				  bank->regs->irqenable_inv ?: false);
>  	}
> -
> -	__raw_writel(l, reg);
>  }
>  
>  static inline void _set_gpio_irqenable(struct gpio_bank *bank, int gpio, int enable)
> @@ -556,12 +511,8 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
>  	}
>  
>  	spin_lock_irqsave(&bank->lock, flags);
> -	if (enable)
> -		bank->context.wake_en |= gpio_bit;
> -	else
> -		bank->context.wake_en &= ~gpio_bit;
> -
> -	__raw_writel(bank->context.wake_en, bank->base + bank->regs->wkup_en);
> +	bank->context.wake_en = _gpio_rmw(bank->base, bank->regs->wkup_en,
> +					  gpio_bit, enable);
>  	spin_unlock_irqrestore(&bank->lock, flags);
>  
>  	return 0;
> @@ -604,21 +555,14 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
>  	_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
>  
>  	if (bank->regs->pinctrl) {
> -		void __iomem *reg = bank->base + bank->regs->pinctrl;
> -
>  		/* Claim the pin for MPU */
> -		__raw_writel(__raw_readl(reg) | (1 << offset), reg);
> +		_gpio_rmw(bank->base, bank->regs->pinctrl, 1 << offset, 1);
>  	}
>  
>  	if (bank->regs->ctrl && !bank->mod_usage) {
> -		void __iomem *reg = bank->base + bank->regs->ctrl;
> -		u32 ctrl;
> -
> -		ctrl = __raw_readl(reg);
>  		/* Module is enabled, clocks are not gated */
> -		ctrl &= ~GPIO_MOD_CTRL_BIT;
> -		__raw_writel(ctrl, reg);
> -		bank->context.ctrl = ctrl;
> +		bank->context.ctrl = _gpio_rmw(bank->base, bank->regs->ctrl,
> +					       GPIO_MOD_CTRL_BIT, 0);
>  	}
>  
>  	bank->mod_usage |= 1 << offset;
> @@ -638,22 +582,16 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
>  
>  	if (bank->regs->wkup_en) {
>  		/* Disable wake-up during idle for dynamic tick */
> -		_gpio_rmw(base, bank->regs->wkup_en, 1 << offset, 0);
>  		bank->context.wake_en =
> -			__raw_readl(bank->base + bank->regs->wkup_en);
> +			_gpio_rmw(base, bank->regs->wkup_en, 1 << offset, 0);
>  	}
>  
>  	bank->mod_usage &= ~(1 << offset);
>  
>  	if (bank->regs->ctrl && !bank->mod_usage) {
> -		void __iomem *reg = bank->base + bank->regs->ctrl;
> -		u32 ctrl;
> -
> -		ctrl = __raw_readl(reg);
>  		/* Module is disabled, clocks are gated */
> -		ctrl |= GPIO_MOD_CTRL_BIT;
> -		__raw_writel(ctrl, reg);
> -		bank->context.ctrl = ctrl;
> +		bank->context.ctrl = _gpio_rmw(base, bank->regs->ctrl,
> +					       GPIO_MOD_CTRL_BIT, 1);
>  	}
>  
>  	_reset_gpio(bank, bank->chip.base + offset);
> -- 
> 1.7.10.4
> 

Re: [PATCH v3 2/3] gpio/omap: modify wake-up register with interrupt enable.

[Kevin Hilman]

Andreas Fenkart <andreas.fenkart@streamunlimited.com> writes:

> OMAP4430 TRM chap. 25.4.5.2
> To reduce dynamic consumption, an efficient idle scheme is based on the
> following:
> • An efficient local autoclock gating for each module
> • The implementation of control sideband signals between the PRCM module
>   and each module
> This enhanced idle control allows clocks to be activated and deactivated
> safely without requiring complex software management. The idle mode
> request, idle acknowledge, and wake-up request are sideband signals
> between the PRCM module and the general-purpose interface
>
> OMAP4430 TRM chap. 25.4.6.2
> There must be a correlation between the wake-up enable and interrupt
> enable register. If a GPIO pin has a wake-up configured on it, it must
> also have the corresponding interrupt enabled. Otherwise, it is possible
> there is a wake-up event, but after exiting the IDLE state, no interrupt
> is generated; the corresponding bit from the interrupt status register is
> not cleared, and the module does not acknowledge a future idle request.
>
> Up to now _set_gpio_triggering() is also handling the wake-up enable
> register. According the TRM this should be in sync with the interrupt
> enable. Wakeup is still enabled by default, since the module would not
> wake from idle otherwise.
> The enabled_wakeup_gpios was introduced to remember an explicit
> _set_gpio_wakeup beyond a mask/unmask cycle. Calling the flag flag
> disabled_wakeup_gpios would spare the problem of initializing it, but
> feels very unnatural to read.

There's a lot of description here, but it still fails to describe the
problem that is being solved and the motiviation for this change.  

> Wakeup functionality is completely untested, since the AM335x
> lacks a IRQWAKEN register.

So in addtion to a better description of the problem, and the changes,
this needs much broader testing, including on platforms with off-mode.

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