Skip to content

Allow the default user to access the new gpio character device #2289

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

Open
gohai opened this issue Nov 26, 2017 · 23 comments
Open

Allow the default user to access the new gpio character device #2289

gohai opened this issue Nov 26, 2017 · 23 comments
Assignees
Labels
Waiting for external input Waiting for a comment from the originator of the issue, or a collaborator. Waiting for internal comment Waiting for comment from a member of the Raspberry Pi engineering team

Comments

@gohai
Copy link
Contributor

gohai commented Nov 26, 2017

I want to switch Processing from using the legacy sysfs interface to the new character device interface (/dev/gpiochip*). To make this possible, those devices should be owned by the gpio group.

The following line, added to 99-com.rules achieves this:

SUBSYSTEM=="gpio", GROUP="gpio", MODE="0660"

Could this be added for upcoming Raspbian releases? Thanks!

@pelwell
Copy link
Contributor

pelwell commented Nov 27, 2017

I'm in favour - an efficient user-space GPIO API has been long overdue.

@XECDesign Can we insert this as line 6 of /etc/udev/rules.d/99-com.rules? At some point we should also add the new kernel-supplied tools (lsgpio, gpio-hammer and gpio-event-mon) to the distro, but the current output isn't pretty - all the pins are marked as unused, with no helpful labels(*).

(*) Minor correction - on 4.14 the brcmvirt-gpio driver shows "led0" for line 0, but that's all.

@gohai
Copy link
Contributor Author

gohai commented Nov 27, 2017 via email

@pelwell
Copy link
Contributor

pelwell commented Nov 27, 2017

I want to see BRCM GPIO/"pin" numbering everywhere - it's used exclusively by the firmware and the kernel, and it's guaranteed to make sense on any arbitrary hardware design (how would you use 40-pin header values on a Compute Module?).

If you have any feedback for us in terms of how to make the new GPIO interface more usable, or warnings about problems you have encountered, then please let us know.

@XECDesign
Copy link
Contributor

Added: RPi-Distro/raspberrypi-sys-mods@bd5a201
It should show up in apt in a week or two.

I'll need to create a package for lsgpio and the others, but I'll save that for later.

@gohai
Copy link
Contributor Author

gohai commented Nov 27, 2017 via email

@lategoodbye
Copy link
Contributor

As a reference here is the lsgpio output of a Raspberry Pi 1 B with upstream 4.14:

GPIO chip: gpiochip0, "pinctrl-bcm2835", 54 GPIO lines
        line  0: "SDA0" unused
        line  1: "SCL0" unused
        line  2: "SDA1" unused
        line  3: "SCL1" unused
        line  4: "GPIO_GCLK" unused
        line  5: "CAM_GPIO1" unused [output]
        line  6: "LAN_RUN" unused [output]
        line  7: "SPI_CE1_N" unused
        line  8: "SPI_CE0_N" unused
        line  9: "SPI_MISO" unused
        line 10: "SPI_MOSI" unused
        line 11: "SPI_SCLK" unused
        line 12: "NC" unused
        line 13: "NC" unused
        line 14: "TXD0" unused
        line 15: "RXD0" unused
        line 16: "STATUS_LED_N" "ACT" [kernel output active-low]
        line 17: "GPIO17" unused
        line 18: "GPIO18" unused
        line 19: "NC" unused
        line 20: "NC" unused
        line 21: "GPIO21" unused
        line 22: "GPIO22" unused
        line 23: "GPIO23" unused
        line 24: "GPIO24" unused
        line 25: "GPIO25" unused
        line 26: "NC" unused
        line 27: "CAM_GPIO0" unused [output]
        line 28: "CONFIG0" unused
        line 29: "CONFIG1" unused
        line 30: "CONFIG2" unused
        line 31: "CONFIG3" unused
        line 32: "NC" unused
        line 33: "NC" unused
        line 34: "NC" unused
        line 35: "NC" unused
        line 36: "NC" unused
        line 37: "NC" unused
        line 38: "NC" unused
        line 39: "NC" unused
        line 40: "PWM0_OUT" unused
        line 41: "NC" unused
        line 42: "NC" unused
        line 43: "NC" unused
        line 44: "NC" unused
        line 45: "PWM1_OUT" unused
        line 46: "HDMI_HPD_P" unused
        line 47: "SD_CARD_DET" unused
        line 48: "SD_CLK_R" unused
        line 49: "SD_CMD_R" unused
        line 50: "SD_DATA0_R" unused
        line 51: "SD_DATA1_R" unused
        line 52: "SD_DATA2_R" unused
        line 53: "SD_DATA3_R" unused

