Hi,
This patch series contains set of trivial fixes and improvements, and also
patches which fixes wrong APIs usage in atomic context as for -RT as for
non-RT kernel. The final goal of this series is to make TI OMAP GPIO
driver compatible with -RT kernel as much as possible.
Patch 1-4: trivial fixes and improvements
Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet
Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime
can't be used in atimic context on -RT.
Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq
handler instead of chained IRQ handler. This way OMAP GPIO driver will be
compatible with RT kernel where it will be forced thread IRQ handler
while in non-RT kernel it still will be executed in HW IRQ context.
Based on
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
branch: devel
commit: 929550b gpio: mxc: fix section mismatch warning
Boot, basic gpio functionality tested on:
dra7-evm, BeagleBone(white), am43xx-gpevm, am437x-sk
Manually tested on dra7-evm including suspend/resume and wakeup.
Grygorii Strashko (7):
gpio: omap: remove wrong irq_domain_remove usage in probe
gpio: omap: switch to use platform_get_irq
gpio: omap: fix omap2_set_gpio_debounce
gpio: omap: protect regs access in omap_gpio_irq_handler
gpio: omap: fix clk_prepare/unprepare usage
gpio: omap: move pm runtime in irq_chip.irq_bus_lock/sync_unlock
gpio: omap: convert to use generic irq handler
drivers/gpio/gpio-omap.c | 146 +++++++++++++++++++++++++++--------------------
1 file changed, 85 insertions(+), 61 deletions(-)
--
2.5.0
The bank->chip.irqdomain is uninitialized at the moment when
irq_domain_remove() is called, so remove this call.
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/gpio/gpio-omap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 466fe70..f38b01b 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1219,7 +1219,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
bank->base = devm_ioremap_resource(dev, res);
if (IS_ERR(bank->base)) {
- irq_domain_remove(bank->chip.irqdomain);
return PTR_ERR(bank->base);
}
--
2.5.0
Switch OMAP GPIO driver to use platform_get_irq(), because
it is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ, ..)
for requesting IRQ resources any more, as they can be not ready yet
in case of DT-boot.
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/gpio/gpio-omap.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index f38b01b..03fd111 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1178,13 +1178,16 @@ static int omap_gpio_probe(struct platform_device *pdev)
irqc->irq_set_wake = omap_gpio_wake_enable,
irqc->name = dev_name(&pdev->dev);
- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (unlikely(!res)) {
- dev_err(dev, "Invalid IRQ resource\n");
- return -ENODEV;
+ bank->irq = platform_get_irq(pdev, 0);
+ if (bank->irq <= 0) {
+ if (!bank->irq)
+ bank->irq = -ENXIO;
+ if (bank->irq != -EPROBE_DEFER)
+ dev_err(dev,
+ "can't get irq resource ret=%d\n", bank->irq);
+ return bank->irq;
}
- bank->irq = res->start;
bank->dev = dev;
bank->chip.dev = dev;
bank->chip.owner = THIS_MODULE;
--
2.5.0
According to TRMs:
Required input line stable =
(the value of the GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) × 31,
where the value of the GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME bit field
is from 0 to 255.
But now omap2_set_gpio_debounce() will calculate debounce time and
behave incorrectly in the following cases:
1) requested debounce time is !0 and <32
calculated DEBOUNCETIME = 0x1 == 62 us;
expected value of DEBOUNCETIME = 0x0 == 31us
2) requested debounce time is 0
calculated DEBOUNCETIME = 0x1 == 62 us;
expected: disable debounce and DEBOUNCETIME = 0x0
3) requested debounce time is >32 and <63
calculated DEBOUNCETIME = 0x0 and debounce will be disabled;
expected: enable debounce and DEBOUNCETIME = 0x1 == 62 us
Hence, rework omap2_set_gpio_debounce() to fix above cases:
1) introduce local variable "enable" and use it to identify
when debounce need to be enabled or disabled. Disable debounce
if requested debounce time is 0.
2) use below formula for debounce time calculation:
debounce = (DIV_ROUND_UP(debounce, 31) - 1) & 0xFF;
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/gpio/gpio-omap.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 03fd111..9ed5a67 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -29,6 +29,7 @@
#include <linux/platform_data/gpio-omap.h>
#define OFF_MODE 1
+#define OMAP4_GPIO_DEBOUNCINGTIME_MASK 0xFF
static LIST_HEAD(omap_gpio_list);
@@ -204,8 +205,9 @@ static inline void omap_gpio_dbck_disable(struct gpio_bank *bank)
* @offset: the gpio number on this @bank
* @debounce: debounce time to use
*
- * OMAP's debounce time is in 31us steps so we need
- * to convert and round up to the closest unit.
+ * OMAP's debounce time is in 31us steps
+ * <debounce time> = (GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) x 31
+ * so we need to convert and round up to the closest unit.
*/
static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
unsigned debounce)
@@ -213,16 +215,15 @@ static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
void __iomem *reg;
u32 val;
u32 l;
+ bool enable = !!debounce;
if (!bank->dbck_flag)
return;
- if (debounce < 32)
- debounce = 0x01;
- else if (debounce > 7936)
- debounce = 0xff;
- else
- debounce = (debounce / 0x1f) - 1;
+ if (enable) {
+ debounce = DIV_ROUND_UP(debounce, 31) - 1;
+ debounce &= OMAP4_GPIO_DEBOUNCINGTIME_MASK;
+ }
l = BIT(offset);
@@ -233,7 +234,7 @@ static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
reg = bank->base + bank->regs->debounce_en;
val = readl_relaxed(reg);
- if (debounce)
+ if (enable)
val |= l;
else
val &= ~l;
--
2.5.0
The access to HW registers has to be be protected in
omap_gpio_irq_handler(), as it may race with code executed on
another CPUs.
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/gpio/gpio-omap.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 9ed5a67..1f02acd 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -718,6 +718,7 @@ static void omap_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
int unmasked = 0;
struct irq_chip *irqchip = irq_desc_get_chip(desc);
struct gpio_chip *chip = irq_desc_get_handler_data(desc);
+ unsigned long lock_flags;
chained_irq_enter(irqchip, desc);
@@ -732,6 +733,8 @@ static void omap_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
u32 isr_saved, level_mask = 0;
u32 enabled;
+ raw_spin_lock_irqsave(&bank->lock, lock_flags);
+
enabled = omap_get_gpio_irqbank_mask(bank);
isr_saved = isr = readl_relaxed(isr_reg) & enabled;
@@ -745,6 +748,8 @@ static void omap_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
omap_clear_gpio_irqbank(bank, isr_saved & ~level_mask);
omap_enable_gpio_irqbank(bank, isr_saved & ~level_mask);
+ raw_spin_unlock_irqrestore(&bank->lock, lock_flags);
+
/* if there is only edge sensitive GPIO pin interrupts
configured, we could unmask GPIO bank interrupt immediately */
if (!level_mask && !unmasked) {
@@ -759,6 +764,7 @@ static void omap_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
bit = __ffs(isr);
isr &= ~(BIT(bit));
+ raw_spin_lock_irqsave(&bank->lock, lock_flags);
/*
* Some chips can't respond to both rising and falling
* at the same time. If this irq was requested with
@@ -769,6 +775,8 @@ static void omap_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
if (bank->toggle_mask & (BIT(bit)))
omap_toggle_gpio_edge_triggering(bank, bit);
+ raw_spin_unlock_irqrestore(&bank->lock, lock_flags);
+
generic_handle_irq(irq_find_mapping(bank->chip.irqdomain,
bit));
}
--
2.5.0
As per CCF documentation (clk.txt) the clk_prepare/unprepare APIs
are not allowed in atomic context. But now OMAP GPIO driver
uses them while applying debounce settings and as part
of PM runtime irqsafe operations:
- omap_gpio_debounce() is holding the lock with IRQs off.
+ omap2_set_gpio_debounce()
+ clk_prepare_enable()
+ clk_prepare() this one might sleep.
- pm_runtime_get_sync() is holding the lock with IRQs off
+ omap_gpio_runtime_suspend()
+ raw_spin_lock_irqsave()
+ omap_gpio_dbck_disable()
+ clk_disable_unprepare()
Hence, fix it by moeving dbclk prepare/unprepare in OMAP GPIO
omap_gpio_probe/omap_gpio_remove. Also, while here, ensure that
debounce functionality is disabled if clk_get() failed,
because otherwise kernel will carsh in omap2_set_gpio_debounce().
Reported-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/gpio/gpio-omap.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 1f02acd..2ae0d47 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -176,7 +176,7 @@ static inline void omap_gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set
static inline void omap_gpio_dbck_enable(struct gpio_bank *bank)
{
if (bank->dbck_enable_mask && !bank->dbck_enabled) {
- clk_prepare_enable(bank->dbck);
+ clk_enable(bank->dbck);
bank->dbck_enabled = true;
writel_relaxed(bank->dbck_enable_mask,
@@ -194,7 +194,7 @@ static inline void omap_gpio_dbck_disable(struct gpio_bank *bank)
*/
writel_relaxed(0, bank->base + bank->regs->debounce_en);
- clk_disable_unprepare(bank->dbck);
+ clk_disable(bank->dbck);
bank->dbck_enabled = false;
}
}
@@ -227,7 +227,7 @@ static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
l = BIT(offset);
- clk_prepare_enable(bank->dbck);
+ clk_enable(bank->dbck);
reg = bank->base + bank->regs->debounce;
writel_relaxed(debounce, reg);
@@ -241,7 +241,7 @@ static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
bank->dbck_enable_mask = val;
writel_relaxed(val, reg);
- clk_disable_unprepare(bank->dbck);
+ clk_disable(bank->dbck);
/*
* Enable debounce clock per module.
* This call is mandatory because in omap_gpio_request() when
@@ -286,7 +286,7 @@ static void omap_clear_gpio_debounce(struct gpio_bank *bank, unsigned offset)
bank->context.debounce = 0;
writel_relaxed(bank->context.debounce, bank->base +
bank->regs->debounce);
- clk_disable_unprepare(bank->dbck);
+ clk_disable(bank->dbck);
bank->dbck_enabled = false;
}
}
@@ -1070,10 +1070,6 @@ static void omap_gpio_mod_init(struct gpio_bank *bank)
/* Initialize interface clk ungated, module enabled */
if (bank->regs->ctrl)
writel_relaxed(0, base + bank->regs->ctrl);
-
- bank->dbck = clk_get(bank->dev, "dbclk");
- if (IS_ERR(bank->dbck))
- dev_err(bank->dev, "Could not get gpio dbck\n");
}
static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
@@ -1234,6 +1230,17 @@ static int omap_gpio_probe(struct platform_device *pdev)
return PTR_ERR(bank->base);
}
+ if (bank->dbck_flag) {
+ bank->dbck = devm_clk_get(bank->dev, "dbclk");
+ if (IS_ERR(bank->dbck)) {
+ dev_err(bank->dev,
+ "Could not get gpio dbck. Disable debounce\n");
+ bank->dbck_flag = false;
+ } else {
+ clk_prepare(bank->dbck);
+ }
+ }
+
platform_set_drvdata(pdev, bank);
pm_runtime_enable(bank->dev);
@@ -1265,6 +1272,8 @@ static int omap_gpio_remove(struct platform_device *pdev)
list_del(&bank->node);
gpiochip_remove(&bank->chip);
pm_runtime_disable(bank->dev);
+ if (bank->dbck_flag)
+ clk_unprepare(bank->dbck);
return 0;
}
--
2.5.0
The PM runtime API can't be used in atomic contex on -RT even if
it's configured as irqsafe. As result, below error report can
be seen when PM runtime API called from IRQ chip's callbacks
irq_startup/irq_shutdown/irq_set_type, because they are
protected by RAW spinlock:
BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:917
in_atomic(): 1, irqs_disabled(): 128, pid: 96, name: insmod
3 locks held by insmod/96:
#0: (&dev->mutex){......}, at: [<c04752c8>] __driver_attach+0x54/0xa0
#1: (&dev->mutex){......}, at: [<c04752d4>] __driver_attach+0x60/0xa0
#2: (class){......}, at: [<c00a408c>] __irq_get_desc_lock+0x60/0xa4
irq event stamp: 1834
hardirqs last enabled at (1833): [<c06ab2a4>] _raw_spin_unlock_irqrestore+0x88/0x90
hardirqs last disabled at (1834): [<c06ab068>] _raw_spin_lock_irqsave+0x2c/0x64
softirqs last enabled at (0): [<c003d220>] copy_process.part.52+0x410/0x19d8
softirqs last disabled at (0): [< (null)>] (null)
Preemption disabled at:[< (null)>] (null)
CPU: 1 PID: 96 Comm: insmod Tainted: G W O 4.1.3-rt3-00618-g57e2387-dirty #184
Hardware name: Generic DRA74X (Flattened Device Tree)
[<c00190f4>] (unwind_backtrace) from [<c0014734>] (show_stack+0x20/0x24)
[<c0014734>] (show_stack) from [<c06a62ec>] (dump_stack+0x88/0xdc)
[<c06a62ec>] (dump_stack) from [<c006ca44>] (___might_sleep+0x198/0x2a8)
[<c006ca44>] (___might_sleep) from [<c06ab6d4>] (rt_spin_lock+0x30/0x70)
[<c06ab6d4>] (rt_spin_lock) from [<c04815ac>] (__pm_runtime_resume+0x68/0xa4)
[<c04815ac>] (__pm_runtime_resume) from [<c04123f4>] (omap_gpio_irq_type+0x188/0x1d8)
[<c04123f4>] (omap_gpio_irq_type) from [<c00a64e4>] (__irq_set_trigger+0x68/0x130)
[<c00a64e4>] (__irq_set_trigger) from [<c00a7bc4>] (irq_set_irq_type+0x44/0x6c)
[<c00a7bc4>] (irq_set_irq_type) from [<c00abbf8>] (irq_create_of_mapping+0x120/0x174)
[<c00abbf8>] (irq_create_of_mapping) from [<c0577b74>] (of_irq_get+0x48/0x58)
[<c0577b74>] (of_irq_get) from [<c0540a14>] (i2c_device_probe+0x54/0x15c)
[<c0540a14>] (i2c_device_probe) from [<c04750dc>] (driver_probe_device+0x184/0x2c8)
[<c04750dc>] (driver_probe_device) from [<c0475310>] (__driver_attach+0x9c/0xa0)
[<c0475310>] (__driver_attach) from [<c0473238>] (bus_for_each_dev+0x7c/0xb0)
[<c0473238>] (bus_for_each_dev) from [<c0474af4>] (driver_attach+0x28/0x30)
[<c0474af4>] (driver_attach) from [<c0474760>] (bus_add_driver+0x154/0x200)
[<c0474760>] (bus_add_driver) from [<c0476348>] (driver_register+0x88/0x108)
[<c0476348>] (driver_register) from [<c0541600>] (i2c_register_driver+0x3c/0x90)
[<c0541600>] (i2c_register_driver) from [<bf003018>] (pcf857x_init+0x18/0x24 [gpio_pcf857x])
[<bf003018>] (pcf857x_init [gpio_pcf857x]) from [<c000998c>] (do_one_initcall+0x128/0x1e8)
[<c000998c>] (do_one_initcall) from [<c06a4220>] (do_init_module+0x6c/0x1bc)
[<c06a4220>] (do_init_module) from [<c00dd0c8>] (load_module+0x18e8/0x21c4)
[<c00dd0c8>] (load_module) from [<c00ddaa0>] (SyS_init_module+0xfc/0x158)
[<c00ddaa0>] (SyS_init_module) from [<c000ff40>] (ret_fast_syscall+0x0/0x54)
The IRQ chip interface defines only two callbacks which are executed in
non-atomic contex - irq_bus_lock/irq_bus_sync_unlock, so lets move
PM runtime calls there.
Cc: <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/gpio/gpio-omap.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 2ae0d47..c33595e 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -496,9 +496,6 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned type)
(type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
return -EINVAL;
- if (!BANK_USED(bank))
- pm_runtime_get_sync(bank->dev);
-
raw_spin_lock_irqsave(&bank->lock, flags);
retval = omap_set_gpio_triggering(bank, offset, type);
if (retval) {
@@ -521,8 +518,6 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned type)
return 0;
error:
- if (!BANK_USED(bank))
- pm_runtime_put(bank->dev);
return retval;
}
@@ -797,9 +792,6 @@ static unsigned int omap_gpio_irq_startup(struct irq_data *d)
unsigned long flags;
unsigned offset = d->hwirq;
- if (!BANK_USED(bank))
- pm_runtime_get_sync(bank->dev);
-
raw_spin_lock_irqsave(&bank->lock, flags);
if (!LINE_USED(bank->mod_usage, offset))
@@ -815,8 +807,6 @@ static unsigned int omap_gpio_irq_startup(struct irq_data *d)
return 0;
err:
raw_spin_unlock_irqrestore(&bank->lock, flags);
- if (!BANK_USED(bank))
- pm_runtime_put(bank->dev);
return -EINVAL;
}
@@ -835,6 +825,19 @@ static void omap_gpio_irq_shutdown(struct irq_data *d)
omap_clear_gpio_debounce(bank, offset);
omap_disable_gpio_module(bank, offset);
raw_spin_unlock_irqrestore(&bank->lock, flags);
+}
+
+static void omap_gpio_irq_bus_lock(struct irq_data *data)
+{
+ struct gpio_bank *bank = omap_irq_data_get_bank(data);
+
+ if (!BANK_USED(bank))
+ pm_runtime_get_sync(bank->dev);
+}
+
+static void gpio_irq_bus_sync_unlock(struct irq_data *data)
+{
+ struct gpio_bank *bank = omap_irq_data_get_bank(data);
/*
* If this is the last IRQ to be freed in the bank,
@@ -1181,6 +1184,8 @@ static int omap_gpio_probe(struct platform_device *pdev)
irqc->irq_unmask = omap_gpio_unmask_irq,
irqc->irq_set_type = omap_gpio_irq_type,
irqc->irq_set_wake = omap_gpio_wake_enable,
+ irqc->irq_bus_lock = omap_gpio_irq_bus_lock,
+ irqc->irq_bus_sync_unlock = gpio_irq_bus_sync_unlock,
irqc->name = dev_name(&pdev->dev);
bank->irq = platform_get_irq(pdev, 0);
--
2.5.0
This patch converts TI OMAP GPIO driver to use generic irq handler
instead of chained IRQ handler. This way OMAP GPIO driver will be
compatible with RT kernel where it will be forced thread IRQ handler
while in non-RT kernel it still will be executed in HW IRQ context.
As part of this change the IRQ wakeup configuration is applied to
GPIO Bank IRQ as it now will be under control of IRQ PM Core during
suspend.
There are also additional benefits:
- on-RT kernel there will be no complains any more about PM runtime usage
in atomic context "BUG: sleeping function called from invalid context";
- GPIO bank IRQs will appear in /proc/interrupts and its usage statistic
will be visible;
- GPIO bank IRQs could be configured through IRQ proc_fs interface and,
as result, could be a part of IRQ balancing process if needed;
- GPIO bank IRQs will be under control of IRQ PM Core during
suspend to RAM.
Disadvantage:
- additional runtime overhed as call chain till
omap_gpio_irq_handler() will be longer now
- necessity to use wa_lock in omap_gpio_irq_handler() to W/A warning
in handle_irq_event_percpu()
WARNING: CPU: 1 PID: 35 at kernel/irq/handle.c:149 handle_irq_event_percpu+0x51c/0x638()
This patch doesn't fully follows recommendations provided by Sebastian
Andrzej Siewior [1], because It's required to go through and check all
GPIO IRQ pin states as fast as possible and pass control to handle_level_irq
or handle_edge_irq. handle_level_irq or handle_edge_irq will perform actions
specific for IRQ triggering type and wakeup corresponding registered
threaded IRQ handler (at least it's expected to be threaded).
IRQs can be lost if handle_nested_irq() will be used, because excecution
time of some pin specific GPIO IRQ handler can be very significant and
require accessing ext. devices (I2C).
Idea of such kind reworking was also discussed in [2].
[1] http://www.spinics.net/lists/linux-omap/msg120665.html
[2] http://www.spinics.net/lists/linux-omap/msg119516.html
Cc: <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
---
drivers/gpio/gpio-omap.c | 55 ++++++++++++++++++++++++------------------------
1 file changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index c33595e..0ef0180 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -59,6 +59,7 @@ struct gpio_bank {
u32 level_mask;
u32 toggle_mask;
raw_spinlock_t lock;
+ raw_spinlock_t wa_lock;
struct gpio_chip chip;
struct clk *dbck;
u32 mod_usage;
@@ -649,8 +650,13 @@ static int omap_gpio_wake_enable(struct irq_data *d, unsigned int enable)
{
struct gpio_bank *bank = omap_irq_data_get_bank(d);
unsigned offset = d->hwirq;
+ int ret;
+
+ ret = omap_set_gpio_wakeup(bank, offset, enable);
+ if (!ret)
+ ret = irq_set_irq_wake(bank->irq, enable);
- return omap_set_gpio_wakeup(bank, offset, enable);
+ return ret;
}
static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
@@ -704,26 +710,21 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
* line's interrupt handler has been run, we may miss some nested
* interrupts.
*/
-static void omap_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
+static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank)
{
void __iomem *isr_reg = NULL;
u32 isr;
unsigned int bit;
- struct gpio_bank *bank;
- int unmasked = 0;
- struct irq_chip *irqchip = irq_desc_get_chip(desc);
- struct gpio_chip *chip = irq_desc_get_handler_data(desc);
+ struct gpio_bank *bank = gpiobank;
+ unsigned long wa_lock_flags;
unsigned long lock_flags;
- chained_irq_enter(irqchip, desc);
-
- bank = container_of(chip, struct gpio_bank, chip);
isr_reg = bank->base + bank->regs->irqstatus;
- pm_runtime_get_sync(bank->dev);
-
if (WARN_ON(!isr_reg))
goto exit;
+ pm_runtime_get_sync(bank->dev);
+
while (1) {
u32 isr_saved, level_mask = 0;
u32 enabled;
@@ -745,13 +746,6 @@ static void omap_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
raw_spin_unlock_irqrestore(&bank->lock, lock_flags);
- /* if there is only edge sensitive GPIO pin interrupts
- configured, we could unmask GPIO bank interrupt immediately */
- if (!level_mask && !unmasked) {
- unmasked = 1;
- chained_irq_exit(irqchip, desc);
- }
-
if (!isr)
break;
@@ -772,18 +766,18 @@ static void omap_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
raw_spin_unlock_irqrestore(&bank->lock, lock_flags);
+ raw_spin_lock_irqsave(&bank->wa_lock, wa_lock_flags);
+
generic_handle_irq(irq_find_mapping(bank->chip.irqdomain,
bit));
+
+ raw_spin_unlock_irqrestore(&bank->wa_lock,
+ wa_lock_flags);
}
}
- /* if bank has any level sensitive GPIO pin interrupt
- configured, we must unmask the bank interrupt only after
- handler(s) are executed in order to avoid spurious bank
- interrupt */
exit:
- if (!unmasked)
- chained_irq_exit(irqchip, desc);
pm_runtime_put(bank->dev);
+ return IRQ_HANDLED;
}
static unsigned int omap_gpio_irq_startup(struct irq_data *d)
@@ -1133,7 +1127,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
}
ret = gpiochip_irqchip_add(&bank->chip, irqc,
- irq_base, omap_gpio_irq_handler,
+ irq_base, handle_bad_irq,
IRQ_TYPE_NONE);
if (ret) {
@@ -1142,10 +1136,14 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
return -ENODEV;
}
- gpiochip_set_chained_irqchip(&bank->chip, irqc,
- bank->irq, omap_gpio_irq_handler);
+ gpiochip_set_chained_irqchip(&bank->chip, irqc, bank->irq, NULL);
- return 0;
+ ret = devm_request_irq(bank->dev, bank->irq, omap_gpio_irq_handler,
+ 0, dev_name(bank->dev), bank);
+ if (ret)
+ gpiochip_remove(&bank->chip);
+
+ return ret;
}
static const struct of_device_id omap_gpio_match[];
@@ -1227,6 +1225,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
bank->set_dataout = omap_set_gpio_dataout_mask;
raw_spin_lock_init(&bank->lock);
+ raw_spin_lock_init(&bank->wa_lock);
/* Static mapping, never released */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
2.5.0
On 8/18/2015 4:10 AM, Grygorii Strashko wrote:
> Hi,
>
> This patch series contains set of trivial fixes and improvements, and also
> patches which fixes wrong APIs usage in atomic context as for -RT as for
> non-RT kernel. The final goal of this series is to make TI OMAP GPIO
> driver compatible with -RT kernel as much as possible.
>
> Patch 1-4: trivial fixes and improvements
> Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet
> Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime
> can't be used in atimic context on -RT.
> Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq
> handler instead of chained IRQ handler. This way OMAP GPIO driver will be
> compatible with RT kernel where it will be forced thread IRQ handler
> while in non-RT kernel it still will be executed in HW IRQ context.
>
> Based on
> git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
> branch: devel
> commit: 929550b gpio: mxc: fix section mismatch warning
>
> Boot, basic gpio functionality tested on:
> dra7-evm, BeagleBone(white), am43xx-gpevm, am437x-sk
> Manually tested on dra7-evm including suspend/resume and wakeup.
>
> Grygorii Strashko (7):
> gpio: omap: remove wrong irq_domain_remove usage in probe
> gpio: omap: switch to use platform_get_irq
> gpio: omap: fix omap2_set_gpio_debounce
> gpio: omap: protect regs access in omap_gpio_irq_handler
> gpio: omap: fix clk_prepare/unprepare usage
> gpio: omap: move pm runtime in irq_chip.irq_bus_lock/sync_unlock
> gpio: omap: convert to use generic irq handler
>
> drivers/gpio/gpio-omap.c | 146 +++++++++++++++++++++++++++--------------------
> 1 file changed, 85 insertions(+), 61 deletions(-)
>
Patch 1 to 5 looks fine to me. You can have that one as a series.
Am not convinced about 6 and 7. Will look at it again in detail.
For 1 to 5,
Acked-by: Santosh Shilimkar <[email protected]>
Hi,
* Grygorii Strashko <[email protected]> [150818 04:14]:
> Hi,
>
> This patch series contains set of trivial fixes and improvements, and also
> patches which fixes wrong APIs usage in atomic context as for -RT as for
> non-RT kernel. The final goal of this series is to make TI OMAP GPIO
> driver compatible with -RT kernel as much as possible.
>
> Patch 1-4: trivial fixes and improvements
> Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet
> Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime
> can't be used in atimic context on -RT.
> Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq
> handler instead of chained IRQ handler. This way OMAP GPIO driver will be
> compatible with RT kernel where it will be forced thread IRQ handler
> while in non-RT kernel it still will be executed in HW IRQ context.
Based on quick testing this series breaks at least core off idle for omap3.
You probably should add a beagle xm to your test devices so you can
properly test PM features.
Regards,
Tony
* Tony Lindgren <[email protected]> [150818 23:42]:
> Hi,
>
> * Grygorii Strashko <[email protected]> [150818 04:14]:
> > Hi,
> >
> > This patch series contains set of trivial fixes and improvements, and also
> > patches which fixes wrong APIs usage in atomic context as for -RT as for
> > non-RT kernel. The final goal of this series is to make TI OMAP GPIO
> > driver compatible with -RT kernel as much as possible.
> >
> > Patch 1-4: trivial fixes and improvements
> > Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet
> > Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime
> > can't be used in atimic context on -RT.
> > Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq
> > handler instead of chained IRQ handler. This way OMAP GPIO driver will be
> > compatible with RT kernel where it will be forced thread IRQ handler
> > while in non-RT kernel it still will be executed in HW IRQ context.
>
> Based on quick testing this series breaks at least core off idle for omap3.
> You probably should add a beagle xm to your test devices so you can
> properly test PM features.
Sorry I take that back, after trying to figure out which patch breaks PM
I noticed I had some other patches applied also. Looks like PM works just
fine with this series for me, so please feel free to add:
Tested-by: Tony Lindgren <[email protected]>
Note that I have not been able to test this with gpio button as my
boards are in a rack. You may want to do some gpio button tests to
make sure things wake up properly from off idle if you can get hold
of a beagle xm. I posted some instructions how to test earlier today
for Kishon in the "[PATCH v2 00/16] omap_hsmmc: regulator usage
cleanup and fixes" thread.
Regards,
Tony
On 08/21/2015 11:13 AM, Tony Lindgren wrote:
> * Tony Lindgren <[email protected]> [150818 23:42]:
>> Hi,
>>
>> * Grygorii Strashko <[email protected]> [150818 04:14]:
>>> Hi,
>>>
>>> This patch series contains set of trivial fixes and improvements, and also
>>> patches which fixes wrong APIs usage in atomic context as for -RT as for
>>> non-RT kernel. The final goal of this series is to make TI OMAP GPIO
>>> driver compatible with -RT kernel as much as possible.
>>>
>>> Patch 1-4: trivial fixes and improvements
>>> Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet
>>> Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime
>>> can't be used in atimic context on -RT.
>>> Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq
>>> handler instead of chained IRQ handler. This way OMAP GPIO driver will be
>>> compatible with RT kernel where it will be forced thread IRQ handler
>>> while in non-RT kernel it still will be executed in HW IRQ context.
>>
>> Based on quick testing this series breaks at least core off idle for omap3.
>> You probably should add a beagle xm to your test devices so you can
>> properly test PM features.
>
> Sorry I take that back, after trying to figure out which patch breaks PM
> I noticed I had some other patches applied also. Looks like PM works just
> fine with this series for me, so please feel free to add:
>
Uh... :) Thanks Tony a lot and sorry for delayed reply (was ooo).
You've restored my heartbeat :)
> Tested-by: Tony Lindgren <[email protected]>
>
> Note that I have not been able to test this with gpio button as my
> boards are in a rack. You may want to do some gpio button tests to
> make sure things wake up properly from off idle if you can get hold
> of a beagle xm. I posted some instructions how to test earlier today
> for Kishon in the "[PATCH v2 00/16] omap_hsmmc: regulator usage
> cleanup and fixes" thread.
I've tested it actually using GPIO buttons and UART console wake-up
(Results - https://lkml.org/lkml/2015/8/14/194 :)
And I'd try to retest it on beagle xm also, but not sure if i'd be able
to test button's wake-up - have shared access only now to beagle xm
which is rack also.
--
regards,
-grygorii
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko
<[email protected]> wrote:
> The bank->chip.irqdomain is uninitialized at the moment when
> irq_domain_remove() is called, so remove this call.
>
> Signed-off-by: Grygorii Strashko <[email protected]>
Obviously correct, patch applied.
Yours,
Linus Walleij
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko
<[email protected]> wrote:
> Switch OMAP GPIO driver to use platform_get_irq(), because
> it is not recommened to use platform_get_resource(pdev, IORESOURCE_IRQ, ..)
> for requesting IRQ resources any more, as they can be not ready yet
> in case of DT-boot.
>
> Signed-off-by: Grygorii Strashko <[email protected]>
Sane handling of deffred probe.
Patch applied.
Yours,
Linus Walleij
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko
<[email protected]> wrote:
> According to TRMs:
>
> Required input line stable =
> (the value of the GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) × 31,
> where the value of the GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME bit field
> is from 0 to 255.
>
> But now omap2_set_gpio_debounce() will calculate debounce time and
> behave incorrectly in the following cases:
> 1) requested debounce time is !0 and <32
> calculated DEBOUNCETIME = 0x1 == 62 us;
> expected value of DEBOUNCETIME = 0x0 == 31us
> 2) requested debounce time is 0
> calculated DEBOUNCETIME = 0x1 == 62 us;
> expected: disable debounce and DEBOUNCETIME = 0x0
> 3) requested debounce time is >32 and <63
> calculated DEBOUNCETIME = 0x0 and debounce will be disabled;
> expected: enable debounce and DEBOUNCETIME = 0x1 == 62 us
>
> Hence, rework omap2_set_gpio_debounce() to fix above cases:
> 1) introduce local variable "enable" and use it to identify
> when debounce need to be enabled or disabled. Disable debounce
> if requested debounce time is 0.
> 2) use below formula for debounce time calculation:
> debounce = (DIV_ROUND_UP(debounce, 31) - 1) & 0xFF;
>
> Signed-off-by: Grygorii Strashko <[email protected]>
Very hightech. Patch applied.
Yours,
Linus Walleij
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko
<[email protected]> wrote:
> The access to HW registers has to be be protected in
> omap_gpio_irq_handler(), as it may race with code executed on
> another CPUs.
>
> Signed-off-by: Grygorii Strashko <[email protected]>
Patch applied.
Yours,
Linus Walleij
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko
<[email protected]> wrote:
> As per CCF documentation (clk.txt) the clk_prepare/unprepare APIs
> are not allowed in atomic context. But now OMAP GPIO driver
> uses them while applying debounce settings and as part
> of PM runtime irqsafe operations:
>
> - omap_gpio_debounce() is holding the lock with IRQs off.
> + omap2_set_gpio_debounce()
> + clk_prepare_enable()
> + clk_prepare() this one might sleep.
>
> - pm_runtime_get_sync() is holding the lock with IRQs off
> + omap_gpio_runtime_suspend()
> + raw_spin_lock_irqsave()
> + omap_gpio_dbck_disable()
> + clk_disable_unprepare()
>
> Hence, fix it by moeving dbclk prepare/unprepare in OMAP GPIO
> omap_gpio_probe/omap_gpio_remove. Also, while here, ensure that
> debounce functionality is disabled if clk_get() failed,
> because otherwise kernel will carsh in omap2_set_gpio_debounce().
>
> Reported-by: Sebastian Andrzej Siewior <[email protected]>
> Signed-off-by: Grygorii Strashko <[email protected]>
Patch applied.
Yours,
Linus Walleij
On Tue, Aug 18, 2015 at 1:10 PM, Grygorii Strashko
<[email protected]> wrote:
> This patch series contains set of trivial fixes and improvements, and also
> patches which fixes wrong APIs usage in atomic context as for -RT as for
> non-RT kernel. The final goal of this series is to make TI OMAP GPIO
> driver compatible with -RT kernel as much as possible.
>
> Patch 1-4: trivial fixes and improvements
> Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet
I've applied patches 1-5 with Santosh's ACK and Tony's Tested-by.
> Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime
> can't be used in atimic context on -RT.
> Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq
> handler instead of chained IRQ handler. This way OMAP GPIO driver will be
> compatible with RT kernel where it will be forced thread IRQ handler
> while in non-RT kernel it still will be executed in HW IRQ context.
Waiting for more feedback here.
Yours,
Linus Walleij
* Grygorii Strashko <[email protected]> [150825 04:44]:
> On 08/21/2015 11:13 AM, Tony Lindgren wrote:
> > * Tony Lindgren <[email protected]> [150818 23:42]:
> >> Hi,
> >>
> >> * Grygorii Strashko <[email protected]> [150818 04:14]:
> >>> Hi,
> >>>
> >>> This patch series contains set of trivial fixes and improvements, and also
> >>> patches which fixes wrong APIs usage in atomic context as for -RT as for
> >>> non-RT kernel. The final goal of this series is to make TI OMAP GPIO
> >>> driver compatible with -RT kernel as much as possible.
> >>>
> >>> Patch 1-4: trivial fixes and improvements
> >>> Patch 5: fixes wrong CLK clk_prepare/unprepare APIs usage in atomic contexet
> >>> Patch 6(rfc): required to be compatible with -RT kernel, because PM runtime
> >>> can't be used in atimic context on -RT.
> >>> Patch 7(rfc): This patch converts TI OMAP GPIO driver to use generic irq
> >>> handler instead of chained IRQ handler. This way OMAP GPIO driver will be
> >>> compatible with RT kernel where it will be forced thread IRQ handler
> >>> while in non-RT kernel it still will be executed in HW IRQ context.
> >>
> >> Based on quick testing this series breaks at least core off idle for omap3.
> >> You probably should add a beagle xm to your test devices so you can
> >> properly test PM features.
> >
> > Sorry I take that back, after trying to figure out which patch breaks PM
> > I noticed I had some other patches applied also. Looks like PM works just
> > fine with this series for me, so please feel free to add:
> >
>
> Uh... :) Thanks Tony a lot and sorry for delayed reply (was ooo).
> You've restored my heartbeat :)
>
> > Tested-by: Tony Lindgren <[email protected]>
> >
> > Note that I have not been able to test this with gpio button as my
> > boards are in a rack. You may want to do some gpio button tests to
> > make sure things wake up properly from off idle if you can get hold
> > of a beagle xm. I posted some instructions how to test earlier today
> > for Kishon in the "[PATCH v2 00/16] omap_hsmmc: regulator usage
> > cleanup and fixes" thread.
>
>
> I've tested it actually using GPIO buttons and UART console wake-up
> (Results - https://lkml.org/lkml/2015/8/14/194 :)
OK great good to hear :)
> And I'd try to retest it on beagle xm also, but not sure if i'd be able
> to test button's wake-up - have shared access only now to beagle xm
> which is rack also.
OK. FYI, we're still missing a clean solution to omap3 errata 1.158
that requires remuxing GPIO pins to safe mode with pull for off idle
to avoid glitches:
http://www.spinics.net/lists/linux-omap/msg11669.html
Regards,
Tony