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 - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Add documentation and a pwm(8) utility.

This revision now requires review to proceed.Nov 13 2018, 8:51 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
58 ↗(On Diff #50384)

interfal == interface or interval?

58 ↗(On Diff #50384)

s/allow device drive/allows the device driver/

59 ↗(On Diff #50384)

s/device/devices/
Sentence stop missing?

66 ↗(On Diff #50384)

s/is/if/

67 ↗(On Diff #50384)

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

79 ↗(On Diff #50384)

s/channel/channels/

82 ↗(On Diff #50384)

s/appear/appeared/

manu marked 7 inline comments as done.Nov 13 2018, 9:21 PM

Allow setting period and duty at the same time.

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
63 ↗(On Diff #50386)

Missing ..

65 ↗(On Diff #50386)

s/pwm/PWM/

66 ↗(On Diff #50386)

...
.Er EINVAL
...

67 ↗(On Diff #50386)

.Er EBUSY

75 ↗(On Diff #50386)

s/pwm/PWM/

77 ↗(On Diff #50386)

s/pwm/PWM/

82 ↗(On Diff #50386)

s/pwm/PWM/

84 ↗(On Diff #50386)

s/pwm/PWM/

share/man/man9/pwmbus.9
59 ↗(On Diff #50386)

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
93 ↗(On Diff #50386)

stray ';'

usr.sbin/pwm/pwm.c
1 ↗(On Diff #50386)

no license in this file.

46 ↗(On Diff #50386)

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

86 ↗(On Diff #50386)

';' not needed.

This revision now requires changes to proceed.Nov 14 2018, 11:02 AM

Address comment and add capsicum.

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

Two comments for the updated version.

share/man/man9/pwm.9
58 ↗(On Diff #50384)

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
59 ↗(On Diff #50386)

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

manu marked 3 inline comments as done.Nov 23 2018, 3:13 PM
manu added inline comments.
share/man/man9/pwm.9
58 ↗(On Diff #50384)

Yeah sorry I missed this part earlier.

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
65 ↗(On Diff #50999)

.Nm pwmbus

67 ↗(On Diff #50999)

s/pwm/PWM/

68 ↗(On Diff #50999)

.Er EINVAL

79 ↗(On Diff #50999)

s/pwm/PWM

81 ↗(On Diff #50999)

s/pwm/PWM

86 ↗(On Diff #50999)

.Nm pwmbus

88 ↗(On Diff #50999)

.Nm pwmbus

This revision now requires changes to proceed.Nov 23 2018, 3:30 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

last s/pwm/PWM/ (I hope)

andrew added inline comments.
usr.sbin/pwm/pwm.c
69 ↗(On Diff #51009)

Where is HAVE_CAPSICUM defined?

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
usr.sbin/pwm/pwm.c
130 ↗(On Diff #51019)

I would use caph_enter

135 ↗(On Diff #51019)

You probably want to use a caph_cache_catpages and caph_limit_stdio

204 ↗(On Diff #51019)

extra space.

Address oshgobo@ comments.

manu marked 3 inline comments as done.Dec 12 2018, 8:07 PM
usr.sbin/pwm/pwm.c
137 ↗(On Diff #51927)

s/cap_rights_limit/caph_rights_limit/

Otherwise we need to check the errno.

Use more caph* functions

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
This revision was automatically updated to reflect the committed changes.