Page MenuHomeFreeBSD

Rockchip RK3399 PWM driver
Needs ReviewPublic

Authored by git_bdragon.rtk0.net on Thu, Jan 31, 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

andrew added inline comments.Thu, Jan 31, 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.Mon, Feb 4, 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.

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.Mon, Feb 18, 2:03 PM
sys/arm64/rockchip/rk_pwm.c
280

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