Page MenuHomeFreeBSD
Differential D17938

Add a PWM subsystem
ClosedPublic
Actions

Authored by manu on Nov 10 2018, 7:31 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 31, 1:33 PM
Unknown Object (File)
Mon, Dec 23, 10:44 AM
Unknown Object (File)
Sat, Dec 21, 5:24 PM
Unknown Object (File)
Thu, Dec 19, 12:17 PM
Unknown Object (File)
Sat, Dec 7, 2:56 PM
Unknown Object (File)
Thu, Dec 5, 8:48 PM
Unknown Object (File)
Thu, Dec 5, 12:03 PM
Unknown Object (File)
Wed, Dec 4, 7:28 PM
View All Files
Subscribers
0mp
andrew
bcr
imp
jonathan
loos
mizhka

Details

Reviewers
mizhka
loos
oshogbo
bcr
0mp
Group Reviewers
ARM
arm64
manpages
capsicum
Commits
rS349415: MFC r341268, r342003-r342007, r342087-r342088, r342091
rS342003: Add a pwm subsystem so we can configure pwm controller from kernel and userland.
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 - subversion
Lint
Lint Skipped
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
Herald added a reviewer: manpages. · View Herald TranscriptNov 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.

Herald added a subscriber: jonathan. · View Herald TranscriptNov 23 2018, 1:19 PM
manu added a reviewer: oshogbo.Nov 23 2018, 1:19 PM
manu updated this revision to Diff 50993.Nov 23 2018, 1:23 PM
manu marked 4 inline comments as done.

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 updated this revision to Diff 51008.Nov 23 2018, 4:00 PM
manu marked an inline comment as done.

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
138

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.Dec 12 2018, 8:47 PM
oshogbo accepted this revision.
oshogbo added a reviewer: capsicum.
This revision was not accepted when it landed; it landed in state Needs Review.Dec 12 2018, 8:59 PM
Closed by commit rS342003: Add a pwm subsystem so we can configure pwm controller from kernel and userland. (authored by manu). · Explain Why
This revision was automatically updated to reflect the committed changes.
• ian added a commit: rS349415: MFC r341268, r342003-r342007, r342087-r342088, r342091.Jun 26 2019, 3:34 PM

Revision Contents
Changeset List

PathSize
share/
man/
man9/
2 lines
89 lines
88 lines
sys/
conf/
5 lines
dev/
pwm/
109 lines
106 lines
62 lines
244 lines
111 lines
161 lines
sys/
51 lines
usr.sbin/
1 line
pwm/
6 lines
96 lines
214 lines

Diff 50992

View Options

share/man/man9/Makefile

Loading...
View Options

share/man/man9/pwm.9

Loading...
View Options

share/man/man9/pwmbus.9

Loading...
View Options

sys/conf/files

Loading...
View Options

sys/dev/pwm/ofw_pwm.c

Loading...
View Options

sys/dev/pwm/pwm_if.m

Loading...
View Options

sys/dev/pwm/pwmbus.h

Loading...
View Options

sys/dev/pwm/pwmbus.c

Loading...
View Options

sys/dev/pwm/pwmbus_if.m

Loading...
View Options

sys/dev/pwm/pwmc.c

Loading...
View Options

sys/sys/pwm.h

Loading...
View Options

usr.sbin/Makefile

Loading...
View Options

usr.sbin/pwm/Makefile

Loading...
View Options

usr.sbin/pwm/pwm.8

Loading...
View Options

usr.sbin/pwm/pwm.c

Loading...