Page MenuHomeFreeBSD

Add a PWM subsystem
ClosedPublic

Authored by manu on Nov 10 2018, 7:31 PM.

Details

Summary

Add a pwm subsystem so we can configure pwm controller from kernel and userland.

Test Plan

tested on pine64 with a few other patches from https://github.com/evadot/freebsd/tree/pwm

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
manu added a comment.Nov 13 2018, 8:51 PM

Add documentation and a pwm(8) utility.

This revision now requires review to proceed.Nov 13 2018, 8:51 PM
manu edited the summary of this revision. (Show Details)Nov 13 2018, 8:52 PM
bcr added a subscriber: bcr.Nov 13 2018, 9:01 PM

A few easy fixes for both man pages (I annotated only, but they apply to both).
Thanks for the writeup.

share/man/man9/pwm.9
59

interfal == interface or interval?

59

s/allow device drive/allows the device driver/

60

s/device/devices/
Sentence stop missing?

67

s/is/if/

68

s/is/if/
s/doesn't/does not/

80

s/channel/channels/

83

s/appear/appeared/

manu updated this revision to Diff 50385.Nov 13 2018, 9:20 PM

Fix manpages.

manu marked 7 inline comments as done.Nov 13 2018, 9:21 PM
manu updated this revision to Diff 50386.Nov 13 2018, 9:48 PM

Allow setting period and duty at the same time.

0mp added a subscriber: 0mp.Nov 14 2018, 10:05 AM

Sometimes you write pwm and sometimes PWM.

I'd either change everything to PWM or to .Nm pwm (I am not sure if sole .Nm is going to work).

Cheers :)

share/man/man9/pwm.9
64

Missing ..

66

s/pwm/PWM/

67

...
.Er EINVAL
...

68

.Er EBUSY

76

s/pwm/PWM/

78

s/pwm/PWM/

83

s/pwm/PWM/

85

s/pwm/PWM/

share/man/man9/pwmbus.9
60

This paragraph has similar issues to the one from the other manual page reviewed by @bcr.

loos requested changes to this revision.Nov 14 2018, 11:02 AM
loos added a subscriber: loos.

Looks good Manu!

What do you think about a method to get the channel resolution ? Could be in bits.

sys/dev/pwm/pwmbus_if.m
94

stray ';'

usr.sbin/pwm/pwm.c
2

no license in this file.

47

I guess the '-f' option is missing here.

87

';' not needed.

This revision now requires changes to proceed.Nov 14 2018, 11:02 AM
manu updated this revision to Diff 50992.Nov 23 2018, 1:19 PM

Address comment and add capsicum.

manu marked 4 inline comments as done.Nov 23 2018, 1:23 PM
manu updated this revision to Diff 50993.

Address few more mandoc -Tlint warnings

manu marked 9 inline comments as done.Nov 23 2018, 1:24 PM
bcr added a comment.Nov 23 2018, 2:58 PM

Two comments for the updated version.

share/man/man9/pwm.9
59

It still reads "device drive" instead of "device driver" here. Or is it just device without the driver? "drive" is definitely wrong here.

share/man/man9/pwmbus.9
60

This is still the old version? Should be fixing the same issue as above (s/allow device/allows the device/)

manu updated this revision to Diff 50999.Nov 23 2018, 3:13 PM

Address bcr comments.

manu marked 3 inline comments as done.Nov 23 2018, 3:13 PM
manu added inline comments.
share/man/man9/pwm.9
59

Yeah sorry I missed this part earlier.

bcr accepted this revision.Nov 23 2018, 3:23 PM

Now it's good and I can approve it from manpages. ;-)

0mp requested changes to this revision.Nov 23 2018, 3:30 PM

It looks like you forgot to update the other manual page as well. :)

share/man/man9/pwmbus.9
66

.Nm pwmbus

68

s/pwm/PWM/

69

.Er EINVAL

80

s/pwm/PWM

82

s/pwm/PWM

87

.Nm pwmbus

89

.Nm pwmbus

This revision now requires changes to proceed.Nov 23 2018, 3:30 PM
manu marked an inline comment as done.Nov 23 2018, 4:00 PM
manu updated this revision to Diff 51008.

I need some sleep ...

manu marked 7 inline comments as done.Nov 23 2018, 4:00 PM
0mp accepted this revision.Nov 23 2018, 4:04 PM
manu updated this revision to Diff 51009.Nov 23 2018, 4:09 PM

last s/pwm/PWM/ (I hope)

0mp accepted this revision.Nov 23 2018, 4:12 PM

Great catch!

andrew added a subscriber: andrew.Nov 23 2018, 4:17 PM
andrew added inline comments.
usr.sbin/pwm/pwm.c
70

Where is HAVE_CAPSICUM defined?

manu updated this revision to Diff 51019.Nov 23 2018, 7:32 PM

Remove HAVE_CAPSICUM as it's deprecated.
While here correct a typo in cap_right/s_init

manu marked an inline comment as done.Nov 23 2018, 7:32 PM
oshogbo added inline comments.Dec 2 2018, 6:34 PM
usr.sbin/pwm/pwm.c
131

I would use caph_enter

136

You probably want to use a caph_cache_catpages and caph_limit_stdio

205

extra space.

manu updated this revision to Diff 51927.Dec 12 2018, 8:07 PM

Address oshgobo@ comments.

manu marked 3 inline comments as done.Dec 12 2018, 8:07 PM
oshogbo added inline comments.Dec 12 2018, 8:18 PM
usr.sbin/pwm/pwm.c
137

s/cap_rights_limit/caph_rights_limit/

Otherwise we need to check the errno.

manu updated this revision to Diff 51930.Dec 12 2018, 8:35 PM

Use more caph* functions

manu updated this revision to Diff 51931.Dec 12 2018, 8:43 PM

s/sizeof/nitems/

oshogbo accepted this revision.
oshogbo accepted this revision.
This revision was not accepted when it landed; it landed in state Needs Review.Dec 12 2018, 8:59 PM
This revision was automatically updated to reflect the committed changes.