| Message ID | 1389777042-2363-1-git-send-email-linus.walleij@linaro.org |
|---|---|
| State | Accepted |
| Commit | 655dada62779917e2337b9abf8ab95e891cc0938 |
| Headers | show |
On Mon, Jan 27, 2014 at 8:01 AM, Barry Song <21cnbao@gmail.com> wrote: > i read the patch "gpio: add API to be strict about GPIO IRQ usage" > again, it seems by checking if (test_bit(FLAG_USED_AS_IRQ, > &desc->flags)), we do be able to stop users setting irqline to output. Yes that is the specific use of that flag right now. > but my concern is actually that: when users use request_irq to get the > irq line from gpio, in that case, gpio_request() probably will not be > called at all, This question has come up a lot recently as we recently established that irqchip and gpiolib APIs are orthogonal, just like you say. > who will fix the availability of this pin if it is > muxed with other functionality except gpio? So the problem is with the established semantic that irqchips and gpiochips are orthogonal, not with this particular patch, right? Muxing is the responsibility of the pin control pin mux subsystem and not GPIO. If you require pinctrl_request_gpio() to be called from the gpiolib driver for the pin to be muxed in, then this function must be called from the irqchip side as well, e.g. from .irq_startup(). > but if gpio_request() is called, in it, pinctrl_request_gpio() is > called as well, which will mark the pin as not-free. so do you think > we need to call pinctrl_request_gpio() in startup() of irqline as > well? I think so. > but this might be buggy again since people might call > gpio_request() before calling request_irq(). Hm, you should study this problem and suggest the best solution. In general I do not think it hurts to call pinctrl_request_gpio() multiple times, and if it does maybe that is something we want to fix? Yours, Linus Walleij -- 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
diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c index b637f5ab022c..a0d6152701cd 100644 --- a/drivers/pinctrl/sirf/pinctrl-sirf.c +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c @@ -594,12 +594,34 @@ static int sirfsoc_gpio_irq_type(struct irq_data *d, unsigned type) return 0; } +static unsigned int sirfsoc_gpio_irq_startup(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)) + dev_err(bank->chip.gc.dev, + "unable to lock HW IRQ %lu for IRQ\n", + d->hwirq); + sirfsoc_gpio_irq_unmask(d); + return 0; +} + +static void sirfsoc_gpio_irq_shutdown(struct irq_data *d) +{ + struct sirfsoc_gpio_bank *bank = irq_data_get_irq_chip_data(d); + + sirfsoc_gpio_irq_mask(d); + gpio_unlock_as_irq(&bank->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_startup = sirfsoc_gpio_irq_startup, + .irq_shutdown = sirfsoc_gpio_irq_shutdown, }; static void sirfsoc_gpio_handle_irq(unsigned int irq, struct irq_desc *desc) @@ -877,6 +899,7 @@ static int sirfsoc_gpio_probe(struct device_node *np) 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;
This uses the new API for tagging GPIO lines as in use by IRQs. This enforces a few semantic checks on how the underlying GPIO line is used. Also assign the gpio_chip.dev pointer to be used for error messages. Cc: Barry Song <Baohua.Song@csr.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Hi Barry, it's be great if you could test this so I can just add it to the v3.14 series. --- drivers/pinctrl/sirf/pinctrl-sirf.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)