Page MenuHomeFreeBSD

Rockchip RK3399 PWM driver
ClosedPublic

Authored by manu on Jan 31 2019, 4:35 PM.
Tags
Referenced Files
Unknown Object (File)
Sun, Jun 29, 5:42 PM
Unknown Object (File)
Mon, Jun 9, 11:36 PM
Unknown Object (File)
Tue, Jun 3, 5:40 AM
Unknown Object (File)
Tue, Jun 3, 5:39 AM
Unknown Object (File)
May 12 2025, 9:21 PM
Unknown Object (File)
May 9 2025, 10:34 AM
Unknown Object (File)
Apr 28 2025, 8:47 AM
Unknown Object (File)
Apr 27 2025, 5:43 AM

Details

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";
 };
Test Plan

Tested on the PinebookPro with the backlight.

Diff Detail

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

Event Timeline

sys/arm64/rockchip/rk_pwm.c
159

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

238

This should be:

/*
 * The device ...
275–276

Multi line comments should be in the format

/*
 * First line.
 * Second line.
 */
sys/arm64/rockchip/rk_pwm.c
167

you should check for error here.

231

clock should be released here.

244

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

281

Why ?

296

This should only happen in pwm_enable.

299

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

301

same here, prescaler is always 0.

303

same here, scaler is always 0.

sys/arm64/rockchip/rk_pwm.c
281

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.

sys/arm64/rockchip/rk_pwm.c
281

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.

manu updated this revision to Diff 66196.
manu edited reviewers, added: bdragon; removed: manu.

Update the latest pwm api.

manu added reviewers: arm64, mmel, ganbold, gonzo.
This revision is now accepted and ready to land.Jan 8 2020, 9:13 AM
This revision now requires review to proceed.Jan 8 2020, 8:14 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 16 2020, 9:25 PM
This revision was automatically updated to reflect the committed changes.