mbox series

[0/7] Add Rockchip RK3576 PWM Support Through MFPWM

Message ID 20250408-rk3576-pwm-v1-0-a49286c2ca8e@collabora.com
Headers show
Series Add Rockchip RK3576 PWM Support Through MFPWM | expand

Message

Nicolas Frattaroli April 8, 2025, 12:32 p.m. UTC
This series introduces support for some of the functions of the new PWM
silicon found on Rockchip's RK3576 SoC. Due to the wide range of
functionalities offered by it, including many parts which this series'
first iteration does not attempt to implement for now, it uses multiple
drivers hanging on a platform bus on which the parent "mfpwm" driver
registers them.

Here's some of the features of the hardware:
- Continuous PWM output (implemented in this series)
- One-shot/Finite repetition PWM output
- PWM capture by counting high/low cycles (implemented in this series)
- Sending IR transmissions in several TV remote protocols
- Generating an interrupt based on the input being one of 16
  user-specified values ("Power key capture")
- Biphasic counter support
- Using the hardware to measure a clock signal's frequency
- Using the hardware to count a clock signal's pulses
- Generating PWM output waveforms through a user-specified lookup table

As you can tell, there's a lot. I've focused on continuous PWM output
for now as the most important one for things like controlling fans. The
PWM capture driver is an added bonus, because I needed at least two
drivers to test things. Anyone doing consumer electronic devices like
tablets based on the RK3576 may need to do the power key stuff at some
stage, but I'm still not entirely clear on what that'd look like in a
schematic. The IR transmission stuff may be a funny weekend project for
someone at some point; I assume it's there so TV boxes can turn on and
off TVs without needing the HDMI control stuff.

At first, I considered simply integrating support for this new IP into
the old pwm-rockchip driver, as the downstream vendor kernel did.
However, the IP is significantly different from previous iterations.
Especially if the goal is to support some of the additional
functionality that the new silicon brings, doing it all in a single pwm
driver would be untenable. Especially one that already supports other
hardware with a way different set of registers.

Hence, the mfpwm pattern: each device functionality is its own driver,
and they all get registered as platform drivers by the parent mfpwm
driver, which is the one that binds to the DT compatible. Each device
function driver then has to _acquire and _release the hardware when it
needs control of it. If some other device function is using the device
already, -EBUSY is returned, which the device function driver can then
forward to the user and everyone is happy.

The PWM output driver, pwm-rockchip-v4, uses the new waveform APIs. I
thought while writing a new driver that I might as well use the new
APIs.

The PWM capture driver, implemented as a counter driver, is somewhat
primitive, in that it doesn't make use of things like the biphasic
counter support or clock measuring, but it serves as a good way to
showcase and test the mutual exclusion that the mfpwm framework tries to
achieve. It also goes one step beyond just exposing the raw LPC/HPC
counts and actually makes them usable in a non-theoretically-racey way
by turning them into nanosecond period/duty cycle values based on the
clock's rate. Shoutouts to the counter subsystem's documentation by the
way, it is some of the best subsystem documentation I've come across so
far, and was a great help.

Some instances of the PWM silicon allow switching between a fixed
crystal oscillator as the PWM clock, and the default PLL clock, which is
just a mux between that very same crystal oscillator and two other fixed
oscillators on RK3576. The downstream vendor driver does not implement
this feature, but I did because it seemed funny, so now there's a sysfs
interface to switch between them and it makes sure you don't
accidentally ruin any PWM user's day while switching. The potential
benefit is that this switching is per-PWM-channel, but aside from that
it doesn't seem super useful and should've probably just been some
per-channel clock muxes in the hardware's CRU instead.

Along the way, I also finally took the time to give us The One True
HIWORD_UPDATE macro, which aims to replace all other copies of the
HIWORD_UPDATE macro, except it's not called HIWORD_UPDATE because all of
them have slightly different semantics and I don't want to introduce
even more confusion. It does, however, do some compile-time checking of
the function-like macro parameters.

