* [PATCH 1/9] ARM: sa1100 collie: use gpio-charger instead of pda-power
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
@ 2013-11-15 8:47 ` Dmitry Eremin-Solenikov
2013-11-15 8:47 ` [PATCH 2/9] ARM: locomo: don't clobber chip data for chained irq Dmitry Eremin-Solenikov
` (9 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:47 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
Use gpio-charger driver instead of pda-power: it automatically cares
about used gpio and since collie does not differentiate between usb and
ac chargers, pda-power is an overkill for it.
As a bonus this allows us to remove gpio_to_irq calls from machine init
call - it is fragile. These gpio_to_irq calls will fail if gpios are
registered later, via device driver mechanisms.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/mach-sa1100/collie.c | 55 ++++++-------------------------------------
1 file changed, 7 insertions(+), 48 deletions(-)
diff --git a/arch/arm/mach-sa1100/collie.c b/arch/arm/mach-sa1100/collie.c
index 7fb96eb..f902b8e 100644
--- a/arch/arm/mach-sa1100/collie.c
+++ b/arch/arm/mach-sa1100/collie.c
@@ -28,7 +28,7 @@
#include <linux/mtd/partitions.h>
#include <linux/timer.h>
#include <linux/gpio.h>
-#include <linux/pda_power.h>
+#include <linux/power/gpio-charger.h>
#include <video/sa1100fb.h>
@@ -97,62 +97,24 @@ static struct mcp_plat_data collie_mcp_data = {
/*
* Collie AC IN
*/
-static int collie_power_init(struct device *dev)
-{
- int ret = gpio_request(COLLIE_GPIO_AC_IN, "ac in");
- if (ret)
- goto err_gpio_req;
-
- ret = gpio_direction_input(COLLIE_GPIO_AC_IN);
- if (ret)
- goto err_gpio_in;
-
- return 0;
-
-err_gpio_in:
- gpio_free(COLLIE_GPIO_AC_IN);
-err_gpio_req:
- return ret;
-}
-
-static void collie_power_exit(struct device *dev)
-{
- gpio_free(COLLIE_GPIO_AC_IN);
-}
-
-static int collie_power_ac_online(void)
-{
- return gpio_get_value(COLLIE_GPIO_AC_IN) == 2;
-}
-
static char *collie_ac_supplied_to[] = {
"main-battery",
"backup-battery",
};
-static struct pda_power_pdata collie_power_data = {
- .init = collie_power_init,
- .is_ac_online = collie_power_ac_online,
- .exit = collie_power_exit,
+
+static struct gpio_charger_platform_data collie_power_data = {
+ .name = "charger",
+ .type = POWER_SUPPLY_TYPE_MAINS,
+ .gpio = COLLIE_GPIO_AC_IN,
.supplied_to = collie_ac_supplied_to,
.num_supplicants = ARRAY_SIZE(collie_ac_supplied_to),
};
-static struct resource collie_power_resource[] = {
- {
- .name = "ac",
- .flags = IORESOURCE_IRQ |
- IORESOURCE_IRQ_HIGHEDGE |
- IORESOURCE_IRQ_LOWEDGE,
- },
-};
-
static struct platform_device collie_power_device = {
- .name = "pda-power",
+ .name = "gpio-charger",
.id = -1,
.dev.platform_data = &collie_power_data,
- .resource = collie_power_resource,
- .num_resources = ARRAY_SIZE(collie_power_resource),
};
#ifdef CONFIG_SHARP_LOCOMO
@@ -348,9 +310,6 @@ static void __init collie_init(void)
GPSR |= _COLLIE_GPIO_UCB1x00_RESET;
- collie_power_resource[0].start = gpio_to_irq(COLLIE_GPIO_AC_IN);
- collie_power_resource[0].end = gpio_to_irq(COLLIE_GPIO_AC_IN);
-
sa11x0_ppc_configure_mcp();
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 2/9] ARM: locomo: don't clobber chip data for chained irq
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
2013-11-15 8:47 ` [PATCH 1/9] ARM: sa1100 collie: use gpio-charger instead of pda-power Dmitry Eremin-Solenikov
@ 2013-11-15 8:47 ` Dmitry Eremin-Solenikov
2013-11-15 8:47 ` [PATCH 3/9] ARM: sa1100: switch to MULTI_IRQ_HANDLER Dmitry Eremin-Solenikov
` (8 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:47 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
Currently locomo uses chip data to pass private data to chained irq
handler. Thus it clobbers the private data of the corresponding chip
(sa1100 or pxa). Make locomo use handler data for this purpose.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/common/locomo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c
index b55c362..f26bd50 100644
--- a/arch/arm/common/locomo.c
+++ b/arch/arm/common/locomo.c
@@ -140,7 +140,7 @@ static struct locomo_dev_info locomo_devices[] = {
static void locomo_handler(unsigned int irq, struct irq_desc *desc)
{
- struct locomo *lchip = irq_get_chip_data(irq);
+ struct locomo *lchip = irq_get_handler_data(irq);
int req, i;
/* Acknowledge the parent IRQ */
@@ -198,7 +198,7 @@ static void locomo_setup_irq(struct locomo *lchip)
* Install handler for IRQ_LOCOMO_HW.
*/
irq_set_irq_type(lchip->irq, IRQ_TYPE_EDGE_FALLING);
- irq_set_chip_data(lchip->irq, lchip);
+ irq_set_handler_data(lchip->irq, lchip);
irq_set_chained_handler(lchip->irq, locomo_handler);
/* Install handlers for IRQ_LOCOMO_* */
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 3/9] ARM: sa1100: switch to MULTI_IRQ_HANDLER
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
2013-11-15 8:47 ` [PATCH 1/9] ARM: sa1100 collie: use gpio-charger instead of pda-power Dmitry Eremin-Solenikov
2013-11-15 8:47 ` [PATCH 2/9] ARM: locomo: don't clobber chip data for chained irq Dmitry Eremin-Solenikov
@ 2013-11-15 8:47 ` Dmitry Eremin-Solenikov
2013-11-15 8:47 ` [PATCH 4/9] ARM: sa1100: convert gpio driver to be a proper platform driver Dmitry Eremin-Solenikov
` (7 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:47 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
Add sa1100_handle_irq implementating handle_irq for sa1100 platform.
Install this irq handler from sa1100_init_irq(). Also drop now unused
entry-macro.S
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/Kconfig | 1 +
arch/arm/mach-sa1100/include/mach/entry-macro.S | 41 -------------------------
arch/arm/mach-sa1100/irq.c | 20 ++++++++++++
3 files changed, 21 insertions(+), 41 deletions(-)
delete mode 100644 arch/arm/mach-sa1100/include/mach/entry-macro.S
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 6a1d2b4..2e6521927 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -719,6 +719,7 @@ config ARCH_SA1100
select GENERIC_CLOCKEVENTS
select HAVE_IDE
select ISA
+ select MULTI_IRQ_HANDLER
select NEED_MACH_MEMORY_H
select SPARSE_IRQ
help
diff --git a/arch/arm/mach-sa1100/include/mach/entry-macro.S b/arch/arm/mach-sa1100/include/mach/entry-macro.S
deleted file mode 100644
index 8cf7630..0000000
--- a/arch/arm/mach-sa1100/include/mach/entry-macro.S
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * arch/arm/mach-sa1100/include/mach/entry-macro.S
- *
- * Low-level IRQ helper macros for SA1100-based platforms
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
- .macro get_irqnr_preamble, base, tmp
- mov \base, #0xfa000000 @ ICIP = 0xfa050000
- add \base, \base, #0x00050000
- .endm
-
- .macro get_irqnr_and_base, irqnr, irqstat, base, tmp
- ldr \irqstat, [\base] @ get irqs
- ldr \irqnr, [\base, #4] @ ICMR = 0xfa050004
- ands \irqstat, \irqstat, \irqnr
- mov \irqnr, #0
- beq 1001f
- tst \irqstat, #0xff
- moveq \irqstat, \irqstat, lsr #8
- addeq \irqnr, \irqnr, #8
- tsteq \irqstat, #0xff
- moveq \irqstat, \irqstat, lsr #8
- addeq \irqnr, \irqnr, #8
- tsteq \irqstat, #0xff
- moveq \irqstat, \irqstat, lsr #8
- addeq \irqnr, \irqnr, #8
- tst \irqstat, #0x0f
- moveq \irqstat, \irqstat, lsr #4
- addeq \irqnr, \irqnr, #4
- tst \irqstat, #0x03
- moveq \irqstat, \irqstat, lsr #2
- addeq \irqnr, \irqnr, #2
- tst \irqstat, #0x01
- addeqs \irqnr, \irqnr, #1
-1001:
- .endm
-
diff --git a/arch/arm/mach-sa1100/irq.c b/arch/arm/mach-sa1100/irq.c
index 2124f1fc..68e8f9d 100644
--- a/arch/arm/mach-sa1100/irq.c
+++ b/arch/arm/mach-sa1100/irq.c
@@ -20,6 +20,7 @@
#include <mach/hardware.h>
#include <mach/irqs.h>
#include <asm/mach/irq.h>
+#include <asm/exception.h>
#include "generic.h"
@@ -291,6 +292,23 @@ static int __init sa1100irq_init_devicefs(void)
device_initcall(sa1100irq_init_devicefs);
+static asmlinkage void __exception_irq_entry
+sa1100_handle_irq(struct pt_regs *regs)
+{
+ uint32_t icip, icmr, mask;
+
+ do {
+ icip = (ICIP);
+ icmr = (ICMR);
+ mask = icip & icmr;
+
+ if (mask == 0)
+ break;
+
+ handle_IRQ(fls(mask) - 1, regs);
+ } while (1);
+}
+
void __init sa1100_init_irq(void)
{
unsigned int irq;
@@ -338,5 +356,7 @@ void __init sa1100_init_irq(void)
irq_set_chip(IRQ_GPIO11_27, &sa1100_normal_chip);
irq_set_chained_handler(IRQ_GPIO11_27, sa1100_high_gpio_handler);
+ set_handle_irq(sa1100_handle_irq);
+
sa1100_init_gpio();
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 4/9] ARM: sa1100: convert gpio driver to be a proper platform driver
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
` (2 preceding siblings ...)
2013-11-15 8:47 ` [PATCH 3/9] ARM: sa1100: switch to MULTI_IRQ_HANDLER Dmitry Eremin-Solenikov
@ 2013-11-15 8:47 ` Dmitry Eremin-Solenikov
2013-11-19 10:08 ` Linus Walleij
2013-11-15 8:47 ` [PATCH 5/9] ARM: sa1100: add platform functions to handle PWER settings Dmitry Eremin-Solenikov
` (6 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:47 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
Start cleaning up/refreshing SA1100 GPIO driver.
* Changed to bind through device model.
* Replaced direct register access with readl/writel.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/mach-sa1100/generic.c | 13 +++-
arch/arm/mach-sa1100/generic.h | 1 -
arch/arm/mach-sa1100/include/mach/SA-1100.h | 2 +
arch/arm/mach-sa1100/irq.c | 2 -
drivers/gpio/Kconfig | 6 ++
drivers/gpio/Makefile | 2 +-
drivers/gpio/gpio-sa1100.c | 110 ++++++++++++++++++++++------
7 files changed, 110 insertions(+), 26 deletions(-)
diff --git a/arch/arm/mach-sa1100/generic.c b/arch/arm/mach-sa1100/generic.c
index d4ea142..c2718f0 100644
--- a/arch/arm/mach-sa1100/generic.c
+++ b/arch/arm/mach-sa1100/generic.c
@@ -9,7 +9,6 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
-#include <linux/gpio.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>
@@ -299,6 +298,17 @@ static struct platform_device sa11x0dma_device = {
.resource = sa11x0dma_resources,
};
+static struct resource sa11x0_gpio_resources[] = {
+ DEFINE_RES_MEM(GPIO_PHYS, GPIO_SIZE),
+};
+
+static struct platform_device sa11x0gpio_device = {
+ .name = "sa1100-gpio",
+ .id = -1,
+ .num_resources = ARRAY_SIZE(sa11x0_gpio_resources),
+ .resource = sa11x0_gpio_resources,
+};
+
static struct platform_device *sa11x0_devices[] __initdata = {
&sa11x0udc_device,
&sa11x0uart1_device,
@@ -307,6 +317,7 @@ static struct platform_device *sa11x0_devices[] __initdata = {
&sa11x0pcmcia_device,
&sa11x0rtc_device,
&sa11x0dma_device,
+ &sa11x0gpio_device,
};
static int __init sa1100_init(void)
diff --git a/arch/arm/mach-sa1100/generic.h b/arch/arm/mach-sa1100/generic.h
index 0d92e11..0b68b95 100644
--- a/arch/arm/mach-sa1100/generic.h
+++ b/arch/arm/mach-sa1100/generic.h
@@ -9,7 +9,6 @@
extern void sa1100_timer_init(void);
extern void __init sa1100_map_io(void);
extern void __init sa1100_init_irq(void);
-extern void __init sa1100_init_gpio(void);
extern void sa11x0_restart(enum reboot_mode, const char *);
extern void sa11x0_init_late(void);
diff --git a/arch/arm/mach-sa1100/include/mach/SA-1100.h b/arch/arm/mach-sa1100/include/mach/SA-1100.h
index 0ac6cc0..70d01a2 100644
--- a/arch/arm/mach-sa1100/include/mach/SA-1100.h
+++ b/arch/arm/mach-sa1100/include/mach/SA-1100.h
@@ -1134,6 +1134,8 @@
* Clock
* fcpu, Tcpu Frequency, period of the CPU core clock (CCLK).
*/
+#define GPIO_PHYS 0x90040000
+#define GPIO_SIZE 0x20
#define GPLR __REG(0x90040000) /* GPIO Pin Level Reg. */
#define GPDR __REG(0x90040004) /* GPIO Pin Direction Reg. */
diff --git a/arch/arm/mach-sa1100/irq.c b/arch/arm/mach-sa1100/irq.c
index 68e8f9d..f055a6b 100644
--- a/arch/arm/mach-sa1100/irq.c
+++ b/arch/arm/mach-sa1100/irq.c
@@ -357,6 +357,4 @@ void __init sa1100_init_irq(void)
irq_set_chained_handler(IRQ_GPIO11_27, sa1100_high_gpio_handler);
set_handle_irq(sa1100_handle_irq);
-
- sa1100_init_gpio();
}
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a0471e6..1abea36 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -224,6 +224,12 @@ config GPIO_PXA
help
Say yes here to support the PXA GPIO device
+config GPIO_SA1100
+ bool "SA1100 GPIO support"
+ depends on ARCH_SA1100
+ help
+ Say yes here to support the StrongARM 11x0 GPIO device.
+
config GPIO_RCAR
tristate "Renesas R-Car GPIO"
depends on ARM
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 7971e36..57755e6 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -65,7 +65,7 @@ obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
obj-$(CONFIG_GPIO_RCAR) += gpio-rcar.o
obj-$(CONFIG_GPIO_SAMSUNG) += gpio-samsung.o
-obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o
+obj-$(CONFIG_GPIO_SA1100) += gpio-sa1100.o
obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
obj-$(CONFIG_GPIO_SODAVILLE) += gpio-sodaville.o
obj-$(CONFIG_GPIO_SPEAR_SPICS) += gpio-spear-spics.o
diff --git a/drivers/gpio/gpio-sa1100.c b/drivers/gpio/gpio-sa1100.c
index a90be34..f4e881a 100644
--- a/drivers/gpio/gpio-sa1100.c
+++ b/drivers/gpio/gpio-sa1100.c
@@ -1,5 +1,5 @@
/*
- * linux/arch/arm/mach-sa1100/gpio.c
+ * drivers/gpio/gpio-sa1100.c
*
* Generic SA-1100 GPIO handling
*
@@ -11,60 +11,128 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/io.h>
-#include <mach/hardware.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
#include <mach/irqs.h>
+#define SA1100_NGPIO 28
+
+struct sa1100_gpio_chip {
+ struct gpio_chip gc;
+ void __iomem *regbase;
+};
+
+#define to_sgc(chip) container_of(chip, struct sa1100_gpio_chip, gc)
+
+#define GPLR_OFFSET 0x00 /* GPIO Pin Level Reg. */
+#define GPDR_OFFSET 0x04 /* GPIO Pin Direction Reg. */
+#define GPSR_OFFSET 0x08 /* GPIO Pin output Set Reg. */
+#define GPCR_OFFSET 0x0C /* GPIO Pin output Clear Reg. */
+#define GRER_OFFSET 0x10 /* GPIO Rising-Edge detect Reg. */
+#define GFER_OFFSET 0x14 /* GPIO Falling-Edge detect Reg. */
+#define GEDR_OFFSET 0x18 /* GPIO Edge Detect status Reg. */
+#define GAFR_OFFSET 0x1C /* GPIO Alternate Function Reg. */
+
static int sa1100_gpio_get(struct gpio_chip *chip, unsigned offset)
{
- return GPLR & GPIO_GPIO(offset);
+ struct sa1100_gpio_chip *sgc = to_sgc(chip);
+ return readl_relaxed(sgc->regbase + GPLR_OFFSET) & BIT(offset);
}
static void sa1100_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
{
- if (value)
- GPSR = GPIO_GPIO(offset);
- else
- GPCR = GPIO_GPIO(offset);
+ struct sa1100_gpio_chip *sgc = to_sgc(chip);
+ writel_relaxed(BIT(offset), sgc->regbase +
+ (value ? GPSR_OFFSET : GPCR_OFFSET));
}
static int sa1100_direction_input(struct gpio_chip *chip, unsigned offset)
{
+ struct sa1100_gpio_chip *sgc = to_sgc(chip);
unsigned long flags;
+ uint32_t tmp;
local_irq_save(flags);
- GPDR &= ~GPIO_GPIO(offset);
+
+ tmp = readl_relaxed(sgc->regbase + GPDR_OFFSET);
+ tmp &= ~BIT(offset);
+ writel_relaxed(tmp, sgc->regbase + GPDR_OFFSET);
+
local_irq_restore(flags);
return 0;
}
static int sa1100_direction_output(struct gpio_chip *chip, unsigned offset, int value)
{
+ struct sa1100_gpio_chip *sgc = to_sgc(chip);
unsigned long flags;
+ uint32_t tmp;
local_irq_save(flags);
sa1100_gpio_set(chip, offset, value);
- GPDR |= GPIO_GPIO(offset);
+
+ tmp = readl_relaxed(sgc->regbase + GPDR_OFFSET);
+ tmp |= BIT(offset);
+ writel_relaxed(tmp, sgc->regbase + GPDR_OFFSET);
+
local_irq_restore(flags);
return 0;
}
-static int sa1100_to_irq(struct gpio_chip *chip, unsigned offset)
+static int sa1100_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
{
return offset < 11 ? (IRQ_GPIO0 + offset) : (IRQ_GPIO11 - 11 + offset);
}
-static struct gpio_chip sa1100_gpio_chip = {
- .label = "gpio",
- .direction_input = sa1100_direction_input,
- .direction_output = sa1100_direction_output,
- .set = sa1100_gpio_set,
- .get = sa1100_gpio_get,
- .to_irq = sa1100_to_irq,
- .base = 0,
- .ngpio = GPIO_MAX + 1,
+static int sa1100_gpio_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ int ret;
+ struct sa1100_gpio_chip *sgc;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
+ sgc = kzalloc(sizeof(*sgc), GFP_KERNEL);
+ if (!sgc)
+ return -ENOMEM;
+
+ sgc->regbase = ioremap(res->start, resource_size(res));
+ if (!sgc->regbase) {
+ kfree(sgc);
+ return -EINVAL;
+ }
+
+ sgc->gc.label = "gpio";
+ sgc->gc.direction_input = sa1100_direction_input;
+ sgc->gc.direction_output = sa1100_direction_output;
+ sgc->gc.set = sa1100_gpio_set;
+ sgc->gc.get = sa1100_gpio_get;
+ sgc->gc.to_irq = sa1100_gpio_to_irq;
+
+ sgc->gc.base = 0;
+ sgc->gc.ngpio = SA1100_NGPIO;
+
+ /* Initialize GPIO chips */
+ ret = gpiochip_add(&sgc->gc);
+ if (ret) {
+ iounmap(sgc->regbase);
+ kfree(sgc);
+ }
+
+ return ret;
+}
+
+static struct platform_driver sa1100_gpio_driver = {
+ .probe = sa1100_gpio_probe,
+ .driver = {
+ .name = "sa1100-gpio",
+ },
};
-void __init sa1100_init_gpio(void)
+static int __init sa1100_gpio_init(void)
{
- gpiochip_add(&sa1100_gpio_chip);
+ return platform_driver_register(&sa1100_gpio_driver);
}
+postcore_initcall(sa1100_gpio_init);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 5/9] ARM: sa1100: add platform functions to handle PWER settings
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
` (3 preceding siblings ...)
2013-11-15 8:47 ` [PATCH 4/9] ARM: sa1100: convert gpio driver to be a proper platform driver Dmitry Eremin-Solenikov
@ 2013-11-15 8:47 ` Dmitry Eremin-Solenikov
2013-11-15 8:47 ` [PATCH 6/9] ARM: sa1100: enable IRQ domains Dmitry Eremin-Solenikov
` (5 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:47 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
PWER settings logically belongs neither to GPIO nor to system IRQ code.
Add special functions to handle PWER (for GPIO and for system IRQs)
from platform code.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/mach-sa1100/generic.c | 21 +++++++++++++++++++++
arch/arm/mach-sa1100/generic.h | 2 ++
2 files changed, 23 insertions(+)
diff --git a/arch/arm/mach-sa1100/generic.c b/arch/arm/mach-sa1100/generic.c
index c2718f0..702f18c 100644
--- a/arch/arm/mach-sa1100/generic.c
+++ b/arch/arm/mach-sa1100/generic.c
@@ -421,3 +421,24 @@ void sa1110_mb_enable(void)
local_irq_restore(flags);
}
+int sa11x0_gpio_set_wake(unsigned int gpio, unsigned int on)
+{
+ if (on)
+ PWER |= 1 << gpio;
+ else
+ PWER &= ~(1 << gpio);
+
+ return 0;
+}
+
+int sa11x0_sc_set_wake(unsigned int irq, unsigned int on)
+{
+ if (irq != IRQ_RTCAlrm)
+ return -EINVAL;
+
+ if (on)
+ PWER |= PWER_RTC;
+ else
+ PWER &= ~PWER_RTC;
+ return 0;
+}
diff --git a/arch/arm/mach-sa1100/generic.h b/arch/arm/mach-sa1100/generic.h
index 0b68b95..39753c4 100644
--- a/arch/arm/mach-sa1100/generic.h
+++ b/arch/arm/mach-sa1100/generic.h
@@ -11,6 +11,8 @@ extern void __init sa1100_map_io(void);
extern void __init sa1100_init_irq(void);
extern void sa11x0_restart(enum reboot_mode, const char *);
extern void sa11x0_init_late(void);
+extern int sa11x0_gpio_set_wake(unsigned int gpio, unsigned int on);
+extern int sa11x0_sc_set_wake(unsigned int irq, unsigned int on);
#define SET_BANK(__nr,__start,__size) \
mi->bank[__nr].start = (__start), \
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 6/9] ARM: sa1100: enable IRQ domains
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
` (4 preceding siblings ...)
2013-11-15 8:47 ` [PATCH 5/9] ARM: sa1100: add platform functions to handle PWER settings Dmitry Eremin-Solenikov
@ 2013-11-15 8:47 ` Dmitry Eremin-Solenikov
2013-11-15 8:47 ` [PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver Dmitry Eremin-Solenikov
` (4 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:47 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2e6521927..a00b3c7 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -718,6 +718,7 @@ config ARCH_SA1100
select CPU_SA1100
select GENERIC_CLOCKEVENTS
select HAVE_IDE
+ select IRQ_DOMAIN
select ISA
select MULTI_IRQ_HANDLER
select NEED_MACH_MEMORY_H
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
` (5 preceding siblings ...)
2013-11-15 8:47 ` [PATCH 6/9] ARM: sa1100: enable IRQ domains Dmitry Eremin-Solenikov
@ 2013-11-15 8:47 ` Dmitry Eremin-Solenikov
2013-11-22 17:45 ` Russell King - ARM Linux
2013-11-15 8:47 ` [PATCH 8/9] ARM: sa1100: move per-IRQ PWER settings to core code Dmitry Eremin-Solenikov
` (3 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:47 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
mach-sa1100's irq.c contains a mixture of system and GPIO irqs handling.
Split out GPIO irqchip to gpio-sa1100.c. To decouple first 11 GPIO IRQs
handling, make IRQ0-IRQ10 use chained irq handler that just passes the
IRQ to GPIO IRQs.
Also during this refactoring introduce irq domain support for sa1100
gpio irqs.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/mach-sa1100/generic.c | 12 ++
arch/arm/mach-sa1100/include/mach/irqs.h | 73 ++++++----
arch/arm/mach-sa1100/irq.c | 202 +-------------------------
drivers/gpio/gpio-sa1100.c | 238 +++++++++++++++++++++++++++++--
4 files changed, 284 insertions(+), 241 deletions(-)
diff --git a/arch/arm/mach-sa1100/generic.c b/arch/arm/mach-sa1100/generic.c
index 702f18c..098593c 100644
--- a/arch/arm/mach-sa1100/generic.c
+++ b/arch/arm/mach-sa1100/generic.c
@@ -300,6 +300,18 @@ static struct platform_device sa11x0dma_device = {
static struct resource sa11x0_gpio_resources[] = {
DEFINE_RES_MEM(GPIO_PHYS, GPIO_SIZE),
+ DEFINE_RES_IRQ(IRQ_GPIO0_),
+ DEFINE_RES_IRQ(IRQ_GPIO1_),
+ DEFINE_RES_IRQ(IRQ_GPIO2_),
+ DEFINE_RES_IRQ(IRQ_GPIO3_),
+ DEFINE_RES_IRQ(IRQ_GPIO4_),
+ DEFINE_RES_IRQ(IRQ_GPIO5_),
+ DEFINE_RES_IRQ(IRQ_GPIO6_),
+ DEFINE_RES_IRQ(IRQ_GPIO7_),
+ DEFINE_RES_IRQ(IRQ_GPIO8_),
+ DEFINE_RES_IRQ(IRQ_GPIO9_),
+ DEFINE_RES_IRQ(IRQ_GPIO10_),
+ DEFINE_RES_IRQ(IRQ_GPIO11_27),
};
static struct platform_device sa11x0gpio_device = {
diff --git a/arch/arm/mach-sa1100/include/mach/irqs.h b/arch/arm/mach-sa1100/include/mach/irqs.h
index 3790298..01aed94 100644
--- a/arch/arm/mach-sa1100/include/mach/irqs.h
+++ b/arch/arm/mach-sa1100/include/mach/irqs.h
@@ -8,17 +8,17 @@
* 2001/11/14 RMK Cleaned up and standardised a lot of the IRQs.
*/
-#define IRQ_GPIO0 0
-#define IRQ_GPIO1 1
-#define IRQ_GPIO2 2
-#define IRQ_GPIO3 3
-#define IRQ_GPIO4 4
-#define IRQ_GPIO5 5
-#define IRQ_GPIO6 6
-#define IRQ_GPIO7 7
-#define IRQ_GPIO8 8
-#define IRQ_GPIO9 9
-#define IRQ_GPIO10 10
+#define IRQ_GPIO0_ 0
+#define IRQ_GPIO1_ 1
+#define IRQ_GPIO2_ 2
+#define IRQ_GPIO3_ 3
+#define IRQ_GPIO4_ 4
+#define IRQ_GPIO5_ 5
+#define IRQ_GPIO6_ 6
+#define IRQ_GPIO7_ 7
+#define IRQ_GPIO8_ 8
+#define IRQ_GPIO9_ 9
+#define IRQ_GPIO10_ 10
#define IRQ_GPIO11_27 11
#define IRQ_LCD 12 /* LCD controller */
#define IRQ_Ser0UDC 13 /* Ser. port 0 UDC */
@@ -41,32 +41,43 @@
#define IRQ_RTC1Hz 30 /* RTC 1 Hz clock */
#define IRQ_RTCAlrm 31 /* RTC Alarm */
-#define IRQ_GPIO11 32
-#define IRQ_GPIO12 33
-#define IRQ_GPIO13 34
-#define IRQ_GPIO14 35
-#define IRQ_GPIO15 36
-#define IRQ_GPIO16 37
-#define IRQ_GPIO17 38
-#define IRQ_GPIO18 39
-#define IRQ_GPIO19 40
-#define IRQ_GPIO20 41
-#define IRQ_GPIO21 42
-#define IRQ_GPIO22 43
-#define IRQ_GPIO23 44
-#define IRQ_GPIO24 45
-#define IRQ_GPIO25 46
-#define IRQ_GPIO26 47
-#define IRQ_GPIO27 48
+#define IRQ_GPIO0 32
+#define IRQ_GPIO1 33
+#define IRQ_GPIO2 34
+#define IRQ_GPIO3 35
+#define IRQ_GPIO4 36
+#define IRQ_GPIO5 37
+#define IRQ_GPIO6 38
+#define IRQ_GPIO7 39
+#define IRQ_GPIO8 40
+#define IRQ_GPIO9 41
+#define IRQ_GPIO10 42
+#define IRQ_GPIO11 43
+#define IRQ_GPIO12 44
+#define IRQ_GPIO13 45
+#define IRQ_GPIO14 46
+#define IRQ_GPIO15 47
+#define IRQ_GPIO16 48
+#define IRQ_GPIO17 49
+#define IRQ_GPIO18 50
+#define IRQ_GPIO19 51
+#define IRQ_GPIO20 52
+#define IRQ_GPIO21 53
+#define IRQ_GPIO22 54
+#define IRQ_GPIO23 55
+#define IRQ_GPIO24 56
+#define IRQ_GPIO25 57
+#define IRQ_GPIO26 58
+#define IRQ_GPIO27 59
/*
* The next 16 interrupts are for board specific purposes. Since
* the kernel can only run on one machine at a time, we can re-use
* these. If you need more, increase IRQ_BOARD_END, but keep it
- * within sensible limits. IRQs 49 to 64 are available.
+ * within sensible limits. IRQs 60 to 75 are available.
*/
-#define IRQ_BOARD_START 49
-#define IRQ_BOARD_END 65
+#define IRQ_BOARD_START 60
+#define IRQ_BOARD_END 76
/*
* Figure out the MAX IRQ number.
diff --git a/arch/arm/mach-sa1100/irq.c b/arch/arm/mach-sa1100/irq.c
index f055a6b..59c1617 100644
--- a/arch/arm/mach-sa1100/irq.c
+++ b/arch/arm/mach-sa1100/irq.c
@@ -24,169 +24,6 @@
#include "generic.h"
-
-/*
- * SA1100 GPIO edge detection for IRQs:
- * IRQs are generated on Falling-Edge, Rising-Edge, or both.
- * Use this instead of directly setting GRER/GFER.
- */
-static int GPIO_IRQ_rising_edge;
-static int GPIO_IRQ_falling_edge;
-static int GPIO_IRQ_mask = (1 << 11) - 1;
-
-/*
- * To get the GPIO number from an IRQ number
- */
-#define GPIO_11_27_IRQ(i) ((i) - 21)
-#define GPIO11_27_MASK(irq) (1 << GPIO_11_27_IRQ(irq))
-
-static int sa1100_gpio_type(struct irq_data *d, unsigned int type)
-{
- unsigned int mask;
-
- if (d->irq <= 10)
- mask = 1 << d->irq;
- else
- mask = GPIO11_27_MASK(d->irq);
-
- if (type == IRQ_TYPE_PROBE) {
- if ((GPIO_IRQ_rising_edge | GPIO_IRQ_falling_edge) & mask)
- return 0;
- type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
- }
-
- if (type & IRQ_TYPE_EDGE_RISING) {
- GPIO_IRQ_rising_edge |= mask;
- } else
- GPIO_IRQ_rising_edge &= ~mask;
- if (type & IRQ_TYPE_EDGE_FALLING) {
- GPIO_IRQ_falling_edge |= mask;
- } else
- GPIO_IRQ_falling_edge &= ~mask;
-
- GRER = GPIO_IRQ_rising_edge & GPIO_IRQ_mask;
- GFER = GPIO_IRQ_falling_edge & GPIO_IRQ_mask;
-
- return 0;
-}
-
-/*
- * GPIO IRQs must be acknowledged. This is for IRQs from 0 to 10.
- */
-static void sa1100_low_gpio_ack(struct irq_data *d)
-{
- GEDR = (1 << d->irq);
-}
-
-static void sa1100_low_gpio_mask(struct irq_data *d)
-{
- ICMR &= ~(1 << d->irq);
-}
-
-static void sa1100_low_gpio_unmask(struct irq_data *d)
-{
- ICMR |= 1 << d->irq;
-}
-
-static int sa1100_low_gpio_wake(struct irq_data *d, unsigned int on)
-{
- if (on)
- PWER |= 1 << d->irq;
- else
- PWER &= ~(1 << d->irq);
- return 0;
-}
-
-static struct irq_chip sa1100_low_gpio_chip = {
- .name = "GPIO-l",
- .irq_ack = sa1100_low_gpio_ack,
- .irq_mask = sa1100_low_gpio_mask,
- .irq_unmask = sa1100_low_gpio_unmask,
- .irq_set_type = sa1100_gpio_type,
- .irq_set_wake = sa1100_low_gpio_wake,
-};
-
-/*
- * IRQ11 (GPIO11 through 27) handler. We enter here with the
- * irq_controller_lock held, and IRQs disabled. Decode the IRQ
- * and call the handler.
- */
-static void
-sa1100_high_gpio_handler(unsigned int irq, struct irq_desc *desc)
-{
- unsigned int mask;
-
- mask = GEDR & 0xfffff800;
- do {
- /*
- * clear down all currently active IRQ sources.
- * We will be processing them all.
- */
- GEDR = mask;
-
- irq = IRQ_GPIO11;
- mask >>= 11;
- do {
- if (mask & 1)
- generic_handle_irq(irq);
- mask >>= 1;
- irq++;
- } while (mask);
-
- mask = GEDR & 0xfffff800;
- } while (mask);
-}
-
-/*
- * Like GPIO0 to 10, GPIO11-27 IRQs need to be handled specially.
- * In addition, the IRQs are all collected up into one bit in the
- * interrupt controller registers.
- */
-static void sa1100_high_gpio_ack(struct irq_data *d)
-{
- unsigned int mask = GPIO11_27_MASK(d->irq);
-
- GEDR = mask;
-}
-
-static void sa1100_high_gpio_mask(struct irq_data *d)
-{
- unsigned int mask = GPIO11_27_MASK(d->irq);
-
- GPIO_IRQ_mask &= ~mask;
-
- GRER &= ~mask;
- GFER &= ~mask;
-}
-
-static void sa1100_high_gpio_unmask(struct irq_data *d)
-{
- unsigned int mask = GPIO11_27_MASK(d->irq);
-
- GPIO_IRQ_mask |= mask;
-
- GRER = GPIO_IRQ_rising_edge & GPIO_IRQ_mask;
- GFER = GPIO_IRQ_falling_edge & GPIO_IRQ_mask;
-}
-
-static int sa1100_high_gpio_wake(struct irq_data *d, unsigned int on)
-{
- if (on)
- PWER |= GPIO11_27_MASK(d->irq);
- else
- PWER &= ~GPIO11_27_MASK(d->irq);
- return 0;
-}
-
-static struct irq_chip sa1100_high_gpio_chip = {
- .name = "GPIO-h",
- .irq_ack = sa1100_high_gpio_ack,
- .irq_mask = sa1100_high_gpio_mask,
- .irq_unmask = sa1100_high_gpio_unmask,
- .irq_set_type = sa1100_gpio_type,
- .irq_set_wake = sa1100_high_gpio_wake,
-};
-
/*
* We don't need to ACK IRQs on the SA1100 unless they're GPIOs
* this is for internal IRQs i.e. from 11 to 31.
@@ -250,16 +87,6 @@ static int sa1100irq_suspend(void)
IC_GPIO6|IC_GPIO5|IC_GPIO4|IC_GPIO3|IC_GPIO2|
IC_GPIO1|IC_GPIO0);
- /*
- * Set the appropriate edges for wakeup.
- */
- GRER = PWER & GPIO_IRQ_rising_edge;
- GFER = PWER & GPIO_IRQ_falling_edge;
-
- /*
- * Clear any pending GPIO interrupts.
- */
- GEDR = GEDR;
return 0;
}
@@ -271,10 +98,6 @@ static void sa1100irq_resume(void)
if (st->saved) {
ICCR = st->iccr;
ICLR = st->iclr;
-
- GRER = GPIO_IRQ_rising_edge & GPIO_IRQ_mask;
- GFER = GPIO_IRQ_falling_edge & GPIO_IRQ_mask;
-
ICMR = st->icmr;
}
}
@@ -321,40 +144,17 @@ void __init sa1100_init_irq(void)
/* all IRQs are IRQ, not FIQ */
ICLR = 0;
- /* clear all GPIO edge detects */
- GFER = 0;
- GRER = 0;
- GEDR = -1;
-
/*
* Whatever the doc says, this has to be set for the wait-on-irq
* instruction to work... on a SA1100 rev 9 at least.
*/
ICCR = 1;
- for (irq = 0; irq <= 10; irq++) {
- irq_set_chip_and_handler(irq, &sa1100_low_gpio_chip,
- handle_edge_irq);
- set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
- }
-
- for (irq = 12; irq <= 31; irq++) {
+ for (irq = 0; irq <= 31; irq++) {
irq_set_chip_and_handler(irq, &sa1100_normal_chip,
handle_level_irq);
set_irq_flags(irq, IRQF_VALID);
}
- for (irq = 32; irq <= 48; irq++) {
- irq_set_chip_and_handler(irq, &sa1100_high_gpio_chip,
- handle_edge_irq);
- set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
- }
-
- /*
- * Install handler for GPIO 11-27 edge detect interrupts
- */
- irq_set_chip(IRQ_GPIO11_27, &sa1100_normal_chip);
- irq_set_chained_handler(IRQ_GPIO11_27, sa1100_high_gpio_handler);
-
set_handle_irq(sa1100_handle_irq);
}
diff --git a/drivers/gpio/gpio-sa1100.c b/drivers/gpio/gpio-sa1100.c
index f4e881a..89719fb1 100644
--- a/drivers/gpio/gpio-sa1100.c
+++ b/drivers/gpio/gpio-sa1100.c
@@ -12,14 +12,29 @@
#include <linux/module.h>
#include <linux/io.h>
#include <linux/slab.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
#include <linux/platform_device.h>
+#include <linux/syscore_ops.h>
#include <mach/irqs.h>
+#include <mach/generic.h>
+
+/*
+ * SA1100 GPIO edge detection for IRQs:
+ * IRQs are generated on Falling-Edge, Rising-Edge, or both.
+ * Use this instead of directly setting GRER/GFER.
+ */
#define SA1100_NGPIO 28
struct sa1100_gpio_chip {
struct gpio_chip gc;
void __iomem *regbase;
+ struct irq_domain *domain;
+ int gpio_rising;
+ int gpio_falling;
+ int gpio_mask;
+ int irq_base;
};
#define to_sgc(chip) container_of(chip, struct sa1100_gpio_chip, gc)
@@ -81,28 +96,145 @@ static int sa1100_direction_output(struct gpio_chip *chip, unsigned offset, int
static int sa1100_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
{
- return offset < 11 ? (IRQ_GPIO0 + offset) : (IRQ_GPIO11 - 11 + offset);
+ struct sa1100_gpio_chip *sgc = to_sgc(chip);
+ return irq_find_mapping(sgc->domain, offset);
+}
+
+static int sa1100_gpio_type(struct irq_data *d, unsigned int type)
+{
+ struct sa1100_gpio_chip *sgc = irq_data_get_irq_chip_data(d);
+ unsigned int mask = BIT(d->hwirq);
+
+ if (type == IRQ_TYPE_PROBE) {
+ if ((sgc->gpio_rising | sgc->gpio_falling) & mask)
+ return 0;
+ type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
+ }
+
+ if (type & IRQ_TYPE_EDGE_RISING)
+ sgc->gpio_rising |= mask;
+ else
+ sgc->gpio_rising &= ~mask;
+
+ if (type & IRQ_TYPE_EDGE_FALLING)
+ sgc->gpio_falling |= mask;
+ else
+ sgc->gpio_falling &= ~mask;
+
+ writel_relaxed(sgc->gpio_rising & sgc->gpio_mask,
+ sgc->regbase + GRER_OFFSET);
+ writel_relaxed(sgc->gpio_falling & sgc->gpio_mask,
+ sgc->regbase + GFER_OFFSET);
+
+ return 0;
+}
+
+/*
+ * GPIO IRQs must be acknowledged.
+ */
+static void sa1100_gpio_ack(struct irq_data *d)
+{
+ struct sa1100_gpio_chip *sgc = irq_data_get_irq_chip_data(d);
+
+ writel_relaxed(BIT(d->hwirq), sgc->regbase + GEDR_OFFSET);
+}
+
+static int sa1100_gpio_wake(struct irq_data *d, unsigned int on)
+{
+ return sa11x0_gpio_set_wake(d->hwirq, on);
+}
+
+static void sa1100_gpio_mask(struct irq_data *d)
+{
+ struct sa1100_gpio_chip *sgc = irq_data_get_irq_chip_data(d);
+
+ sgc->gpio_mask &= ~BIT(d->hwirq);
+
+ writel_relaxed(sgc->gpio_rising & sgc->gpio_mask,
+ sgc->regbase + GRER_OFFSET);
+ writel_relaxed(sgc->gpio_falling & sgc->gpio_mask,
+ sgc->regbase + GFER_OFFSET);
+}
+
+static void sa1100_gpio_unmask(struct irq_data *d)
+{
+ struct sa1100_gpio_chip *sgc = irq_data_get_irq_chip_data(d);
+
+ sgc->gpio_mask |= BIT(d->hwirq);
+
+ writel_relaxed(sgc->gpio_rising & sgc->gpio_mask,
+ sgc->regbase + GRER_OFFSET);
+ writel_relaxed(sgc->gpio_falling & sgc->gpio_mask,
+ sgc->regbase + GFER_OFFSET);
+}
+
+static void
+sa1100_gpio_handler(unsigned int irq, struct irq_desc *desc)
+{
+ struct sa1100_gpio_chip *sgc = irq_get_handler_data(irq);
+ unsigned int hwirq = 0;
+ unsigned int mask = readl_relaxed(sgc->regbase + GEDR_OFFSET);
+ /*
+ * clear down all currently active IRQ sources.
+ * We will be processing them all.
+ */
+ writel_relaxed(mask, sgc->regbase + GEDR_OFFSET);
+
+ while (mask) {
+ if (mask & 1)
+ generic_handle_irq(irq_find_mapping(sgc->domain,
+ hwirq));
+ mask >>= 1;
+ hwirq++;
+ }
}
+static struct irq_chip sa1100_gpio_irq_chip = {
+ .name = "GPIO",
+ .irq_ack = sa1100_gpio_ack,
+ .irq_mask = sa1100_gpio_mask,
+ .irq_unmask = sa1100_gpio_unmask,
+ .irq_set_type = sa1100_gpio_type,
+ .irq_set_wake = sa1100_gpio_wake,
+};
+
+static int sa1100_gpio_irqdomain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ struct sa1100_gpio_chip *sgc = d->host_data;
+
+ irq_set_chip_data(irq, sgc);
+ irq_set_chip_and_handler(irq, &sa1100_gpio_irq_chip, handle_edge_irq);
+ set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
+
+ return 0;
+}
+
+static struct irq_domain_ops sa1100_gpio_irqdomain_ops = {
+ .map = sa1100_gpio_irqdomain_map,
+ .xlate = irq_domain_xlate_onetwocell,
+};
+
+static struct sa1100_gpio_chip *chip;
+
static int sa1100_gpio_probe(struct platform_device *pdev)
{
struct resource *res;
int ret;
+ unsigned int i, irq;
struct sa1100_gpio_chip *sgc;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -EINVAL;
- sgc = kzalloc(sizeof(*sgc), GFP_KERNEL);
+ sgc = devm_kzalloc(&pdev->dev, sizeof(*sgc), GFP_KERNEL);
if (!sgc)
return -ENOMEM;
- sgc->regbase = ioremap(res->start, resource_size(res));
- if (!sgc->regbase) {
- kfree(sgc);
+ sgc->regbase = devm_ioremap_resource(&pdev->dev, res);
+ if (!sgc->regbase)
return -EINVAL;
- }
sgc->gc.label = "gpio";
sgc->gc.direction_input = sa1100_direction_input;
@@ -114,12 +246,45 @@ static int sa1100_gpio_probe(struct platform_device *pdev)
sgc->gc.base = 0;
sgc->gc.ngpio = SA1100_NGPIO;
+ sgc->irq_base = IRQ_GPIO0;
+
+ /* clear all GPIO edge detects */
+ writel_relaxed(0, sgc->regbase + GFER_OFFSET);
+ writel_relaxed(0, sgc->regbase + GRER_OFFSET);
+ writel_relaxed(-1, sgc->regbase + GEDR_OFFSET);
+
/* Initialize GPIO chips */
ret = gpiochip_add(&sgc->gc);
- if (ret) {
- iounmap(sgc->regbase);
- kfree(sgc);
+ if (ret)
+ return ret;
+
+ sgc->domain = irq_domain_add_legacy(NULL, SA1100_NGPIO, IRQ_GPIO0, 0,
+ &sa1100_gpio_irqdomain_ops, sgc);
+ /*
+ * Install handler for GPIO 11-27 edge detect interrupts
+ */
+ for (i = 0; i < 11; i++) {
+ irq = platform_get_irq(pdev, i);
+ if (irq < 0)
+ goto err_irq;
+ irq_set_handler_data(irq, sgc);
+ irq_set_chained_handler(irq, sa1100_gpio_handler);
}
+ irq = platform_get_irq(pdev, 11);
+ if (irq < 0)
+ goto err_irq;
+ irq_set_handler_data(irq, sgc);
+ irq_set_chained_handler(irq, sa1100_gpio_handler);
+
+ chip = sgc;
+
+ return 0;
+
+err_irq:
+ dev_err(&pdev->dev, "Error retrieving irq resource %d (%d)\n", i, irq);
+ i = gpiochip_remove(&sgc->gc);
+ if (i)
+ dev_err(&pdev->dev, "Error removing gpio chip (%d)!\n", i);
return ret;
}
@@ -136,3 +301,58 @@ static int __init sa1100_gpio_init(void)
return platform_driver_register(&sa1100_gpio_driver);
}
postcore_initcall(sa1100_gpio_init);
+
+#ifdef CONFIG_PM
+static unsigned long saved_gplr;
+static unsigned long saved_gpdr;
+static unsigned long saved_grer;
+static unsigned long saved_gfer;
+
+static int sa1100_gpio_suspend(void)
+{
+ struct sa1100_gpio_chip *sgc = chip;
+ saved_gplr = readl_relaxed(sgc->regbase + GPLR_OFFSET);
+ saved_gpdr = readl_relaxed(sgc->regbase + GPDR_OFFSET);
+ saved_grer = readl_relaxed(sgc->regbase + GRER_OFFSET);
+ saved_gfer = readl_relaxed(sgc->regbase + GFER_OFFSET);
+
+ /* Clear GPIO transition detect bits */
+ writel_relaxed(0xffffffff, sgc->regbase + GEDR_OFFSET);
+
+ /* FIXME: Original code also reprogramed GRER/GFER here,
+ * I don't see the purpose though.
+ GRER = PWER & sgc->gpio_rising;
+ GFER = PWER & sgc->gpio_falling;
+ */
+
+ return 0;
+}
+
+static void sa1100_gpio_resume(void)
+{
+ struct sa1100_gpio_chip *sgc = chip;
+ /* restore level with set/clear */
+ writel_relaxed(saved_gplr, sgc->regbase + GPSR_OFFSET);
+ writel_relaxed(~saved_gplr, sgc->regbase + GPCR_OFFSET);
+
+ writel_relaxed(saved_grer, sgc->regbase + GRER_OFFSET);
+ writel_relaxed(saved_gfer, sgc->regbase + GFER_OFFSET);
+ writel_relaxed(saved_gpdr, sgc->regbase + GPDR_OFFSET);
+}
+#else
+#define sa1100_gpio_suspend NULL
+#define sa1100_gpio_resume NULL
+#endif
+
+static struct syscore_ops sa1100_gpio_syscore_ops = {
+ .suspend = sa1100_gpio_suspend,
+ .resume = sa1100_gpio_resume,
+};
+
+static int __init sa1100_gpio_sysinit(void)
+{
+ if (chip)
+ register_syscore_ops(&sa1100_gpio_syscore_ops);
+ return 0;
+}
+postcore_initcall(sa1100_gpio_sysinit);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver
2013-11-15 8:47 ` [PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver Dmitry Eremin-Solenikov
@ 2013-11-22 17:45 ` Russell King - ARM Linux
2013-11-22 19:46 ` Dmitry Eremin-Solenikov
0 siblings, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2013-11-22 17:45 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: linux-arm-kernel, linux-gpio, Linus Walleij, Dmitry Artamonow
On Fri, Nov 15, 2013 at 12:47:58PM +0400, Dmitry Eremin-Solenikov wrote:
> mach-sa1100's irq.c contains a mixture of system and GPIO irqs handling.
> Split out GPIO irqchip to gpio-sa1100.c. To decouple first 11 GPIO IRQs
> handling, make IRQ0-IRQ10 use chained irq handler that just passes the
> IRQ to GPIO IRQs.
>
> Also during this refactoring introduce irq domain support for sa1100
> gpio irqs.
I'm not sure I quite like this change... How much testing has this had?
I'd much prefer that this code just isn't touched because of all the
problems we've had here in years back with IRQs from CF cards being
dropped and the like. This was very hard to get right, especially when
interacting with the SA1111.
> @@ -136,3 +301,58 @@ static int __init sa1100_gpio_init(void)
> return platform_driver_register(&sa1100_gpio_driver);
> }
> postcore_initcall(sa1100_gpio_init);
> +
> +#ifdef CONFIG_PM
> +static unsigned long saved_gplr;
> +static unsigned long saved_gpdr;
> +static unsigned long saved_grer;
> +static unsigned long saved_gfer;
> +
> +static int sa1100_gpio_suspend(void)
> +{
> + struct sa1100_gpio_chip *sgc = chip;
> + saved_gplr = readl_relaxed(sgc->regbase + GPLR_OFFSET);
> + saved_gpdr = readl_relaxed(sgc->regbase + GPDR_OFFSET);
> + saved_grer = readl_relaxed(sgc->regbase + GRER_OFFSET);
> + saved_gfer = readl_relaxed(sgc->regbase + GFER_OFFSET);
> +
> + /* Clear GPIO transition detect bits */
> + writel_relaxed(0xffffffff, sgc->regbase + GEDR_OFFSET);
> +
> + /* FIXME: Original code also reprogramed GRER/GFER here,
> + * I don't see the purpose though.
> + GRER = PWER & sgc->gpio_rising;
> + GFER = PWER & sgc->gpio_falling;
> + */
So you thought you'd just comment it out because you don't understand...
Not really the way things are done. If you don't understand something,
you shouldn't be touching the code.
In this case, it's quite simple. GRER and GFER need to be programmed
with the interrupts which we want to be active for _each_ mode of the
system.
Therefore, if we want to have certain GPIOs triggering wakeups (iow,
those GPIOs which have had enable_irq_wake() called on them) but not
those which haven't, we need to reprogram GRER and GFER with the
GPIOs which can wake the system up.
Quite simple and obvious really.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver
2013-11-22 17:45 ` Russell King - ARM Linux
@ 2013-11-22 19:46 ` Dmitry Eremin-Solenikov
2013-11-22 20:02 ` Russell King - ARM Linux
0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-22 19:46 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-arm-kernel, linux-gpio@vger.kernel.org, Linus Walleij,
Dmitry Artamonow
Hello,
On Fri, Nov 22, 2013 at 9:45 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Nov 15, 2013 at 12:47:58PM +0400, Dmitry Eremin-Solenikov wrote:
>> mach-sa1100's irq.c contains a mixture of system and GPIO irqs handling.
>> Split out GPIO irqchip to gpio-sa1100.c. To decouple first 11 GPIO IRQs
>> handling, make IRQ0-IRQ10 use chained irq handler that just passes the
>> IRQ to GPIO IRQs.
>>
>> Also during this refactoring introduce irq domain support for sa1100
>> gpio irqs.
>
> I'm not sure I quite like this change... How much testing has this had?
> I'd much prefer that this code just isn't touched because of all the
> problems we've had here in years back with IRQs from CF cards being
> dropped and the like. This was very hard to get right, especially when
> interacting with the SA1111.
How can we just check that? And what was the problem with CF IRQs?
Sorry, if I'm asking dumb questions, I really missed that part of the history.
I can go the way Linus Walleij originally did:
1) switch to irq domain / hwirq
2) integrate static variables to containers
And only after that in a separate patch(set)
3) Move GPIO IRQ handling to gpio-sa1100.
What do you think?
>
>> @@ -136,3 +301,58 @@ static int __init sa1100_gpio_init(void)
>> return platform_driver_register(&sa1100_gpio_driver);
>> }
>> postcore_initcall(sa1100_gpio_init);
>> +
>> +#ifdef CONFIG_PM
>> +static unsigned long saved_gplr;
>> +static unsigned long saved_gpdr;
>> +static unsigned long saved_grer;
>> +static unsigned long saved_gfer;
>> +
>> +static int sa1100_gpio_suspend(void)
>> +{
>> + struct sa1100_gpio_chip *sgc = chip;
>> + saved_gplr = readl_relaxed(sgc->regbase + GPLR_OFFSET);
>> + saved_gpdr = readl_relaxed(sgc->regbase + GPDR_OFFSET);
>> + saved_grer = readl_relaxed(sgc->regbase + GRER_OFFSET);
>> + saved_gfer = readl_relaxed(sgc->regbase + GFER_OFFSET);
>> +
>> + /* Clear GPIO transition detect bits */
>> + writel_relaxed(0xffffffff, sgc->regbase + GEDR_OFFSET);
>> +
>> + /* FIXME: Original code also reprogramed GRER/GFER here,
>> + * I don't see the purpose though.
>> + GRER = PWER & sgc->gpio_rising;
>> + GFER = PWER & sgc->gpio_falling;
>> + */
>
> So you thought you'd just comment it out because you don't understand...
> Not really the way things are done. If you don't understand something,
> you shouldn't be touching the code.
Thus I have the big FIXME.
> In this case, it's quite simple. GRER and GFER need to be programmed
> with the interrupts which we want to be active for _each_ mode of the
> system.
I see. I will put this back in the next iteration, but see below please.
> Therefore, if we want to have certain GPIOs triggering wakeups (iow,
> those GPIOs which have had enable_irq_wake() called on them) but not
> those which haven't, we need to reprogram GRER and GFER with the
> GPIOs which can wake the system up.
I thought so. I was puzzled, because that would mean, that we want to wake
up from a GPIO, which is masked in GPIO_IRQ_mask, but enabled in PWER.
Is that the real scenario/usecase? If we/kernel has disabled IRQ
through _mask_irq,
I thought, we didn't expect events from that pin (including wakeup), did we?
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver
2013-11-22 19:46 ` Dmitry Eremin-Solenikov
@ 2013-11-22 20:02 ` Russell King - ARM Linux
2013-11-22 21:20 ` Dmitry Eremin-Solenikov
0 siblings, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2013-11-22 20:02 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: linux-arm-kernel, linux-gpio@vger.kernel.org, Linus Walleij,
Dmitry Artamonow
On Fri, Nov 22, 2013 at 11:46:10PM +0400, Dmitry Eremin-Solenikov wrote:
> Hello,
>
> On Fri, Nov 22, 2013 at 9:45 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > Therefore, if we want to have certain GPIOs triggering wakeups (iow,
> > those GPIOs which have had enable_irq_wake() called on them) but not
> > those which haven't, we need to reprogram GRER and GFER with the
> > GPIOs which can wake the system up.
>
> I thought so. I was puzzled, because that would mean, that we want to wake
> up from a GPIO, which is masked in GPIO_IRQ_mask, but enabled in PWER.
> Is that the real scenario/usecase? If we/kernel has disabled IRQ
> through _mask_irq,
> I thought, we didn't expect events from that pin (including wakeup), did we?
So, if a device driver decides that it wants to be woken up by a GPIO
and therefore calls enable_irq_wake() on it, but also calls disable_irq()
on that same interrupt to avoid _receiving_ the interrupt until it's
ready, does that mean that the system should _not_ be woken up?
I doubt many systems work this way - later PXA devices certainly don't,
where the wakeup is triggered from the silicon on the pad interface
rather than the interrupt controller - where the state of the IRQ masks
have no bearing what so ever on it.
Please, stop thinking about changing the behaviour of any of the SA11x0
stuff. Treat the code as-is as authoritive on the way things should be
done, and _carefully_ "adjust it" to how you'd like to see it - so we
can have a higher confidence that stuff isn't being broken or behaviour
isn't changed.
Such behaviour changes would include looping differently while handling
interrupts.
And more importantly, treat SA11x0 as a reference implementation for
IRQ stuff; that's the platform I developed the IRQ handling stuff on
which became the basis for tglx's IRQ layer in the kernel.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver
2013-11-22 20:02 ` Russell King - ARM Linux
@ 2013-11-22 21:20 ` Dmitry Eremin-Solenikov
0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-22 21:20 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-arm-kernel, linux-gpio@vger.kernel.org, Linus Walleij,
Dmitry Artamonow
Hello,
On Sat, Nov 23, 2013 at 12:02 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Nov 22, 2013 at 11:46:10PM +0400, Dmitry Eremin-Solenikov wrote:
>> On Fri, Nov 22, 2013 at 9:45 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > Therefore, if we want to have certain GPIOs triggering wakeups (iow,
>> > those GPIOs which have had enable_irq_wake() called on them) but not
>> > those which haven't, we need to reprogram GRER and GFER with the
>> > GPIOs which can wake the system up.
>>
>> I thought so. I was puzzled, because that would mean, that we want to wake
>> up from a GPIO, which is masked in GPIO_IRQ_mask, but enabled in PWER.
>> Is that the real scenario/usecase? If we/kernel has disabled IRQ
>> through _mask_irq,
>> I thought, we didn't expect events from that pin (including wakeup), did we?
>
> So, if a device driver decides that it wants to be woken up by a GPIO
> and therefore calls enable_irq_wake() on it, but also calls disable_irq()
> on that same interrupt to avoid _receiving_ the interrupt until it's
> ready, does that mean that the system should _not_ be woken up?
>
> I doubt many systems work this way - later PXA devices certainly don't,
> where the wakeup is triggered from the silicon on the pad interface
> rather than the interrupt controller - where the state of the IRQ masks
> have no bearing what so ever on it.
>
> Please, stop thinking about changing the behaviour of any of the SA11x0
> stuff. Treat the code as-is as authoritive on the way things should be
> done, and _carefully_ "adjust it" to how you'd like to see it - so we
> can have a higher confidence that stuff isn't being broken or behaviour
> isn't changed.
>
> Such behaviour changes would include looping differently while handling
> interrupts.
>
> And more importantly, treat SA11x0 as a reference implementation for
> IRQ stuff; that's the platform I developed the IRQ handling stuff on
> which became the basis for tglx's IRQ layer in the kernel.
OK, I got your message. I agree that I don't have enough level of knowledge
to rework SA11x0 it in such big steps.
Thank you for your explanations.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 8/9] ARM: sa1100: move per-IRQ PWER settings to core code
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
` (6 preceding siblings ...)
2013-11-15 8:47 ` [PATCH 7/9] ARM: sa1100: move gpio irq handling to GPIO driver Dmitry Eremin-Solenikov
@ 2013-11-15 8:47 ` Dmitry Eremin-Solenikov
2013-11-15 8:48 ` [PATCH 9/9] ARM: sa1100: refactor irq driver Dmitry Eremin-Solenikov
` (2 subsequent siblings)
10 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:47 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
In attempt to limit register access from non-core code, move PWER access
to generic.c, having irqchip code call sa11x0_sc_set_wake() function
to set wake status.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/mach-sa1100/irq.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/arch/arm/mach-sa1100/irq.c b/arch/arm/mach-sa1100/irq.c
index 59c1617..d96f65f 100644
--- a/arch/arm/mach-sa1100/irq.c
+++ b/arch/arm/mach-sa1100/irq.c
@@ -18,8 +18,6 @@
#include <linux/syscore_ops.h>
#include <mach/hardware.h>
-#include <mach/irqs.h>
-#include <asm/mach/irq.h>
#include <asm/exception.h>
#include "generic.h"
@@ -43,14 +41,7 @@ static void sa1100_unmask_irq(struct irq_data *d)
*/
static int sa1100_set_wake(struct irq_data *d, unsigned int on)
{
- if (d->irq == IRQ_RTCAlrm) {
- if (on)
- PWER |= PWER_RTC;
- else
- PWER &= ~PWER_RTC;
- return 0;
- }
- return -EINVAL;
+ return sa11x0_sc_set_wake(d->irq, on);
}
static struct irq_chip sa1100_normal_chip = {
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 9/9] ARM: sa1100: refactor irq driver
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
` (7 preceding siblings ...)
2013-11-15 8:47 ` [PATCH 8/9] ARM: sa1100: move per-IRQ PWER settings to core code Dmitry Eremin-Solenikov
@ 2013-11-15 8:48 ` Dmitry Eremin-Solenikov
2013-11-19 13:00 ` [PATCH 0/9] ARM: sa1100: Rework IRQ handling Linus Walleij
2013-11-22 21:35 ` Dmitry Eremin-Solenikov
10 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-15 8:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio
Cc: Russell King, Linus Walleij, Dmitry Artamonow
* Replace direct-mapped access with proper ioremap.
* Introduce irq domain for system controller irqs
* Merge "save" state to common newly create private data.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
arch/arm/mach-sa1100/irq.c | 111 +++++++++++++++++++++++++++++----------------
1 file changed, 71 insertions(+), 40 deletions(-)
diff --git a/arch/arm/mach-sa1100/irq.c b/arch/arm/mach-sa1100/irq.c
index d96f65f..a26a6a2 100644
--- a/arch/arm/mach-sa1100/irq.c
+++ b/arch/arm/mach-sa1100/irq.c
@@ -14,26 +14,50 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/irq.h>
+#include <linux/irqdomain.h>
#include <linux/ioport.h>
#include <linux/syscore_ops.h>
-#include <mach/hardware.h>
#include <asm/exception.h>
#include "generic.h"
+#define ICIP 0x00 /* IC IRQ Pending reg. */
+#define ICMR 0x04 /* IC Mask Reg. */
+#define ICLR 0x08 /* IC Level Reg. */
+#define ICCR 0x0C /* IC Control Reg. */
+#define ICFP 0x10 /* IC FIQ Pending reg. */
+#define ICPR 0x20 /* IC Pending Reg. */
+
+struct sa1100_sc {
+ struct irq_domain *domain;
+ void __iomem *regbase;
+
+ unsigned int saved_icmr;
+ unsigned int saved_iclr;
+ unsigned int saved_iccr;
+};
+
/*
* We don't need to ACK IRQs on the SA1100 unless they're GPIOs
* this is for internal IRQs i.e. from 11 to 31.
*/
static void sa1100_mask_irq(struct irq_data *d)
{
- ICMR &= ~(1 << d->irq);
+ struct sa1100_sc *sc = irq_data_get_irq_chip_data(d);
+ uint32_t icmr = readl_relaxed(sc->regbase + ICMR);
+
+ icmr &= ~BIT(d->hwirq);
+ writel_relaxed(icmr, sc->regbase + ICMR);
}
static void sa1100_unmask_irq(struct irq_data *d)
{
- ICMR |= (1 << d->irq);
+ struct sa1100_sc *sc = irq_data_get_irq_chip_data(d);
+ uint32_t icmr = readl_relaxed(sc->regbase + ICMR);
+
+ icmr |= BIT(d->hwirq);
+ writel_relaxed(icmr, sc->regbase + ICMR);
}
/*
@@ -41,7 +65,7 @@ static void sa1100_unmask_irq(struct irq_data *d)
*/
static int sa1100_set_wake(struct irq_data *d, unsigned int on)
{
- return sa11x0_sc_set_wake(d->irq, on);
+ return sa11x0_sc_set_wake(d->hwirq, on);
}
static struct irq_chip sa1100_normal_chip = {
@@ -55,28 +79,37 @@ static struct irq_chip sa1100_normal_chip = {
static struct resource irq_resource =
DEFINE_RES_MEM_NAMED(0x90050000, SZ_64K, "irqs");
-static struct sa1100irq_state {
- unsigned int saved;
- unsigned int icmr;
- unsigned int iclr;
- unsigned int iccr;
-} sa1100irq_state;
+static int sa1100_irqdomain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ struct sa1100_sc *sc = d->host_data;
+
+ irq_set_chip_data(irq, sc);
+ irq_set_chip_and_handler(irq, &sa1100_normal_chip, handle_level_irq);
+ set_irq_flags(irq, IRQF_VALID);
+
+ return 0;
+}
+
+static struct irq_domain_ops sa1100_irqdomain_ops = {
+ .map = sa1100_irqdomain_map,
+ .xlate = irq_domain_xlate_onetwocell,
+};
+
+static struct sa1100_sc state;
static int sa1100irq_suspend(void)
{
- struct sa1100irq_state *st = &sa1100irq_state;
+ struct sa1100_sc *sc = &state;
- st->saved = 1;
- st->icmr = ICMR;
- st->iclr = ICLR;
- st->iccr = ICCR;
+ sc->saved_icmr = readl_relaxed(sc->regbase + ICMR);
+ sc->saved_iclr = readl_relaxed(sc->regbase + ICLR);
+ sc->saved_iccr = readl_relaxed(sc->regbase + ICCR);
/*
* Disable all GPIO-based interrupts.
*/
- ICMR &= ~(IC_GPIO11_27|IC_GPIO10|IC_GPIO9|IC_GPIO8|IC_GPIO7|
- IC_GPIO6|IC_GPIO5|IC_GPIO4|IC_GPIO3|IC_GPIO2|
- IC_GPIO1|IC_GPIO0);
+ writel_relaxed(sc->saved_icmr & ~0xFFF, sc->regbase + ICMR);
return 0;
@@ -84,13 +117,11 @@ static int sa1100irq_suspend(void)
static void sa1100irq_resume(void)
{
- struct sa1100irq_state *st = &sa1100irq_state;
+ struct sa1100_sc *sc = &state;
- if (st->saved) {
- ICCR = st->iccr;
- ICLR = st->iclr;
- ICMR = st->icmr;
- }
+ writel_relaxed(sc->saved_iccr, sc->regbase + ICCR);
+ writel_relaxed(sc->saved_iclr, sc->regbase + ICLR);
+ writel_relaxed(sc->saved_icmr, sc->regbase + ICMR);
}
static struct syscore_ops sa1100irq_syscore_ops = {
@@ -110,42 +141,42 @@ static asmlinkage void __exception_irq_entry
sa1100_handle_irq(struct pt_regs *regs)
{
uint32_t icip, icmr, mask;
+ int irq;
- do {
- icip = (ICIP);
- icmr = (ICMR);
+ while (1) {
+ icip = readl_relaxed(state.regbase + ICIP);
+ icmr = readl_relaxed(state.regbase + ICMR);
mask = icip & icmr;
if (mask == 0)
break;
- handle_IRQ(fls(mask) - 1, regs);
- } while (1);
+ irq = fls(mask) - 1;
+ handle_IRQ(irq_find_mapping(state.domain, irq), regs);
+ }
}
void __init sa1100_init_irq(void)
{
- unsigned int irq;
-
request_resource(&iomem_resource, &irq_resource);
+ state.regbase = ioremap(irq_resource.start,
+ resource_size(&irq_resource));
+
/* disable all IRQs */
- ICMR = 0;
+ writel_relaxed(0, state.regbase + ICMR);
/* all IRQs are IRQ, not FIQ */
- ICLR = 0;
+ writel_relaxed(0, state.regbase + ICLR);
/*
* Whatever the doc says, this has to be set for the wait-on-irq
- * instruction to work... on a SA1100 rev 9 at least.
+ * instruction to work... on a SA1100 rev 9 at leastate.
*/
- ICCR = 1;
+ writel_relaxed(1, state.regbase + ICCR);
- for (irq = 0; irq <= 31; irq++) {
- irq_set_chip_and_handler(irq, &sa1100_normal_chip,
- handle_level_irq);
- set_irq_flags(irq, IRQF_VALID);
- }
+ state.domain = irq_domain_add_legacy(NULL, 32, 0, 0,
+ &sa1100_irqdomain_ops, &state);
set_handle_irq(sa1100_handle_irq);
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
` (8 preceding siblings ...)
2013-11-15 8:48 ` [PATCH 9/9] ARM: sa1100: refactor irq driver Dmitry Eremin-Solenikov
@ 2013-11-19 13:00 ` Linus Walleij
2013-11-19 15:17 ` Dmitry Eremin-Solenikov
2013-11-22 21:35 ` Dmitry Eremin-Solenikov
10 siblings, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2013-11-19 13:00 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
Russell King, Dmitry Artamonow
On Fri, Nov 15, 2013 at 9:47 AM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:
> This is a considerable rework of my previous attempt to update sa1100 irq
> handling. IRQ code is updated to support MULTI_IRQ_HANDLER and is mostly
> prepared to be converted to irqchip driver (if the need arises in future).
> I have integrated idea of Linus Waleij to use irq domains. GPIO irq handling
> is split to gpio driver, which also undergo an update/rewrite.
Overall this is looking very nice. However when I apply this on top
of (a working and booting) mainline HEAD, I get this:
sa11x0-uart.3: ttySA0 at MMIO 0x80050000 (irq = 17, base_baud =
230400) is a SA1100
Unable to handle kernel NULL pointer dereference at virtual address 0000004c
pgd = c0004000
[0000004c] *pgd=00000000
Internal error: Oops: 40cd7017 [#1] PREEMPT ARM
Modules linked in:
task: c1832000 ti: c1834000 task.ti: c1834000
PC is at gpiod_set_raw_value+0x14/0x4c
LR is at uart_add_one_port+0x2c4/0x3e8
pc : [<c017b7ec>] lr : [<c01ac7d8>] psr: a0000093
sp : c1835db8 ip : 00000001 fp : c0359df8
r10: c1862e20 r9 : a0000013 r8 : c1834008
r7 : c1926078 r6 : c0659f2c r5 : 00000001 r4 : c0656438
r3 : 00000000 r2 : 80050000 r1 : 00000001 r0 : c0656438
Flags: NzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: c0cd717f Table: c0cd717f DAC: 00000017
Process swapper (pid: 1, stack limit = 0xc18341c0)
Stack: (0xc1835db8 to 0xc1836000)
5da0: 80050000
c1926000
5dc0: c06415fc c01ac7d8 00000000 c1835df0 00000011 00038400 c0351738
c0cf35e0
5de0: 00038400 00000011 00000000 c0352e2c 4f494d4d 38783020 30353030
00303030
5e00: c0cf3540 c0c0c400 c0c0c400 c0c0c540 c1863cc0 c00f973c 00000000
c0c0c540
5e20: c1863cc0 c00fa14c c0c5dea0 c00f989c ff0a0004 c0659f2c c0635d20
c0635d54
5e40: c06415b8 c03af454 c03b5280 00000000 c1834030 c01ae8d8 c01b68b4
c0635d20
5e60: c06415b8 c01b68cc c01b68b4 c0635d20 c065a550 c01b4fcc 00000007
c0635d20
5e80: c06415b8 c0635d54 00000000 c01b524c 00000000 c01b51b4 c06415b8
c01b36cc
5ea0: c1807f4c c1874a50 c06415b8 c0c5dea0 c0641c90 c01b4884 c033c704
c06415b8
5ec0: 00000006 c06415b8 00000006 c064a320 c064a320 c01b58bc 00000000
00000000
5ee0: 00000006 c03af48c c03ba968 c0008738 c033eb60 c1805f80 c0652094
c18584e0
5f00: c02c1000 0000001f 00000000 00000000 00000000 c00efdd8 c1835f40
c1858480
5f20: c0002178 c02cd440 00000072 c003420c c0383568 00000006 c0002184
00000006
5f40: 00000000 c03ba968 00000006 c064a320 c064a320 c039c47c c03b5280
00000072
5f60: c03b5274 c039cbc8 00000006 00000006 c039c47c e64cf708 e9a321ec
c15a1bcf
5f80: 63932b15 00000000 c02b759c 00000000 00000000 00000000 00000000
00000000
5fa0: 00000000 c02b75a4 00000000 c000e9f0 00000000 00000000 00000000
00000000
5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
5fe0: 00000000 00000000 00000000 00000000 00000013 00000000 7fc33b8e
cc8612cc
[<c017b7ec>] (gpiod_set_raw_value+0x14/0x4c) from [<c01ac7d8>]
(uart_add_one_port+0x2c4/0x3e8)
[<c01ac7d8>] (uart_add_one_port+0x2c4/0x3e8) from [<c01ae8d8>]
(sa1100_serial_probe+0x98/0xc4)
[<c01ae8d8>] (sa1100_serial_probe+0x98/0xc4) from [<c01b68cc>]
(platform_drv_probe+0x18/0x4c)
[<c01b68cc>] (platform_drv_probe+0x18/0x4c) from [<c01b4fcc>]
(really_probe+0x6c/0x1f8)
[<c01b4fcc>] (really_probe+0x6c/0x1f8) from [<c01b524c>]
(__driver_attach+0x98/0x9c)
[<c01b524c>] (__driver_attach+0x98/0x9c) from [<c01b36cc>]
(bus_for_each_dev+0x68/0x98)
[<c01b36cc>] (bus_for_each_dev+0x68/0x98) from [<c01b4884>]
(bus_add_driver+0x13c/0x1e8)
[<c01b4884>] (bus_add_driver+0x13c/0x1e8) from [<c01b58bc>]
(driver_register+0x78/0xf8)
[<c01b58bc>] (driver_register+0x78/0xf8) from [<c03af48c>]
(sa1100_serial_init+0x38/0x58)
[<c03af48c>] (sa1100_serial_init+0x38/0x58) from [<c0008738>]
(do_one_initcall+0x104/0x16c)
[<c0008738>] (do_one_initcall+0x104/0x16c) from [<c039cbc8>]
(kernel_init_freeable+0xf4/0x1b0)
[<c039cbc8>] (kernel_init_freeable+0xf4/0x1b0) from [<c02b75a4>]
(kernel_init+0x8/0x118)
[<c02b75a4>] (kernel_init+0x8/0x118) from [<c000e9f0>]
(ret_from_fork+0x14/0x24)
Code: e2504000 0a000008 e5943000 e1a05001 (e5d3304c)
---[ end trace 42b376e5a40c702b ]---
note: swapper[1] exited with preempt_count 1
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
This is caused by the patch:
"ARM: sa1100: convert gpio driver to be a proper platform driver"
The reason: UARTs are initialized *very* early and the UART quirk used on the
h3600 is using GPIOs through the .set_mctrl hooks back into
arch/arm/mach-sa1100/h3xxx.c functions
h3xxx_uart_set_mctrl()
h3xxx_uart_get_mctrl()
And that happens before the GPIO driver gets registered -> crash.
This is why the device is registered from the machine in the first place
I think.
I don't know what the proper solution is, but I think you can keep most
of the platform device conversion if you also keep the special initialization
call.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-19 13:00 ` [PATCH 0/9] ARM: sa1100: Rework IRQ handling Linus Walleij
@ 2013-11-19 15:17 ` Dmitry Eremin-Solenikov
2013-11-19 20:24 ` Linus Walleij
0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-19 15:17 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
Russell King, Dmitry Artamonow
On 11/19/2013 05:00 PM, Linus Walleij wrote:
> On Fri, Nov 15, 2013 at 9:47 AM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
>
>> This is a considerable rework of my previous attempt to update sa1100 irq
>> handling. IRQ code is updated to support MULTI_IRQ_HANDLER and is mostly
>> prepared to be converted to irqchip driver (if the need arises in future).
>> I have integrated idea of Linus Waleij to use irq domains. GPIO irq handling
>> is split to gpio driver, which also undergo an update/rewrite.
>
> Overall this is looking very nice. However when I apply this on top
> of (a working and booting) mainline HEAD, I get this:
[skipped]
> This is caused by the patch:
> "ARM: sa1100: convert gpio driver to be a proper platform driver"
>
> The reason: UARTs are initialized *very* early and the UART quirk used on the
> h3600 is using GPIOs through the .set_mctrl hooks back into
> arch/arm/mach-sa1100/h3xxx.c functions
> h3xxx_uart_set_mctrl()
> h3xxx_uart_get_mctrl()
>
> And that happens before the GPIO driver gets registered -> crash.
That is not the issue. The real issue is h3xxx using those gpio's
without previously calling gpio_request. Unfortunately sa1100 driver
doesn't have a good place to request gpios. When faced this problem
during locomo refactoring, I ended up with the following piece of code:
========== Cut here ============
static struct gpio collie_uart_gpio[] = {
{ COLLIE_GPIO_CTS, GPIOF_IN, "CTS" },
{ COLLIE_GPIO_RTS, GPIOF_OUT_INIT_LOW, "RTS" },
{ COLLIE_GPIO_DTR, GPIOF_OUT_INIT_LOW, "DTR" },
{ COLLIE_GPIO_DSR, GPIOF_IN, "DSR" },
};
static bool collie_uart_gpio_ok;
static void collie_uart_set_mctrl(struct uart_port *port, u_int mctrl)
{
if (!collie_uart_gpio_ok) {
int rc = gpio_request_array(collie_uart_gpio,
ARRAY_SIZE(collie_uart_gpio));
if (rc)
printk("collie_uart_set_mctrl: gpio request
%d\n", rc);
else
collie_uart_gpio_ok = true;
}
if (collie_uart_gpio_ok) {
gpio_set_value(COLLIE_GPIO_RTS, !(mctrl & TIOCM_RTS));
gpio_set_value(COLLIE_GPIO_DTR, !(mctrl & TIOCM_DTR));
}
}
=========== Cut here ===============
Same goes for get_mctr().
Russell, Linus, what do you think about the previous solution?
Another solution would be to make sa1100 distinguish between console
output (where it doesn't have to control rts/dtr) and normal uart work
(where 'modem' lines should be used).
I have the feeling that sa11x0-serial driver would need to be somewhat
modified at least. I understand the conception behind unified
"port_fns", but I think that those should be split to a per-port
platform data.
> This is why the device is registered from the machine in the first place
> I think.
>
> I don't know what the proper solution is, but I think you can keep most
> of the platform device conversion if you also keep the special initialization
> call.
That would be possible solution if we can't come up with usefull way to
handle h3xxx (and collie) gpios.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-19 15:17 ` Dmitry Eremin-Solenikov
@ 2013-11-19 20:24 ` Linus Walleij
2013-11-20 0:20 ` Russell King - ARM Linux
` (2 more replies)
0 siblings, 3 replies; 28+ messages in thread
From: Linus Walleij @ 2013-11-19 20:24 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
Russell King, Dmitry Artamonow
On Tue, Nov 19, 2013 at 4:17 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:
> On 11/19/2013 05:00 PM, Linus Walleij wrote:
>> And that happens before the GPIO driver gets registered -> crash.
>
> That is not the issue. The real issue is h3xxx using those gpio's without
> previously calling gpio_request.
Really? But that wasn't done before this patch either.
It is basically OK to use the gpio_* functions before or
without requesting the GPIOs, it won't look nice but it
works.
> Unfortunately sa1100 driver doesn't have a
> good place to request gpios. When faced this problem during locomo
> refactoring, I ended up with the following piece of code:
(...)
> Russell, Linus, what do you think about the previous solution?
That looks OK but is that really the problem?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-19 20:24 ` Linus Walleij
@ 2013-11-20 0:20 ` Russell King - ARM Linux
2013-11-20 0:45 ` Dmitry Eremin-Solenikov
2013-11-20 0:40 ` Dmitry Eremin-Solenikov
2013-11-22 17:33 ` Dmitry Eremin-Solenikov
2 siblings, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2013-11-20 0:20 UTC (permalink / raw)
To: Linus Walleij
Cc: Dmitry Eremin-Solenikov, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, Dmitry Artamonow
On Tue, Nov 19, 2013 at 09:24:31PM +0100, Linus Walleij wrote:
> On Tue, Nov 19, 2013 at 4:17 PM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
> > On 11/19/2013 05:00 PM, Linus Walleij wrote:
>
> >> And that happens before the GPIO driver gets registered -> crash.
> >
> > That is not the issue. The real issue is h3xxx using those gpio's without
> > previously calling gpio_request.
>
> Really? But that wasn't done before this patch either.
>
> It is basically OK to use the gpio_* functions before or
> without requesting the GPIOs, it won't look nice but it
> works.
Using gpio_* functions without first having the GPIO requested causes
a complaint at boot time... so it's really undesirable to do this.
Also note that we _do_ have sa11x0 platforms which request their GPIOs
in their arch_initcall callback, so the sa11x0 GPIO driver better be
around at that point otherwise things *will* fail.
I suspect that means the SA11x0 GPIO driver must be initialised early.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-20 0:20 ` Russell King - ARM Linux
@ 2013-11-20 0:45 ` Dmitry Eremin-Solenikov
2013-11-20 7:43 ` Dmitry Artamonow
2013-11-22 17:58 ` Russell King - ARM Linux
0 siblings, 2 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-20 0:45 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Linus Walleij, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, Dmitry Artamonow
On Wed, Nov 20, 2013 at 4:20 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Also note that we _do_ have sa11x0 platforms which request their GPIOs
> in their arch_initcall callback, so the sa11x0 GPIO driver better be
> around at that point otherwise things *will* fail.
Which ones if you have them in your mind?
> I suspect that means the SA11x0 GPIO driver must be initialised early.
Anyway in these patches gpio-sa1100 driver is registered at postcore
initcall, so gpios should
be available before arch_initcall functions.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-20 0:45 ` Dmitry Eremin-Solenikov
@ 2013-11-20 7:43 ` Dmitry Artamonow
2013-11-22 17:58 ` Russell King - ARM Linux
1 sibling, 0 replies; 28+ messages in thread
From: Dmitry Artamonow @ 2013-11-20 7:43 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov, Russell King - ARM Linux
Cc: linux-gpio@vger.kernel.org, Linus Walleij,
linux-arm-kernel@lists.infradead.org, Dmitry Artamonow
20.11.2013 04:45, Dmitry Eremin-Solenikov пишет:
> On Wed, Nov 20, 2013 at 4:20 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> Also note that we _do_ have sa11x0 platforms which request their GPIOs
>> in their arch_initcall callback, so the sa11x0 GPIO driver better be
>> around at that point otherwise things *will* fail.
> Which ones if you have them in your mind?
FYI, h3100 and h3600 do exactly this - they request UART gpios at
arch_initcall time (see h3600_mach_init() in arch/arm/mach-sa1100/h3600.c)
---
Best regards,
Dmitry Artamonow
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-20 0:45 ` Dmitry Eremin-Solenikov
2013-11-20 7:43 ` Dmitry Artamonow
@ 2013-11-22 17:58 ` Russell King - ARM Linux
2013-11-22 19:12 ` Dmitry Eremin-Solenikov
1 sibling, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2013-11-22 17:58 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Linus Walleij, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, Dmitry Artamonow
On Wed, Nov 20, 2013 at 04:45:40AM +0400, Dmitry Eremin-Solenikov wrote:
> On Wed, Nov 20, 2013 at 4:20 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > Also note that we _do_ have sa11x0 platforms which request their GPIOs
> > in their arch_initcall callback, so the sa11x0 GPIO driver better be
> > around at that point otherwise things *will* fail.
>
> Which ones if you have them in your mind?
>
> > I suspect that means the SA11x0 GPIO driver must be initialised early.
>
> Anyway in these patches gpio-sa1100 driver is registered at postcore
> initcall, so gpios should
> be available before arch_initcall functions.
No it won't - quite simply because the platforms get called before
sa1100_init() is run - they have to be because the platforms get to
customise the platform data passed to the SA11x0 devices, which has
to happen before sa1100_init() registers the devices.
By adding the GPIO device to the sa11x0_devices list, you're registering
it afterwards which means that the GPIO driver isn't up and running
at this time. That means that various platforms (such as the h3xxx
ones which call gpio_request on built-in GPIOs at this time) will fail.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-22 17:58 ` Russell King - ARM Linux
@ 2013-11-22 19:12 ` Dmitry Eremin-Solenikov
2013-11-22 19:51 ` Russell King - ARM Linux
0 siblings, 1 reply; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-22 19:12 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Linus Walleij, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, Dmitry Artamonow
On Fri, Nov 22, 2013 at 9:58 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Nov 20, 2013 at 04:45:40AM +0400, Dmitry Eremin-Solenikov wrote:
>> On Wed, Nov 20, 2013 at 4:20 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > Also note that we _do_ have sa11x0 platforms which request their GPIOs
>> > in their arch_initcall callback, so the sa11x0 GPIO driver better be
>> > around at that point otherwise things *will* fail.
>>
>> Which ones if you have them in your mind?
>>
>> > I suspect that means the SA11x0 GPIO driver must be initialised early.
>>
>> Anyway in these patches gpio-sa1100 driver is registered at postcore
>> initcall, so gpios should
>> be available before arch_initcall functions.
>
> No it won't - quite simply because the platforms get called before
> sa1100_init() is run - they have to be because the platforms get to
> customise the platform data passed to the SA11x0 devices, which has
> to happen before sa1100_init() registers the devices.
They both belong to arch_initicall level, aren't they? So ordering is guarded
only by having customize_machine(which calls init_machine) from arch/arm/kernel/
before sa1100_init from arch/arm/mach-sa1100/?
> By adding the GPIO device to the sa11x0_devices list, you're registering
> it afterwards which means that the GPIO driver isn't up and running
> at this time. That means that various platforms (such as the h3xxx
> ones which call gpio_request on built-in GPIOs at this time) will fail.
For sa1100 there are only 2.5 platforms which use gpiolib - collie and h3xxx.
(The half is for simpad, which registers gpio chip from platform code, but
doesn't use it otherwise -- in platform code, of course).
Collie is changed not to call gpio_to_irq at init_machine time.
Separate patchset
reworks h3xxx (I missed them first time) to call gpio_* functions later.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-22 19:12 ` Dmitry Eremin-Solenikov
@ 2013-11-22 19:51 ` Russell King - ARM Linux
2013-11-22 21:23 ` Dmitry Eremin-Solenikov
0 siblings, 1 reply; 28+ messages in thread
From: Russell King - ARM Linux @ 2013-11-22 19:51 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Linus Walleij, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, Dmitry Artamonow
On Fri, Nov 22, 2013 at 11:12:05PM +0400, Dmitry Eremin-Solenikov wrote:
> On Fri, Nov 22, 2013 at 9:58 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Wed, Nov 20, 2013 at 04:45:40AM +0400, Dmitry Eremin-Solenikov wrote:
> >> On Wed, Nov 20, 2013 at 4:20 AM, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > Also note that we _do_ have sa11x0 platforms which request their GPIOs
> >> > in their arch_initcall callback, so the sa11x0 GPIO driver better be
> >> > around at that point otherwise things *will* fail.
> >>
> >> Which ones if you have them in your mind?
> >>
> >> > I suspect that means the SA11x0 GPIO driver must be initialised early.
> >>
> >> Anyway in these patches gpio-sa1100 driver is registered at postcore
> >> initcall, so gpios should
> >> be available before arch_initcall functions.
> >
> > No it won't - quite simply because the platforms get called before
> > sa1100_init() is run - they have to be because the platforms get to
> > customise the platform data passed to the SA11x0 devices, which has
> > to happen before sa1100_init() registers the devices.
>
> They both belong to arch_initicall level, aren't they? So ordering is
> guarded only by having customize_machine(which calls init_machine)
> from arch/arm/kernel/ before sa1100_init from arch/arm/mach-sa1100/?
Here:
c05b5194 T __initcall3_start
c05b5194 t __initcall_gate_vma_init3
c05b5198 t __initcall_customize_machine3
c05b519c t __initcall_exceptions_init3
c05b51a0 t __initcall_sa1100_init3
c05b51a4 t __initcall_dma_bus_init3
c05b51a8 t __initcall_dma_channel_table_init3
c05b51ac t __initcall_sa1110_clk_init3
* There we can see customize_machine() is called before sa1100_init().
* customize_machine() calls the machine specific .init_machine function.
* Several platforms call gpio_request() from their .init_machine function
for SA11x0 GPIOs.
* You're adding the SA11x0 GPIO device in sa1100_init().
Therefore, those platforms which call gpio_request() will fail.
What is soo difficult to understand about this?
> For sa1100 there are only 2.5 platforms which use gpiolib - collie and h3xxx.
> (The half is for simpad, which registers gpio chip from platform code, but
> doesn't use it otherwise -- in platform code, of course).
Oh well, I guess it doesn't matter then, if we're only breaking 2.5
platforms.
No. It does matter. This needs fixing.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-22 19:51 ` Russell King - ARM Linux
@ 2013-11-22 21:23 ` Dmitry Eremin-Solenikov
0 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-22 21:23 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Linus Walleij, linux-arm-kernel@lists.infradead.org,
linux-gpio@vger.kernel.org, Dmitry Artamonow
On Fri, Nov 22, 2013 at 11:51 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Nov 22, 2013 at 11:12:05PM +0400, Dmitry Eremin-Solenikov wrote:
>> For sa1100 there are only 2.5 platforms which use gpiolib - collie and h3xxx.
>> (The half is for simpad, which registers gpio chip from platform code, but
>> doesn't use it otherwise -- in platform code, of course).
>
> Oh well, I guess it doesn't matter then, if we're only breaking 2.5
> platforms.
>
> No. It does matter. This needs fixing.
Collie was updated during this serie. Simpad was not broken.
And h3xxx was updated in a separate patchset (I would like to
hear yours and Linuses opinion about it anyway).
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-19 20:24 ` Linus Walleij
2013-11-20 0:20 ` Russell King - ARM Linux
@ 2013-11-20 0:40 ` Dmitry Eremin-Solenikov
2013-11-22 17:33 ` Dmitry Eremin-Solenikov
2 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-20 0:40 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
Russell King, Dmitry Artamonow
On Wed, Nov 20, 2013 at 12:24 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Tue, Nov 19, 2013 at 4:17 PM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
>> On 11/19/2013 05:00 PM, Linus Walleij wrote:
>
>>> And that happens before the GPIO driver gets registered -> crash.
>>
>> That is not the issue. The real issue is h3xxx using those gpio's without
>> previously calling gpio_request.
>
> Really? But that wasn't done before this patch either.
I checked 4 different configurations.
3.12 + my patches, GPIO_SA1100 = y => ok
3.12 + my patches, GPIO_SA1100 = n => ok (few error messages, but no oops)
HEAD + my patches, GPIO_SA1100 = y => ok
HEAD + my patches, GPIO_SA1100 = n => Oops
It looks like something in v3.12..HEAD has increased requirements on the
gpio_set_value handling.
>
> It is basically OK to use the gpio_* functions before or
> without requesting the GPIOs, it won't look nice but it
> works.
Quoting comments from gpiolib.c: it is
'...legal but ill-advised when setting direction, and otherwise illegal.'
Indeed gpiod_direction_output checks for (!desc || !desc->chip).
gpiod_set_raw_value only checks for (!desc). So:
1) We can add checks to gpio_{g,s}et_value().
2) We can change h3xxx code to call gpio_direction_output instead
of gpio_set_value and add a call to gpio_direction_input before
gpio_get_value.
3) Add request-check code as I showed before
4) Make GPIO_SA1100 always-enabled on StrongARM
>> Unfortunately sa1100 driver doesn't have a
>> good place to request gpios. When faced this problem during locomo
>> refactoring, I ended up with the following piece of code:
> (...)
>> Russell, Linus, what do you think about the previous solution?
>
> That looks OK but is that really the problem?
Yes and no. Trigger is GPIO_SA1100=n. Problem In my opinion is
absense of gpio_request before.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-19 20:24 ` Linus Walleij
2013-11-20 0:20 ` Russell King - ARM Linux
2013-11-20 0:40 ` Dmitry Eremin-Solenikov
@ 2013-11-22 17:33 ` Dmitry Eremin-Solenikov
2 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-22 17:33 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
Russell King, Dmitry Artamonow
On 11/20/2013 12:24 AM, Linus Walleij wrote:
> On Tue, Nov 19, 2013 at 4:17 PM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
>> On 11/19/2013 05:00 PM, Linus Walleij wrote:
>
>>> And that happens before the GPIO driver gets registered -> crash.
>>
>> That is not the issue. The real issue is h3xxx using those gpio's without
>> previously calling gpio_request.
>
> Really? But that wasn't done before this patch either.
>
> It is basically OK to use the gpio_* functions before or
> without requesting the GPIOs, it won't look nice but it
> works.
>
>> Unfortunately sa1100 driver doesn't have a
>> good place to request gpios. When faced this problem during locomo
>> refactoring, I ended up with the following piece of code:
> (...)
>> Russell, Linus, what do you think about the previous solution?
>
> That looks OK but is that really the problem?
I have reworked h3xxx GPIO handling to (hopefully) not to cause the
problem. I have sent the patches yesterday.
Linus, Could you please check if both series work (and make sense to you)?
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/9] ARM: sa1100: Rework IRQ handling
2013-11-15 8:47 [PATCH 0/9] ARM: sa1100: Rework IRQ handling Dmitry Eremin-Solenikov
` (9 preceding siblings ...)
2013-11-19 13:00 ` [PATCH 0/9] ARM: sa1100: Rework IRQ handling Linus Walleij
@ 2013-11-22 21:35 ` Dmitry Eremin-Solenikov
10 siblings, 0 replies; 28+ messages in thread
From: Dmitry Eremin-Solenikov @ 2013-11-22 21:35 UTC (permalink / raw)
To: linux-arm-kernel, linux-gpio@vger.kernel.org
Cc: Russell King, Linus Walleij, Dmitry Artamonow
On Fri, Nov 15, 2013 at 12:47 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:
> Dmitry Eremin-Solenikov (9):
> ARM: sa1100 collie: use gpio-charger instead of pda-power
> ARM: locomo: don't clobber chip data for chained irq
Russell, could you please reivew and ack these first two patches.
I would like to get them independently of the fate of the rest of this
patch serie.
> ARM: sa1100: switch to MULTI_IRQ_HANDLER
> ARM: sa1100: convert gpio driver to be a proper platform driver
> ARM: sa1100: add platform functions to handle PWER settings
> ARM: sa1100: enable IRQ domains
> ARM: sa1100: move gpio irq handling to GPIO driver
> ARM: sa1100: move per-IRQ PWER settings to core code
> ARM: sa1100: refactor irq driver
>
> arch/arm/Kconfig | 2 +
> arch/arm/common/locomo.c | 4 +-
> arch/arm/mach-sa1100/collie.c | 55 ++-------------
> arch/arm/mach-sa1100/generic.c | 46 +++++++++++-
> arch/arm/mach-sa1100/generic.h | 3 +-
> arch/arm/mach-sa1100/include/mach/SA-1100.h | 2 +
> arch/arm/mach-sa1100/include/mach/entry-macro.S | 41 -----------
> arch/arm/mach-sa1100/include/mach/irqs.h | 73 +++++++++++--------
> arch/arm/mach-sa1100/irq.c | 322 +++++++++++++++++++++--------------------------------------------------------------
> drivers/gpio/Kconfig | 6 ++
> drivers/gpio/Makefile | 2 +-
> drivers/gpio/gpio-sa1100.c | 332 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
> 12 files changed, 500 insertions(+), 388 deletions(-)
> delete mode 100644 arch/arm/mach-sa1100/include/mach/entry-macro.S
>
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 28+ messages in thread