Skip to content
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

Add Adafruit's rpi_power_switch: Optionally uses GPIO pin to trigger shutdown #1014

Closed

Conversation

brennen
Copy link

@brennen brennen commented Jun 8, 2015

This adds the rpi_power_switch module, which lets a GPIO pin be configured to run shutdown -h.

This is mainly intended for use in conjunction with an Adafruit PiTFT and a tactile switch, but should be usable with just about any pushbutton or switch.

Submitting this here since it's definitely Pi-specific.

Code by @xobs, cc: @ladyada, @toddtreece.

This is a squashed commit, rounding up @ladyada's powerswitch patch and
@toddtreece's later import of a slight fix to that from a tarball.
@ladyada
Copy link

ladyada commented Jun 8, 2015

hiya folx! one of the neat things about this kernel module is it is always running, so even if there's a userspace lockup in someone's project, its still possible to get the pi to shutdown nicely. also, it will turn the pi back on after its been shutdown, which you cannot do in user space.

@pelwell
Copy link
Contributor

pelwell commented Jun 9, 2015

Although I'm happy with the goals of this PR, the implementation needs some work before we'll accept it:

  1. We're not going to accept any code that accesses the GPIO registers directly like this, when there is a generic GPIO API for linux drivers. See drivers/leds/leds-gpio.c for an example of how to use it. drivers/staging/media/lirc/lirc_rpi.c might also be useful, but this isn't the best example of kernel coding standards. The gpiolib API doesn't include a generic way to set the pull direction on a pin, but that's OK because...

  2. Use Device Tree. The pinctrl infrastructure allows you to define the pins used by a module and their configuration for a number of states. The configuration includes the pull direction, so you don't need an API to set it dynamically. Device Tree parameters allow things like the GPIO number to be patched using entries in config.txt.
    Although we still support non-DT configurations, this won't always be the case, and choosing to go DT-only (by assuming that the pins are configured already) will make your code simpler.
    Another way to solve the pull problem for generic non-DT configurations is to make use of the default pull of the GPIOs - GPIO0-GPIO8 pull high, while GPIO9-GPIO27 pull low.

  3. A bit of a tidy up wouldn't hurt - simply removing all of the commented out printk()s (you should be using pr_info, pr_err etc. anyway) would be an improvement,

  4. A Copyright notice at the top of the C file would be a good idea.

  5. Make the configuration option dependent on one of the Raspberry Pi platforms:

    depends on MACH_BCM2708 || MACH_BCM2709 || ARCH_BCM2835

I hope this list isn't too disheartening. With a bit of effort (mainly deleting code) you will end up with something that is usable on other platforms as well.

I'm happy to answer questions if you get stuck and need a few pointers - phil at raspberrypi dot org.

@brennen
Copy link
Author

brennen commented Jun 9, 2015

Thanks for taking the time to give detailed feedback. We'll see what we can do.

brennen added 3 commits June 9, 2015 10:02
- Adds copyright notice (I'm assuming work for hire here, maybe should
  be Copyright Sean Cross instead?)
- Adds GPL notice
- Removes a pile of printk()s
- Removes some blank lines
- K&Rifies some functions
@Ruffio
Copy link

Ruffio commented Nov 18, 2016

@brennen is this still relevant? If not, please close it.
@pelwell if this it still relevant, are the performed updates ok, and it can be merged?

@pelwell
Copy link
Contributor

pelwell commented Nov 18, 2016

The driver still accesses the GPIO hardware directly to set pull directions, and it isn't even slightly Device Tree-aware. We won't accept this as-is.

@Ruffio
Copy link

Ruffio commented Nov 18, 2016

@brennen do you want to 'fix' else I think this PR should be closed. Thanks

@brennen
Copy link
Author

brennen commented Nov 18, 2016

Hey folks. Sorry - it's been a while. We'll re-open when we get the bandwidth to beat this into shape, but for the moment I think it can stop cluttering your open PRs.

@brennen brennen closed this Nov 18, 2016
popcornmix pushed a commit that referenced this pull request Mar 22, 2018
Scatter-gather needs to be disabled when using dma_declare_coherent_memory
and HCD_LOCAL_MEM. Andrea Righi made the equivalent fix for EHCI drivers
in commit 4307a28 "USB: EHCI: fix NULL pointer dererence in HCDs
that use HCD_LOCAL_MEM".

The following NULL pointer WARN_ON_ONCE triggered with OHCI drivers:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 49 at drivers/usb/core/hcd.c:1379 hcd_alloc_coherent+0x4c/0xc8
Modules linked in:
CPU: 0 PID: 49 Comm: usb-storage Not tainted 4.15.0+ #1014
Stack : 00000000 00000000 805a78d2 0000003a 81f5c2cc 8053d367 804d77fc 00000031
        805a3a08 00000563 81ee9400 805a0000 00000000 10058c00 81f61b10 805c0000
        00000000 00000000 805a0000 00d9038e 00000004 803ee818 00000006 312e3420
        805c0000 00000000 00000073 81f61958 00000000 00000000 802eb380 804fd538
        00000009 00000563 81ee9400 805a0000 00000002 80056148 00000000 805a0000
        ...
Call Trace:
[<578af360>] show_stack+0x74/0x104
[<2f3702c6>] __warn+0x118/0x120
[<ae93fc9e>] warn_slowpath_null+0x44/0x58
[<a891a517>] hcd_alloc_coherent+0x4c/0xc8
[<3578fa36>] usb_hcd_map_urb_for_dma+0x4d8/0x534
[<110bc94c>] usb_hcd_submit_urb+0x82c/0x834
[<02eb5baf>] usb_sg_wait+0x14c/0x1a0
[<ccd09e85>] usb_stor_bulk_transfer_sglist.part.1+0xac/0x124
[<87a5c34c>] usb_stor_bulk_srb+0x40/0x60
[<ff1792ac>] usb_stor_Bulk_transport+0x160/0x37c
[<b9e2709c>] usb_stor_invoke_transport+0x3c/0x500
[<004754f4>] usb_stor_control_thread+0x258/0x28c
[<22edf42e>] kthread+0x134/0x13c
[<a419ffd0>] ret_from_kernel_thread+0x14/0x1c
---[ end trace bcdb825805eefdcc ]---

Signed-off-by: Fredrik Noring <noring@nocrew.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
nathanchance pushed a commit to nathanchance/pi-kernel that referenced this pull request May 28, 2018
[ Upstream commit d6c931e ]

Scatter-gather needs to be disabled when using dma_declare_coherent_memory
and HCD_LOCAL_MEM. Andrea Righi made the equivalent fix for EHCI drivers
in commit 4307a28 "USB: EHCI: fix NULL pointer dererence in HCDs
that use HCD_LOCAL_MEM".

The following NULL pointer WARN_ON_ONCE triggered with OHCI drivers:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 49 at drivers/usb/core/hcd.c:1379 hcd_alloc_coherent+0x4c/0xc8
Modules linked in:
CPU: 0 PID: 49 Comm: usb-storage Not tainted 4.15.0+ raspberrypi#1014
Stack : 00000000 00000000 805a78d2 0000003a 81f5c2cc 8053d367 804d77fc 00000031
        805a3a08 00000563 81ee9400 805a0000 00000000 10058c00 81f61b10 805c0000
        00000000 00000000 805a0000 00d9038e 00000004 803ee818 00000006 312e3420
        805c0000 00000000 00000073 81f61958 00000000 00000000 802eb380 804fd538
        00000009 00000563 81ee9400 805a0000 00000002 80056148 00000000 805a0000
        ...
Call Trace:
[<578af360>] show_stack+0x74/0x104
[<2f3702c6>] __warn+0x118/0x120
[<ae93fc9e>] warn_slowpath_null+0x44/0x58
[<a891a517>] hcd_alloc_coherent+0x4c/0xc8
[<3578fa36>] usb_hcd_map_urb_for_dma+0x4d8/0x534
[<110bc94c>] usb_hcd_submit_urb+0x82c/0x834
[<02eb5baf>] usb_sg_wait+0x14c/0x1a0
[<ccd09e85>] usb_stor_bulk_transfer_sglist.part.1+0xac/0x124
[<87a5c34c>] usb_stor_bulk_srb+0x40/0x60
[<ff1792ac>] usb_stor_Bulk_transport+0x160/0x37c
[<b9e2709c>] usb_stor_invoke_transport+0x3c/0x500
[<004754f4>] usb_stor_control_thread+0x258/0x28c
[<22edf42e>] kthread+0x134/0x13c
[<a419ffd0>] ret_from_kernel_thread+0x14/0x1c
---[ end trace bcdb825805eefdcc ]---

Signed-off-by: Fredrik Noring <noring@nocrew.org>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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

Successfully merging this pull request may close these issues.

4 participants