| Message ID | 1413384491-23703-4-git-send-email-octavian.purdila@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Oct 15, 2014 at 11:48 PM, Octavian Purdila <octavian.purdila@intel.com> wrote: > Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set > operation but do not need a threaded irq handler. Sorry if you already explained this (I have been a little bit late with the GPIO reviews recently), but does this optimization bring a significant benefit that justifies adding another field in struct gpio_chip? If so it would be nice to have it in the commit message. If not, do we need this at all? -- 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
On Mon, Oct 20, 2014 at 8:08 AM, Alexandre Courbot <gnurou@gmail.com> wrote: > > On Wed, Oct 15, 2014 at 11:48 PM, Octavian Purdila > <octavian.purdila@intel.com> wrote: > > Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set > > operation but do not need a threaded irq handler. > > Sorry if you already explained this (I have been a little bit late > with the GPIO reviews recently), but does this optimization bring a > significant benefit that justifies adding another field in struct > gpio_chip? If so it would be nice to have it in the commit message. If > not, do we need this at all? Hi Alexandre, In the case DLN2 USB GPIO adapter the GPIO interrupt is generated in the completion routine of a receive URB, which means that we are in interrupt context. If a threaded irq is used, we would have to schedule work instead of running to interrupt handler directly which is unnecessary and adds latency. BTW, AFAIC Linus W already merged this patch in his next tree, I am keeping it in this series because it was not pulled in the mfd-next tree. Thanks, Tavi -- 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
On Mon, Oct 20, 2014 at 7:19 PM, Octavian Purdila <octavian.purdila@intel.com> wrote: > On Mon, Oct 20, 2014 at 8:08 AM, Alexandre Courbot <gnurou@gmail.com> wrote: >> >> On Wed, Oct 15, 2014 at 11:48 PM, Octavian Purdila >> <octavian.purdila@intel.com> wrote: >> > Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set >> > operation but do not need a threaded irq handler. >> >> Sorry if you already explained this (I have been a little bit late >> with the GPIO reviews recently), but does this optimization bring a >> significant benefit that justifies adding another field in struct >> gpio_chip? If so it would be nice to have it in the commit message. If >> not, do we need this at all? > > Hi Alexandre, > > In the case DLN2 USB GPIO adapter the GPIO interrupt is generated in > the completion routine of a receive URB, which means that we are in > interrupt context. If a threaded irq is used, we would have to > schedule work instead of running to interrupt handler directly which > is unnecessary and adds latency. > > BTW, AFAIC Linus W already merged this patch in his next tree, I am > keeping it in this series because it was not pulled in the mfd-next > tree. You're right, it's all good then. Thanks for the explanation. -- 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/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 15cc0bb..3fa7e73 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -447,7 +447,7 @@ static int gpiochip_irq_map(struct irq_domain *d, unsigned int irq, irq_set_lockdep_class(irq, &gpiochip_irq_lock_class); irq_set_chip_and_handler(irq, chip->irqchip, chip->irq_handler); /* Chips that can sleep need nested thread handlers */ - if (chip->can_sleep) + if (chip->can_sleep && !chip->irq_not_threaded) irq_set_nested_thread(irq, 1); #ifdef CONFIG_ARM set_irq_flags(irq, IRQF_VALID); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index e78a237..44161ac 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -56,6 +56,8 @@ struct seq_file; * as the chip access may sleep when e.g. reading out the IRQ status * registers. * @exported: flags if the gpiochip is exported for use from sysfs. Private. + * @irq_not_threaded: flag must be set if @can_sleep is set but the + * IRQs don't need to be threaded * * A gpio_chip can help platforms abstract various sources of GPIOs so * they can all be accessed through a common programing interface. @@ -101,6 +103,7 @@ struct gpio_chip { struct gpio_desc *desc; const char *const *names; bool can_sleep; + bool irq_not_threaded; bool exported; #ifdef CONFIG_GPIOLIB_IRQCHIP
Some GPIO chips (e.g. the DLN2 USB adapter) have blocking get/set operation but do not need a threaded irq handler. Signed-off-by: Octavian Purdila <octavian.purdila@intel.com> --- drivers/gpio/gpiolib.c | 2 +- include/linux/gpio/driver.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)