* gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? @ 2021-06-28 3:36 Vincent Pelletier 2021-06-28 13:40 ` Andy Shevchenko 2021-07-02 0:09 ` Linus Walleij 0 siblings, 2 replies; 10+ messages in thread From: Vincent Pelletier @ 2021-06-28 3:36 UTC (permalink / raw) To: Linus Walleij, Bartosz Golaszewski, linux-gpio, Linux Kernel Mailing List Hello, While trying to debug an IRQ handling issue on a sifive-unmatched board (which is a very recent board on a recent architecture, so I would not be overly surprised if there were bugs in hiding), I realised that I was able to claim via sysfs GPIO pins which are being actively used as IRQ sources. Checking drivers/gpio/gpiolib.c and kernel/irq/chip.c, I believe this is because gpiolib (gpiochip_irq_reqres, gpiochip_reqres_irq, gpiochip_lock_as_irq) does not call gpiod_request_{,commit}, resulting in a pin which is available for use. I could confirm this by adding (just as a debugging aid): WARN_ON(!test_bit(FLAG_REQUESTED, &desc->flags)); early in gpiochip_lock_as_irq, and this statement gets triggered. Is this intentional ? Does this requesting belong to something else in the codepath from request_threaded_irq (and similar) ? Could it be something missing in the devicetree for this board ? Also, I notice that both gpiochip_hierarchy_add_domain and gpiochip_reqres_irq call gpiochip_lock_as_irq, and I am surprised I do not get any error about this: in my understanding only the first call on a given pin should succeed, but with my WARN_ON I am seeing both stack traces and no other warning. FWIW, my builds are based on vanilla 5.13-rc6. Regards, -- Vincent Pelletier ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-06-28 3:36 gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? Vincent Pelletier @ 2021-06-28 13:40 ` Andy Shevchenko 2021-06-28 13:42 ` Andy Shevchenko 2021-07-02 0:09 ` Linus Walleij 1 sibling, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2021-06-28 13:40 UTC (permalink / raw) To: Vincent Pelletier Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Mon, Jun 28, 2021 at 6:37 AM Vincent Pelletier <plr.vincent@gmail.com> wrote: > > Hello, > > While trying to debug an IRQ handling issue on a sifive-unmatched board > (which is a very recent board on a recent architecture, so I would not > be overly surprised if there were bugs in hiding), I realised that I was able > to claim via sysfs GPIO pins which are being actively used as IRQ sources. > > Checking drivers/gpio/gpiolib.c and kernel/irq/chip.c, I believe this is because > gpiolib (gpiochip_irq_reqres, gpiochip_reqres_irq, gpiochip_lock_as_irq) > does not call gpiod_request_{,commit}, resulting in a pin which is available > for use. I could confirm this by adding (just as a debugging aid): > WARN_ON(!test_bit(FLAG_REQUESTED, &desc->flags)); > early in gpiochip_lock_as_irq, and this statement gets triggered. > > Is this intentional ? IIRC the GPIO can be locked as IRQ without being requested (perhaps for legacy/historical reasons). But I forgot all code paths anyway, so I'm expecting that Linus and or Bart can elaborate this better. > Does this requesting belong to something else in the codepath from > request_threaded_irq (and similar) ? > Could it be something missing in the devicetree for this board ? > > Also, I notice that both gpiochip_hierarchy_add_domain and > gpiochip_reqres_irq call gpiochip_lock_as_irq, and I am surprised I do not > get any error about this: in my understanding only the first call on a given pin > should succeed, but with my WARN_ON I am seeing both stack traces and > no other warning. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-06-28 13:40 ` Andy Shevchenko @ 2021-06-28 13:42 ` Andy Shevchenko 2021-06-28 22:52 ` Vincent Pelletier 0 siblings, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2021-06-28 13:42 UTC (permalink / raw) To: Vincent Pelletier Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Mon, Jun 28, 2021 at 4:40 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Jun 28, 2021 at 6:37 AM Vincent Pelletier <plr.vincent@gmail.com> wrote: > > While trying to debug an IRQ handling issue on a sifive-unmatched board > > (which is a very recent board on a recent architecture, so I would not > > be overly surprised if there were bugs in hiding), I realised that I was able > > to claim via sysfs GPIO pins which are being actively used as IRQ sources. And one important note: do NOT use sysfs GPIO interface. Use a GPIO character device instead. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-06-28 13:42 ` Andy Shevchenko @ 2021-06-28 22:52 ` Vincent Pelletier 2021-06-29 10:19 ` Andy Shevchenko 2021-07-02 0:24 ` Linus Walleij 0 siblings, 2 replies; 10+ messages in thread From: Vincent Pelletier @ 2021-06-28 22:52 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Mon, Jun 28, 2021 at 10:42 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > And one important note: do NOT use sysfs GPIO interface. Use a GPIO > character device instead. I am indeed aware of this. My IRQ issue is unrelated to the gpios being claimed by anything, and I was doing it while trying to get more information about the current state of the gpio driver. For more background, the context of my IRQ issue is: PMIC (da9063) /irq -> GPIO pin 1 -> PLIC irq 24 The PMIC has several internal interrupt sources, like the power button being pressed or the ADC conversion completion signal. The first time after a boot that I press the power button, I do get an interrupt and the da9063-onkey driver produces a keypress input event. But any further button press does not produce an IRQ. So something is going wrong in the IRQ acknowledgement. AFAIK the PLIC (platform-level interrupt controller) works: it is used for PCIe interrupts, and those work. The PMIC driver exists since 2013, so I assume any bug would have been identified long ago. But I believe the GPIO level has not handled any interrupt until I enabled the power button event source, and this one is a lot more recent: gpio-sifive.c from late 2019. So this is where I turned my attention. Discovering that the pin is somehow only half-claimed made me wonder if there was some important initialisation step missing, which could maybe be related to these IRQ issues. While on this topic, there is a bullet point in Documentation/driver-api/gpio/driver.rst which I fail to understand: | - Nominally set all handlers to handle_bad_irq() in the setup call and pass | handle_bad_irq() as flow handler parameter in gpiochip_irqchip_add() if it is | expected for GPIO driver that irqchip .set_type() callback will be called | before using/enabling each GPIO IRQ. Then set the handler to | handle_level_irq() and/or handle_edge_irq() in the irqchip .set_type() | callback depending on what your controller supports and what is requested | by the consumer. - why the plural in "set all handlers to handle_bad_irq()" ? Isn't there only a single handler in struct gpio_irq_chip ? - I do not find a function named gpiochip_irqchip_add(), only gpiochip_irqchip_add_domain() - "Then set the handler to [...] in the irqchip .set_type() callback" Isn't set_type per-pin, and isn't the interrupt handler chip-level ? Regards, -- Vincent Pelletier ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-06-28 22:52 ` Vincent Pelletier @ 2021-06-29 10:19 ` Andy Shevchenko 2021-07-01 13:36 ` Vincent Pelletier 2021-07-02 0:24 ` Linus Walleij 1 sibling, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2021-06-29 10:19 UTC (permalink / raw) To: Vincent Pelletier Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Tue, Jun 29, 2021 at 1:52 AM Vincent Pelletier <plr.vincent@gmail.com> wrote: > > On Mon, Jun 28, 2021 at 10:42 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > And one important note: do NOT use sysfs GPIO interface. Use a GPIO > > character device instead. > > I am indeed aware of this. My IRQ issue is unrelated to the gpios > being claimed by anything, and I was doing it while trying to get > more information about the current state of the gpio driver. > > For more background, the context of my IRQ issue is: > PMIC (da9063) /irq -> GPIO pin 1 -> PLIC irq 24 > The PMIC has several internal interrupt sources, like the power > button being pressed or the ADC conversion completion signal. This is the usual case with PMIC. We have similar on x86 machines (PMIC is represented by an MFD driver with regmap IRQ being involved). > The first time after a boot that I press the power button, I do > get an interrupt and the da9063-onkey driver produces a keypress > input event. > But any further button press does not produce an IRQ. So something > is going wrong in the IRQ acknowledgement. > AFAIK the PLIC (platform-level interrupt controller) works: it is > used for PCIe interrupts, and those work. > The PMIC driver exists since 2013, so I assume any bug would have > been identified long ago. > But I believe the GPIO level has not handled any interrupt until I > enabled the power button event source, and this one is a lot more > recent: gpio-sifive.c from late 2019. So this is where I turned my > attention. Discovering that the pin is somehow only half-claimed > made me wonder if there was some important initialisation step > missing, which could maybe be related to these IRQ issues. > > While on this topic, there is a bullet point in > Documentation/driver-api/gpio/driver.rst which I fail to understand: > > | - Nominally set all handlers to handle_bad_irq() in the setup call and pass > | handle_bad_irq() as flow handler parameter in > gpiochip_irqchip_add() if it is > | expected for GPIO driver that irqchip .set_type() callback will be called > | before using/enabling each GPIO IRQ. Then set the handler to > | handle_level_irq() and/or handle_edge_irq() in the irqchip .set_type() > | callback depending on what your controller supports and what is requested > | by the consumer. > > - why the plural in "set all handlers to handle_bad_irq()" ? Isn't > there only a single handler in struct gpio_irq_chip ? Each GPIO line may have its own handler (usually level or edge). I guess it's written from the GPIO point of view. > - I do not find a function named gpiochip_irqchip_add(), only > gpiochip_irqchip_add_domain() Missed during update I suppose. https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git/commit/?h=gpio/for-next&id=f1f37abbe6fc2b1242f78157db76e48dbf9518ee Feel free to submit a patch! > - "Then set the handler to [...] in the irqchip .set_type() callback" > Isn't set_type per-pin, and isn't the interrupt handler chip-level ? The idea behind that initially the chip-level IRQ handler is set to BAD. It means any (spurious) IRQ will be served by it. Now, when one requests IRQ the framework will call ->irq_set_type() of corresponding IRQ chip and change the handler for the certain pin (pin-level). So, the main handler is basically for spurious interrupts only. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-06-29 10:19 ` Andy Shevchenko @ 2021-07-01 13:36 ` Vincent Pelletier 0 siblings, 0 replies; 10+ messages in thread From: Vincent Pelletier @ 2021-07-01 13:36 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List Hello, On Tue, Jun 29, 2021 at 7:19 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > - why the plural in "set all handlers to handle_bad_irq()" ? Isn't > > there only a single handler in struct gpio_irq_chip ? > > Each GPIO line may have its own handler (usually level or edge). I > guess it's written from the GPIO point of view. [...] > > - "Then set the handler to [...] in the irqchip .set_type() callback" > > Isn't set_type per-pin, and isn't the interrupt handler chip-level ? > > The idea behind that initially the chip-level IRQ handler is set to > BAD. It means any (spurious) IRQ will be served by it. Now, when one > requests IRQ the framework will call ->irq_set_type() of corresponding > IRQ chip and change the handler for the certain pin (pin-level). So, > the main handler is basically for spurious interrupts only. I think I found what I was missing: I was only seeing (struct gpio_irq_chip *)->handler = handle_bad_irq; and was completely missing irq_set_handler_locked((struct irq_data *), handle_..._irq); hence my confusion about these points. Thanks for this extra push which led me to these. Maybe the doc should mention this function ? > > - I do not find a function named gpiochip_irqchip_add(), only > > gpiochip_irqchip_add_domain() > > Missed during update I suppose. > https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git/commit/?h=gpio/for-next&id=f1f37abbe6fc2b1242f78157db76e48dbf9518ee > Feel free to submit a patch! Makes sense. Before trying to fix the documentation I intended to get to the end of my IRQ issues though, as I feel I am still lacking a lot of understanding which will be needed to produce a decent documentation patch (or three). But I feel I am as far as I can go, as I cannot even tell what is normal and what is not, for lack of what I would identify as a reference setup in other machines. I have no idea what to try next, and I am ashamed to say I am working on this issue for over a week (outside of work hours, but still). So here goes all I spotted, if anyone reading can tell the good from the bad: The high-level issue: I added a devicetree entry to enable the power-button function of a DA9063 PMIC. This is AFAICT the first subsystem of this chip on this board (hifive unmatched) which produces IRQs in normal use (meaning outside of catastrophic failures on the board, like the PMIC complaining about an overcurrent condition). I initially thought it worked fine: short push of the button, system shuts down cleanly. Then I disabled power button handling in systemd-logind, and only the first press causes a key press event, all further presses do nothing. I hacked a script to peek at IRQ registers behind the kernel, and here is what I see: vincent@riscv:~$ sudo ./unmatched_gpio_irq_debug.py 1 GPIO 1: dir=in in=1 out=0 irq_en=low irq_pending=high PLIC 24: irq=False hart=4 vincent@riscv:~$ sudo ./unmatched_gpio_irq_debug.py 1 GPIO 1: dir=in in=1 out=0 irq_en=low irq_pending=rise,high,low PLIC 24: irq=False hart=4 vincent@riscv:~$ sudo ./unmatched_gpio_irq_debug.py 1 GPIO 1: dir=in in=0 out=0 irq_en=low irq_pending=rise,fall,high,low PLIC 24: irq=False hart=4 Note: - GPIO pin 1 is active low, consistently with the da9063 code and the gpio irq_en line. - after the first press, the gpio pin reads as "high", so the irs is ack'ed at the da9063 level - after the first press, the pending irq events are all but "falling", which to me indicate the GPIO-level IRQ was ack'ed while the pin was still low, so it immediately became pending again. Knowing the GPIO driver clears *all* pending interrupts, I understand "rise,high,low" as meaning the GPIO controller saw the pin go from low to high after it was cleared, which also hints that the da9063's IRQ was cleared after the GPIO, which seems wrong for a level interrupt - but I am not 100% sure. - ...but the PLIC 24 pin (corresponding to GPIO 1 pin's IRQ) does not have a pending irq, which suggests it missed the GPIO IRQ re-triggering - and after a second key press, the only change is that now the GPIO chip did see a falling edge, and it now has all its pending bits active for this pin. On the /proc/interrupts front, here is what I see: CPU0 CPU1 CPU2 CPU3 1: 637 0 0 0 SiFive PLIC 39 10010000.serial 2: 0 0 0 0 SiFive PLIC 40 10011000.serial 3: 1829 0 0 0 SiFive PLIC 52 10030000.i2c 4: 0 0 0 0 SiFive PLIC 41 10040000.spi 5: 7647 10727 9670 11094 RISC-V INTC 5 riscv-timer 6: 96 0 0 0 SiFive PLIC 43 10050000.spi 7: 0 0 0 0 SiFive PLIC 55 eth0 17: 0 0 0 0 SiFive PLIC 19 l2_ecc 18: 1 0 0 0 SiFive PLIC 21 l2_ecc 19: 0 0 0 0 SiFive PLIC 22 l2_ecc 20: 0 0 0 0 SiFive PLIC 20 l2_ecc 46: 0 0 0 0 PCI-MSI 0 PCIe PME, aerdrv 53: 22 0 0 0 PCI-MSI 3145728 nvme0q0 54: 1 0 0 0 sifive-gpio 1 da9063-irq 55: 1 0 0 0 da9063-irq 0 ONKEY 56: 0 0 0 0 da9063-irq 1 ALARM 63: 0 0 0 0 da9063-irq 8 LDO_LIM 84: 694 0 0 0 PCI-MSI 3145729 nvme0q1 85: 1780 0 0 0 PCI-MSI 3145730 nvme0q2 86: 1092 0 0 0 PCI-MSI 3145731 nvme0q3 87: 1303 0 0 0 PCI-MSI 3145732 nvme0q4 88: 6523 0 0 0 PCI-MSI 2097152 xhci_hcd 89: 0 0 0 0 PCI-MSI 2097153 xhci_hcd 90: 0 0 0 0 PCI-MSI 2097154 xhci_hcd 91: 0 0 0 0 PCI-MSI 2097155 xhci_hcd 92: 0 0 0 0 PCI-MSI 2097156 xhci_hcd 93: 0 0 0 0 sifive-gpio 6 lm90 94: 1029 0 0 0 PCI-MSI 2621440 iwlwifi: default queue 95: 282 0 0 0 PCI-MSI 2621441 iwlwifi: queue 1 96: 5 0 0 0 PCI-MSI 2621442 iwlwifi: queue 2 97: 31 0 0 0 PCI-MSI 2621443 iwlwifi: queue 3 98: 8 0 0 0 PCI-MSI 2621444 iwlwifi: queue 4 99: 5 0 0 0 PCI-MSI 2621445 iwlwifi: exception IPI0: 109 125 110 104 Rescheduling interrupts IPI1: 5072 10959 6008 12207 Function call interrupts IPI2: 0 0 0 0 CPU stop interrupts IPI3: 0 0 0 0 IRQ work interrupts IPI4: 0 0 0 0 Timer broadcast interrupts I note the absence of a "SiFive PLIC 24 sifive-gpio" line, but I do not know if this is to be expected. In kernel/debug/irqs, I further see: # cat irqs/55 handler: handle_bad_irq device: (null) status: 0x00008508 _IRQ_NOPROBE _IRQ_NESTED_THREAD istate: 0x00000020 IRQS_ONESHOT ddepth: 0 wdepth: 0 dstate: 0x00402208 IRQ_TYPE_LEVEL_LOW IRQD_LEVEL IRQD_ACTIVATED IRQD_IRQ_STARTED node: 0 affinity: 0-3 domain: :soc:i2c@10030000:pmic@58 hwirq: 0x0 chip: da9063-irq flags: 0x0 # cat irqs/54 handler: handle_level_irq device: (null) status: 0x00000508 _IRQ_NOPROBE istate: 0x00000020 IRQS_ONESHOT ddepth: 0 wdepth: 0 dstate: 0x02403208 IRQ_TYPE_LEVEL_LOW IRQD_LEVEL IRQD_ACTIVATED IRQD_IRQ_STARTED IRQD_AFFINITY_SET IRQD_DEFAULT_TRIGGER_SET node: 0 affinity: 0-3 domain: :soc:gpio@10060000 hwirq: 0x1 chip: sifive-gpio flags: 0x0 parent: domain: :soc:interrupt-controller@c000000 hwirq: 0x18 chip: SiFive PLIC flags: 0x0 # cat irqs/22 handler: handle_fasteoi_irq device: (null) status: 0x00000400 _IRQ_NOPROBE istate: 0x00000000 ddepth: 1 wdepth: 0 dstate: 0x02031000 IRQD_IRQ_DISABLED IRQD_IRQ_MASKED IRQD_AFFINITY_SET IRQD_DEFAULT_TRIGGER_SET node: 0 affinity: 0-3 domain: :soc:interrupt-controller@c000000 hwirq: 0x18 chip: SiFive PLIC flags: 0x0 So: - the GPIO driver seems to have properly told the kernel that it reports to interrupt-controller@c000000's domain, on line 24. - ...but that line (soft irq 22 on this specific boot) is disabled and masked. - the da9063 does not seem to have express its reliance on the gpio's irq domain, but still the gpio irq is enabled What is abnormal here ? What could be a probable cause ? PLIC driver ? GPIO driver ? da9063 driver ? devicetree ? Side note: the above traces are with a few of my hacks, they may not fully represent a clean kernel. I did confirm that the high-level symptom still exists. Regards, -- Vincent Pelletier ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-06-28 22:52 ` Vincent Pelletier 2021-06-29 10:19 ` Andy Shevchenko @ 2021-07-02 0:24 ` Linus Walleij 1 sibling, 0 replies; 10+ messages in thread From: Linus Walleij @ 2021-07-02 0:24 UTC (permalink / raw) To: Vincent Pelletier Cc: Andy Shevchenko, Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Tue, Jun 29, 2021 at 12:52 AM Vincent Pelletier <plr.vincent@gmail.com> wrote: > While on this topic, there is a bullet point in > Documentation/driver-api/gpio/driver.rst which I fail to understand: > > | - Nominally set all handlers to handle_bad_irq() in the setup call and pass > | handle_bad_irq() as flow handler parameter in > gpiochip_irqchip_add() if it is > | expected for GPIO driver that irqchip .set_type() callback will be called > | before using/enabling each GPIO IRQ. Then set the handler to > | handle_level_irq() and/or handle_edge_irq() in the irqchip .set_type() > | callback depending on what your controller supports and what is requested > | by the consumer. > > - why the plural in "set all handlers to handle_bad_irq()" ? Isn't > there only a single handler in struct gpio_irq_chip ? girq->handler will be applied (inside gpiolib) successively to all the irq lines on the irqchip, as many as you have GPIO lines. It will then be set to something usable when we get to ->set_type() when an IRQ is being requested. > - I do not find a function named gpiochip_irqchip_add(), only > gpiochip_irqchip_add_domain() This refers to the old API that didn't add the irqchip with the gpiochip. Needs updating. Patches welcome ;) > - "Then set the handler to [...] in the irqchip .set_type() callback" > Isn't set_type per-pin, and isn't the interrupt handler chip-level ? Those are two different things. The ->set_type() callback is per-line (call it lines, not pins, because terminology can be confusing otherwise), these are the interrupt handlers per-line apart from of course the handler the consumer provides in e.g. request_[threaded_]irq(). There is usually another cascading interrupt handler for the GPIO irqchip itself, usually what is passed in girq->parent_handler *BUT* you are looking at an hierarchical interrupt controller with one line per gpio line to the next level of irq controller, so you do not see this. You don't have a chip-level interrupt handler in the sifive driver, all falls through upward in the hierarchy to the next overarching interrupt controller. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-06-28 3:36 gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? Vincent Pelletier 2021-06-28 13:40 ` Andy Shevchenko @ 2021-07-02 0:09 ` Linus Walleij 2021-07-02 10:48 ` Vincent Pelletier 1 sibling, 1 reply; 10+ messages in thread From: Linus Walleij @ 2021-07-02 0:09 UTC (permalink / raw) To: Vincent Pelletier Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Mon, Jun 28, 2021 at 5:37 AM Vincent Pelletier <plr.vincent@gmail.com> wrote: > gpiolib (gpiochip_irq_reqres, gpiochip_reqres_irq, gpiochip_lock_as_irq) > does not call gpiod_request_{,commit}, resulting in a pin which is available > for use. Nope and they should not. > Is this intentional ? Yes. The basic reason is that gpiochips and irqchips are orthogonal. You can request an IRQ on a GPIO line without requesting the GPIO line for anything else. This is also used when drivers want to inspect the state of a GPIO line (read the value) while the same line triggers IRQs. This is perfectly legal. An extreme example is: drivers/media/cec/platform/cec-gpio/cec-gpio.c There is sometimes confusion around gpiod_to_irq(). This is just a convenience function locating the IRQ for a certain GPIO. Both resources still have to be requested separately and there is no dependency between them, they are just often implemented in the same driver, using two different subsystem APIs, in the end. sysfs can't be used as any guide here since it conflates GPIO lines and IRQs and provides several dangerous ways to shoot oneself in the foot. The chardev does a better job at keeping this in order. > Also, I notice that both gpiochip_hierarchy_add_domain and > gpiochip_reqres_irq call gpiochip_lock_as_irq, and I am surprised I do not > get any error about this: in my understanding only the first call on a given pin > should succeed, but with my WARN_ON I am seeing both stack traces and > no other warning. Hm that may be a subtle bug. The state is just a bool so the first to leave will turn out the lights for whoever is left in the room :P Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-07-02 0:09 ` Linus Walleij @ 2021-07-02 10:48 ` Vincent Pelletier 2021-07-02 21:46 ` Linus Walleij 0 siblings, 1 reply; 10+ messages in thread From: Vincent Pelletier @ 2021-07-02 10:48 UTC (permalink / raw) To: Linus Walleij Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Fri, 2 Jul 2021 02:09:17 +0200, Linus Walleij <linus.walleij@linaro.org> wrote: > The basic reason is that gpiochips and irqchips are orthogonal. > You can request an IRQ on a GPIO line without requesting the > GPIO line for anything else. > > This is also used when drivers want to inspect the state of a GPIO > line (read the value) while the same line triggers IRQs. This is > perfectly legal. An extreme example is: > drivers/media/cec/platform/cec-gpio/cec-gpio.c Interesting, thank you very much. > On Mon, Jun 28, 2021 at 5:37 AM Vincent Pelletier <plr.vincent@gmail.com> wrote: > > Also, I notice that both gpiochip_hierarchy_add_domain and > > gpiochip_reqres_irq call gpiochip_lock_as_irq, and I am surprised I do not > > get any error about this: in my understanding only the first call on a given pin > > should succeed, but with my WARN_ON I am seeing both stack traces and > > no other warning. > > Hm that may be a subtle bug. > > The state is just a bool so the first to leave will turn out the lights > for whoever is left in the room :P Actually my question came from yet another misunderstanding on my side: I expected this function to act as an exclusive access control (because of the "lock" in the name), but I then realised my assumption is wrong. So while this could be a subtle bug indeed (irq_disable without irq_shutdown is not the exact same state as right after irq_startup), it's likely not the one I'm chasing - if it leads to any actual issue at all. Regards, -- Vincent Pelletier GPG fingerprint 983A E8B7 3B91 1598 7A92 3845 CAC9 3691 4257 B0C1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? 2021-07-02 10:48 ` Vincent Pelletier @ 2021-07-02 21:46 ` Linus Walleij 0 siblings, 0 replies; 10+ messages in thread From: Linus Walleij @ 2021-07-02 21:46 UTC (permalink / raw) To: Vincent Pelletier Cc: Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List On Fri, Jul 2, 2021 at 12:48 PM Vincent Pelletier <plr.vincent@gmail.com> wrote: > gpiochip_lock_as_irq, (...) > Actually my question came from yet another misunderstanding on my side: > I expected this function to act as an exclusive access control (because > of the "lock" in the name), but I then realised my assumption is wrong. Well it locks its use to be compliant with being used for IRQ. So it cannot be turned into an output for example. While reading its value as input is fine. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-07-02 21:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-28 3:36 gpiochip_lock_as_irq on pins without FLAG_REQUESTED: bug or feature ? Vincent Pelletier 2021-06-28 13:40 ` Andy Shevchenko 2021-06-28 13:42 ` Andy Shevchenko 2021-06-28 22:52 ` Vincent Pelletier 2021-06-29 10:19 ` Andy Shevchenko 2021-07-01 13:36 ` Vincent Pelletier 2021-07-02 0:24 ` Linus Walleij 2021-07-02 0:09 ` Linus Walleij 2021-07-02 10:48 ` Vincent Pelletier 2021-07-02 21:46 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).