| Message ID | 1397544229-20545-5-git-send-email-21cnbao@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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);