@JamesH65
Copy link
Contributor

JamesH65 commented Dec 4, 2017

@XECDesign Can be closed?

@XECDesign
Copy link
Contributor

@JamesH65 The updated raspberrypi-sys-mods package is in the repo, so I would say yes. However, nothing has been done to add lsgpio and other binaries or change how they display data.

@gohai
Copy link
Contributor Author

gohai commented Dec 4, 2017

Wanted to correct what I posted above: I realized that currently the new API does not allow the setting and clearing of pull-up/down resistors. I confused them with setting open source / open drain, but these things seem to be orthogonal really.

@lategoodbye
Copy link
Contributor

From my point of view it should be easier to add a pull-up/down IOCTL instead of extending the sysfs.

@pelwell
Copy link
Contributor

pelwell commented Dec 4, 2017

I'm happy for us to be supporting the new interface, whether or not it supports the setting of pulls yet. Let's keep this open until we've added the new utilities.

@gohai
Copy link
Contributor Author

gohai commented Dec 5, 2017

@lategoodbye Agree. The new (IOCTL) interface already has an extensible flags field, which is used e.g. for indicating whether the line should be an input or output. This should be sufficient for implementing the most basic pull-up/down configuration at least.

@JamesH65
Copy link
Contributor

Anyone have any progress to report on this one? Would be nice to know what stage we are at.

@JamesH65 JamesH65 added Waiting for external input Waiting for a comment from the originator of the issue, or a collaborator. Waiting for internal comment Waiting for comment from a member of the Raspberry Pi engineering team labels Apr 20, 2018
@JamesH65
Copy link
Contributor

@pelwell Anything to report?

@pelwell
Copy link
Contributor

pelwell commented Jun 27, 2018

Last we heard we were waiting for lsgpio and family to make it into Raspbian.

@JamesH65
Copy link
Contributor

@XECDesign @spl237 Is this now in your court?

@lategoodbye
Copy link
Contributor

I think this is related to @gohai intension, starting with 4.18 the pinctrl driver would be generic.

@XECDesign
Copy link
Contributor

I don't feel a sense of urgency to add the gpio-utils package. Very few distros include them and there are many other binaries we don't include. The original issue is resolved. The new package would be for bonus points.

@lategoodbye
Copy link
Contributor

lategoodbye commented Jun 27, 2018

Sure there is no urgency, but the sysfs-gpio is obsolete since 2016.

Edit: I know you don't care about mainline, but CONFIG_GPIO_SYSFS has been disabled in multi_v7_defconfig and arm64/defconfig since 2016-11-30 (Linux 4.10).

@JamesH65
Copy link
Contributor

This issue will be closed within 30 days unless further interactions are posted. If you wish this issue to remain open, please add a comment. A closed issue may be reopened if requested.

@XECDesign
Copy link
Contributor

Bumping it since it's probably time to package up gpio-utils.

@XECDesign XECDesign self-assigned this Jul 30, 2019
@XECDesign
Copy link
Contributor

gpio-utils added to buster and should show up in the repo within about an hour.

@pelwell
Copy link
Contributor

pelwell commented Aug 8, 2019

Nice.

popcornmix pushed a commit that referenced this issue Feb 6, 2024
[ Upstream commit 2a9de42 ]

======================================================
WARNING: possible circular locking dependency detected
6.5.0-kfd-yangp #2289 Not tainted
------------------------------------------------------
kworker/0:2/996 is trying to acquire lock:
        (srcu){.+.+}-{0:0}, at: __synchronize_srcu+0x5/0x1a0

