* [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers @ 2014-04-15 6:43 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linus.walleij, linux-gpio; +Cc: linux-arm-kernel, workgroup.linux, Barry Song From: Barry Song <Baohua.Song@csr.com> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased from Linus Walleij's patch too. Barry Song (3): pinctrl: sirf: wrap all gpio banks into one gpio_chip gpio: make handler_data configurable while using gpiolib_irqchip pinctrl: sirf: move to use irq_get_handler_data Linus Walleij (1): pinctrl: sirf: switch driver to use gpiolib irqchip helpers drivers/gpio/gpiolib.c | 10 +- drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/sirf/pinctrl-sirf.c | 264 ++++++++++++----------------------- include/linux/gpio/driver.h | 2 +- 4 files changed, 97 insertions(+), 180 deletions(-) -- 1.7.5.4 ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers @ 2014-04-15 6:43 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linux-arm-kernel From: Barry Song <Baohua.Song@csr.com> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased from Linus Walleij's patch too. Barry Song (3): pinctrl: sirf: wrap all gpio banks into one gpio_chip gpio: make handler_data configurable while using gpiolib_irqchip pinctrl: sirf: move to use irq_get_handler_data Linus Walleij (1): pinctrl: sirf: switch driver to use gpiolib irqchip helpers drivers/gpio/gpiolib.c | 10 +- drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/sirf/pinctrl-sirf.c | 264 ++++++++++++----------------------- include/linux/gpio/driver.h | 2 +- 4 files changed, 97 insertions(+), 180 deletions(-) -- 1.7.5.4 ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip 2014-04-15 6:43 ` Barry Song @ 2014-04-15 6:43 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linus.walleij, linux-gpio; +Cc: linux-arm-kernel, workgroup.linux, Barry Song From: Barry Song <Baohua.Song@csr.com> all gpio banks are in one chip, that makes software clean in mapping irq and gpio. Signed-off-by: Barry Song <Baohua.Song@csr.com> --- drivers/pinctrl/sirf/pinctrl-sirf.c | 217 +++++++++++++++-------------------- 1 files changed, 91 insertions(+), 126 deletions(-) diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c index 2c3eb20..cb81b15 100644 --- a/drivers/pinctrl/sirf/pinctrl-sirf.c +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c @@ -34,15 +34,19 @@ #define DRIVER_NAME "pinmux-sirf" struct sirfsoc_gpio_bank { - struct of_mm_gpio_chip chip; - struct irq_domain *domain; int id; int parent_irq; spinlock_t lock; +}; + +struct sirfsoc_gpio_chip { + struct of_mm_gpio_chip chip; + struct irq_domain *domain; bool is_marco; /* for marco, some registers are different with prima2 */ + struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS]; }; -static struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS]; +static struct sirfsoc_gpio_chip sgpio_chip; static DEFINE_SPINLOCK(sgpio_lock); static struct sirfsoc_pin_group *sirfsoc_pin_groups; @@ -258,32 +262,12 @@ static struct pinctrl_desc sirfsoc_pinmux_desc = { /* * Todo: bind irq_chip to every pinctrl_gpio_range */ -static struct pinctrl_gpio_range sirfsoc_gpio_ranges[] = { - { - .name = "sirfsoc-gpio*", - .id = 0, - .base = 0, - .pin_base = 0, - .npins = 32, - }, { - .name = "sirfsoc-gpio*", - .id = 1, - .base = 32, - .pin_base = 32, - .npins = 32, - }, { - .name = "sirfsoc-gpio*", - .id = 2, - .base = 64, - .pin_base = 64, - .npins = 32, - }, { - .name = "sirfsoc-gpio*", - .id = 3, - .base = 96, - .pin_base = 96, - .npins = 19, - }, +static struct pinctrl_gpio_range sirfsoc_gpio_ranges = { + .name = "sirfsoc-gpio*", + .id = 0, + .base = 0, + .pin_base = 0, + .npins = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS, }; static void __iomem *sirfsoc_rsc_of_iomap(void) @@ -303,19 +287,19 @@ static void __iomem *sirfsoc_rsc_of_iomap(void) } static int sirfsoc_gpio_of_xlate(struct gpio_chip *gc, - const struct of_phandle_args *gpiospec, - u32 *flags) + const struct of_phandle_args *gpiospec, + u32 *flags) { - if (gpiospec->args[0] > SIRFSOC_GPIO_NO_OF_BANKS * SIRFSOC_GPIO_BANK_SIZE) + if (gpiospec->args[0] > SIRFSOC_GPIO_NO_OF_BANKS * SIRFSOC_GPIO_BANK_SIZE) return -EINVAL; - if (gc != &sgpio_bank[gpiospec->args[0] / SIRFSOC_GPIO_BANK_SIZE].chip.gc) + if (gc != &sgpio_chip.chip.gc) return -EINVAL; - if (flags) + if (flags) *flags = gpiospec->args[1]; - return gpiospec->args[0] % SIRFSOC_GPIO_BANK_SIZE; + return gpiospec->args[0]; } static const struct of_device_id pinmux_ids[] = { @@ -331,7 +315,6 @@ static int sirfsoc_pinmux_probe(struct platform_device *pdev) struct sirfsoc_pmx *spmx; struct device_node *np = pdev->dev.of_node; const struct sirfsoc_pinctrl_data *pdata; - int i; /* Create state holders etc for this driver */ spmx = devm_kzalloc(&pdev->dev, sizeof(*spmx), GFP_KERNEL); @@ -375,10 +358,8 @@ static int sirfsoc_pinmux_probe(struct platform_device *pdev) goto out_no_pmx; } - for (i = 0; i < ARRAY_SIZE(sirfsoc_gpio_ranges); i++) { - sirfsoc_gpio_ranges[i].gc = &sgpio_bank[i].chip.gc; - pinctrl_add_gpio_range(spmx->pmx, &sirfsoc_gpio_ranges[i]); - } + sirfsoc_gpio_ranges.gc = &sgpio_chip.chip.gc; + pinctrl_add_gpio_range(spmx->pmx, &sirfsoc_gpio_ranges); dev_info(&pdev->dev, "initialized SIRFSOC pinmux driver\n"); @@ -464,33 +445,23 @@ 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) +static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio) { - 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); + return &sgpio_chip.sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE]; } -static inline int sirfsoc_gpio_to_offset(unsigned int gpio) +static int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset) { - return gpio % SIRFSOC_GPIO_BANK_SIZE; + return irq_create_mapping(sgpio_chip.domain, offset); } -static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio) +static inline int sirfsoc_gpio_to_bankoff(unsigned int gpio) { - return &sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE]; -} - -static inline struct sirfsoc_gpio_bank *sirfsoc_gpiochip_to_bank(struct gpio_chip *chip) -{ - return container_of(to_of_mm_gpio_chip(chip), struct sirfsoc_gpio_bank, chip); + return gpio % SIRFSOC_GPIO_BANK_SIZE; } - static void sirfsoc_gpio_irq_ack(struct irq_data *d) { - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE; u32 val, offset; unsigned long flags; @@ -499,9 +470,9 @@ static void sirfsoc_gpio_irq_ack(struct irq_data *d) spin_lock_irqsave(&sgpio_lock, flags); - val = readl(bank->chip.regs + offset); + val = readl(sgpio_chip.chip.regs + offset); - writel(val, bank->chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&sgpio_lock, flags); } @@ -515,24 +486,24 @@ static void __sirfsoc_gpio_irq_mask(struct sirfsoc_gpio_bank *bank, int idx) spin_lock_irqsave(&sgpio_lock, flags); - val = readl(bank->chip.regs + offset); + val = readl(sgpio_chip.chip.regs + offset); val &= ~SIRFSOC_GPIO_CTL_INTR_EN_MASK; val &= ~SIRFSOC_GPIO_CTL_INTR_STS_MASK; - writel(val, bank->chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&sgpio_lock, flags); } static void sirfsoc_gpio_irq_mask(struct irq_data *d) { - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); __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 sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE; u32 val, offset; unsigned long flags; @@ -541,17 +512,17 @@ static void sirfsoc_gpio_irq_unmask(struct irq_data *d) spin_lock_irqsave(&sgpio_lock, flags); - val = readl(bank->chip.regs + offset); + val = readl(sgpio_chip.chip.regs + offset); val &= ~SIRFSOC_GPIO_CTL_INTR_STS_MASK; val |= SIRFSOC_GPIO_CTL_INTR_EN_MASK; - writel(val, bank->chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&sgpio_lock, flags); } 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 sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE; u32 val, offset; unsigned long flags; @@ -560,7 +531,7 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) spin_lock_irqsave(&sgpio_lock, flags); - val = readl(bank->chip.regs + offset); + val = readl(sgpio_chip.chip.regs + offset); val &= ~(SIRFSOC_GPIO_CTL_INTR_STS_MASK | SIRFSOC_GPIO_CTL_OUT_EN_MASK); switch (type) { @@ -588,7 +559,7 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) break; } - writel(val, bank->chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&sgpio_lock, flags); @@ -597,10 +568,8 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) 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, + if (gpio_lock_as_irq(&sgpio_chip.chip.gc, d->hwirq)) { + dev_err(sgpio_chip.chip.gc.dev, "unable to lock HW IRQ %lu for IRQ\n", d->hwirq); return -EINVAL; @@ -610,9 +579,7 @@ static int sirfsoc_gpio_irq_reqres(struct irq_data *d) 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); + gpio_unlock_as_irq(&sgpio_chip.chip.gc, d->hwirq); } static struct irq_chip sirfsoc_irq_chip = { @@ -634,7 +601,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc) chained_irq_enter(chip, desc); - status = readl(bank->chip.regs + SIRFSOC_GPIO_INT_STATUS(bank->id)); + status = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_INT_STATUS(bank->id)); if (!status) { printk(KERN_WARNING "%s: gpio id %d status %#x no interrupt is flaged\n", @@ -644,7 +611,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc) } while (status) { - ctrl = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, idx)); + ctrl = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, idx)); /* * Here we must check whether the corresponding GPIO's interrupt @@ -653,7 +620,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(sgpio_chip.domain, idx + bank->id * SIRFSOC_GPIO_BANK_SIZE)); } @@ -668,14 +635,14 @@ static inline void sirfsoc_gpio_set_input(struct sirfsoc_gpio_bank *bank, unsign { u32 val; - val = readl(bank->chip.regs + ctrl_offset); + val = readl(sgpio_chip.chip.regs + ctrl_offset); val &= ~SIRFSOC_GPIO_CTL_OUT_EN_MASK; - writel(val, bank->chip.regs + ctrl_offset); + writel(val, sgpio_chip.chip.regs + ctrl_offset); } static int sirfsoc_gpio_request(struct gpio_chip *chip, unsigned offset) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset); unsigned long flags; if (pinctrl_request_gpio(chip->base + offset)) @@ -697,7 +664,7 @@ static int sirfsoc_gpio_request(struct gpio_chip *chip, unsigned offset) static void sirfsoc_gpio_free(struct gpio_chip *chip, unsigned offset) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset); unsigned long flags; spin_lock_irqsave(&bank->lock, flags); @@ -712,8 +679,8 @@ static void sirfsoc_gpio_free(struct gpio_chip *chip, unsigned offset) static int sirfsoc_gpio_direction_input(struct gpio_chip *chip, unsigned gpio) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); - int idx = sirfsoc_gpio_to_offset(gpio); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(gpio); + int idx = sirfsoc_gpio_to_bankoff(gpio); unsigned long flags; unsigned offset; @@ -736,7 +703,7 @@ static inline void sirfsoc_gpio_set_output(struct sirfsoc_gpio_bank *bank, unsig spin_lock_irqsave(&bank->lock, flags); - out_ctrl = readl(bank->chip.regs + offset); + out_ctrl = readl(sgpio_chip.chip.regs + offset); if (value) out_ctrl |= SIRFSOC_GPIO_CTL_DATAOUT_MASK; else @@ -744,15 +711,15 @@ static inline void sirfsoc_gpio_set_output(struct sirfsoc_gpio_bank *bank, unsig out_ctrl &= ~SIRFSOC_GPIO_CTL_INTR_EN_MASK; out_ctrl |= SIRFSOC_GPIO_CTL_OUT_EN_MASK; - writel(out_ctrl, bank->chip.regs + offset); + writel(out_ctrl, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&bank->lock, flags); } static int sirfsoc_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int value) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); - int idx = sirfsoc_gpio_to_offset(gpio); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(gpio); + int idx = sirfsoc_gpio_to_bankoff(gpio); u32 offset; unsigned long flags; @@ -769,13 +736,13 @@ static int sirfsoc_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, static int sirfsoc_gpio_get_value(struct gpio_chip *chip, unsigned offset) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset); u32 val; unsigned long flags; spin_lock_irqsave(&bank->lock, flags); - val = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); + val = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); spin_unlock_irqrestore(&bank->lock, flags); @@ -785,18 +752,18 @@ static int sirfsoc_gpio_get_value(struct gpio_chip *chip, unsigned offset) static void sirfsoc_gpio_set_value(struct gpio_chip *chip, unsigned offset, int value) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset); u32 ctrl; unsigned long flags; spin_lock_irqsave(&bank->lock, flags); - ctrl = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); + ctrl = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); if (value) ctrl |= SIRFSOC_GPIO_CTL_DATAOUT_MASK; else ctrl &= ~SIRFSOC_GPIO_CTL_DATAOUT_MASK; - writel(ctrl, bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); + writel(ctrl, sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); spin_unlock_irqrestore(&bank->lock, flags); } @@ -811,7 +778,6 @@ static int sirfsoc_gpio_irq_map(struct irq_domain *d, unsigned int irq, 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; @@ -830,10 +796,10 @@ static void sirfsoc_gpio_set_pullup(const u32 *pullups) for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) { for_each_set_bit(n, p + i, BITS_PER_LONG) { u32 offset = SIRFSOC_GPIO_CTRL(i, n); - u32 val = readl(sgpio_bank[i].chip.regs + offset); + u32 val = readl(sgpio_chip.chip.regs + offset); val |= SIRFSOC_GPIO_CTL_PULL_MASK; val |= SIRFSOC_GPIO_CTL_PULL_HIGH; - writel(val, sgpio_bank[i].chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); } } } @@ -846,10 +812,10 @@ static void sirfsoc_gpio_set_pulldown(const u32 *pulldowns) for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) { for_each_set_bit(n, p + i, BITS_PER_LONG) { u32 offset = SIRFSOC_GPIO_CTRL(i, n); - u32 val = readl(sgpio_bank[i].chip.regs + offset); + u32 val = readl(sgpio_chip.chip.regs + offset); val |= SIRFSOC_GPIO_CTL_PULL_MASK; val &= ~SIRFSOC_GPIO_CTL_PULL_HIGH; - writel(val, sgpio_bank[i].chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); } } } @@ -877,48 +843,47 @@ static int sirfsoc_gpio_probe(struct device_node *np) 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); + &sirfsoc_gpio_irq_simple_ops, &sgpio_chip); if (!domain) { pr_err("%s: Failed to create irqdomain\n", np->full_name); err = -ENOSYS; goto out; } + sgpio_chip.chip.gc.request = sirfsoc_gpio_request; + sgpio_chip.chip.gc.free = sirfsoc_gpio_free; + sgpio_chip.chip.gc.direction_input = sirfsoc_gpio_direction_input; + sgpio_chip.chip.gc.get = sirfsoc_gpio_get_value; + sgpio_chip.chip.gc.direction_output = sirfsoc_gpio_direction_output; + sgpio_chip.chip.gc.set = sirfsoc_gpio_set_value; + sgpio_chip.chip.gc.to_irq = sirfsoc_gpio_to_irq; + sgpio_chip.chip.gc.base = 0; + sgpio_chip.chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS; + sgpio_chip.chip.gc.label = kstrdup(np->full_name, GFP_KERNEL); + sgpio_chip.chip.gc.of_node = np; + sgpio_chip.chip.gc.of_xlate = sirfsoc_gpio_of_xlate; + sgpio_chip.chip.gc.of_gpio_n_cells = 2; + sgpio_chip.chip.gc.dev = &pdev->dev; + sgpio_chip.chip.regs = regs; + sgpio_chip.is_marco = is_marco; + sgpio_chip.domain = domain; + + err = gpiochip_add(&sgpio_chip.chip.gc); + if (err) { + pr_err("%s: error in probe function with status %d\n", + np->full_name, err); + goto out; + } + for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) { - bank = &sgpio_bank[i]; + bank = &sgpio_chip.sgpio_bank[i]; spin_lock_init(&bank->lock); - bank->chip.gc.request = sirfsoc_gpio_request; - bank->chip.gc.free = sirfsoc_gpio_free; - bank->chip.gc.direction_input = sirfsoc_gpio_direction_input; - 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); - bank->chip.gc.of_node = np; - bank->chip.gc.of_xlate = sirfsoc_gpio_of_xlate; - bank->chip.gc.of_gpio_n_cells = 2; - bank->chip.gc.dev = &pdev->dev; - bank->chip.regs = regs; - bank->id = i; - bank->is_marco = is_marco; bank->parent_irq = platform_get_irq(pdev, i); if (bank->parent_irq < 0) { err = bank->parent_irq; goto out; } - err = gpiochip_add(&bank->chip.gc); - if (err) { - pr_err("%s: error in probe function with status %d\n", - np->full_name, err); - goto out; - } - - bank->domain = domain; - irq_set_chained_handler(bank->parent_irq, sirfsoc_gpio_handle_irq); irq_set_handler_data(bank->parent_irq, bank); } -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip @ 2014-04-15 6:43 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linux-arm-kernel From: Barry Song <Baohua.Song@csr.com> all gpio banks are in one chip, that makes software clean in mapping irq and gpio. Signed-off-by: Barry Song <Baohua.Song@csr.com> --- drivers/pinctrl/sirf/pinctrl-sirf.c | 217 +++++++++++++++-------------------- 1 files changed, 91 insertions(+), 126 deletions(-) diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c index 2c3eb20..cb81b15 100644 --- a/drivers/pinctrl/sirf/pinctrl-sirf.c +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c @@ -34,15 +34,19 @@ #define DRIVER_NAME "pinmux-sirf" struct sirfsoc_gpio_bank { - struct of_mm_gpio_chip chip; - struct irq_domain *domain; int id; int parent_irq; spinlock_t lock; +}; + +struct sirfsoc_gpio_chip { + struct of_mm_gpio_chip chip; + struct irq_domain *domain; bool is_marco; /* for marco, some registers are different with prima2 */ + struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS]; }; -static struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS]; +static struct sirfsoc_gpio_chip sgpio_chip; static DEFINE_SPINLOCK(sgpio_lock); static struct sirfsoc_pin_group *sirfsoc_pin_groups; @@ -258,32 +262,12 @@ static struct pinctrl_desc sirfsoc_pinmux_desc = { /* * Todo: bind irq_chip to every pinctrl_gpio_range */ -static struct pinctrl_gpio_range sirfsoc_gpio_ranges[] = { - { - .name = "sirfsoc-gpio*", - .id = 0, - .base = 0, - .pin_base = 0, - .npins = 32, - }, { - .name = "sirfsoc-gpio*", - .id = 1, - .base = 32, - .pin_base = 32, - .npins = 32, - }, { - .name = "sirfsoc-gpio*", - .id = 2, - .base = 64, - .pin_base = 64, - .npins = 32, - }, { - .name = "sirfsoc-gpio*", - .id = 3, - .base = 96, - .pin_base = 96, - .npins = 19, - }, +static struct pinctrl_gpio_range sirfsoc_gpio_ranges = { + .name = "sirfsoc-gpio*", + .id = 0, + .base = 0, + .pin_base = 0, + .npins = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS, }; static void __iomem *sirfsoc_rsc_of_iomap(void) @@ -303,19 +287,19 @@ static void __iomem *sirfsoc_rsc_of_iomap(void) } static int sirfsoc_gpio_of_xlate(struct gpio_chip *gc, - const struct of_phandle_args *gpiospec, - u32 *flags) + const struct of_phandle_args *gpiospec, + u32 *flags) { - if (gpiospec->args[0] > SIRFSOC_GPIO_NO_OF_BANKS * SIRFSOC_GPIO_BANK_SIZE) + if (gpiospec->args[0] > SIRFSOC_GPIO_NO_OF_BANKS * SIRFSOC_GPIO_BANK_SIZE) return -EINVAL; - if (gc != &sgpio_bank[gpiospec->args[0] / SIRFSOC_GPIO_BANK_SIZE].chip.gc) + if (gc != &sgpio_chip.chip.gc) return -EINVAL; - if (flags) + if (flags) *flags = gpiospec->args[1]; - return gpiospec->args[0] % SIRFSOC_GPIO_BANK_SIZE; + return gpiospec->args[0]; } static const struct of_device_id pinmux_ids[] = { @@ -331,7 +315,6 @@ static int sirfsoc_pinmux_probe(struct platform_device *pdev) struct sirfsoc_pmx *spmx; struct device_node *np = pdev->dev.of_node; const struct sirfsoc_pinctrl_data *pdata; - int i; /* Create state holders etc for this driver */ spmx = devm_kzalloc(&pdev->dev, sizeof(*spmx), GFP_KERNEL); @@ -375,10 +358,8 @@ static int sirfsoc_pinmux_probe(struct platform_device *pdev) goto out_no_pmx; } - for (i = 0; i < ARRAY_SIZE(sirfsoc_gpio_ranges); i++) { - sirfsoc_gpio_ranges[i].gc = &sgpio_bank[i].chip.gc; - pinctrl_add_gpio_range(spmx->pmx, &sirfsoc_gpio_ranges[i]); - } + sirfsoc_gpio_ranges.gc = &sgpio_chip.chip.gc; + pinctrl_add_gpio_range(spmx->pmx, &sirfsoc_gpio_ranges); dev_info(&pdev->dev, "initialized SIRFSOC pinmux driver\n"); @@ -464,33 +445,23 @@ 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) +static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio) { - 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); + return &sgpio_chip.sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE]; } -static inline int sirfsoc_gpio_to_offset(unsigned int gpio) +static int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset) { - return gpio % SIRFSOC_GPIO_BANK_SIZE; + return irq_create_mapping(sgpio_chip.domain, offset); } -static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio) +static inline int sirfsoc_gpio_to_bankoff(unsigned int gpio) { - return &sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE]; -} - -static inline struct sirfsoc_gpio_bank *sirfsoc_gpiochip_to_bank(struct gpio_chip *chip) -{ - return container_of(to_of_mm_gpio_chip(chip), struct sirfsoc_gpio_bank, chip); + return gpio % SIRFSOC_GPIO_BANK_SIZE; } - static void sirfsoc_gpio_irq_ack(struct irq_data *d) { - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE; u32 val, offset; unsigned long flags; @@ -499,9 +470,9 @@ static void sirfsoc_gpio_irq_ack(struct irq_data *d) spin_lock_irqsave(&sgpio_lock, flags); - val = readl(bank->chip.regs + offset); + val = readl(sgpio_chip.chip.regs + offset); - writel(val, bank->chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&sgpio_lock, flags); } @@ -515,24 +486,24 @@ static void __sirfsoc_gpio_irq_mask(struct sirfsoc_gpio_bank *bank, int idx) spin_lock_irqsave(&sgpio_lock, flags); - val = readl(bank->chip.regs + offset); + val = readl(sgpio_chip.chip.regs + offset); val &= ~SIRFSOC_GPIO_CTL_INTR_EN_MASK; val &= ~SIRFSOC_GPIO_CTL_INTR_STS_MASK; - writel(val, bank->chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&sgpio_lock, flags); } static void sirfsoc_gpio_irq_mask(struct irq_data *d) { - struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); __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 sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE; u32 val, offset; unsigned long flags; @@ -541,17 +512,17 @@ static void sirfsoc_gpio_irq_unmask(struct irq_data *d) spin_lock_irqsave(&sgpio_lock, flags); - val = readl(bank->chip.regs + offset); + val = readl(sgpio_chip.chip.regs + offset); val &= ~SIRFSOC_GPIO_CTL_INTR_STS_MASK; val |= SIRFSOC_GPIO_CTL_INTR_EN_MASK; - writel(val, bank->chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&sgpio_lock, flags); } 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 sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); int idx = d->hwirq % SIRFSOC_GPIO_BANK_SIZE; u32 val, offset; unsigned long flags; @@ -560,7 +531,7 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) spin_lock_irqsave(&sgpio_lock, flags); - val = readl(bank->chip.regs + offset); + val = readl(sgpio_chip.chip.regs + offset); val &= ~(SIRFSOC_GPIO_CTL_INTR_STS_MASK | SIRFSOC_GPIO_CTL_OUT_EN_MASK); switch (type) { @@ -588,7 +559,7 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) break; } - writel(val, bank->chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&sgpio_lock, flags); @@ -597,10 +568,8 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) 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, + if (gpio_lock_as_irq(&sgpio_chip.chip.gc, d->hwirq)) { + dev_err(sgpio_chip.chip.gc.dev, "unable to lock HW IRQ %lu for IRQ\n", d->hwirq); return -EINVAL; @@ -610,9 +579,7 @@ static int sirfsoc_gpio_irq_reqres(struct irq_data *d) 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); + gpio_unlock_as_irq(&sgpio_chip.chip.gc, d->hwirq); } static struct irq_chip sirfsoc_irq_chip = { @@ -634,7 +601,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc) chained_irq_enter(chip, desc); - status = readl(bank->chip.regs + SIRFSOC_GPIO_INT_STATUS(bank->id)); + status = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_INT_STATUS(bank->id)); if (!status) { printk(KERN_WARNING "%s: gpio id %d status %#x no interrupt is flaged\n", @@ -644,7 +611,7 @@ static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc) } while (status) { - ctrl = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, idx)); + ctrl = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, idx)); /* * Here we must check whether the corresponding GPIO's interrupt @@ -653,7 +620,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(sgpio_chip.domain, idx + bank->id * SIRFSOC_GPIO_BANK_SIZE)); } @@ -668,14 +635,14 @@ static inline void sirfsoc_gpio_set_input(struct sirfsoc_gpio_bank *bank, unsign { u32 val; - val = readl(bank->chip.regs + ctrl_offset); + val = readl(sgpio_chip.chip.regs + ctrl_offset); val &= ~SIRFSOC_GPIO_CTL_OUT_EN_MASK; - writel(val, bank->chip.regs + ctrl_offset); + writel(val, sgpio_chip.chip.regs + ctrl_offset); } static int sirfsoc_gpio_request(struct gpio_chip *chip, unsigned offset) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset); unsigned long flags; if (pinctrl_request_gpio(chip->base + offset)) @@ -697,7 +664,7 @@ static int sirfsoc_gpio_request(struct gpio_chip *chip, unsigned offset) static void sirfsoc_gpio_free(struct gpio_chip *chip, unsigned offset) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset); unsigned long flags; spin_lock_irqsave(&bank->lock, flags); @@ -712,8 +679,8 @@ static void sirfsoc_gpio_free(struct gpio_chip *chip, unsigned offset) static int sirfsoc_gpio_direction_input(struct gpio_chip *chip, unsigned gpio) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); - int idx = sirfsoc_gpio_to_offset(gpio); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(gpio); + int idx = sirfsoc_gpio_to_bankoff(gpio); unsigned long flags; unsigned offset; @@ -736,7 +703,7 @@ static inline void sirfsoc_gpio_set_output(struct sirfsoc_gpio_bank *bank, unsig spin_lock_irqsave(&bank->lock, flags); - out_ctrl = readl(bank->chip.regs + offset); + out_ctrl = readl(sgpio_chip.chip.regs + offset); if (value) out_ctrl |= SIRFSOC_GPIO_CTL_DATAOUT_MASK; else @@ -744,15 +711,15 @@ static inline void sirfsoc_gpio_set_output(struct sirfsoc_gpio_bank *bank, unsig out_ctrl &= ~SIRFSOC_GPIO_CTL_INTR_EN_MASK; out_ctrl |= SIRFSOC_GPIO_CTL_OUT_EN_MASK; - writel(out_ctrl, bank->chip.regs + offset); + writel(out_ctrl, sgpio_chip.chip.regs + offset); spin_unlock_irqrestore(&bank->lock, flags); } static int sirfsoc_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, int value) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); - int idx = sirfsoc_gpio_to_offset(gpio); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(gpio); + int idx = sirfsoc_gpio_to_bankoff(gpio); u32 offset; unsigned long flags; @@ -769,13 +736,13 @@ static int sirfsoc_gpio_direction_output(struct gpio_chip *chip, unsigned gpio, static int sirfsoc_gpio_get_value(struct gpio_chip *chip, unsigned offset) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset); u32 val; unsigned long flags; spin_lock_irqsave(&bank->lock, flags); - val = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); + val = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); spin_unlock_irqrestore(&bank->lock, flags); @@ -785,18 +752,18 @@ static int sirfsoc_gpio_get_value(struct gpio_chip *chip, unsigned offset) static void sirfsoc_gpio_set_value(struct gpio_chip *chip, unsigned offset, int value) { - struct sirfsoc_gpio_bank *bank = sirfsoc_gpiochip_to_bank(chip); + struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(offset); u32 ctrl; unsigned long flags; spin_lock_irqsave(&bank->lock, flags); - ctrl = readl(bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); + ctrl = readl(sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); if (value) ctrl |= SIRFSOC_GPIO_CTL_DATAOUT_MASK; else ctrl &= ~SIRFSOC_GPIO_CTL_DATAOUT_MASK; - writel(ctrl, bank->chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); + writel(ctrl, sgpio_chip.chip.regs + SIRFSOC_GPIO_CTRL(bank->id, offset)); spin_unlock_irqrestore(&bank->lock, flags); } @@ -811,7 +778,6 @@ static int sirfsoc_gpio_irq_map(struct irq_domain *d, unsigned int irq, 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; @@ -830,10 +796,10 @@ static void sirfsoc_gpio_set_pullup(const u32 *pullups) for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) { for_each_set_bit(n, p + i, BITS_PER_LONG) { u32 offset = SIRFSOC_GPIO_CTRL(i, n); - u32 val = readl(sgpio_bank[i].chip.regs + offset); + u32 val = readl(sgpio_chip.chip.regs + offset); val |= SIRFSOC_GPIO_CTL_PULL_MASK; val |= SIRFSOC_GPIO_CTL_PULL_HIGH; - writel(val, sgpio_bank[i].chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); } } } @@ -846,10 +812,10 @@ static void sirfsoc_gpio_set_pulldown(const u32 *pulldowns) for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) { for_each_set_bit(n, p + i, BITS_PER_LONG) { u32 offset = SIRFSOC_GPIO_CTRL(i, n); - u32 val = readl(sgpio_bank[i].chip.regs + offset); + u32 val = readl(sgpio_chip.chip.regs + offset); val |= SIRFSOC_GPIO_CTL_PULL_MASK; val &= ~SIRFSOC_GPIO_CTL_PULL_HIGH; - writel(val, sgpio_bank[i].chip.regs + offset); + writel(val, sgpio_chip.chip.regs + offset); } } } @@ -877,48 +843,47 @@ static int sirfsoc_gpio_probe(struct device_node *np) 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); + &sirfsoc_gpio_irq_simple_ops, &sgpio_chip); if (!domain) { pr_err("%s: Failed to create irqdomain\n", np->full_name); err = -ENOSYS; goto out; } + sgpio_chip.chip.gc.request = sirfsoc_gpio_request; + sgpio_chip.chip.gc.free = sirfsoc_gpio_free; + sgpio_chip.chip.gc.direction_input = sirfsoc_gpio_direction_input; + sgpio_chip.chip.gc.get = sirfsoc_gpio_get_value; + sgpio_chip.chip.gc.direction_output = sirfsoc_gpio_direction_output; + sgpio_chip.chip.gc.set = sirfsoc_gpio_set_value; + sgpio_chip.chip.gc.to_irq = sirfsoc_gpio_to_irq; + sgpio_chip.chip.gc.base = 0; + sgpio_chip.chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS; + sgpio_chip.chip.gc.label = kstrdup(np->full_name, GFP_KERNEL); + sgpio_chip.chip.gc.of_node = np; + sgpio_chip.chip.gc.of_xlate = sirfsoc_gpio_of_xlate; + sgpio_chip.chip.gc.of_gpio_n_cells = 2; + sgpio_chip.chip.gc.dev = &pdev->dev; + sgpio_chip.chip.regs = regs; + sgpio_chip.is_marco = is_marco; + sgpio_chip.domain = domain; + + err = gpiochip_add(&sgpio_chip.chip.gc); + if (err) { + pr_err("%s: error in probe function with status %d\n", + np->full_name, err); + goto out; + } + for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) { - bank = &sgpio_bank[i]; + bank = &sgpio_chip.sgpio_bank[i]; spin_lock_init(&bank->lock); - bank->chip.gc.request = sirfsoc_gpio_request; - bank->chip.gc.free = sirfsoc_gpio_free; - bank->chip.gc.direction_input = sirfsoc_gpio_direction_input; - 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); - bank->chip.gc.of_node = np; - bank->chip.gc.of_xlate = sirfsoc_gpio_of_xlate; - bank->chip.gc.of_gpio_n_cells = 2; - bank->chip.gc.dev = &pdev->dev; - bank->chip.regs = regs; - bank->id = i; - bank->is_marco = is_marco; bank->parent_irq = platform_get_irq(pdev, i); if (bank->parent_irq < 0) { err = bank->parent_irq; goto out; } - err = gpiochip_add(&bank->chip.gc); - if (err) { - pr_err("%s: error in probe function with status %d\n", - np->full_name, err); - goto out; - } - - bank->domain = domain; - irq_set_chained_handler(bank->parent_irq, sirfsoc_gpio_handle_irq); irq_set_handler_data(bank->parent_irq, bank); } -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip 2014-04-15 6:43 ` Barry Song @ 2014-04-22 18:27 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-22 18:27 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > all gpio banks are in one chip, that makes software clean in mapping > irq and gpio. > > Signed-off-by: Barry Song <Baohua.Song@csr.com> These all look good, but for some reason does not apply on top of the pinctrl "devel" branch based on v3.15-rc2. Can you please rebase this set onto my branch or atleast v3.15-rc2 and resend? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip @ 2014-04-22 18:27 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-22 18:27 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > all gpio banks are in one chip, that makes software clean in mapping > irq and gpio. > > Signed-off-by: Barry Song <Baohua.Song@csr.com> These all look good, but for some reason does not apply on top of the pinctrl "devel" branch based on v3.15-rc2. Can you please rebase this set onto my branch or atleast v3.15-rc2 and resend? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip 2014-04-15 6:43 ` Barry Song @ 2014-04-23 20:03 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:03 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > all gpio banks are in one chip, that makes software clean in mapping > irq and gpio. > > Signed-off-by: Barry Song <Baohua.Song@csr.com> Patch applied! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip @ 2014-04-23 20:03 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:03 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > all gpio banks are in one chip, that makes software clean in mapping > irq and gpio. > > Signed-off-by: Barry Song <Baohua.Song@csr.com> Patch applied! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers 2014-04-15 6:43 ` Barry Song @ 2014-04-15 6:43 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linus.walleij, linux-gpio; +Cc: linux-arm-kernel, workgroup.linux, Barry Song From: 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: Barry Song <Baohua.Song@csr.com> --- came from Linus Walleij's patch. drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/sirf/pinctrl-sirf.c | 89 ++++++++++------------------------ 2 files changed, 27 insertions(+), 63 deletions(-) diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index e493240..ddd4e6e 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 cb81b15..c03dcc7 100644 --- a/drivers/pinctrl/sirf/pinctrl-sirf.c +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c @@ -14,8 +14,6 @@ #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 +25,6 @@ #include <linux/bitops.h> #include <linux/gpio.h> #include <linux/of_gpio.h> -#include <asm/mach/irq.h> #include "pinctrl-sirf.h" @@ -41,7 +38,6 @@ struct sirfsoc_gpio_bank { struct sirfsoc_gpio_chip { struct of_mm_gpio_chip chip; - struct irq_domain *domain; bool is_marco; /* for marco, some registers are different with prima2 */ struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS]; }; @@ -450,15 +446,11 @@ static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio) return &sgpio_chip.sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE]; } -static int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset) -{ - return irq_create_mapping(sgpio_chip.domain, offset); -} - static inline int sirfsoc_gpio_to_bankoff(unsigned int gpio) { return gpio % SIRFSOC_GPIO_BANK_SIZE; } + static void sirfsoc_gpio_irq_ack(struct irq_data *d) { struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); @@ -566,38 +558,28 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) return 0; } -static int sirfsoc_gpio_irq_reqres(struct irq_data *d) -{ - if (gpio_lock_as_irq(&sgpio_chip.chip.gc, d->hwirq)) { - dev_err(sgpio_chip.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) -{ - gpio_unlock_as_irq(&sgpio_chip.chip.gc, d->hwirq); -} - 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 sirfsoc_gpio_bank *bank; u32 status, ctrl; int idx = 0; struct irq_chip *chip = irq_get_chip(irq); + int i; + + for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) { + bank = &sgpio_chip.sgpio_bank[i]; + if (bank->parent_irq == irq) + break; + } + BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE); chained_irq_enter(chip, desc); @@ -620,7 +602,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(sgpio_chip.domain, idx + + generic_handle_irq(irq_find_mapping(sgpio_chip.chip.gc.irqdomain, idx + bank->id * SIRFSOC_GPIO_BANK_SIZE)); } @@ -768,26 +750,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); - 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; @@ -826,7 +788,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]; @@ -842,21 +803,12 @@ 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_chip); - if (!domain) { - pr_err("%s: Failed to create irqdomain\n", np->full_name); - err = -ENOSYS; - goto out; - } - sgpio_chip.chip.gc.request = sirfsoc_gpio_request; sgpio_chip.chip.gc.free = sirfsoc_gpio_free; sgpio_chip.chip.gc.direction_input = sirfsoc_gpio_direction_input; sgpio_chip.chip.gc.get = sirfsoc_gpio_get_value; sgpio_chip.chip.gc.direction_output = sirfsoc_gpio_direction_output; sgpio_chip.chip.gc.set = sirfsoc_gpio_set_value; - sgpio_chip.chip.gc.to_irq = sirfsoc_gpio_to_irq; sgpio_chip.chip.gc.base = 0; sgpio_chip.chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS; sgpio_chip.chip.gc.label = kstrdup(np->full_name, GFP_KERNEL); @@ -866,15 +818,24 @@ static int sirfsoc_gpio_probe(struct device_node *np) sgpio_chip.chip.gc.dev = &pdev->dev; sgpio_chip.chip.regs = regs; sgpio_chip.is_marco = is_marco; - sgpio_chip.domain = domain; err = gpiochip_add(&sgpio_chip.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; } + err = gpiochip_irqchip_add(&sgpio_chip.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; + } + for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) { bank = &sgpio_chip.sgpio_bank[i]; spin_lock_init(&bank->lock); @@ -884,8 +845,10 @@ static int sirfsoc_gpio_probe(struct device_node *np) 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(&sgpio_chip.chip.gc, + &sirfsoc_irq_chip, + bank->parent_irq, + sirfsoc_gpio_handle_irq); } if (!of_property_read_u32_array(np, "sirf,pullups", pullups, -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers @ 2014-04-15 6:43 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linux-arm-kernel From: 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: Barry Song <Baohua.Song@csr.com> --- came from Linus Walleij's patch. drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/sirf/pinctrl-sirf.c | 89 ++++++++++------------------------ 2 files changed, 27 insertions(+), 63 deletions(-) diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index e493240..ddd4e6e 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 cb81b15..c03dcc7 100644 --- a/drivers/pinctrl/sirf/pinctrl-sirf.c +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c @@ -14,8 +14,6 @@ #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 +25,6 @@ #include <linux/bitops.h> #include <linux/gpio.h> #include <linux/of_gpio.h> -#include <asm/mach/irq.h> #include "pinctrl-sirf.h" @@ -41,7 +38,6 @@ struct sirfsoc_gpio_bank { struct sirfsoc_gpio_chip { struct of_mm_gpio_chip chip; - struct irq_domain *domain; bool is_marco; /* for marco, some registers are different with prima2 */ struct sirfsoc_gpio_bank sgpio_bank[SIRFSOC_GPIO_NO_OF_BANKS]; }; @@ -450,15 +446,11 @@ static inline struct sirfsoc_gpio_bank *sirfsoc_gpio_to_bank(unsigned int gpio) return &sgpio_chip.sgpio_bank[gpio / SIRFSOC_GPIO_BANK_SIZE]; } -static int sirfsoc_gpio_to_irq(struct gpio_chip *chip, unsigned offset) -{ - return irq_create_mapping(sgpio_chip.domain, offset); -} - static inline int sirfsoc_gpio_to_bankoff(unsigned int gpio) { return gpio % SIRFSOC_GPIO_BANK_SIZE; } + static void sirfsoc_gpio_irq_ack(struct irq_data *d) { struct sirfsoc_gpio_bank *bank = sirfsoc_gpio_to_bank(d->hwirq); @@ -566,38 +558,28 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) return 0; } -static int sirfsoc_gpio_irq_reqres(struct irq_data *d) -{ - if (gpio_lock_as_irq(&sgpio_chip.chip.gc, d->hwirq)) { - dev_err(sgpio_chip.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) -{ - gpio_unlock_as_irq(&sgpio_chip.chip.gc, d->hwirq); -} - 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 sirfsoc_gpio_bank *bank; u32 status, ctrl; int idx = 0; struct irq_chip *chip = irq_get_chip(irq); + int i; + + for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) { + bank = &sgpio_chip.sgpio_bank[i]; + if (bank->parent_irq == irq) + break; + } + BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE); chained_irq_enter(chip, desc); @@ -620,7 +602,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(sgpio_chip.domain, idx + + generic_handle_irq(irq_find_mapping(sgpio_chip.chip.gc.irqdomain, idx + bank->id * SIRFSOC_GPIO_BANK_SIZE)); } @@ -768,26 +750,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); - 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; @@ -826,7 +788,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]; @@ -842,21 +803,12 @@ 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_chip); - if (!domain) { - pr_err("%s: Failed to create irqdomain\n", np->full_name); - err = -ENOSYS; - goto out; - } - sgpio_chip.chip.gc.request = sirfsoc_gpio_request; sgpio_chip.chip.gc.free = sirfsoc_gpio_free; sgpio_chip.chip.gc.direction_input = sirfsoc_gpio_direction_input; sgpio_chip.chip.gc.get = sirfsoc_gpio_get_value; sgpio_chip.chip.gc.direction_output = sirfsoc_gpio_direction_output; sgpio_chip.chip.gc.set = sirfsoc_gpio_set_value; - sgpio_chip.chip.gc.to_irq = sirfsoc_gpio_to_irq; sgpio_chip.chip.gc.base = 0; sgpio_chip.chip.gc.ngpio = SIRFSOC_GPIO_BANK_SIZE * SIRFSOC_GPIO_NO_OF_BANKS; sgpio_chip.chip.gc.label = kstrdup(np->full_name, GFP_KERNEL); @@ -866,15 +818,24 @@ static int sirfsoc_gpio_probe(struct device_node *np) sgpio_chip.chip.gc.dev = &pdev->dev; sgpio_chip.chip.regs = regs; sgpio_chip.is_marco = is_marco; - sgpio_chip.domain = domain; err = gpiochip_add(&sgpio_chip.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; } + err = gpiochip_irqchip_add(&sgpio_chip.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; + } + for (i = 0; i < SIRFSOC_GPIO_NO_OF_BANKS; i++) { bank = &sgpio_chip.sgpio_bank[i]; spin_lock_init(&bank->lock); @@ -884,8 +845,10 @@ static int sirfsoc_gpio_probe(struct device_node *np) 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(&sgpio_chip.chip.gc, + &sirfsoc_irq_chip, + bank->parent_irq, + sirfsoc_gpio_handle_irq); } if (!of_property_read_u32_array(np, "sirf,pullups", pullups, -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers 2014-04-15 6:43 ` Barry Song @ 2014-04-23 20:04 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:04 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: 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: Barry Song <Baohua.Song@csr.com> Patch applied! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers @ 2014-04-23 20:04 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:04 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: 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: Barry Song <Baohua.Song@csr.com> Patch applied! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip 2014-04-15 6:43 ` Barry Song @ 2014-04-15 6:43 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linus.walleij, linux-gpio; +Cc: linux-arm-kernel, workgroup.linux, Barry Song From: Barry Song <Baohua.Song@csr.com> since it is called handler_data, drivers should have the ability to set handler_data based on real hardware. GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not break GPIO core. Signed-off-by: Barry Song <Baohua.Song@csr.com> --- drivers/gpio/gpiolib.c | 10 +++------- include/linux/gpio/driver.h | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 761013f..4b90d14 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1351,24 +1351,20 @@ static struct gpio_chip *find_chip_by_name(const char *name) /** * gpiochip_add_chained_irqchip() - adds a chained irqchip to a gpiochip - * @gpiochip: the gpiochip to add the irqchip to + * @handler_data: handler_data which will be used by ISR of interrupt parent * @irqchip: the irqchip to add to the gpiochip * @parent_irq: the irq number corresponding to the parent IRQ for this * chained irqchip * @parent_handler: the parent interrupt handler for the accumulated IRQ * coming out of the gpiochip */ -void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip, +void gpiochip_set_chained_irqchip(void *handler_data, struct irq_chip *irqchip, int parent_irq, irq_flow_handler_t parent_handler) { irq_set_chained_handler(parent_irq, parent_handler); - /* - * The parent irqchip is already using the chip_data for this - * irqchip, so our callbacks simply use the handler_data. - */ - irq_set_handler_data(parent_irq, gpiochip); + irq_set_handler_data(parent_irq, handler_data); } EXPORT_SYMBOL_GPL(gpiochip_set_chained_irqchip); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..52a0758 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -207,7 +207,7 @@ void gpiod_add_lookup_table(struct gpiod_lookup_table *table); #ifdef CONFIG_GPIOLIB_IRQCHIP -void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip, +void gpiochip_set_chained_irqchip(void *handler_data, struct irq_chip *irqchip, int parent_irq, irq_flow_handler_t parent_handler); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip @ 2014-04-15 6:43 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linux-arm-kernel From: Barry Song <Baohua.Song@csr.com> since it is called handler_data, drivers should have the ability to set handler_data based on real hardware. GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not break GPIO core. Signed-off-by: Barry Song <Baohua.Song@csr.com> --- drivers/gpio/gpiolib.c | 10 +++------- include/linux/gpio/driver.h | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 761013f..4b90d14 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1351,24 +1351,20 @@ static struct gpio_chip *find_chip_by_name(const char *name) /** * gpiochip_add_chained_irqchip() - adds a chained irqchip to a gpiochip - * @gpiochip: the gpiochip to add the irqchip to + * @handler_data: handler_data which will be used by ISR of interrupt parent * @irqchip: the irqchip to add to the gpiochip * @parent_irq: the irq number corresponding to the parent IRQ for this * chained irqchip * @parent_handler: the parent interrupt handler for the accumulated IRQ * coming out of the gpiochip */ -void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip, +void gpiochip_set_chained_irqchip(void *handler_data, struct irq_chip *irqchip, int parent_irq, irq_flow_handler_t parent_handler) { irq_set_chained_handler(parent_irq, parent_handler); - /* - * The parent irqchip is already using the chip_data for this - * irqchip, so our callbacks simply use the handler_data. - */ - irq_set_handler_data(parent_irq, gpiochip); + irq_set_handler_data(parent_irq, handler_data); } EXPORT_SYMBOL_GPL(gpiochip_set_chained_irqchip); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 1827b43..52a0758 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -207,7 +207,7 @@ void gpiod_add_lookup_table(struct gpiod_lookup_table *table); #ifdef CONFIG_GPIOLIB_IRQCHIP -void gpiochip_set_chained_irqchip(struct gpio_chip *gpiochip, +void gpiochip_set_chained_irqchip(void *handler_data, struct irq_chip *irqchip, int parent_irq, irq_flow_handler_t parent_handler); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip 2014-04-15 6:43 ` Barry Song @ 2014-04-23 20:13 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:13 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > since it is called handler_data, drivers should have the ability to > set handler_data based on real hardware. > GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not > break GPIO core. > > Signed-off-by: Barry Song <Baohua.Song@csr.com> NAK, the whole idea with the function gpiochip_set_chained_irqchip() is that its sets things up so that the gpiochip is passed as handler data on the parent IRQ, akin to how the gpiochip is passed as chip data on the cascaded IRQs. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip @ 2014-04-23 20:13 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:13 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > since it is called handler_data, drivers should have the ability to > set handler_data based on real hardware. > GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not > break GPIO core. > > Signed-off-by: Barry Song <Baohua.Song@csr.com> NAK, the whole idea with the function gpiochip_set_chained_irqchip() is that its sets things up so that the gpiochip is passed as handler data on the parent IRQ, akin to how the gpiochip is passed as chip data on the cascaded IRQs. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip 2014-04-23 20:13 ` Linus Walleij @ 2014-04-23 22:55 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-23 22:55 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song 2014-04-24 4:13 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>: > On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > >> From: Barry Song <Baohua.Song@csr.com> >> >> since it is called handler_data, drivers should have the ability to >> set handler_data based on real hardware. >> GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not >> break GPIO core. >> >> Signed-off-by: Barry Song <Baohua.Song@csr.com> > > NAK, the whole idea with the function gpiochip_set_chained_irqchip() > is that its sets things up so that the gpiochip is passed as handler > data on the parent IRQ, akin to how the gpiochip is passed as > chip data on the cascaded IRQs. i think it is a really bad idea as the parent handler might not want the whole chip if the chip has several parents and each parent want a separate handler_data. this patch doesn't break your way as the parameter is void *, drivers which use your way don't need any change. but for drivers which want more special handler_data, it supports better. > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip @ 2014-04-23 22:55 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-23 22:55 UTC (permalink / raw) To: linux-arm-kernel 2014-04-24 4:13 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>: > On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > >> From: Barry Song <Baohua.Song@csr.com> >> >> since it is called handler_data, drivers should have the ability to >> set handler_data based on real hardware. >> GPIOLIB_IRQCHIP doesn't use handler_data by itself, so it will not >> break GPIO core. >> >> Signed-off-by: Barry Song <Baohua.Song@csr.com> > > NAK, the whole idea with the function gpiochip_set_chained_irqchip() > is that its sets things up so that the gpiochip is passed as handler > data on the parent IRQ, akin to how the gpiochip is passed as > chip data on the cascaded IRQs. i think it is a really bad idea as the parent handler might not want the whole chip if the chip has several parents and each parent want a separate handler_data. this patch doesn't break your way as the parameter is void *, drivers which use your way don't need any change. but for drivers which want more special handler_data, it supports better. > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip 2014-04-23 22:55 ` Barry Song @ 2014-04-23 23:09 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 23:09 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Thu, Apr 24, 2014 at 12:55 AM, Barry Song <21cnbao@gmail.com> wrote: >> NAK, the whole idea with the function gpiochip_set_chained_irqchip() >> is that its sets things up so that the gpiochip is passed as handler >> data on the parent IRQ, akin to how the gpiochip is passed as >> chip data on the cascaded IRQs. > > i think it is a really bad idea as the parent handler might not want > the whole chip if the chip has several parents and each parent want a > separate handler_data. > this patch doesn't break your way as the parameter is void *, drivers > which use your way don't need any change. but for drivers which want > more special handler_data, it supports better. This is perfectly possible, just don't use gpiochip_set_chained_irqchip() which has this semantic effect. Use irq_set_chained_handler() and irq_set_handler_data() directly instead. I do see that this is maybe easier for the handler to get the bank passed in. However when I in the RFT patches change the driver to allocate and pass a struct for the gpio chip, it all looks a bit different. You would have to get the sgpio from the bank instead, which may require to introduce a special pointer for that. What we want to get rid of is this: for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) { bank = &sgpio->sgpio_bank[i]; if (bank->parent_irq == irq) break; } BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE); So what about passing something like that to the handler: struct sirf_irq_handler_data { struct sirfsoc_gpio_chip *sgpio; struct sirfsoc_gpio_bank *bank; }; Then the irq handler instantly have pointers to all it needs. But maybe it's better to just pass the bank, whatdoIknow. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip @ 2014-04-23 23:09 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 23:09 UTC (permalink / raw) To: linux-arm-kernel On Thu, Apr 24, 2014 at 12:55 AM, Barry Song <21cnbao@gmail.com> wrote: >> NAK, the whole idea with the function gpiochip_set_chained_irqchip() >> is that its sets things up so that the gpiochip is passed as handler >> data on the parent IRQ, akin to how the gpiochip is passed as >> chip data on the cascaded IRQs. > > i think it is a really bad idea as the parent handler might not want > the whole chip if the chip has several parents and each parent want a > separate handler_data. > this patch doesn't break your way as the parameter is void *, drivers > which use your way don't need any change. but for drivers which want > more special handler_data, it supports better. This is perfectly possible, just don't use gpiochip_set_chained_irqchip() which has this semantic effect. Use irq_set_chained_handler() and irq_set_handler_data() directly instead. I do see that this is maybe easier for the handler to get the bank passed in. However when I in the RFT patches change the driver to allocate and pass a struct for the gpio chip, it all looks a bit different. You would have to get the sgpio from the bank instead, which may require to introduce a special pointer for that. What we want to get rid of is this: for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) { bank = &sgpio->sgpio_bank[i]; if (bank->parent_irq == irq) break; } BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE); So what about passing something like that to the handler: struct sirf_irq_handler_data { struct sirfsoc_gpio_chip *sgpio; struct sirfsoc_gpio_bank *bank; }; Then the irq handler instantly have pointers to all it needs. But maybe it's better to just pass the bank, whatdoIknow. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip 2014-04-23 23:09 ` Linus Walleij @ 2014-04-23 23:23 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-23 23:23 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song 2014-04-24 7:09 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>: > On Thu, Apr 24, 2014 at 12:55 AM, Barry Song <21cnbao@gmail.com> wrote: > >>> NAK, the whole idea with the function gpiochip_set_chained_irqchip() >>> is that its sets things up so that the gpiochip is passed as handler >>> data on the parent IRQ, akin to how the gpiochip is passed as >>> chip data on the cascaded IRQs. >> >> i think it is a really bad idea as the parent handler might not want >> the whole chip if the chip has several parents and each parent want a >> separate handler_data. >> this patch doesn't break your way as the parameter is void *, drivers >> which use your way don't need any change. but for drivers which want >> more special handler_data, it supports better. > > This is perfectly possible, just don't use > gpiochip_set_chained_irqchip() which has this semantic > effect. > > Use irq_set_chained_handler() and irq_set_handler_data() > directly instead. > > I do see that this is maybe easier for the handler to get the bank > passed in. > > However when I in the RFT patches change the driver to allocate > and pass a struct for the gpio chip, it all looks a bit different. > You would have to get the sgpio from the bank instead, which > may require to introduce a special pointer for that. > > What we want to get rid of is this: > > for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) { > bank = &sgpio->sgpio_bank[i]; > if (bank->parent_irq == irq) > break; > } > BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE); > > So what about passing something like that to the handler: > > struct sirf_irq_handler_data { > struct sirfsoc_gpio_chip *sgpio; > struct sirfsoc_gpio_bank *bank; > }; > > Then the irq handler instantly have pointers to all it needs. > But maybe it's better to just pass the bank, whatdoIknow. only if we move to irq_set_chained_handler() and irq_set_handler_data() directly and set bank-specific handler_data manually, it works. my point is that will we make the gpiochip_set_chained_irqchip() more general for this kind of cases too since you have had a good API to wrap things? > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip @ 2014-04-23 23:23 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-23 23:23 UTC (permalink / raw) To: linux-arm-kernel 2014-04-24 7:09 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>: > On Thu, Apr 24, 2014 at 12:55 AM, Barry Song <21cnbao@gmail.com> wrote: > >>> NAK, the whole idea with the function gpiochip_set_chained_irqchip() >>> is that its sets things up so that the gpiochip is passed as handler >>> data on the parent IRQ, akin to how the gpiochip is passed as >>> chip data on the cascaded IRQs. >> >> i think it is a really bad idea as the parent handler might not want >> the whole chip if the chip has several parents and each parent want a >> separate handler_data. >> this patch doesn't break your way as the parameter is void *, drivers >> which use your way don't need any change. but for drivers which want >> more special handler_data, it supports better. > > This is perfectly possible, just don't use > gpiochip_set_chained_irqchip() which has this semantic > effect. > > Use irq_set_chained_handler() and irq_set_handler_data() > directly instead. > > I do see that this is maybe easier for the handler to get the bank > passed in. > > However when I in the RFT patches change the driver to allocate > and pass a struct for the gpio chip, it all looks a bit different. > You would have to get the sgpio from the bank instead, which > may require to introduce a special pointer for that. > > What we want to get rid of is this: > > for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) { > bank = &sgpio->sgpio_bank[i]; > if (bank->parent_irq == irq) > break; > } > BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE); > > So what about passing something like that to the handler: > > struct sirf_irq_handler_data { > struct sirfsoc_gpio_chip *sgpio; > struct sirfsoc_gpio_bank *bank; > }; > > Then the irq handler instantly have pointers to all it needs. > But maybe it's better to just pass the bank, whatdoIknow. only if we move to irq_set_chained_handler() and irq_set_handler_data() directly and set bank-specific handler_data manually, it works. my point is that will we make the gpiochip_set_chained_irqchip() more general for this kind of cases too since you have had a good API to wrap things? > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip 2014-04-23 23:23 ` Barry Song @ 2014-04-24 13:06 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-24 13:06 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Thu, Apr 24, 2014 at 1:23 AM, Barry Song <21cnbao@gmail.com> wrote: > my point is that will we make the gpiochip_set_chained_irqchip() more > general for this kind of cases too since you have had a good API to > wrap things? As it only wraps two function calls I'm leaning toward deleting it as it might create more complexity than it removes. Not sure. We don't have a similar handler for registering nested threaded irq handlers so it's not really helpful. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip @ 2014-04-24 13:06 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-24 13:06 UTC (permalink / raw) To: linux-arm-kernel On Thu, Apr 24, 2014 at 1:23 AM, Barry Song <21cnbao@gmail.com> wrote: > my point is that will we make the gpiochip_set_chained_irqchip() more > general for this kind of cases too since you have had a good API to > wrap things? As it only wraps two function calls I'm leaning toward deleting it as it might create more complexity than it removes. Not sure. We don't have a similar handler for registering nested threaded irq handlers so it's not really helpful. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip 2014-04-24 13:06 ` Linus Walleij @ 2014-04-24 15:43 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-24 15:43 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song 2014-04-24 21:06 GMT+08:00, Linus Walleij <linus.walleij@linaro.org>: > On Thu, Apr 24, 2014 at 1:23 AM, Barry Song <21cnbao@gmail.com> wrote: > >> my point is that will we make the gpiochip_set_chained_irqchip() more >> general for this kind of cases too since you have had a good API to >> wrap things? > > As it only wraps two function calls I'm leaning toward deleting it as it > might create more complexity than it removes. Not sure. i do agree that we can remove it if it doesn't really support general gpiochip. i think it is popular that some gpiochip has several irq parents. so people might get confused by this API as it actually doesn't really help many. > > We don't have a similar handler for registering nested threaded > irq handlers so it's not really helpful. > > Yours, > Linus Walleij > -barry -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip @ 2014-04-24 15:43 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-24 15:43 UTC (permalink / raw) To: linux-arm-kernel 2014-04-24 21:06 GMT+08:00, Linus Walleij <linus.walleij@linaro.org>: > On Thu, Apr 24, 2014 at 1:23 AM, Barry Song <21cnbao@gmail.com> wrote: > >> my point is that will we make the gpiochip_set_chained_irqchip() more >> general for this kind of cases too since you have had a good API to >> wrap things? > > As it only wraps two function calls I'm leaning toward deleting it as it > might create more complexity than it removes. Not sure. i do agree that we can remove it if it doesn't really support general gpiochip. i think it is popular that some gpiochip has several irq parents. so people might get confused by this API as it actually doesn't really help many. > > We don't have a similar handler for registering nested threaded > irq handlers so it's not really helpful. > > Yours, > Linus Walleij > -barry -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data 2014-04-15 6:43 ` Barry Song @ 2014-04-15 6:43 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linus.walleij, linux-gpio; +Cc: linux-arm-kernel, workgroup.linux, Barry Song From: Barry Song <Baohua.Song@csr.com> we set bank as handler_data, now we can get bank in ISR directly instead of looking-up. Signed-off-by: Barry Song <Baohua.Song@csr.com> --- drivers/pinctrl/sirf/pinctrl-sirf.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c index c03dcc7..36cc678 100644 --- a/drivers/pinctrl/sirf/pinctrl-sirf.c +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c @@ -568,18 +568,10 @@ static struct irq_chip sirfsoc_irq_chip = { static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc) { - struct sirfsoc_gpio_bank *bank; + struct sirfsoc_gpio_bank *bank = irq_get_handler_data(irq); u32 status, ctrl; int idx = 0; struct irq_chip *chip = irq_get_chip(irq); - int i; - - for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) { - bank = &sgpio_chip.sgpio_bank[i]; - if (bank->parent_irq == irq) - break; - } - BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE); chained_irq_enter(chip, desc); @@ -845,7 +837,7 @@ static int sirfsoc_gpio_probe(struct device_node *np) goto out; } - gpiochip_set_chained_irqchip(&sgpio_chip.chip.gc, + gpiochip_set_chained_irqchip(bank, &sirfsoc_irq_chip, bank->parent_irq, sirfsoc_gpio_handle_irq); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data @ 2014-04-15 6:43 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-15 6:43 UTC (permalink / raw) To: linux-arm-kernel From: Barry Song <Baohua.Song@csr.com> we set bank as handler_data, now we can get bank in ISR directly instead of looking-up. Signed-off-by: Barry Song <Baohua.Song@csr.com> --- drivers/pinctrl/sirf/pinctrl-sirf.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c index c03dcc7..36cc678 100644 --- a/drivers/pinctrl/sirf/pinctrl-sirf.c +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c @@ -568,18 +568,10 @@ static struct irq_chip sirfsoc_irq_chip = { static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc) { - struct sirfsoc_gpio_bank *bank; + struct sirfsoc_gpio_bank *bank = irq_get_handler_data(irq); u32 status, ctrl; int idx = 0; struct irq_chip *chip = irq_get_chip(irq); - int i; - - for (i = 0; i < SIRFSOC_GPIO_BANK_SIZE; i++) { - bank = &sgpio_chip.sgpio_bank[i]; - if (bank->parent_irq == irq) - break; - } - BUG_ON (i == SIRFSOC_GPIO_BANK_SIZE); chained_irq_enter(chip, desc); @@ -845,7 +837,7 @@ static int sirfsoc_gpio_probe(struct device_node *np) goto out; } - gpiochip_set_chained_irqchip(&sgpio_chip.chip.gc, + gpiochip_set_chained_irqchip(bank, &sirfsoc_irq_chip, bank->parent_irq, sirfsoc_gpio_handle_irq); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data 2014-04-15 6:43 ` Barry Song @ 2014-04-23 20:17 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:17 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > we set bank as handler_data, now we can get bank in ISR directly > instead of looking-up. > > Signed-off-by: Barry Song <Baohua.Song@csr.com> If you want to use some other handler data than the gpiochip, you should just use irq_set_chained_handler() and irq_set_handler_data() directly. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data @ 2014-04-23 20:17 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:17 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > we set bank as handler_data, now we can get bank in ISR directly > instead of looking-up. > > Signed-off-by: Barry Song <Baohua.Song@csr.com> If you want to use some other handler data than the gpiochip, you should just use irq_set_chained_handler() and irq_set_handler_data() directly. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data 2014-04-23 20:17 ` Linus Walleij @ 2014-04-23 22:59 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-23 22:59 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song 2014-04-24 4:17 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>: > On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > >> From: Barry Song <Baohua.Song@csr.com> >> >> we set bank as handler_data, now we can get bank in ISR directly >> instead of looking-up. >> >> Signed-off-by: Barry Song <Baohua.Song@csr.com> > > If you want to use some other handler data than the gpiochip, > you should just use irq_set_chained_handler() and > irq_set_handler_data() directly. i think this should be fixed in the general API but not use one more function call to over-write the handler_data which has been filled in the API. since we have the chance for drivers to set either the whole chip for a simple chip, or bank-specific data for a chip which has multiple parent IRQs. > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data @ 2014-04-23 22:59 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-23 22:59 UTC (permalink / raw) To: linux-arm-kernel 2014-04-24 4:17 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>: > On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > >> From: Barry Song <Baohua.Song@csr.com> >> >> we set bank as handler_data, now we can get bank in ISR directly >> instead of looking-up. >> >> Signed-off-by: Barry Song <Baohua.Song@csr.com> > > If you want to use some other handler data than the gpiochip, > you should just use irq_set_chained_handler() and > irq_set_handler_data() directly. i think this should be fixed in the general API but not use one more function call to over-write the handler_data which has been filled in the API. since we have the chance for drivers to set either the whole chip for a simple chip, or bank-specific data for a chip which has multiple parent IRQs. > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data 2014-04-23 22:59 ` Barry Song @ 2014-04-24 12:59 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-24 12:59 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Thu, Apr 24, 2014 at 12:59 AM, Barry Song <21cnbao@gmail.com> wrote: >> If you want to use some other handler data than the gpiochip, >> you should just use irq_set_chained_handler() and >> irq_set_handler_data() directly. > > i think this should be fixed in the general API but not use one more > function call to over-write the handler_data which has been filled in > the API. > since we have the chance for drivers to set either the whole chip for > a simple chip, or bank-specific data for a chip which has multiple > parent IRQs. I don't think it's worth it for saving one line. The helper is intended for the simple case, i.e. where it's enough to just get the gpio_chip as handler data. Other alternatives need to be open coded. I'm even considering removing this helper if it's confusing, it doesn't really add much, gpiochip_irqchip_add() is the important function to use, not gpiochip_set_chained_irqchip(). Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data @ 2014-04-24 12:59 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-24 12:59 UTC (permalink / raw) To: linux-arm-kernel On Thu, Apr 24, 2014 at 12:59 AM, Barry Song <21cnbao@gmail.com> wrote: >> If you want to use some other handler data than the gpiochip, >> you should just use irq_set_chained_handler() and >> irq_set_handler_data() directly. > > i think this should be fixed in the general API but not use one more > function call to over-write the handler_data which has been filled in > the API. > since we have the chance for drivers to set either the whole chip for > a simple chip, or bank-specific data for a chip which has multiple > parent IRQs. I don't think it's worth it for saving one line. The helper is intended for the simple case, i.e. where it's enough to just get the gpio_chip as handler data. Other alternatives need to be open coded. I'm even considering removing this helper if it's confusing, it doesn't really add much, gpiochip_irqchip_add() is the important function to use, not gpiochip_set_chained_irqchip(). Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data 2014-04-24 12:59 ` Linus Walleij @ 2014-04-24 15:45 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-24 15:45 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song 2014-04-24 20:59 GMT+08:00, Linus Walleij <linus.walleij@linaro.org>: > On Thu, Apr 24, 2014 at 12:59 AM, Barry Song <21cnbao@gmail.com> wrote: > >>> If you want to use some other handler data than the gpiochip, >>> you should just use irq_set_chained_handler() and >>> irq_set_handler_data() directly. >> >> i think this should be fixed in the general API but not use one more >> function call to over-write the handler_data which has been filled in >> the API. >> since we have the chance for drivers to set either the whole chip for >> a simple chip, or bank-specific data for a chip which has multiple >> parent IRQs. > > I don't think it's worth it for saving one line. The helper is intended > for the simple case, i.e. where it's enough to just get the gpio_chip > as handler data. Other alternatives need to be open coded. > gpiochip_set_chained_irqchip() only save one line for simple gpiochip too. it is not worth as well. people can realize what he should set as irq_handler. > I'm even considering removing this helper if it's confusing, it doesn't > really add much, gpiochip_irqchip_add() is the important function > to use, not gpiochip_set_chained_irqchip(). > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data @ 2014-04-24 15:45 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-24 15:45 UTC (permalink / raw) To: linux-arm-kernel 2014-04-24 20:59 GMT+08:00, Linus Walleij <linus.walleij@linaro.org>: > On Thu, Apr 24, 2014 at 12:59 AM, Barry Song <21cnbao@gmail.com> wrote: > >>> If you want to use some other handler data than the gpiochip, >>> you should just use irq_set_chained_handler() and >>> irq_set_handler_data() directly. >> >> i think this should be fixed in the general API but not use one more >> function call to over-write the handler_data which has been filled in >> the API. >> since we have the chance for drivers to set either the whole chip for >> a simple chip, or bank-specific data for a chip which has multiple >> parent IRQs. > > I don't think it's worth it for saving one line. The helper is intended > for the simple case, i.e. where it's enough to just get the gpio_chip > as handler data. Other alternatives need to be open coded. > gpiochip_set_chained_irqchip() only save one line for simple gpiochip too. it is not worth as well. people can realize what he should set as irq_handler. > I'm even considering removing this helper if it's confusing, it doesn't > really add much, gpiochip_irqchip_add() is the important function > to use, not gpiochip_set_chained_irqchip(). > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers 2014-04-15 6:43 ` Barry Song @ 2014-04-16 9:54 ` Javier Martinez Canillas -1 siblings, 0 replies; 48+ messages in thread From: Javier Martinez Canillas @ 2014-04-16 9:54 UTC (permalink / raw) To: Barry Song Cc: Linus Walleij, Linux GPIO List, linux-arm-kernel, workgroup.linux, Barry Song Hello Barry, On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". > and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased > from Linus Walleij's patch too. > > Barry Song (3): > pinctrl: sirf: wrap all gpio banks into one gpio_chip > gpio: make handler_data configurable while using gpiolib_irqchip > pinctrl: sirf: move to use irq_get_handler_data > Linus Walleij (1): > pinctrl: sirf: switch driver to use gpiolib irqchip helpers > > drivers/gpio/gpiolib.c | 10 +- > drivers/pinctrl/Kconfig | 1 + > drivers/pinctrl/sirf/pinctrl-sirf.c | 264 ++++++++++++----------------------- > include/linux/gpio/driver.h | 2 +- > 4 files changed, 97 insertions(+), 180 deletions(-) > > -- > 1.7.5.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Maybe you can first do the parameter change to gpiochip_set_chained_irqchip() and then just convert to gpiolib irqchip passing the struct sirfsoc_gpio_bank pointer instead of a struct gpio_chip. I think there is no need to split PATCH 2 and PATCH 4 but it is up to you. The series looks good to me: Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org> ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers @ 2014-04-16 9:54 ` Javier Martinez Canillas 0 siblings, 0 replies; 48+ messages in thread From: Javier Martinez Canillas @ 2014-04-16 9:54 UTC (permalink / raw) To: linux-arm-kernel Hello Barry, On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". > and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased > from Linus Walleij's patch too. > > Barry Song (3): > pinctrl: sirf: wrap all gpio banks into one gpio_chip > gpio: make handler_data configurable while using gpiolib_irqchip > pinctrl: sirf: move to use irq_get_handler_data > Linus Walleij (1): > pinctrl: sirf: switch driver to use gpiolib irqchip helpers > > drivers/gpio/gpiolib.c | 10 +- > drivers/pinctrl/Kconfig | 1 + > drivers/pinctrl/sirf/pinctrl-sirf.c | 264 ++++++++++++----------------------- > include/linux/gpio/driver.h | 2 +- > 4 files changed, 97 insertions(+), 180 deletions(-) > > -- > 1.7.5.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Maybe you can first do the parameter change to gpiochip_set_chained_irqchip() and then just convert to gpiolib irqchip passing the struct sirfsoc_gpio_bank pointer instead of a struct gpio_chip. I think there is no need to split PATCH 2 and PATCH 4 but it is up to you. The series looks good to me: Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org> ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers 2014-04-16 9:54 ` Javier Martinez Canillas @ 2014-04-16 10:05 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-16 10:05 UTC (permalink / raw) To: Javier Martinez Canillas Cc: Linus Walleij, Linux GPIO List, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song 2014-04-16 17:54 GMT+08:00 Javier Martinez Canillas <javier@dowhile0.org>: > Hello Barry, > > On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: >> From: Barry Song <Baohua.Song@csr.com> >> >> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". >> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased >> from Linus Walleij's patch too. >> >> Barry Song (3): >> pinctrl: sirf: wrap all gpio banks into one gpio_chip >> gpio: make handler_data configurable while using gpiolib_irqchip >> pinctrl: sirf: move to use irq_get_handler_data >> Linus Walleij (1): >> pinctrl: sirf: switch driver to use gpiolib irqchip helpers >> >> drivers/gpio/gpiolib.c | 10 +- >> drivers/pinctrl/Kconfig | 1 + >> drivers/pinctrl/sirf/pinctrl-sirf.c | 264 ++++++++++++----------------------- >> include/linux/gpio/driver.h | 2 +- >> 4 files changed, 97 insertions(+), 180 deletions(-) >> >> -- >> 1.7.5.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > Maybe you can first do the parameter change to > gpiochip_set_chained_irqchip() and then just convert to gpiolib > irqchip passing the struct sirfsoc_gpio_bank pointer instead of a > struct gpio_chip. I think there is no need to split PATCH 2 and PATCH > 4 but it is up to you. > Javier, thanks. the reason for splitting 2 and 4 is that i don't want to lose the contribution of Linus for "gpiolib irqchip helpers". otherwise, i will be the author of the merged patch. > The series looks good to me: > > Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org> -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers @ 2014-04-16 10:05 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-16 10:05 UTC (permalink / raw) To: linux-arm-kernel 2014-04-16 17:54 GMT+08:00 Javier Martinez Canillas <javier@dowhile0.org>: > Hello Barry, > > On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: >> From: Barry Song <Baohua.Song@csr.com> >> >> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". >> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased >> from Linus Walleij's patch too. >> >> Barry Song (3): >> pinctrl: sirf: wrap all gpio banks into one gpio_chip >> gpio: make handler_data configurable while using gpiolib_irqchip >> pinctrl: sirf: move to use irq_get_handler_data >> Linus Walleij (1): >> pinctrl: sirf: switch driver to use gpiolib irqchip helpers >> >> drivers/gpio/gpiolib.c | 10 +- >> drivers/pinctrl/Kconfig | 1 + >> drivers/pinctrl/sirf/pinctrl-sirf.c | 264 ++++++++++++----------------------- >> include/linux/gpio/driver.h | 2 +- >> 4 files changed, 97 insertions(+), 180 deletions(-) >> >> -- >> 1.7.5.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in >> the body of a message to majordomo at vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > Maybe you can first do the parameter change to > gpiochip_set_chained_irqchip() and then just convert to gpiolib > irqchip passing the struct sirfsoc_gpio_bank pointer instead of a > struct gpio_chip. I think there is no need to split PATCH 2 and PATCH > 4 but it is up to you. > Javier, thanks. the reason for splitting 2 and 4 is that i don't want to lose the contribution of Linus for "gpiolib irqchip helpers". otherwise, i will be the author of the merged patch. > The series looks good to me: > > Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org> -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers 2014-04-16 10:05 ` Barry Song @ 2014-04-16 10:20 ` Javier Martinez Canillas -1 siblings, 0 replies; 48+ messages in thread From: Javier Martinez Canillas @ 2014-04-16 10:20 UTC (permalink / raw) To: Barry Song Cc: Linus Walleij, Linux GPIO List, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Wed, Apr 16, 2014 at 12:05 PM, Barry Song <21cnbao@gmail.com> wrote: > 2014-04-16 17:54 GMT+08:00 Javier Martinez Canillas <javier@dowhile0.org>: >> Hello Barry, >> >> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: >>> From: Barry Song <Baohua.Song@csr.com> >>> >>> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". >>> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased >>> from Linus Walleij's patch too. >>> >>> Barry Song (3): >>> pinctrl: sirf: wrap all gpio banks into one gpio_chip >>> gpio: make handler_data configurable while using gpiolib_irqchip >>> pinctrl: sirf: move to use irq_get_handler_data >>> Linus Walleij (1): >>> pinctrl: sirf: switch driver to use gpiolib irqchip helpers >>> >>> drivers/gpio/gpiolib.c | 10 +- >>> drivers/pinctrl/Kconfig | 1 + >>> drivers/pinctrl/sirf/pinctrl-sirf.c | 264 ++++++++++++----------------------- >>> include/linux/gpio/driver.h | 2 +- >>> 4 files changed, 97 insertions(+), 180 deletions(-) >>> >>> -- >>> 1.7.5.4 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> Maybe you can first do the parameter change to >> gpiochip_set_chained_irqchip() and then just convert to gpiolib >> irqchip passing the struct sirfsoc_gpio_bank pointer instead of a >> struct gpio_chip. I think there is no need to split PATCH 2 and PATCH >> 4 but it is up to you. >> > > Javier, thanks. the reason for splitting 2 and 4 is that i don't want > to lose the contribution of Linus for "gpiolib irqchip helpers". > otherwise, i will be the author of the merged patch. > Right, I didn't notice PATCH 2 was from Linus because it didn't have his Signed-off-by but yours (although now I notice that it was sent with him as the author). AFAIK you can still squash both patches and in that case the convention is to add the original author s-o-b and then explain your contributions on top. i.e: From: Linus Walleij <linus.walleij@linaro.org> This switches the SiRF pinctrl driver over to using the gpiolib irqchip helpers simplifying some of the code. [Baohua.Song@csr.com: move to use irq_get_handler_data] Signed-off-by: Linus Walleij <linus.walleij@linaro.org> But it's also OK to have split in two patches, I just asked out of curiosity :-) Best regards, Javier > >> The series looks good to me: >> >> Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org> > > -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers @ 2014-04-16 10:20 ` Javier Martinez Canillas 0 siblings, 0 replies; 48+ messages in thread From: Javier Martinez Canillas @ 2014-04-16 10:20 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 16, 2014 at 12:05 PM, Barry Song <21cnbao@gmail.com> wrote: > 2014-04-16 17:54 GMT+08:00 Javier Martinez Canillas <javier@dowhile0.org>: >> Hello Barry, >> >> On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: >>> From: Barry Song <Baohua.Song@csr.com> >>> >>> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". >>> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased >>> from Linus Walleij's patch too. >>> >>> Barry Song (3): >>> pinctrl: sirf: wrap all gpio banks into one gpio_chip >>> gpio: make handler_data configurable while using gpiolib_irqchip >>> pinctrl: sirf: move to use irq_get_handler_data >>> Linus Walleij (1): >>> pinctrl: sirf: switch driver to use gpiolib irqchip helpers >>> >>> drivers/gpio/gpiolib.c | 10 +- >>> drivers/pinctrl/Kconfig | 1 + >>> drivers/pinctrl/sirf/pinctrl-sirf.c | 264 ++++++++++++----------------------- >>> include/linux/gpio/driver.h | 2 +- >>> 4 files changed, 97 insertions(+), 180 deletions(-) >>> >>> -- >>> 1.7.5.4 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in >>> the body of a message to majordomo at vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> Maybe you can first do the parameter change to >> gpiochip_set_chained_irqchip() and then just convert to gpiolib >> irqchip passing the struct sirfsoc_gpio_bank pointer instead of a >> struct gpio_chip. I think there is no need to split PATCH 2 and PATCH >> 4 but it is up to you. >> > > Javier, thanks. the reason for splitting 2 and 4 is that i don't want > to lose the contribution of Linus for "gpiolib irqchip helpers". > otherwise, i will be the author of the merged patch. > Right, I didn't notice PATCH 2 was from Linus because it didn't have his Signed-off-by but yours (although now I notice that it was sent with him as the author). AFAIK you can still squash both patches and in that case the convention is to add the original author s-o-b and then explain your contributions on top. i.e: From: Linus Walleij <linus.walleij@linaro.org> This switches the SiRF pinctrl driver over to using the gpiolib irqchip helpers simplifying some of the code. [Baohua.Song at csr.com: move to use irq_get_handler_data] Signed-off-by: Linus Walleij <linus.walleij@linaro.org> But it's also OK to have split in two patches, I just asked out of curiosity :-) Best regards, Javier > >> The series looks good to me: >> >> Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org> > > -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers 2014-04-15 6:43 ` Barry Song @ 2014-04-22 21:47 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-22 21:47 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". > and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased > from Linus Walleij's patch too. > > Barry Song (3): > pinctrl: sirf: wrap all gpio banks into one gpio_chip > gpio: make handler_data configurable while using gpiolib_irqchip > pinctrl: sirf: move to use irq_get_handler_data > Linus Walleij (1): > pinctrl: sirf: switch driver to use gpiolib irqchip helpers As mentioned I can't get this series to apply so please rebase, resend and take this opportunity to also add relevant ACKs. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers @ 2014-04-22 21:47 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-22 21:47 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > From: Barry Song <Baohua.Song@csr.com> > > This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". > and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased > from Linus Walleij's patch too. > > Barry Song (3): > pinctrl: sirf: wrap all gpio banks into one gpio_chip > gpio: make handler_data configurable while using gpiolib_irqchip > pinctrl: sirf: move to use irq_get_handler_data > Linus Walleij (1): > pinctrl: sirf: switch driver to use gpiolib irqchip helpers As mentioned I can't get this series to apply so please rebase, resend and take this opportunity to also add relevant ACKs. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers 2014-04-22 21:47 ` Linus Walleij @ 2014-04-23 12:09 ` Barry Song -1 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-23 12:09 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song 2014-04-23 5:47 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>: > On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > >> From: Barry Song <Baohua.Song@csr.com> >> >> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". >> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased >> from Linus Walleij's patch too. >> >> Barry Song (3): >> pinctrl: sirf: wrap all gpio banks into one gpio_chip >> gpio: make handler_data configurable while using gpiolib_irqchip >> pinctrl: sirf: move to use irq_get_handler_data >> Linus Walleij (1): >> pinctrl: sirf: switch driver to use gpiolib irqchip helpers > > As mentioned I can't get this series to apply so please rebase, > resend and take this opportunity to also add relevant ACKs. linus, this patchset depends on your "pinctrl: sirf: rename inlined accessor" you sent to me. it seems it is in neither your devel nor 3.5-rc2. https://lkml.org/lkml/2014/4/8/156 > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers @ 2014-04-23 12:09 ` Barry Song 0 siblings, 0 replies; 48+ messages in thread From: Barry Song @ 2014-04-23 12:09 UTC (permalink / raw) To: linux-arm-kernel 2014-04-23 5:47 GMT+08:00 Linus Walleij <linus.walleij@linaro.org>: > On Tue, Apr 15, 2014 at 8:43 AM, Barry Song <21cnbao@gmail.com> wrote: > >> From: Barry Song <Baohua.Song@csr.com> >> >> This patchset depends on Linus Walleij's "pinctrl: sirf: rename inlined accessor". >> and "pinctrl: sirf: switch driver to use gpiolib irqchip helpers" is rebased >> from Linus Walleij's patch too. >> >> Barry Song (3): >> pinctrl: sirf: wrap all gpio banks into one gpio_chip >> gpio: make handler_data configurable while using gpiolib_irqchip >> pinctrl: sirf: move to use irq_get_handler_data >> Linus Walleij (1): >> pinctrl: sirf: switch driver to use gpiolib irqchip helpers > > As mentioned I can't get this series to apply so please rebase, > resend and take this opportunity to also add relevant ACKs. linus, this patchset depends on your "pinctrl: sirf: rename inlined accessor" you sent to me. it seems it is in neither your devel nor 3.5-rc2. https://lkml.org/lkml/2014/4/8/156 > > Yours, > Linus Walleij -barry ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers 2014-04-23 12:09 ` Barry Song @ 2014-04-23 20:02 ` Linus Walleij -1 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:02 UTC (permalink / raw) To: Barry Song Cc: linux-gpio, linux-arm-kernel, DL-SHA-WorkGroupLinux, Barry Song On Wed, Apr 23, 2014 at 2:09 PM, Barry Song <21cnbao@gmail.com> wrote: > linus, this patchset depends on your "pinctrl: sirf: rename inlined > accessor" you sent to me. it seems it is in neither your devel nor > 3.5-rc2. Ah, sorry I had that on a separate branch. Retrying... Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers @ 2014-04-23 20:02 ` Linus Walleij 0 siblings, 0 replies; 48+ messages in thread From: Linus Walleij @ 2014-04-23 20:02 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 23, 2014 at 2:09 PM, Barry Song <21cnbao@gmail.com> wrote: > linus, this patchset depends on your "pinctrl: sirf: rename inlined > accessor" you sent to me. it seems it is in neither your devel nor > 3.5-rc2. Ah, sorry I had that on a separate branch. Retrying... Yours, Linus Walleij ^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2014-04-24 15:45 UTC | newest] Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-04-15 6:43 [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers Barry Song 2014-04-15 6:43 ` Barry Song 2014-04-15 6:43 ` [PATCH 1/4] pinctrl: sirf: wrap all gpio banks into one gpio_chip Barry Song 2014-04-15 6:43 ` Barry Song 2014-04-22 18:27 ` Linus Walleij 2014-04-22 18:27 ` Linus Walleij 2014-04-23 20:03 ` Linus Walleij 2014-04-23 20:03 ` Linus Walleij 2014-04-15 6:43 ` [PATCH 2/4] pinctrl: sirf: switch driver to use gpiolib irqchip helpers Barry Song 2014-04-15 6:43 ` Barry Song 2014-04-23 20:04 ` Linus Walleij 2014-04-23 20:04 ` Linus Walleij 2014-04-15 6:43 ` [PATCH 3/4] gpio: make handler_data configurable while using gpiolib_irqchip Barry Song 2014-04-15 6:43 ` Barry Song 2014-04-23 20:13 ` Linus Walleij 2014-04-23 20:13 ` Linus Walleij 2014-04-23 22:55 ` Barry Song 2014-04-23 22:55 ` Barry Song 2014-04-23 23:09 ` Linus Walleij 2014-04-23 23:09 ` Linus Walleij 2014-04-23 23:23 ` Barry Song 2014-04-23 23:23 ` Barry Song 2014-04-24 13:06 ` Linus Walleij 2014-04-24 13:06 ` Linus Walleij 2014-04-24 15:43 ` Barry Song 2014-04-24 15:43 ` Barry Song 2014-04-15 6:43 ` [PATCH 4/4] pinctrl: sirf: move to use irq_get_handler_data Barry Song 2014-04-15 6:43 ` Barry Song 2014-04-23 20:17 ` Linus Walleij 2014-04-23 20:17 ` Linus Walleij 2014-04-23 22:59 ` Barry Song 2014-04-23 22:59 ` Barry Song 2014-04-24 12:59 ` Linus Walleij 2014-04-24 12:59 ` Linus Walleij 2014-04-24 15:45 ` Barry Song 2014-04-24 15:45 ` Barry Song 2014-04-16 9:54 ` [PATCH 0/4] pinctrl: sirf: switch to one gpiolib and gpiolib_irqchip helpers Javier Martinez Canillas 2014-04-16 9:54 ` Javier Martinez Canillas 2014-04-16 10:05 ` Barry Song 2014-04-16 10:05 ` Barry Song 2014-04-16 10:20 ` Javier Martinez Canillas 2014-04-16 10:20 ` Javier Martinez Canillas 2014-04-22 21:47 ` Linus Walleij 2014-04-22 21:47 ` Linus Walleij 2014-04-23 12:09 ` Barry Song 2014-04-23 12:09 ` Barry Song 2014-04-23 20:02 ` Linus Walleij 2014-04-23 20:02 ` Linus Walleij
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.