Page MenuHomeFreeBSD

Rockchip RK3399 PWM driver
Needs ReviewPublic

Authored by bdragon on Jan 31 2019, 4:35 PM.

Details

Reviewers
manu
Summary

Here's a stab at a PWM driver for the rockchip rk3399, using the new PWM api. I have never written a driver before, but it seemed pretty straightforward.

The hardware itself is four channel, but the dts assumes multiple attachment for some reason.

I can control the fan on my ROCKPRO64 by enabling the second pwm in the device tree:

Index: sys/gnu/dts/arm64/rockchip/rk3399-rockpro64.dts
===================================================================
--- sys/gnu/dts/arm64/rockchip/rk3399-rockpro64.dts	(revision 343327)
+++ sys/gnu/dts/arm64/rockchip/rk3399-rockpro64.dts	(working copy)
@@ -560,6 +560,10 @@
 	status = "okay";
 };
 
+&pwm1 {
+	status = "okay";
+};
+
 &pwm2 {
 	status = "okay";
 };

Diff Detail

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

Event Timeline

bdragon created this revision.Jan 31 2019, 4:35 PM
andrew added inline comments.Jan 31 2019, 7:24 PM
sys/arm64/rockchip/rk_pwm.c
158

Do we need sc->dev? Everywhere it's used we have access the it as an argument.

237

This should be:

/*
 * The device ...
274–275

Multi line comments should be in the format

/*
 * First line.
 * Second line.
 */
manu added inline comments.Feb 4 2019, 2:39 PM
sys/arm64/rockchip/rk_pwm.c
166

you should check for error here.

230

clock should be released here.

243

This comment isn't needed, we are respecting the bindings.

280

Why ?

295

This should only happen in pwm_enable.

298

You always set using_scaler to false, just clear the clock source bit and remove the variable.

300

same here, prescaler is always 0.

302

same here, scaler is always 0.

bdragon added inline comments.Feb 14 2019, 6:09 PM
sys/arm64/rockchip/rk_pwm.c
280

So the period can be longer than a couple seconds or so. If this isn't desired in a PWM driver, I can just simplify stuff and leave scaling and prescaling off at all times instead of implementing it, since the API will overflow before the hardware will. I was assuming people would be using this for stuff like watering timers etc where the period would be in minutes or hours, but personally I just wanted the fan to spin on my rockpro64.

manu added inline comments.Feb 18 2019, 2:03 PM
sys/arm64/rockchip/rk_pwm.c
280

I really don't need that we need period > 1s honestly.

Will address comments and catch up with the latest API changes soon. Poking at it this evening.