/snapd

interfaces/builtin: use udev to export GPIOs to userspace #2065

Closed
wants to merge 1 commit into
from

Conversation

Reviewers

@jocave jocave

@niemeyer niemeyer

Assignees
No one assigned
Labels
Blocked Reviewed
Projects
None yet
Milestone
No milestone
3 participants
@zyga @jocave @niemeyer
@zyga
Contributor

zyga commented Oct 3, 2016

This patch changes the GPIO interface to use udev for exporting devices
to userspace.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com
@zyga
interfaces/builtin: use udev to export GPIOs to userspace 
This patch changes the GPIO interface to use udev for exporting devices
to userspace.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
03e6eba
@zyga
Contributor

zyga commented Oct 3, 2016

@jocave can you test this to ensure that it really works in practice please?
@jocave

jocave suggested changes Oct 3, 2016
View changes

Tested, but found the action in RUN wasn't applied. See inline comments for what I found works.

interfaces/builtin/gpio.go
+ case interfaces.SecurityUDev:
+ // NOTE: nothing unexports this GPIO when the slot is disconnected but
+ // AFAIK this doesn't hurt.
+ snippet := fmt.Sprintf(`ACTION=="add", SUBSYSTEM=="gpio", RUN+="/bin/echo %v > /syc/class/gpio/export"`, slot.Attrs["number"])
@jocave

jocave Oct 3, 2016

Contributor

This doesn't work - you need to pass it to a shell e.g.:

RUN+="/bin/sh -c '/bin/echo 346 > /sys/class/gpio/export'"

interfaces/builtin/gpio_test.go
+ plug := s.gadgetPlug
+ content, err := s.iface.ConnectedSlotSnippet(plug, slot, interfaces.SecurityUDev)
+ c.Assert(err, IsNil)
+ expected := `ACTION=="add", SUBSYSTEM=="gpio", RUN+="/bin/echo 100 > /syc/class/gpio/export"`
@jocave

jocave Oct 3, 2016

Contributor

Modify RUN action to match the above.

@jocave
Contributor

jocave commented Oct 3, 2016

Testing note: just calling a "udevadm trigger" only creates "change" events so the rules used here are not run. "udevadm trigger -c add" causes them to be invoked

Does this mean the rule needs to be changed or are we ok with "add"?
@zyga
Contributor

zyga commented Oct 3, 2016

Can you do a quick test to see what happens after reboot (it should be exported with your changed RUN line).

As for -c add, no we cannot currently use that (I was hoping we don't need to modify the udev backend for this). I'll check with pitti to see if there's a better way of associating this with some udev events.
@niemeyer

niemeyer approved these changes Oct 3, 2016
View changes

LGTM assuming it actually works. Leaving that to @jocave to review.

@niemeyer niemeyer added the Reviewed label Oct 3, 2016

@niemeyer
Contributor

niemeyer commented Oct 5, 2016

We need to discuss this to see how to approach the problem. Let's work on it together next week.

@niemeyer niemeyer added the Blocked label Oct 5, 2016

@zyga zyga closed this Nov 15, 2016

@zyga
Contributor

zyga commented Nov 15, 2016

This was actually implemented and landed as systemd-based job

@zyga zyga deleted the zyga:udev-gpio branch Aug 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment