* [PATCH 2/2] pinctrl: sirf: switch driver to use gpiolib irqchip helpers
@ 2014-04-08 12:27 Linus Walleij
2014-04-10 3:48 ` Barry Song
0 siblings, 1 reply; 4+ messages in thread
From: Linus Walleij @ 2014-04-08 12:27 UTC (permalink / raw)
To: linux-kernel, Barry Song, Barry Song; +Cc: linux-gpio, Linus Walleij
This switches the Sirf pinctrl driver over to using the gpiolib
irqchip helpers simplifying some of the code.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
This really needs testing on real hardware before I dare to
merge it, but the driver seems simple and straight-forward to
convert.
---
drivers/pinctrl/Kconfig | 1 +
drivers/pinctrl/sirf/pinctrl-sirf.c | 103 +++++++++---------------------------
2 files changed, 27 insertions(+), 77 deletions(-)
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index e49324032611..ddd4e6eae3c1 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -270,6 +270,7 @@ config PINCTRL_SIRF
bool "CSR SiRFprimaII/SiRFmarco pin controller driver"
depends on ARCH_SIRF
select PINMUX
+ select GPIOLIB_IRQCHIP
config PINCTRL_SUNXI
bool
diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
index 2c3eb207ff87..99dbe61f5fd1 100644
--- a/drivers/pinctrl/sirf/pinctrl-sirf.c
+++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
@@ -9,13 +9,10 @@
#include <linux/init.h>
#include <linux/module.h>
-#include <linux/irq.h>
#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/slab.h>
#include <linux/err.h>
-#include <linux/irqdomain.h>
-#include <linux/irqchip/chained_irq.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
#include <linux/pinctrl/consumer.h>
@@ -27,7 +24,6 @@
#include <linux/bitops.h>
#include <linux/gpio.h>
#include <linux/of_gpio.h>
-#include <asm/mach/irq.h>
#include "pinctrl-sirf.h"
@@ -35,7 +31,6 @@
struct sirfsoc_gpio_bank {
struct of_mm_gpio_chip chip;
- struct irq_domain *domain;
int id;
int parent_irq;
spinlock_t lock;
@@ -464,15 +459,6 @@ static int __init sirfsoc_pinmux_init(void)
}
arch_initcall(sirfsoc_pinmux_init);
-static inline int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
-{
- struct sirfsoc_gpio_bank *bank = container_of(to_of_mm_gpio_chip(chip),
- struct sirfsoc_gpio_bank, chip);
-
- return irq_create_mapping(bank->domain, offset + bank->id *
- SIRFSOC_GPIO_BANK_SIZE);
-}
-
static inline int sirfsoc_gpio_to_offset(unsigned int gpio)
{
return gpio % SIRFSOC_GPIO_BANK_SIZE;
@@ -490,7 +476,8 @@ static inline struct sirfsoc_gpio_bank *sirfsoc_gpiochip_to_bank(struct gpio_chi
static void sirfsoc_gpio_irq_ack(struct irq_data *d)
{
- struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
u32 val, offset;
unsigned long flags;
@@ -525,14 +512,16 @@ static void __sirfsoc_gpio_irq_mask(struct sirfsoc_gpio_bank *bank, int idx)
static void sirfsoc_gpio_irq_mask(struct irq_data *d)
{
- struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
__sirfsoc_gpio_irq_mask(bank, d->hwirq % SIRFSOC_GPIO_BANK_SIZE);
}
static void sirfsoc_gpio_irq_unmask(struct irq_data *d)
{
- struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
u32 val, offset;
unsigned long flags;
@@ -551,7 +540,8 @@ static void sirfsoc_gpio_irq_unmask(struct irq_data *d)
static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
{
- struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
u32 val, offset;
unsigned long flags;
@@ -595,39 +585,18 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
return 0;
}
-static int sirfsoc_gpio_irq_reqres(struct irq_data *d)
-{
- struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
-
- if (gpio_lock_as_irq(&bank->chip.gc, d->hwirq % SIRFSOC_GPIO_BANK_SIZE)) {
- dev_err(bank->chip.gc.dev,
- "unable to lock HW IRQ %lu for IRQ\n",
- d->hwirq);
- return -EINVAL;
- }
- return 0;
-}
-
-static void sirfsoc_gpio_irq_relres(struct irq_data *d)
-{
- struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
-
- gpio_unlock_as_irq(&bank->chip.gc, d->hwirq % SIRFSOC_GPIO_BANK_SIZE);
-}
-
static struct irq_chip sirfsoc_irq_chip = {
.name = "sirf-gpio-irq",
.irq_ack = sirfsoc_gpio_irq_ack,
.irq_mask = sirfsoc_gpio_irq_mask,
.irq_unmask = sirfsoc_gpio_irq_unmask,
.irq_set_type = sirfsoc_gpio_irq_type,
- .irq_request_resources = sirfsoc_gpio_irq_reqres,
- .irq_release_resources = sirfsoc_gpio_irq_relres,
};
static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
{
- struct sirfsoc_gpio_bank *bank = irq_get_handler_data(irq);
+ struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+ struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
u32 status, ctrl;
int idx = 0;
struct irq_chip *chip = irq_get_chip(irq);
@@ -653,7 +622,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
if ((status & 0x1) && (ctrl & SIRFSOC_GPIO_CTL_INTR_EN_MASK)) {
pr_debug("%s: gpio id %d idx %d happens\n",
__func__, bank->id, idx);
- generic_handle_irq(irq_find_mapping(bank->domain, idx +
+ generic_handle_irq(irq_find_mapping(gc->irqdomain, idx +
bank->id * SIRFSOC_GPIO_BANK_SIZE));
}
@@ -801,27 +770,6 @@ static void sirfsoc_gpio_set_value(struct gpio_chip *chip, unsigned offset,
spin_unlock_irqrestore(&bank->lock, flags);
}
-static int sirfsoc_gpio_irq_map(struct irq_domain *d, unsigned int irq,
- irq_hw_number_t hwirq)
-{
- struct sirfsoc_gpio_bank *bank = d->host_data;
-
- if (!bank)
- return -EINVAL;
-
- irq_set_chip(irq, &sirfsoc_irq_chip);
- irq_set_handler(irq, handle_level_irq);
- irq_set_chip_data(irq, bank + hwirq / SIRFSOC_GPIO_BANK_SIZE);
- set_irq_flags(irq, IRQF_VALID);
-
- return 0;
-}
-
-static const struct irq_domain_ops sirfsoc_gpio_irq_simple_ops = {
- .map = sirfsoc_gpio_irq_map,
- .xlate = irq_domain_xlate_twocell,
-};
-
static void sirfsoc_gpio_set_pullup(const u32 *pullups)
{
int i, n;
@@ -860,7 +808,6 @@ static int sirfsoc_gpio_probe(struct device_node *np)
struct sirfsoc_gpio_bank *bank;
void __iomem *regs;
struct platform_device *pdev;
- struct irq_domain *domain;
bool is_marco = false;
u32 pullups[SIRFSOC_GPIO_NO_OF_BANKS], pulldowns[SIRFSOC_GPIO_NO_OF_BANKS];
@@ -876,14 +823,6 @@ static int sirfsoc_gpio_probe(struct device_node *np)
if (of_device_is_compatible(np, "sirf,marco-pinctrl"))
is_marco = 1;
- domain = irq_domain_add_linear(np, SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS,
- &sirfsoc_gpio_irq_simple_ops, sgpio_bank);
- if (!domain) {
- pr_err("%s: Failed to create irqdomain\n", np->full_name);
- err = -ENOSYS;
- goto out;
- }
-
for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
bank = &sgpio_bank[i];
spin_lock_init(&bank->lock);
@@ -893,7 +832,6 @@ static int sirfsoc_gpio_probe(struct device_node *np)
bank->chip.gc.get = sirfsoc_gpio_get_value;
bank->chip.gc.direction_output = sirfsoc_gpio_direction_output;
bank->chip.gc.set = sirfsoc_gpio_set_value;
- bank->chip.gc.to_irq = sirfsoc_gpio_to_irq;
bank->chip.gc.base = i * SIRFSOC_GPIO_BANK_SIZE;
bank->chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE;
bank->chip.gc.label = kstrdup(np->full_name, GFP_KERNEL);
@@ -912,15 +850,26 @@ static int sirfsoc_gpio_probe(struct device_node *np)
err = gpiochip_add(&bank->chip.gc);
if (err) {
- pr_err("%s: error in probe function with status %d\n",
+ dev_err(&pdev->dev,
+ "%s: error in probe function with status %d\n",
np->full_name, err);
goto out;
}
- bank->domain = domain;
+ err = gpiochip_irqchip_add(&bank->chip.gc,
+ &sirfsoc_irq_chip,
+ 0, handle_level_irq,
+ IRQ_TYPE_NONE);
+ if (err) {
+ dev_err(&pdev->dev,
+ "could not connect irqchip to gpiochip\n");
+ goto out;
+ }
- irq_set_chained_handler(bank->parent_irq, sirfsoc_gpio_handle_irq);
- irq_set_handler_data(bank->parent_irq, bank);
+ gpiochip_set_chained_irqchip(&bank->chip.gc,
+ &sirfsoc_irq_chip,
+ bank->parent_irq,
+ sirfsoc_gpio_handle_irq);
}
if (!of_property_read_u32_array(np, "sirf,pullups", pullups,
--
1.9.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] pinctrl: sirf: switch driver to use gpiolib irqchip helpers
2014-04-08 12:27 [PATCH 2/2] pinctrl: sirf: switch driver to use gpiolib irqchip helpers Linus Walleij
@ 2014-04-10 3:48 ` Barry Song
2014-04-10 9:30 ` Barry Song
0 siblings, 1 reply; 4+ messages in thread
From: Barry Song @ 2014-04-10 3:48 UTC (permalink / raw)
To: Linus Walleij; +Cc: LKML, Barry Song, linux-gpio, DL-SHA-WorkGroupLinux
2014-04-08 20:27 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
> This switches the Sirf pinctrl driver over to using the gpiolib
> irqchip helpers simplifying some of the code.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This really needs testing on real hardware before I dare to
> merge it, but the driver seems simple and straight-forward to
> convert.
Linus, this broke the irq/gpio mapping. for example:
without this:
# cat /proc/interrupts
CPU0
16: 181 irq_sirfsoc 0 sirfsoc_timer0
...
62: 0 sirf-gpio-irq 45 extcon-gpio
...
with this:
# cat /proc/interrupts
CPU0
16: 944 irq_sirfsoc 0 sirfsoc_timer0
...
105: 0 sirf-gpio-irq 13 extcon-gpio
...
i will do a debug to find why. any idea from you?
-barry
> ---
> drivers/pinctrl/Kconfig | 1 +
> drivers/pinctrl/sirf/pinctrl-sirf.c | 103 +++++++++---------------------------
> 2 files changed, 27 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index e49324032611..ddd4e6eae3c1 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -270,6 +270,7 @@ config PINCTRL_SIRF
> bool "CSR SiRFprimaII/SiRFmarco pin controller driver"
> depends on ARCH_SIRF
> select PINMUX
> + select GPIOLIB_IRQCHIP
>
> config PINCTRL_SUNXI
> bool
> diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c
> index 2c3eb207ff87..99dbe61f5fd1 100644
> --- a/drivers/pinctrl/sirf/pinctrl-sirf.c
> +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c
> @@ -9,13 +9,10 @@
>
> #include <linux/init.h>
> #include <linux/module.h>
> -#include <linux/irq.h>
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/slab.h>
> #include <linux/err.h>
> -#include <linux/irqdomain.h>
> -#include <linux/irqchip/chained_irq.h>
> #include <linux/pinctrl/pinctrl.h>
> #include <linux/pinctrl/pinmux.h>
> #include <linux/pinctrl/consumer.h>
> @@ -27,7 +24,6 @@
> #include <linux/bitops.h>
> #include <linux/gpio.h>
> #include <linux/of_gpio.h>
> -#include <asm/mach/irq.h>
>
> #include "pinctrl-sirf.h"
>
> @@ -35,7 +31,6 @@
>
> struct sirfsoc_gpio_bank {
> struct of_mm_gpio_chip chip;
> - struct irq_domain *domain;
> int id;
> int parent_irq;
> spinlock_t lock;
> @@ -464,15 +459,6 @@ static int __init sirfsoc_pinmux_init(void)
> }
> arch_initcall(sirfsoc_pinmux_init);
>
> -static inline int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> -{
> - struct sirfsoc_gpio_bank *bank = container_of(to_of_mm_gpio_chip(chip),
> - struct sirfsoc_gpio_bank, chip);
> -
> - return irq_create_mapping(bank->domain, offset + bank->id *
> - SIRFSOC_GPIO_BANK_SIZE);
> -}
> -
> static inline int sirfsoc_gpio_to_offset(unsigned int gpio)
> {
> return gpio % SIRFSOC_GPIO_BANK_SIZE;
> @@ -490,7 +476,8 @@ static inline struct sirfsoc_gpio_bank *sirfsoc_gpiochip_to_bank(struct gpio_chi
>
> static void sirfsoc_gpio_irq_ack(struct irq_data *d)
> {
> - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
> int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
> u32 val, offset;
> unsigned long flags;
> @@ -525,14 +512,16 @@ static void __sirfsoc_gpio_irq_mask(struct sirfsoc_gpio_bank *bank, int idx)
>
> static void sirfsoc_gpio_irq_mask(struct irq_data *d)
> {
> - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
>
> __sirfsoc_gpio_irq_mask(bank, d->hwirq % SIRFSOC_GPIO_BANK_SIZE);
> }
>
> static void sirfsoc_gpio_irq_unmask(struct irq_data *d)
> {
> - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
> int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
> u32 val, offset;
> unsigned long flags;
> @@ -551,7 +540,8 @@ static void sirfsoc_gpio_irq_unmask(struct irq_data *d)
>
> static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
> {
> - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
> int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE;
> u32 val, offset;
> unsigned long flags;
> @@ -595,39 +585,18 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type)
> return 0;
> }
>
> -static int sirfsoc_gpio_irq_reqres(struct irq_data *d)
> -{
> - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> -
> - if (gpio_lock_as_irq(&bank->chip.gc, d->hwirq % SIRFSOC_GPIO_BANK_SIZE)) {
> - dev_err(bank->chip.gc.dev,
> - "unable to lock HW IRQ %lu for IRQ\n",
> - d->hwirq);
> - return -EINVAL;
> - }
> - return 0;
> -}
> -
> -static void sirfsoc_gpio_irq_relres(struct irq_data *d)
> -{
> - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> -
> - gpio_unlock_as_irq(&bank->chip.gc, d->hwirq % SIRFSOC_GPIO_BANK_SIZE);
> -}
> -
> static struct irq_chip sirfsoc_irq_chip = {
> .name = "sirf-gpio-irq",
> .irq_ack = sirfsoc_gpio_irq_ack,
> .irq_mask = sirfsoc_gpio_irq_mask,
> .irq_unmask = sirfsoc_gpio_irq_unmask,
> .irq_set_type = sirfsoc_gpio_irq_type,
> - .irq_request_resources = sirfsoc_gpio_irq_reqres,
> - .irq_release_resources = sirfsoc_gpio_irq_relres,
> };
>
> static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
> {
> - struct sirfsoc_gpio_bank *bank = irq_get_handler_data(irq);
> + struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> + struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(gc);
> u32 status, ctrl;
> int idx = 0;
> struct irq_chip *chip = irq_get_chip(irq);
> @@ -653,7 +622,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc)
> if ((status & 0x1) && (ctrl & SIRFSOC_GPIO_CTL_INTR_EN_MASK)) {
> pr_debug("%s: gpio id %d idx %d happens\n",
> __func__, bank->id, idx);
> - generic_handle_irq(irq_find_mapping(bank->domain, idx +
> + generic_handle_irq(irq_find_mapping(gc->irqdomain, idx +
> bank->id * SIRFSOC_GPIO_BANK_SIZE));
> }
>
> @@ -801,27 +770,6 @@ static void sirfsoc_gpio_set_value(struct gpio_chip *chip, unsigned offset,
> spin_unlock_irqrestore(&bank->lock, flags);
> }
>
> -static int sirfsoc_gpio_irq_map(struct irq_domain *d, unsigned int irq,
> - irq_hw_number_t hwirq)
> -{
> - struct sirfsoc_gpio_bank *bank = d->host_data;
> -
> - if (!bank)
> - return -EINVAL;
> -
> - irq_set_chip(irq, &sirfsoc_irq_chip);
> - irq_set_handler(irq, handle_level_irq);
> - irq_set_chip_data(irq, bank + hwirq / SIRFSOC_GPIO_BANK_SIZE);
> - set_irq_flags(irq, IRQF_VALID);
> -
> - return 0;
> -}
> -
> -static const struct irq_domain_ops sirfsoc_gpio_irq_simple_ops = {
> - .map = sirfsoc_gpio_irq_map,
> - .xlate = irq_domain_xlate_twocell,
> -};
> -
> static void sirfsoc_gpio_set_pullup(const u32 *pullups)
> {
> int i, n;
> @@ -860,7 +808,6 @@ static int sirfsoc_gpio_probe(struct device_node *np)
> struct sirfsoc_gpio_bank *bank;
> void __iomem *regs;
> struct platform_device *pdev;
> - struct irq_domain *domain;
> bool is_marco = false;
>
> u32 pullups[SIRFSOC_GPIO_NO_OF_BANKS], pulldowns[SIRFSOC_GPIO_NO_OF_BANKS];
> @@ -876,14 +823,6 @@ static int sirfsoc_gpio_probe(struct device_node *np)
> if (of_device_is_compatible(np, "sirf,marco-pinctrl"))
> is_marco = 1;
>
> - domain = irq_domain_add_linear(np, SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS,
> - &sirfsoc_gpio_irq_simple_ops, sgpio_bank);
> - if (!domain) {
> - pr_err("%s: Failed to create irqdomain\n", np->full_name);
> - err = -ENOSYS;
> - goto out;
> - }
> -
> for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) {
> bank = &sgpio_bank[i];
> spin_lock_init(&bank->lock);
> @@ -893,7 +832,6 @@ static int sirfsoc_gpio_probe(struct device_node *np)
> bank->chip.gc.get = sirfsoc_gpio_get_value;
> bank->chip.gc.direction_output = sirfsoc_gpio_direction_output;
> bank->chip.gc.set = sirfsoc_gpio_set_value;
> - bank->chip.gc.to_irq = sirfsoc_gpio_to_irq;
> bank->chip.gc.base = i * SIRFSOC_GPIO_BANK_SIZE;
> bank->chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE;
> bank->chip.gc.label = kstrdup(np->full_name, GFP_KERNEL);
> @@ -912,15 +850,26 @@ static int sirfsoc_gpio_probe(struct device_node *np)
>
> err = gpiochip_add(&bank->chip.gc);
> if (err) {
> - pr_err("%s: error in probe function with status %d\n",
> + dev_err(&pdev->dev,
> + "%s: error in probe function with status %d\n",
> np->full_name, err);
> goto out;
> }
>
> - bank->domain = domain;
> + err = gpiochip_irqchip_add(&bank->chip.gc,
> + &sirfsoc_irq_chip,
> + 0, handle_level_irq,
> + IRQ_TYPE_NONE);
> + if (err) {
> + dev_err(&pdev->dev,
> + "could not connect irqchip to gpiochip\n");
> + goto out;
> + }
>
> - irq_set_chained_handler(bank->parent_irq, sirfsoc_gpio_handle_irq);
> - irq_set_handler_data(bank->parent_irq, bank);
> + gpiochip_set_chained_irqchip(&bank->chip.gc,
> + &sirfsoc_irq_chip,
> + bank->parent_irq,
> + sirfsoc_gpio_handle_irq);
> }
>
> if (!of_property_read_u32_array(np, "sirf,pullups", pullups,
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] pinctrl: sirf: switch driver to use gpiolib irqchip helpers
2014-04-10 3:48 ` Barry Song
@ 2014-04-10 9:30 ` Barry Song
2014-04-10 18:32 ` Linus Walleij
0 siblings, 1 reply; 4+ messages in thread
From: Barry Song @ 2014-04-10 9:30 UTC (permalink / raw)
To: Linus Walleij; +Cc: LKML, Barry Song, linux-gpio, DL-SHA-WorkGroupLinux
2014-04-10 11:48 GMT+08:00 Barry Song <baohua@kernel.org>:
> 2014-04-08 20:27 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>:
>> This switches the Sirf pinctrl driver over to using the gpiolib
>> irqchip helpers simplifying some of the code.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> This really needs testing on real hardware before I dare to
>> merge it, but the driver seems simple and straight-forward to
>> convert.
>
> Linus, this broke the irq/gpio mapping. for example:
>
> without this:
> # cat /proc/interrupts
> CPU0
> 16: 181 irq_sirfsoc 0 sirfsoc_timer0
> ...
> 62: 0 sirf-gpio-irq 45 extcon-gpio
> ...
>
> with this:
> # cat /proc/interrupts
> CPU0
> 16: 944 irq_sirfsoc 0 sirfsoc_timer0
> ...
> 105: 0 sirf-gpio-irq 13 extcon-gpio
> ...
>
> i will do a debug to find why. any idea from you?
hi linus, after reading the source codes of GPIOLIB_IRQCHIP, i think
the new irq number after applying your patch should not be a problem.
the reason is that you create irq mapping earlier in
gpiochip_irqchip_add(), but the old codes did it later on demand in
sirfsoc_gpio_to_irq().
the real problem here is that we have several gpio banks, but there is
only one device node, so i think this should be not good and will make
the gpiochip_irq_reqres(), gpiochip_irq_relres() fail since
sub-devices can only point its interrupt parent to the only gpio node,
this makes hwirq bigger than the size of the gpio bank.
so will you wait for me to send a patch to merge all banks into one
gpio_chip just as i have done by commit
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8daeffb058f78deb0b0ef2cb67ef741c38788bf9
to merge irq_domain?
then i think your patch will work. thank you!
-barry
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] pinctrl: sirf: switch driver to use gpiolib irqchip helpers
2014-04-10 9:30 ` Barry Song
@ 2014-04-10 18:32 ` Linus Walleij
0 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2014-04-10 18:32 UTC (permalink / raw)
To: Barry Song
Cc: LKML, Barry Song, linux-gpio@vger.kernel.org,
DL-SHA-WorkGroupLinux
On Thu, Apr 10, 2014 at 11:30 AM, Barry Song <baohua@kernel.org> wrote:
>> with this:
>> # cat /proc/interrupts
>> CPU0
>> 16: 944 irq_sirfsoc 0 sirfsoc_timer0
>> ...
>> 105: 0 sirf-gpio-irq 13 extcon-gpio
>> ...
>>
>> i will do a debug to find why. any idea from you?
>
> hi linus, after reading the source codes of GPIOLIB_IRQCHIP, i think
> the new irq number after applying your patch should not be a problem.
> the reason is that you create irq mapping earlier in
> gpiochip_irqchip_add(), but the old codes did it later on demand in
> sirfsoc_gpio_to_irq().
Yeah it's just some number, it's dynamically allocated to you should
not need to worry about that. It could even change with probe order
due to unpredictable things.
> the real problem here is that we have several gpio banks, but there is
> only one device node, so i think this should be not good and will make
> the gpiochip_irq_reqres(), gpiochip_irq_relres() fail since
> sub-devices can only point its interrupt parent to the only gpio node,
> this makes hwirq bigger than the size of the gpio bank.
>
> so will you wait for me to send a patch to merge all banks into one
> gpio_chip just as i have done by commit
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8daeffb058f78deb0b0ef2cb67ef741c38788bf9
> to merge irq_domain?
Sure I'm standing by.
Bonus points if you rebase my patch on top of it and send it
as patch 2/2 :-D
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-04-10 18:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-08 12:27 [PATCH 2/2] pinctrl: sirf: switch driver to use gpiolib irqchip helpers Linus Walleij
2014-04-10 3:48 ` Barry Song
2014-04-10 9:30 ` Barry Song
2014-04-10 18:32 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).