Skip to content

Kernel module cannot request interrupt 79 after kernel update from 4.9 to 4.14 #2590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tz77 opened this issue Jun 25, 2018 · 12 comments
Closed

Comments

@tz77
Copy link

tz77 commented Jun 25, 2018

After a kernel upgrade from 4.9 to 4.14 via apt-get upgrade on a Raspberry Pi 3, loading my uio-based kernel module fails with uio_register_device() returning -EINVAL. The kernel module tries to request IRQ 79 for the edge detection of all GPIOs on bank 0. The other uio_info structure fields name and version are also populated, and irq_flags is set to IRQF_SHARED.

A more detailed analysis shows that uio_register_device() fails because request_irq() returns -EINVAL as the outcome of request_threaded_irq(), which in turns fails, because irq_settings_can_request(desc) returns false due to desc->status_use_accessors having the _IRQ_NOREQUEST bit set.

This seems to indicate that IRQ 79 (for gpio_int[0], used to be 49 as in the BCM2835 ARM Peripherals document) cannot be requested any longer with kernel 4.14.

An observation that might be related to this is that the 4.14 content of /proc/interrupts does not have the two lines
79: 0 0 0 0 ARMCTRL-level 81 Edge 3f200000.gpio:bank0
80: 0 0 0 0 ARMCTRL-level 82 Edge 3f200000.gpio:bank1
whereas the 4.9 kernel does have them.

@pelwell
Copy link
Contributor

pelwell commented Jun 26, 2018

That is not how you are supposed to use interrupts in Linux, for two reasons:

  1. Interrupts numbers should be considered to be randomly chosen at boot time. They aren't, of course - if the system configuration doesn't change then they will consistent (and logical) between boots - but code shouldn't make assumptions about the numbering.
  2. The GPIO interrupts are managed by the gpio driver - pinctrl-bcm2835 - which appears as a second interrupt controller, and your driver should be configured (ideally by device tree) to claim the interrupt for a specified GPIO from it. The gpio-keys driver and the gpio-shutdown overlay are relatively simple examples of how it all works.

@tz77
Copy link
Author

tz77 commented Jun 27, 2018

Thanks for the quick reply. I see your point, but the UIO-framework requires an interrupt number to be passed. Moreover, I am wondering what happened to IRQs 79 and 80 in /proc/interrupts, so far I could not figure out why they are no longer assigned after the kernel upgrade.

@pelwell
Copy link
Contributor

pelwell commented Jun 28, 2018

Without diving into the kernel source code:

Those IRQs are claimed by the gpio driver and only used to generate GPIO-specific "chained" IRQs. I would imagine that interrupts which are used in that way are omitted from the /proc/interrupts list - possibly to avoid double counting.

@tz77
Copy link
Author

tz77 commented Jul 5, 2018

Meanwhile, I was able to analyze the issue a bit further and I would like to tell you the insights I have gained from that.

The GPIO bank interrupts are managed by the module pinctrl-bcm2835 as pelwell said. For the 4.9 Raspbian kernel, each GPIO bank interrupts (79 for bank 0 and 80 for bank 1) were assigned using devm_request_irq() with IRQF_SHARED as irqflags, which in the end allocates the interrupt by means of request_threaded_irq(). As a consequence, those two interrupts may be shared and can be requested for additional handlers for example as in the case of my UIO-based module.

With commit 85ae9e512f437cd09bf61564bdba29ab88bab3e3, which is part of the Raspbian kernel 4.14, the GPIO bank interrupt handling changed. As pelwell said, the interrupts are now chained irqchips, which are allocated using gpiochip_set_chained_irqchip(). The two bank interrupts still have interrupt numbers 79 and 80, but they are registered differently: IRQ 79 is chained to the parent IRQ 0 of the interrupt controller in the irq domain soc:gpio@7e200000, and IRQ 80 is chained to the parent IRQ 79. Due to the fact that gpiochip_set_chained_irqchip() finally calls __irq_do_set_handler() with the is_chained parameter set to 1, the irq_desc structure element status_use_accessors will have the _IRQ_NOREQUEST bit set which prevents this IRQ from being requested via request_threaded_irq() as it would be needed by the UIO framework as described in my very first post. In a nutshell, the GPIO bank interrupts cannot be requested any longer.