This series went through two substantial rewrites, because after I
realised it needed to be multiple drivers (first rewrite) I then first
wrote it as a couple of auxiliary bus drivers, until I became unsure
about whether this should be auxiliary bus drivers at all, so it became
a bunch of platform bus drivers instead. If anything looks like a weird
vestigial leftover in the code, then that's probably why, but I made
sure to get rid of all the ones I could find before submitting this.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Nicolas Frattaroli (7):
      dt-bindings: pinctrl: rockchip: increase max amount of device functions
      dt-bindings: pwm: Add a new binding for rockchip,rk3576-pwm
      soc: rockchip: add utils header for things shared across drivers
      soc: rockchip: add mfpwm driver
      pwm: Add rockchip PWMv4 driver
      counter: Add rockchip-pwm-capture driver
      arm64: dts: rockchip: add PWM nodes to RK3576 SoC dtsi

 .../bindings/pinctrl/rockchip,pinctrl.yaml         |   2 +-
 .../bindings/pwm/rockchip,rk3576-pwm.yaml          |  94 ++++
 MAINTAINERS                                        |  11 +
 arch/arm64/boot/dts/rockchip/rk3576.dtsi           | 192 +++++++
 drivers/counter/Kconfig                            |  13 +
 drivers/counter/Makefile                           |   1 +
 drivers/counter/rockchip-pwm-capture.c             | 341 ++++++++++++
 drivers/pwm/Kconfig                                |  13 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/pwm-rockchip-v4.c                      | 336 ++++++++++++
 drivers/soc/rockchip/Kconfig                       |  13 +
 drivers/soc/rockchip/Makefile                      |   1 +
 drivers/soc/rockchip/mfpwm.c                       | 608 +++++++++++++++++++++
 include/soc/rockchip/mfpwm.h                       | 505 +++++++++++++++++
 include/soc/rockchip/utils.h                       |  76 +++
 15 files changed, 2206 insertions(+), 1 deletion(-)
---
base-commit: 64e9fdfc89a76fed38d8ddeed72d42ec71957ed9
change-id: 20250407-rk3576-pwm-46761bd0deaa

Best regards,

Comments

Conor Dooley April 8, 2025, 4:07 p.m. UTC | #1
On Tue, Apr 08, 2025 at 02:32:14PM +0200, Nicolas Frattaroli wrote:
> The Rockchip RK3576 SoC has a newer PWM controller IP revision than
> previous Rockchip SoCs. This IP, called "PWMv4" by Rockchip, introduces
> several new features, and consequently differs in its bindings.
> 
> Instead of expanding the ever-growing rockchip-pwm binding that already
> has an if-condition, add an entirely new binding to handle this.
> 
> The "osc" clock is an optional clock available on some instances of the
> PWM controller. If present, it allows the controller to switch between
> either the "pwm" clock or the "osc" clock for deriving its PWM signal
> on a per-channel basis, through a hardware register write.
> 
> However, not all instances have this feature, and the hardware copes
> just fine without this additional clock, so it's optional.
> 
> The PWM controller also comes with an interrupt now. This interrupt is
> used to signal various conditions.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Rob Herring (Arm) April 8, 2025, 5:27 p.m. UTC | #2
On Tue, Apr 08, 2025 at 02:32:13PM +0200, Nicolas Frattaroli wrote:
> With the introduction of the RK3576, the maximum device function ID used
> increased to 14, as anyone can easily verify for themselves with:
> 
>   rg -g '*-pinctrl.dtsi' '<\d+\s+RK_P..\s+(?<func>\d+)\s.*>;$' --trim \
>   -NI -r '$func' arch/arm64/boot/dts/rockchip/ | sort -g | uniq
> 
> Unfortunately, this wasn't caught by dt-validate as those pins are
> omit-if-no-ref and we had no reference to them in any tree so far.

Sounds like we need a way to disable that for validation. We'd need a 
dtc flag to ignore that and then set that flag for CHECK_DTBS.

Rob