but task is already holding lock:
        ((work_completion)(&svms->deferred_list_work)){+.+.}-{0:0}, at:
	process_one_work+0x211/0x560

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #3 ((work_completion)(&svms->deferred_list_work)){+.+.}-{0:0}:
        __flush_work+0x88/0x4f0
        svm_range_list_lock_and_flush_work+0x3d/0x110 [amdgpu]
        svm_range_set_attr+0xd6/0x14c0 [amdgpu]
        kfd_ioctl+0x1d1/0x630 [amdgpu]
        __x64_sys_ioctl+0x88/0xc0

-> #2 (&info->lock#2){+.+.}-{3:3}:
        __mutex_lock+0x99/0xc70
        amdgpu_amdkfd_gpuvm_restore_process_bos+0x54/0x740 [amdgpu]
        restore_process_helper+0x22/0x80 [amdgpu]
        restore_process_worker+0x2d/0xa0 [amdgpu]
        process_one_work+0x29b/0x560
        worker_thread+0x3d/0x3d0

-> #1 ((work_completion)(&(&process->restore_work)->work)){+.+.}-{0:0}:
        __flush_work+0x88/0x4f0
        __cancel_work_timer+0x12c/0x1c0
        kfd_process_notifier_release_internal+0x37/0x1f0 [amdgpu]
        __mmu_notifier_release+0xad/0x240
        exit_mmap+0x6a/0x3a0
        mmput+0x6a/0x120
        do_exit+0x322/0xb90
        do_group_exit+0x37/0xa0
        __x64_sys_exit_group+0x18/0x20
        do_syscall_64+0x38/0x80

-> #0 (srcu){.+.+}-{0:0}:
        __lock_acquire+0x1521/0x2510
        lock_sync+0x5f/0x90
        __synchronize_srcu+0x4f/0x1a0
        __mmu_notifier_release+0x128/0x240
        exit_mmap+0x6a/0x3a0
        mmput+0x6a/0x120
        svm_range_deferred_list_work+0x19f/0x350 [amdgpu]
        process_one_work+0x29b/0x560
        worker_thread+0x3d/0x3d0