This behavior is also described in the documentation Documentation/gpio/driver.txt:

CHAINED GPIO irqchips: these are usually the type that is embedded on
an SoC. This means that there is a fast IRQ flow handler for the GPIOs that
gets called in a chain from the parent IRQ handler, most typically the
system interrupt controller. This means that the GPIO irqchip handler will
be called immediately from the parent irqchip, while holding the IRQs
disabled. The GPIO irqchip will then end up calling something like this
sequence in its interrupt handler:

and moreover:

Note, chained IRQ handlers will not be forced threaded on -RT.
As result, spinlock_t or any sleepable APIs (like PM runtime) can't be used
in chained IRQ handler.
If required (and if it can't be converted to the nested threaded GPIO irqchip)
a chained IRQ handler can be converted to generic irq handler and this way
it will be a threaded IRQ handler on -RT and a hard IRQ handler on non-RT

The documentation also cites an example for that on the LKML, where the chained interrupt handlers are reverted to generic ones again with the following benefit (amongst others):

GPIO bank IRQs will appear in /proc/interrupts and its usage statistic
will be visible;

But the more important benefit in my opinion is the fact that generic handlers can be shared and thus be requested by other kernel modules for example as in my UIO-based case.

Without such a patch for the pinctrl-bcm2835 module, I do not see any further possibility to hook any of the GPIO bank interrupts for my own purpose. Would you agree on that, pelwell, or is there any other possibility which I haven't seen yet?
If not, I will sadly have to stick to kernel 4.9 to keep my UIO-based kernel module running...

@pelwell
Copy link
Contributor

pelwell commented Jul 5, 2018

Are your uses of UIO drivers limited to GPIOs 0-27? If so, it should be possible to use an overlay to overwrite the interrupt declaration for GPIO bank, preventing it from being claimed., without interfering with the other, system uses of GPIO interrupts.

@tz77
Copy link
Author

tz77 commented Jul 6, 2018

Yes, they are limited to this range. I only need a couple of them, but a common handler. How does the overlay concept work? Would be great if this works out...

@pelwell
Copy link
Contributor

pelwell commented Jul 6, 2018

An overlay is just a set of patches that can be applied to a Device Tree. We need to do find a modification to the Device Tree that will cause the GPIO driver to not claim the interrupt for bank 0.

The device node for the GPIO driver declares is interrupt usage using a standard interrupts property:

interrupts = <2 17>, <2 18>;

I think we should be able to overwrite that first interrupt with something invalid to cause it to be skipped without affecting GPIO bank 1.

@pelwell
Copy link
Contributor

pelwell commented Jul 19, 2018

I'm sorry it's taken so long to get back to this. Here is a new overlay - gpio-no-bank0-irq - that stops the GPIO driver claiming IRQ79 in a way that should allow your module to claim it. Copy it into /boot/overlays, add dtoverlay=gpio-no-bank0-irq to config.txt, and reboot.

Let me know how you get on, and if it works I'll make it part of the standard image.

@tz77
Copy link
Author

tz77 commented Jul 25, 2018

Many thanks for the overlay. I tested it on a PI 2B with the 4.14.52 kernel.
Unfortunately, there is already an error during the boot process. dmesg shows the following trace output:

[ 0.038024] bcm2835-mbox 3f00b880.mailbox: mailbox enabled
[ 0.038211] ------------[ cut here ]------------
[ 0.038255] WARNING: CPU: 0 PID: 1 at drivers/irqchip/irq-bcm2835.c:191 armctrl_xlate+0xa4/0xc4
[ 0.038289] Modules linked in:
[ 0.038324] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.14.52-v7+ #1123
[ 0.038349] Hardware name: BCM2835
[ 0.038400] [<8010ffd8>] (unwind_backtrace) from [<8010c240>] (show_stack+0x20/0x24)
[ 0.038445] [<8010c240>] (show_stack) from [<80785e84>] (dump_stack+0xd4/0x118)
[ 0.038489] [<80785e84>] (dump_stack) from [<8011da4c>] (__warn+0xf8/0x110)
[ 0.038526] [<8011da4c>] (__warn) from [<8011db34>] (warn_slowpath_null+0x30/0x38)
[ 0.038568] [<8011db34>] (warn_slowpath_null) from [<804c3248>] (armctrl_xlate+0xa4/0xc4)
[ 0.038615] [<804c3248>] (armctrl_xlate) from [<8017c5f4>] (irq_create_fwspec_mapping+0xa0/0x2b0)
[ 0.038660] [<8017c5f4>] (irq_create_fwspec_mapping) from [<8017c868>] (irq_create_of_mapping+0x64/0x6c)
[ 0.038706] [<8017c868>] (irq_create_of_mapping) from [<806566d8>] (of_irq_get+0x68/0x78)
[ 0.038750] [<806566d8>] (of_irq_get) from [<80656710>] (of_irq_to_resource+0x28/0xcc)
[ 0.038792] [<80656710>] (of_irq_to_resource) from [<80656800>] (of_irq_to_resource_table+0x4c/0x5c)
[ 0.038840] [<80656800>] (of_irq_to_resource_table) from [<8065133c>] (of_device_alloc+0x100/0x184)
[ 0.038888] [<8065133c>] (of_device_alloc) from [<80651418>] (of_platform_device_create_pdata+0x58/0xac)
[ 0.038937] [<80651418>] (of_platform_device_create_pdata) from [<806515e8>] (of_platform_bus_create+0x110/0x368)
[ 0.038986] [<806515e8>] (of_platform_bus_create) from [<80651644>] (of_platform_bus_create+0x16c/0x368)
[ 0.039034] [<80651644>] (of_platform_bus_create) from [<806519c8>] (of_platform_populate+0x7c/0xdc)
[ 0.039085] [<806519c8>] (of_platform_populate) from [<80b3fde0>] (of_platform_default_populate_init+0x80/0x90)
[ 0.039136] [<80b3fde0>] (of_platform_default_populate_init) from [<80101bf0>] (do_one_initcall+0x50/0x17c)
[ 0.039184] [<80101bf0>] (do_one_initcall) from [<80b01020>] (kernel_init_freeable+0x21c/0x2bc)
[ 0.039231] [<80b01020>] (kernel_init_freeable) from [<8079af70>] (kernel_init+0x18/0x128)
[ 0.039277] [<8079af70>] (kernel_init) from [<8010810c>] (ret_from_fork+0x14/0x28)
[ 0.039321] ---[ end trace 677e3d5c24021b57 ]---
[ 0.081448] bcm2835-dma 3f007000.dma: DMA legacy API manager at bb813000, dmachans=0x1

@pelwell
Copy link
Contributor

pelwell commented Jul 26, 2018

That is an unavoidable(?) warning that one of the interrupt specifiers is invalid, but that is what we want and should be no cause for concern. Did you actually try to use the system?

@tz77
Copy link
Author

tz77 commented Aug 1, 2018

No, I had not tried to use the system with your overlay being active at the time of my last post because I did not expect it to work with the given error message during bootup.

Meanwhile, I tested the kernel module on my PI 3 with your overlay on the latest 4.14.52 kernel (where it shows up the same error message). I am more than glad to report that everything works as predicted by you: the uio_register_device() command does not fail any longer, IRQ 79 is actually requested and shown in /proc/interrupts including its call-statistics, the interrupt is hooked upon a rising/falling edge of the GPIO pin, and therefore, you made it possible for me to use my kernel module with the latest kernel. Thank you so much!

Of course, I would appreciate it if you could make the overlay part of the standard image.
Best regards!

pelwell pushed a commit that referenced this issue Oct 12, 2018
@pelwell
Copy link
Contributor

pelwell commented Oct 12, 2018

This patch slipped down the cracks but it's in the 4.14 tree now, so the overlay will eventually become part of the standard images.

@pelwell pelwell closed this as completed Oct 12, 2018
ahmedradaideh pushed a commit to ahmedradaideh/Pi-Kernel that referenced this issue Oct 15, 2018
See: raspberrypi/linux#2590

Signed-off-by: Phil Elwell <[email protected]>
Signed-off-by: ahmedradaideh <[email protected]>
pelwell pushed a commit to pelwell/linux that referenced this issue Nov 13, 2018
pelwell pushed a commit to pelwell/linux that referenced this issue Nov 13, 2018
pelwell pushed a commit to pelwell/linux that referenced this issue Nov 13, 2018
pelwell pushed a commit to pelwell/linux that referenced this issue Nov 13, 2018
pelwell pushed a commit to pelwell/linux that referenced this issue Nov 13, 2018
pelwell pushed a commit that referenced this issue Nov 13, 2018
popcornmix pushed a commit that referenced this issue Nov 13, 2018
popcornmix pushed a commit that referenced this issue Nov 15, 2018
popcornmix pushed a commit that referenced this issue Nov 19, 2018
popcornmix pushed a commit that referenced this issue Nov 21, 2018
popcornmix pushed a commit that referenced this issue Nov 23, 2018
popcornmix pushed a commit that referenced this issue Nov 28, 2018
popcornmix pushed a commit that referenced this issue Nov 28, 2018
lyakh pushed a commit to lyakh/linux that referenced this issue Nov 30, 2018
popcornmix pushed a commit that referenced this issue Dec 4, 2018
popcornmix pushed a commit that referenced this issue Dec 14, 2018
popcornmix pushed a commit that referenced this issue Dec 17, 2018
popcornmix pushed a commit that referenced this issue Dec 17, 2018
popcornmix pushed a commit that referenced this issue Dec 21, 2018
popcornmix pushed a commit that referenced this issue Dec 24, 2018
popcornmix pushed a commit that referenced this issue Jan 1, 2019
lyakh pushed a commit to lyakh/linux that referenced this issue Jan 7, 2019
popcornmix pushed a commit that referenced this issue Jan 9, 2019
popcornmix pushed a commit that referenced this issue Jan 14, 2019
popcornmix pushed a commit that referenced this issue Jan 22, 2019
popcornmix pushed a commit that referenced this issue Jan 23, 2019
lyakh pushed a commit to lyakh/linux that referenced this issue Jan 24, 2019
popcornmix pushed a commit that referenced this issue Feb 4, 2019
popcornmix pushed a commit that referenced this issue Feb 12, 2019
popcornmix pushed a commit that referenced this issue Feb 18, 2019
popcornmix pushed a commit that referenced this issue Feb 24, 2019
popcornmix pushed a commit that referenced this issue Mar 6, 2019
popcornmix pushed a commit that referenced this issue Mar 12, 2019
popcornmix pushed a commit that referenced this issue Mar 15, 2019
popcornmix pushed a commit that referenced this issue Mar 21, 2019
popcornmix pushed a commit that referenced this issue Apr 2, 2019
popcornmix pushed a commit that referenced this issue Apr 8, 2019
Gadgetoid pushed a commit to Gadgetoid/linux that referenced this issue Apr 10, 2019
popcornmix pushed a commit that referenced this issue Apr 23, 2019
popcornmix pushed a commit that referenced this issue Apr 30, 2019
popcornmix pushed a commit that referenced this issue Apr 30, 2019
popcornmix pushed a commit that referenced this issue May 7, 2019
popcornmix pushed a commit that referenced this issue May 13, 2019
TiejunChina pushed a commit that referenced this issue Jun 19, 2019
TiejunChina pushed a commit that referenced this issue Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants