From 381a752f291763bd6971521fa44c76ad9e937f7b Mon Sep 17 00:00:00 2001 From: Tarun Kanti DebBarma Date: Wed, 29 Feb 2012 21:49:21 +0530 Subject: gpio/omap: fix wakeup_en register update in _set_gpio_wakeup() There are two ways through which wakeup_en register can be programmed using gpiolib APIs as shown below. It is seen that in the second case in _set_gpio_wakeup(), even though bank->suspend_wakeup is updated correctly, its value is not programmed in wakeup_en register. Fix this. irq_set_type()->gpio_irq_type()->_set_gpio_triggering()->set_gpio_trigger() irq_set_wake()->gpio_wake_enable()->_set_gpio_wakeup() Signed-off-by: Tarun Kanti DebBarma Reviewed-by: Kevin Hilman Signed-off-by: Kevin Hilman --- drivers/gpio/gpio-omap.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpio') diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 7cbad856926..1a144ac3b34 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -511,6 +511,7 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable) else bank->suspend_wakeup &= ~gpio_bit; + __raw_writel(bank->suspend_wakeup, bank->base + bank->regs->wkup_en); spin_unlock_irqrestore(&bank->lock, flags); return 0; -- cgit v1.2.3 From 00ece7e4826e631565eae089d3c813120c6535ef Mon Sep 17 00:00:00 2001 From: Tarun Kanti DebBarma Date: Fri, 25 Nov 2011 15:41:06 +0530 Subject: gpio/omap: fix trigger type to unsigned The GPIO trigger parameter is of type unsigned. enum { IRQ_TYPE_NONE = 0x00000000, IRQ_TYPE_EDGE_RISING = 0x00000001, IRQ_TYPE_EDGE_FALLING = 0x00000002, IRQ_TYPE_EDGE_BOTH = (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING), IRQ_TYPE_LEVEL_HIGH = 0x00000004, IRQ_TYPE_LEVEL_LOW = 0x00000008, IRQ_TYPE_LEVEL_MASK = (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH), IRQ_TYPE_SENSE_MASK = 0x0000000f, IRQ_TYPE_PROBE = 0x00000010, ... }; Even though gpio_irq_type(struct irq_data *d, unsigned type) has the right type of parameter, the subsequent called functions set_gpio_triggering() and set_gpio_trigger() wrongly makes it signed integer. Fix this. Signed-off-by: Tarun Kanti DebBarma Reviewed-by: Santosh Shilimkar Acked-by: Felipe Balbi Reviewed-by: Kevin Hilman Signed-off-by: Kevin Hilman --- drivers/gpio/gpio-omap.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/gpio') diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 1a144ac3b34..2042857a366 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -245,7 +245,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, } static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio, - int trigger) + unsigned trigger) { void __iomem *base = bank->base; u32 gpio_bit = 1 << gpio; @@ -327,7 +327,8 @@ static void _toggle_gpio_edge_triggering(struct gpio_bank *bank, int gpio) static void _toggle_gpio_edge_triggering(struct gpio_bank *bank, int gpio) {} #endif -static int _set_gpio_triggering(struct gpio_bank *bank, int gpio, int trigger) +static int _set_gpio_triggering(struct gpio_bank *bank, int gpio, + unsigned trigger) { void __iomem *reg = bank->base; void __iomem *base = bank->base; -- cgit v1.2.3 From 8276536cec38bc6bde30d0aa67716f22b9b9705a Mon Sep 17 00:00:00 2001 From: Tarun Kanti DebBarma Date: Fri, 25 Nov 2011 15:27:37 +0530 Subject: gpio/omap: fix _set_gpio_irqenable implementation This function should be capable of both enabling and disabling interrupts based upon the *enable* parameter. Right now the function only enables the interrupt and *enable* is not used at all. So add the interrupt disable capability also using the parameter. Signed-off-by: Tarun Kanti DebBarma Reviewed-by: Santosh Shilimkar Acked-by: Felipe Balbi Reviewed-by: Kevin Hilman Signed-off-by: Kevin Hilman --- drivers/gpio/gpio-omap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/gpio') diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 2042857a366..8901d576cb7 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -484,7 +484,10 @@ 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) { - _enable_gpio_irqbank(bank, GPIO_BIT(bank, gpio)); + if (enable) + _enable_gpio_irqbank(bank, GPIO_BIT(bank, gpio)); + else + _disable_gpio_irqbank(bank, GPIO_BIT(bank, gpio)); } /* -- cgit v1.2.3 From 2c836f7ea5e7b5eec2a798e02b1d76ea791fa094 Mon Sep 17 00:00:00 2001 From: Tarun Kanti DebBarma Date: Fri, 2 Mar 2012 12:52:52 +0530 Subject: gpio/omap: fix missing dataout context save in _set_gpio_dataout_reg There are two functions, _set_gpio_dataout_reg() and _set_gpio_dataout_mask() which writes to dataout register and the dataout context must be saved. It is missing in the first function, _set_gpio_dataout_reg(). Fix this. Signed-off-by: Tarun Kanti DebBarma Reviewed-by: Santosh Shilimkar Reviewed-by: Kevin Hilman Signed-off-by: Kevin Hilman --- drivers/gpio/gpio-omap.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/gpio') diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 8901d576cb7..bbe96489901 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -120,10 +120,13 @@ static void _set_gpio_dataout_reg(struct gpio_bank *bank, int gpio, int enable) void __iomem *reg = bank->base; u32 l = GPIO_BIT(bank, gpio); - if (enable) + if (enable) { reg += bank->regs->set_dataout; - else + bank->context.dataout |= l; + } else { reg += bank->regs->clr_dataout; + bank->context.dataout &= ~l; + } __raw_writel(l, reg); } -- cgit v1.2.3 From 960edffe29dfd845ee532ee51398592cba96d701 Mon Sep 17 00:00:00 2001 From: Tarun Kanti DebBarma Date: Mon, 5 Mar 2012 16:00:54 +0530 Subject: gpio/omap: fix incorrect context restore logic in omap_gpio_runtime_* In omap_gpio_runtime_suspend/resume() the context save/restore should be independent of bank->enabled_non_wakeup_gpios. This was preventing context restore of GPIO lines which are not wakeup enabled. Reported-by: Govindraj Raja Signed-off-by: Tarun Kanti DebBarma Reviewed-by: Santosh Shilimkar Reviewed-by: Kevin Hilman Signed-off-by: Kevin Hilman --- drivers/gpio/gpio-omap.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/gpio') diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index bbe96489901..bcb1061dbd6 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1247,9 +1247,6 @@ static int omap_gpio_runtime_suspend(struct device *dev) * non-wakeup GPIOs. Otherwise spurious IRQs will be * generated. See OMAP2420 Errata item 1.101. */ - if (!(bank->enabled_non_wakeup_gpios)) - goto update_gpio_context_count; - bank->saved_datain = __raw_readl(bank->base + bank->regs->datain); l1 = __raw_readl(bank->base + bank->regs->fallingdetect); @@ -1298,7 +1295,7 @@ static int omap_gpio_runtime_resume(struct device *dev) __raw_writel(bank->context.risingdetect, bank->base + bank->regs->risingdetect); - if (!bank->enabled_non_wakeup_gpios || !bank->workaround_enabled) { + if (!bank->workaround_enabled) { spin_unlock_irqrestore(&bank->lock, flags); return 0; } -- cgit v1.2.3 From 2a900eb74c123a21054836ab2c63d6ff46f854c6 Mon Sep 17 00:00:00 2001 From: Tarun Kanti DebBarma Date: Tue, 6 Mar 2012 12:08:16 +0530 Subject: gpio/omap: fix incorrect update to context.irqenable1 In _enable_gpio_irqbank() when bank->regs->set_irqenable is TRUE, gpio_mask can be directly set by writing to set_irqenable register without overwriting current value. In order to ensure the same is stored in context.irqenable1, we must avoid overwriting it with gpio_mask at the end of the function. Instead, update irqenable1 appropriately by OR'ing with gpio_mask. For the case where bank->regs->set_irqenable is FALSE, irqenable1 can be directly overwritten with 'l' which holds correct computed value. if (bank->regs->set_irqenable) { reg += bank->regs->set_irqenable; l = gpio_mask; } else { reg += bank->regs->irqenable; l = __raw_readl(reg); if (bank->regs->irqenable_inv) l &= ~gpio_mask; else l |= gpio_mask; } Make similar change for _disable_gpio_irqbank(). Signed-off-by: Tarun Kanti DebBarma Reviewed-by: Santosh Shilimkar Reviewed-by: Kevin Hilman Signed-off-by: Kevin Hilman --- drivers/gpio/gpio-omap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/gpio') diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index bcb1061dbd6..6c17e581231 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -451,6 +451,7 @@ static void _enable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask) if (bank->regs->set_irqenable) { reg += bank->regs->set_irqenable; l = gpio_mask; + bank->context.irqenable1 |= gpio_mask; } else { reg += bank->regs->irqenable; l = __raw_readl(reg); @@ -458,10 +459,10 @@ static void _enable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask) l &= ~gpio_mask; else l |= gpio_mask; + bank->context.irqenable1 = l; } __raw_writel(l, reg); - bank->context.irqenable1 = l; } static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask) @@ -472,6 +473,7 @@ static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask) if (bank->regs->clr_irqenable) { reg += bank->regs->clr_irqenable; l = gpio_mask; + bank->context.irqenable1 &= ~gpio_mask; } else { reg += bank->regs->irqenable; l = __raw_readl(reg); @@ -479,10 +481,10 @@ static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask) l |= gpio_mask; else l &= ~gpio_mask; + bank->context.irqenable1 = l; } __raw_writel(l, reg); - bank->context.irqenable1 = l; } static inline void _set_gpio_irqenable(struct gpio_bank *bank, int gpio, int enable) -- cgit v1.2.3 From 7fcca715de3438b8fc3c8a144702f3a95c8ff63c Mon Sep 17 00:00:00 2001 From: Tarun Kanti DebBarma Date: Mon, 27 Feb 2012 11:46:09 +0530 Subject: gpio/omap: fix redundant decoding of gpio offset In gpio_get(), _get_gpio_datain() and _get_gpio_dataout() get rid of un-necessary operation to compute gpio mask. The gpio offset passed to gpio_get() is sufficient to do that. Here is Russell's original comment: Can someone explain to me this: static int _get_gpio_datain(struct gpio_bank *bank, int gpio) { void __iomem *reg = bank->base + bank->regs->datain; return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0; } static int gpio_get(struct gpio_chip *chip, unsigned offset) { struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip); void __iomem *reg = bank->base; int gpio = chip->base + offset; u32 mask = GPIO_BIT(bank, gpio); if (gpio_is_input(bank, mask)) return _get_gpio_datain(bank, gpio); else return _get_gpio_dataout(bank, gpio); } Given that bank->width on OMAP is either 32 or 16, and GPIO numbers for any GPIO chip are always aligned to 32 or 16, why does this code bother adding the chips base gpio number and then modulo the width? Surely this means if - for argument sake - you registered a GPIO chip with 8 lines followed by one with 16 lines, GPIO0..7 would be chip 0 bit 0..7, GPIO8..15 would be chip 1 bit 8..15, GPIO16..23 would be chip 1 bit 0..7. However, if you registered a GPIO chip with 16 lines first, it would mean GPIO0..15 would be chip 0 bit 0..15, and GPIO16..31 would be chip 1 bit 0..15. Surely this kind of behaviour is not intended? Is there a reason why the bitmask can't just be (1 << offset) where offset is passed into these functions as GPIO number - chip->base ? Reported-by: Russell King - ARM Linux Signed-off-by: Tarun Kanti DebBarma Reviewed-by: Santosh Shilimkar Reviewed-by: Kevin Hilman Signed-off-by: Kevin Hilman --- drivers/gpio/gpio-omap.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'drivers/gpio') diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 6c17e581231..1adc2ec1e38 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -147,18 +147,18 @@ static void _set_gpio_dataout_mask(struct gpio_bank *bank, int gpio, int enable) bank->context.dataout = l; } -static int _get_gpio_datain(struct gpio_bank *bank, int gpio) +static int _get_gpio_datain(struct gpio_bank *bank, int offset) { void __iomem *reg = bank->base + bank->regs->datain; - return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0; + return (__raw_readl(reg) & (1 << offset)) != 0; } -static int _get_gpio_dataout(struct gpio_bank *bank, int gpio) +static int _get_gpio_dataout(struct gpio_bank *bank, int offset) { void __iomem *reg = bank->base + bank->regs->dataout; - return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0; + return (__raw_readl(reg) & (1 << offset)) != 0; } static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set) @@ -865,19 +865,15 @@ static int gpio_is_input(struct gpio_bank *bank, int mask) static int gpio_get(struct gpio_chip *chip, unsigned offset) { struct gpio_bank *bank; - void __iomem *reg; - int gpio; u32 mask; - gpio = chip->base + offset; bank = container_of(chip, struct gpio_bank, chip); - reg = bank->base; - mask = GPIO_BIT(bank, gpio); + mask = (1 << offset); if (gpio_is_input(bank, mask)) - return _get_gpio_datain(bank, gpio); + return _get_gpio_datain(bank, offset); else - return _get_gpio_dataout(bank, gpio); + return _get_gpio_dataout(bank, offset); } static int gpio_output(struct gpio_chip *chip, unsigned offset, int value) -- cgit v1.2.3