Re: [PATCH v3 3/5] pwm: Add rockchip PWMv4 driver

Uwe Kleine-König
2025-11-14T09:51:30Z

[-- Attachment #1: Type: text/plain, Size: 7127 bytes --]

On Tue, Oct 28, 2025 at 04:16:26PM +0800, Damon Ding wrote:
> Hi Nicolas,
>
> On 10/28/2025 1:11 AM, Nicolas Frattaroli wrote:
> > The Rockchip RK3576 brings with it a new PWM IP, in downstream code
> > referred to as "v4". This new IP is different enough from the previous
> > Rockchip IP that I felt it necessary to add a new driver for it, instead
> > of shoehorning it in the old one.
> >
> > Add this new driver, based on the PWM core's waveform APIs. Its platform
> > device is registered by the parent mfpwm driver, from which it also
> > receives a little platform data struct, so that mfpwm can guarantee that
> > all the platform device drivers spread across different subsystems for
> > this specific hardware IP do not interfere with each other.
> >
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/pwm/Kconfig | 13 ++
> > drivers/pwm/Makefile | 1 +
> > drivers/pwm/pwm-rockchip-v4.c | 353 ++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 368 insertions(+)
> >
>
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 8f3235ba825e..2079c2d51d5c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -22372,6 +22372,7 @@ L: linux-rockchip@lists.infradead.org
> > L: linux-pwm@vger.kernel.org
> > S: Maintained
> > F: Documentation/devicetree/bindings/pwm/rockchip,rk3576-pwm.yaml
> > +F: drivers/pwm/pwm-rockchip-v4.c
> > F: drivers/soc/rockchip/mfpwm.c
> > F: include/soc/rockchip/mfpwm.h
> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > index c2fd3f4b62d9..b852a7b2a29d 100644
> > --- a/drivers/pwm/Kconfig
> > +++ b/drivers/pwm/Kconfig
> > @@ -615,6 +615,19 @@ config PWM_ROCKCHIP
> > Generic PWM framework driver for the PWM controller found on
> > Rockchip SoCs.
> > +config PWM_ROCKCHIP_V4
> > + tristate "Rockchip PWM v4 support"
> > + depends on ARCH_ROCKCHIP || COMPILE_TEST
> > + depends on HAS_IOMEM
> > + depends on MFD_ROCKCHIP_MFPWM
> > + help
> > + Generic PWM framework driver for the PWM controller found on
> > + later Rockchip SoCs such as the RK3576.
> > +
> > + Uses the Rockchip Multi-function PWM controller driver infrastructure
> > + to guarantee fearlessly concurrent operation with other functions of
> > + the same device implemented by drivers in other subsystems.
> > +
> > config PWM_SAMSUNG
> > tristate "Samsung PWM support"
> > depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST
> > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> > index dfa8b4966ee1..fe0d16558d99 100644
> > --- a/drivers/pwm/Makefile
> > +++ b/drivers/pwm/Makefile
> > @@ -55,6 +55,7 @@ obj-$(CONFIG_PWM_RENESAS_RZG2L_GPT) += pwm-rzg2l-gpt.o
> > obj-$(CONFIG_PWM_RENESAS_RZ_MTU3) += pwm-rz-mtu3.o
> > obj-$(CONFIG_PWM_RENESAS_TPU) += pwm-renesas-tpu.o
> > obj-$(CONFIG_PWM_ROCKCHIP) += pwm-rockchip.o
> > +obj-$(CONFIG_PWM_ROCKCHIP_V4) += pwm-rockchip-v4.o
> > obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
> > obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o
> > obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o
> > diff --git a/drivers/pwm/pwm-rockchip-v4.c b/drivers/pwm/pwm-rockchip-v4.c
> > new file mode 100644
> > index 000000000000..7afa83f12b6a
> > --- /dev/null
> > +++ b/drivers/pwm/pwm-rockchip-v4.c
> > @@ -0,0 +1,353 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2025 Collabora Ltd.
> > + *
> > + * A Pulse-Width-Modulation (PWM) generator driver for the generators found in
> > + * Rockchip SoCs such as the RK3576, internally referred to as "PWM v4". Uses
> > + * the MFPWM infrastructure to guarantee exclusive use over the device without
> > + * other functions of the device from different drivers interfering with its
> > + * operation while it's active.
> > + *
> > + * Technical Reference Manual: Chapter 31 of the RK3506 TRM Part 1, a SoC which
> > + * uses the same PWM hardware and has a publicly available TRM.
> > + * https://opensource.rock-chips.com/images/3/36/Rockchip_RK3506_TRM_Part_1_V1.2-20250811.pdf
> > + *
> > + * Authors:
> > + * Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > + *
> > + * Limitations:
> > + * - When the output is disabled, it will end abruptly without letting the
> > + * current period complete.
> > + * TODO: This can be fixed in the driver in the future by having the enable-
> > + * to-disable transition switch to oneshot mode with one repetition,
> > + * and then disable the pwmclk and release mfpwm when the oneshot
> > + * complete interrupt fires.
> > + * - When the output is disabled, the pin will remain driven to whatever state
> > + * it last had.
> > + * - Adjustments to the duty cycle will only take effect during the next period.
> > + * - Adjustments to the period length will only take effect during the next
> > + * period.
> > + * - offset should be between 0 and (period - duty)
> > + */
> > +
> > +#include <linux/math64.h>
> > +#include <linux/mfd/rockchip-mfpwm.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pwm.h>
> > +
> > +struct rockchip_pwm_v4 {
> > + struct rockchip_mfpwm_func *pwmf;
> > + struct pwm_chip chip;
> > +};
> > +
> > +struct rockchip_pwm_v4_wf {
> > + u32 period;
> > + u32 duty;
> > + u32 offset;
> > + u8 enable;
> > +};
> > +
>
> ...
>
> > +
> > +static int rockchip_pwm_v4_read_wf(struct pwm_chip *chip, struct pwm_device *pwm,
> > + void *_wfhw)
> > +{
> > + struct rockchip_pwm_v4 *pc = to_rockchip_pwm_v4(chip);
> > + struct rockchip_pwm_v4_wf *wfhw = _wfhw;
> > + int ret = 0;
> > +
>
> Redundant blank lin. ;-)
>
> > +
> > + ret = mfpwm_acquire(pc->pwmf);
> > + if (ret)
> > + return ret;
> > +
> > + wfhw->period = mfpwm_reg_read(pc->pwmf->base, PWMV4_REG_PERIOD);
> > + wfhw->duty = mfpwm_reg_read(pc->pwmf->base, PWMV4_REG_DUTY);
> > + wfhw->offset = mfpwm_reg_read(pc->pwmf->base, PWMV4_REG_OFFSET);
> > + wfhw->enable = mfpwm_reg_read(pc->pwmf->base, PWMV4_REG_ENABLE) & PWMV4_EN_BOTH_MASK;
> > +
> > + mfpwm_release(pc->pwmf);
> > +
> > + return 0;
> > +}
> > +
>
> ...
>
> > +MODULE_AUTHOR("Nicolas Frattaroli <nicolas.frattaroli@collabora.com>");
> > +MODULE_DESCRIPTION("Rockchip PWMv4 Driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_IMPORT_NS("ROCKCHIP_MFPWM");
> > +MODULE_ALIAS("platform:pwm-rockchip-v4");
> >
>
> Tested-by: Damon Ding <damon.ding@rock-chips.com>
>
> I have tested all the PWM channels in continuous mode on my RK3576-IOTEST
> board.
>
> Test commands are like:
>
> cd /sys/class/pwm/pwmchip0/
> echo 0 > export
> cd pwm0
> echo 10000 > period
> echo 5000 > duty_cycle
> echo normal > polarity
> echo 1 > enable

Thanks for the test, very appreciated.

I wonder what made you test using sysfs instead of
https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/libpwm.git/.
Is it unknown? Too complicated? Other problems?

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]