interfaces: add pwm-control interface #12347
Conversation
zyga
force-pushed
the
feature/pwm-control
branch
2 times, most recently
from
a28c237
to
5e3bc4c
Compare
bboozzoo
added
the
Needs security review
|
@degville shall I write a documentation snippet about this interface on the forum? |
|
I just had a quick look, and my main question is what are the cases that the existing |
|
@mardy the relation between the (so far missing) |
Yes please! I'll edit it there if needed. I think it's great to see usernames attached to the work they're responsible for. |
zyga
force-pushed
the
feature/pwm-control
branch
from
5e3bc4c
to
b145c91
Compare
mardy
added
the
Needs Samuele review
interfaces/builtin/pwm_control.go
Outdated
|
|
||
| /sys/class/pwm/pwmchip[0-9]*/{,un}export rw, | ||
| /sys/class/pwm/pwmchip[0-9]*/npwm r, | ||
| /sys/class/pwm/pwmchip[0-9]*/pwm[0-9]*/{period,duty_cycle,polarity,enable} rw, |
There was a problem hiding this comment.
afaict pwm itself, used /* for the last component here?
There was a problem hiding this comment.
Yes, I can adjust that. I just wanted to be precise, as it is easier to extend but harder to take the permissions away.
If you want I can change that to the general glob.
There was a problem hiding this comment.
I agree, but given the choice for pwm itself, I think it's better to be consistent at this point, also it would be strange for the -control variant to be more restricted in this way than the non-control one
There was a problem hiding this comment.
Ack, I'll send an update in a moment.
There was a problem hiding this comment.
@pedronis done. I have also added the k permission to match the pwm interface better (k allows locking AFAIR).
I've created https://forum.snapcraft.io/t/the-pwm-control-interface/32908 based on the documentation of the gpio-control interface. I think both interfaces could use some technical details. I'll do another round of proposed updates to those threads to see what you think @degville EDIT: I think the sort of thing now present on under the "developer details" section of the PWM interface is what I was thinking of. |
The pwm-control interface is similar in spirit to the gpio-control interface, allowing access to any and all pulse-width-modulation channels. The interface is equally privileged. The only difference as compared to GPIO control is the set of permissions granted. Those are modeled after the kernel documentation referenced from a code comment. Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
zyga
force-pushed
the
feature/pwm-control
branch
from
b145c91
to
27c16ed
Compare
alexmurray
removed
the
Needs security review
|
Please hold on with merging this. I had to apply one more change for this to really work. I totally missed the fact that sysfs is full of symlinks. I made a change that looks almost identical to what |
|
I'll pick this up mid next week. I have an urgent thing to address outside of the scope of snapd. |
|
I still have a few things to do but I will push an update with the new look of the code for possible feedback. |
| # potentially impact the system and other snaps, and allows privileged access | ||
| # to hardware. | ||
|
|
||
| /sys/class/pwm/pwmchip[0-9]*/{,un}export rw, |
There was a problem hiding this comment.
The three patterns look good, but fail on the symlink:
On a random Raspberry Pi 5:
zyga@pi5-1:/sys/class/pwm$ ls -ld pwmchip*
lrwxrwxrwx 1 root root 0 Nov 8 18:38 pwmchip0 -> ../../devices/platform/axi/1000120000.pcie/1f0009c000.pwm/pwm/pwmchip0
lrwxrwxrwx 1 root root 0 Feb 6 23:48 pwmchip4 -> ../../devices/platform/soc/107d517a80.pwm/pwm/pwmchip4
So this pattern alone would not work.
zyga
removed
the
Needs Samuele review
|
This needs re-design to avoid the problem of symlinks and apparmor. |
The pwm-control interface is similar in spirit to the gpio-control interface, allowing access to any and all pulse-width-modulation channels.
The interface is equally privileged. The only difference as compared to GPIO control is the set of permissions granted. Those are modeled after the kernel documentation referenced from a code comment.
Signed-off-by: Zygmunt Krynicki me@zygoon.pl