other info that might help us debug this:
Chain exists of:
  srcu --> &info->lock#2 --> (work_completion)(&svms->deferred_list_work)

Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
        lock((work_completion)(&svms->deferred_list_work));
                        lock(&info->lock#2);
			lock((work_completion)(&svms->deferred_list_work));
        sync(srcu);

Signed-off-by: Philip Yang <[email protected]>
Reviewed-by: Felix Kuehling <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
popcornmix pushed a commit that referenced this issue Feb 6, 2024
[ Upstream commit 2a9de42 ]

======================================================
WARNING: possible circular locking dependency detected
6.5.0-kfd-yangp #2289 Not tainted
------------------------------------------------------
kworker/0:2/996 is trying to acquire lock:
        (srcu){.+.+}-{0:0}, at: __synchronize_srcu+0x5/0x1a0

but task is already holding lock:
        ((work_completion)(&svms->deferred_list_work)){+.+.}-{0:0}, at:
	process_one_work+0x211/0x560

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #3 ((work_completion)(&svms->deferred_list_work)){+.+.}-{0:0}:
        __flush_work+0x88/0x4f0
        svm_range_list_lock_and_flush_work+0x3d/0x110 [amdgpu]
        svm_range_set_attr+0xd6/0x14c0 [amdgpu]
        kfd_ioctl+0x1d1/0x630 [amdgpu]
        __x64_sys_ioctl+0x88/0xc0

-> #2 (&info->lock#2){+.+.}-{3:3}:
        __mutex_lock+0x99/0xc70
        amdgpu_amdkfd_gpuvm_restore_process_bos+0x54/0x740 [amdgpu]
        restore_process_helper+0x22/0x80 [amdgpu]
        restore_process_worker+0x2d/0xa0 [amdgpu]
        process_one_work+0x29b/0x560
        worker_thread+0x3d/0x3d0

-> #1 ((work_completion)(&(&process->restore_work)->work)){+.+.}-{0:0}:
        __flush_work+0x88/0x4f0
        __cancel_work_timer+0x12c/0x1c0
        kfd_process_notifier_release_internal+0x37/0x1f0 [amdgpu]
        __mmu_notifier_release+0xad/0x240
        exit_mmap+0x6a/0x3a0
        mmput+0x6a/0x120
        do_exit+0x322/0xb90
        do_group_exit+0x37/0xa0
        __x64_sys_exit_group+0x18/0x20
        do_syscall_64+0x38/0x80

-> #0 (srcu){.+.+}-{0:0}:
        __lock_acquire+0x1521/0x2510
        lock_sync+0x5f/0x90
        __synchronize_srcu+0x4f/0x1a0
        __mmu_notifier_release+0x128/0x240
        exit_mmap+0x6a/0x3a0
        mmput+0x6a/0x120
        svm_range_deferred_list_work+0x19f/0x350 [amdgpu]
        process_one_work+0x29b/0x560
        worker_thread+0x3d/0x3d0

other info that might help us debug this:
Chain exists of:
  srcu --> &info->lock#2 --> (work_completion)(&svms->deferred_list_work)

Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
        lock((work_completion)(&svms->deferred_list_work));
                        lock(&info->lock#2);
			lock((work_completion)(&svms->deferred_list_work));
        sync(srcu);

Signed-off-by: Philip Yang <[email protected]>
Reviewed-by: Felix Kuehling <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
popcornmix pushed a commit that referenced this issue Feb 8, 2024
[ Upstream commit 2a9de42 ]

======================================================
WARNING: possible circular locking dependency detected
6.5.0-kfd-yangp #2289 Not tainted
------------------------------------------------------
kworker/0:2/996 is trying to acquire lock:
        (srcu){.+.+}-{0:0}, at: __synchronize_srcu+0x5/0x1a0

but task is already holding lock:
        ((work_completion)(&svms->deferred_list_work)){+.+.}-{0:0}, at:
	process_one_work+0x211/0x560

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #3 ((work_completion)(&svms->deferred_list_work)){+.+.}-{0:0}:
        __flush_work+0x88/0x4f0
        svm_range_list_lock_and_flush_work+0x3d/0x110 [amdgpu]
        svm_range_set_attr+0xd6/0x14c0 [amdgpu]
        kfd_ioctl+0x1d1/0x630 [amdgpu]
        __x64_sys_ioctl+0x88/0xc0

-> #2 (&info->lock#2){+.+.}-{3:3}:
        __mutex_lock+0x99/0xc70
        amdgpu_amdkfd_gpuvm_restore_process_bos+0x54/0x740 [amdgpu]
        restore_process_helper+0x22/0x80 [amdgpu]
        restore_process_worker+0x2d/0xa0 [amdgpu]
        process_one_work+0x29b/0x560
        worker_thread+0x3d/0x3d0

-> #1 ((work_completion)(&(&process->restore_work)->work)){+.+.}-{0:0}:
        __flush_work+0x88/0x4f0
        __cancel_work_timer+0x12c/0x1c0
        kfd_process_notifier_release_internal+0x37/0x1f0 [amdgpu]
        __mmu_notifier_release+0xad/0x240
        exit_mmap+0x6a/0x3a0
        mmput+0x6a/0x120
        do_exit+0x322/0xb90
        do_group_exit+0x37/0xa0
        __x64_sys_exit_group+0x18/0x20
        do_syscall_64+0x38/0x80

-> #0 (srcu){.+.+}-{0:0}:
        __lock_acquire+0x1521/0x2510
        lock_sync+0x5f/0x90
        __synchronize_srcu+0x4f/0x1a0
        __mmu_notifier_release+0x128/0x240
        exit_mmap+0x6a/0x3a0
        mmput+0x6a/0x120
        svm_range_deferred_list_work+0x19f/0x350 [amdgpu]
        process_one_work+0x29b/0x560
        worker_thread+0x3d/0x3d0

other info that might help us debug this:
Chain exists of:
  srcu --> &info->lock#2 --> (work_completion)(&svms->deferred_list_work)

Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
        lock((work_completion)(&svms->deferred_list_work));
                        lock(&info->lock#2);
			lock((work_completion)(&svms->deferred_list_work));
        sync(srcu);

Signed-off-by: Philip Yang <[email protected]>
Reviewed-by: Felix Kuehling <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting for external input Waiting for a comment from the originator of the issue, or a collaborator. Waiting for internal comment Waiting for comment from a member of the Raspberry Pi engineering team
Projects
None yet
Development

No branches or pull requests

5 participants