pwm: Add Nuvoton MA35D1 PWM controller support
Subsystems (3)
v4 cover pwm: Add Nuvoton MA35D1 PWM controller support
From: Chi-Wen Weng <[email protected]>
This series adds support for the Nuvoton MA35D1 PWM controller.
The MA35D1 PWM controller provides 6 PWM channels. The hardware supports several counter types and output modes. This driver configures the controller to use up-counting mode, auto-reload mode and independent output mode. The waveform generator is configured to drive the output high at the zero point and low at the compare-up point.
For the up-counting mode used by this driver, the counter counts from 0 to PERIOD inclusive. Therefore, the programmed period is PERIOD + 1 cycles. The hardware can generate 0% duty cycle with CMPDAT = 0 and 100% duty cycle with CMPDAT > PERIOD. To keep 100% duty cycle representable, the driver limits the maximum PERIOD value to 0xfffe and reserves CMPDAT = 0xffff for the full-duty case.
The hardware buffers PERIOD and CMPDAT updates when IMMLDENn is disabled. The driver keeps IMMLDENn disabled, so period and duty cycle updates take effect at the end of the current period. Polarity and waveform-control changes are applied directly and may cause transient output changes if the PWM is running.
When the PWM output is disabled by clearing POENn, the output pin is put into tri-state according to the MA35D1 reference manual.
Changes in v4:
- Add a Limitations section to describe the hardware capabilities and driver limitations.
- Add a link to the MA35D1 reference manual.
- Replace register address macros containing the base pointer with register offset macros.
- Add readl/writel/rmw helper functions.
- Rename TOTAL_CHANNELS to NUM_CHANNELS.
- Use unsigned long for the cached clock rate.
- Use devm_clk_rate_exclusive_get().
- Configure polarity before enabling the counter and output.
- Add controller initialization for up-counting, auto-reload and independent output mode.
- Configure the waveform generator for zero-point-high and compare-up-low output.
- Fix the period conversion because the hardware period is PERIOD + 1 cycles.
- Limit the maximum PERIOD value to 0xfffe so that CMPDAT = 0xffff can be used to generate 100% duty cycle.
- Use CNTEN and POEN bits to report the enabled state in .get_state().
- Disable the PWM by clearing POENn and CNTENn.
- Fix error message capitalization and trailing newlines.
- Fix coding style issues reported by review/checkpatch-style tools.
Changes in v3:
- Update nuvoton,ma35d1-pwm.yaml
- Add maintainers entry
- Increse "#pwm-cells" to 3
- Update ma35d1 pwm driver
- Make include header and macros definitions organized alphabetically
- Rename macros REG_PWM_XXXX to MA35D1_REG_PWM_XXXX
- Add macros for register address
v2 resend:
- Remove wrong 'Reviewed-by' tags.
Changes in v2:
- Update nuvoton,ma35d1-pwm.yaml
- Fix 'maxItems' of 'reg' to 1.
- Remove unused label
- Update ma35d1 pwm driver
- Remove MODULE_ALIAS()
- Add chip->atomic = true
Chi-Wen Weng (2):
dt-bindings: pwm: Add Nuvoton MA35D1 PWM controller
pwm: Add Nuvoton MA35D1 PWM controller support
.../bindings/pwm/nuvoton,ma35d1-pwm.yaml | 45 +++ drivers/pwm/Kconfig | 9 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-ma35d1.c | 344 ++++++++++++++++++ 4 files changed, 399 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml create mode 100644 drivers/pwm/pwm-ma35d1.c
Signature · 1 line
2.25.1
PATCH v4 1/2 dt-bindings: pwm: Add Nuvoton MA35D1 PWM controller
From: Chi-Wen Weng <[email protected]>
Add device tree binding for the Nuvoton MA35D1 PWM controller.
The MA35D1 PWM controller provides 6 PWM channels and uses one register region and one functional clock. The binding uses the standard PWM binding with three PWM cells.
--- .../bindings/pwm/nuvoton,ma35d1-pwm.yaml | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +45 -0
@@ -0,0 +1,45 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/nuvoton,ma35d1-pwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton MA35D1 PWM controller + +maintainers: + - Chi-Wen Weng <[email protected]> + +allOf: + - $ref: pwm.yaml# + +properties: + compatible: + enum: + - nuvoton,ma35d1-pwm + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + "#pwm-cells": + const: 3 + +required: + - compatible + - reg + - clocks + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> + + pwm@40580000 { + compatible = "nuvoton,ma35d1-pwm"; + reg = <0x40580000 0x400>; + clocks = <&clk EPWM0_GATE>; + #pwm-cells = <3>; + };
Signature · 1 line
2.25.1
PATCH v4 2/2 pwm: Add Nuvoton MA35D1 PWM controller support
In reply to Chi-Wen WengFrom: Chi-Wen Weng <[email protected]>
Add a PWM framework driver for the Nuvoton MA35D1 PWM controller.
The MA35D1 PWM controller provides 6 PWM channels. The hardware supports up, down and up-down counter types, auto-reload and one-shot modes, and independent and complementary output modes. This driver configures all channels to up-counting mode, auto-reload mode and independent output mode.
The waveform generator is configured to drive the output high at the zero point and low at the compare-up point. In up-counting mode the counter counts from 0 to PERIOD inclusive, so the PWM period is PERIOD + 1 cycles. With the selected waveform actions, CMPDAT = 0 generates 0% duty cycle and CMPDAT > PERIOD generates 100% duty cycle. Limit PERIOD to 0xfffe so that CMPDAT = 0xffff can be used for the full-duty case.
PERIOD and CMPDAT updates are buffered by the hardware and take effect at the end of the current period because IMMLDENn is left disabled. When the PWM output is disabled, POENn is cleared and the output pin is put into tri-state.
--- drivers/pwm/Kconfig | 9 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-ma35d1.c | 344 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 354 insertions(+) create mode 100644 drivers/pwm/pwm-ma35d1.c
drivers/pwm/Kconfig +9 -0
@@ -463,6 +463,15 @@ config PWM_LPSS_PLATFORM To compile this driver as a module, choose M here: the module will be called pwm-lpss-platform. +config PWM_MA35D1 + tristate "Nuvoton MA35D1 PWM support" + depends on ARCH_MA35 || COMPILE_TEST + help + Generic PWM framework driver for Nuvoton MA35D1. + + To compile this driver as a module, choose M here: the module + will be called pwm-ma35d1. + config PWM_MAX7360 tristate "MAX7360 PWMs" depends on MFD_MAX7360
drivers/pwm/Makefile +1 -0
@@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_LPC32XX) += pwm-lpc32xx.o obj-$(CONFIG_PWM_LPSS) += pwm-lpss.o obj-$(CONFIG_PWM_LPSS_PCI) += pwm-lpss-pci.o obj-$(CONFIG_PWM_LPSS_PLATFORM) += pwm-lpss-platform.o +obj-$(CONFIG_PWM_MA35D1) += pwm-ma35d1.o obj-$(CONFIG_PWM_MAX7360) += pwm-max7360.o obj-$(CONFIG_PWM_MC33XS2410) += pwm-mc33xs2410.o obj-$(CONFIG_PWM_MEDIATEK) += pwm-mediatek.o
drivers/pwm/pwm-ma35d1.c new file +344 -0
344 lines not on this page · raw message
Signature · 1 line
2.25.1
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The driver unconditionally clobbers PWM waveform generator and control registers in probe()…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The driver unconditionally clobbers PWM waveform generator and control registers in probe(), breaking seamless handover for active PWM channels. --
commit 1c5d12cc5f94bf9ab4e36bc34c306ce883661f7e
Author: Chi-Wen Weng <[email protected]>
pwm: Add Nuvoton MA35D1 PWM controller support
Add a PWM framework driver for the Nuvoton MA35D1 PWM controller.
Quoted 5 lines
drivers/pwm/pwm-ma35d1.c new file +0
-0
[ ... ]
Quoted 42 lines
drivers/pwm/pwm-ma35d1.c new file +42
-0+static void nuvoton_pwm_init (struct nuvoton_pwm *nvtpwm) +{ +u32 ctl1_mask =0 ; +u32 wgctl0_mask =0 ; +u32 wgctl0_val =0 ; +u32 wgctl1_mask =0 ; +u32 wgctl1_val =0 ; +int ch; + +for (ch =0 ; ch <MA35D1_PWM_NUM_CHANNELS ; ch++) { +/* CNTTYPEn = 00: up counter type */ + ctl1_mask |=nuvoton_pwm_ctl1_cnttype_mask (ch); + +/* CNTMODEn = 0: auto-reload mode */ + ctl1_mask |=MA35D1_PWM_CTL1_CNTMODE_MASK (ch); + +/* ZPCTLn = 10: output high at zero point */ + wgctl0_mask |=nuvoton_pwm_wgctl_zero_mask (ch); + wgctl0_val |=MA35D1_PWM_WGCTL_ZERO_HIGH (ch); + +/* PRDPCTLn = 00: do nothing at period point */ + wgctl0_mask |=nuvoton_pwm_wgctl_period_mask (ch); + +/* CMPUCTLn = 01: output low at compare up point */ + wgctl1_mask |=nuvoton_pwm_wgctl_cmp_up_mask (ch); + wgctl1_val |=MA35D1_PWM_WGCTL_CMP_UP_LOW (ch); + +/* CMPDCTLn = 00: do nothing at compare down point */ + wgctl1_mask |=nuvoton_pwm_wgctl_cmp_down_mask (ch); + } + +for (ch =0 ; ch <MA35D1_PWM_NUM_CHANNELS ; ch +=2 ) { +/* OUTMODEn = 0: independent mode */ + ctl1_mask |=MA35D1_PWM_CTL1_OUTMODE_MASK (ch); + } + +nuvoton_pwm_rmw (nvtpwm,MA35D1_REG_PWM_CTL1 , ctl1_mask,0 ); +nuvoton_pwm_rmw (nvtpwm,MA35D1_REG_PWM_WGCTL0 , + wgctl0_mask, wgctl0_val); +nuvoton_pwm_rmw (nvtpwm,MA35D1_REG_PWM_WGCTL1 , + wgctl1_mask, wgctl1_val); +}
[ ... ]
Quoted 2 lines
drivers/pwm/pwm-ma35d1.c new file +2
-0+static int nuvoton_pwm_probe (struct platform_device *pdev) +{
[ ... ]
drivers/pwm/pwm-ma35d1.c line 315
Quoted 6 lines
drivers/pwm/pwm-ma35d1.c new file +6
-0+if (nvtpwm->clkrate >NSEC_PER_SEC ) +return dev_err_probe (dev, -EINVAL , +"PWM clock out of range (%lu)\n" , + nvtpwm->clkrate); + +nuvoton_pwm_init (nvtpwm);
[Severity: Medium]
Will this unconditionally clobber the waveform generator and control registers for all channels during probe()?
If the bootloader has already enabled a PWM channel (such as for a regulator or display backlight) with a different waveform configuration, altering these global control registers while the channel is running might corrupt its output.
Also, since nuvoton_pwm_get_state() relies on PERIOD and CMPDAT values, altering their hardware interpretation here means the PWM core will inherit an incorrect initial state.
Should the driver check if channels are already active via CNTEN and POEN before modifying their configuration to allow for a seamless handover?
Quoted 3 lines
drivers/pwm/pwm-ma35d1.c new file +3
-0+ + chip->ops = &nuvoton_pwm_ops; + chip->atomic = true;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Uwe Kleine-König
maintainerHello,
drivers/pwm/pwm-ma35d1.c line 38
Quoted 5 lines · Chi-Wen Weng
drivers/pwm/pwm-ma35d1.c new file +5
-0+#include <linux/bits.h> +#include <linux/clk.h> +#include <linux/io.h> +#include <linux/math64.h> +#include <linux/mod_devicetable.h>
Please don't include that file, <linux/platform_device.h> should pull in the things you need from that file.
Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10
-0+#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> + +#define MA35D1_REG_PWM_CTL0 0x00 +#define MA35D1_REG_PWM_CTL1 0x04 +#define MA35D1_REG_PWM_CNTEN 0x20 +#define MA35D1_REG_PWM_PERIOD (ch) (0x30 + 4 * (ch)) +#define MA35D1_REG_PWM_CMPDAT (ch) (0x50 + 4 * (ch)) +#define MA35D1_REG_PWM_WGCTL0 0xb0
drivers/pwm/pwm-ma35d1.c line 58
Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10
-0+#define MA35D1_REG_PWM_WGCTL1 0xb4 +#define MA35D1_REG_PWM_POLCTL 0xd4 +#define MA35D1_REG_PWM_POEN 0xd8 + +#define MA35D1_PWM_CTL1_CNTMODE_MASK (ch) BIT(16 + (ch)) +#define MA35D1_PWM_CTL1_OUTMODE_MASK (ch) BIT(24 + ((ch) / 2)) + +#define MA35D1_PWM_WGCTL_ACTION_MASK 0x3 +#define MA35D1_PWM_WGCTL_ACTION_LOW 1 +#define MA35D1_PWM_WGCTL_ACTION_HIGH 2
If you make this:
#define MA35D1_PWM_WGCTL_ACTION(ch) GENMASK(2 * (ch) + 2, 2 * (ch)) #define MA35D1_PWM_WGCTL_ACTION_LOW 1 #define MA35D1_PWM_WGCTL_ACTION_HIGH 2
you can drop the static inlines below.
drivers/pwm/pwm-ma35d1.c line 71
Quoted 13 lines
drivers/pwm/pwm-ma35d1.c new file +13
-0+ +#define MA35D1_PWM_WGCTL_ZERO_HIGH (ch) \ + (MA35D1_PWM_WGCTL_ACTION_HIGH << (2 * (ch))) +#define MA35D1_PWM_WGCTL_CMP_UP_LOW (ch) \ + (MA35D1_PWM_WGCTL_ACTION_LOW << (2 * (ch))) + +#define MA35D1_PWM_CNTEN_EN (ch) BIT(ch) +#define MA35D1_PWM_POEN_EN (ch) BIT(ch) +#define MA35D1_PWM_POLCTL_INV (ch) BIT(ch) + +#define MA35D1_PWM_MAX_CMPDAT 0xffff +#define MA35D1_PWM_MAX_PERIOD 0xfffe +#define MA35D1_PWM_MAX_PERIOD_CYCLES (MA35D1_PWM_MAX_PERIOD + 1)
This is irritating with similar names and different values/semantic.
drivers/pwm/pwm-ma35d1.c line 315
Quoted 7 lines
drivers/pwm/pwm-ma35d1.c new file +5
-0+#define MA35D1_PWM_NUM_CHANNELS 6 + +static int nuvoton_pwm_probe (struct platform_device *pdev) +{ +nuvoton_pwm_init (nvtpwm);
This clobbers what the hardware is doing. The idea here is to not modify the hardware settings at probe time to keep e.g. a backlight configured as it was setup by the bootloader and only modify on explicit calls to .apply().
drivers/pwm/pwm-ma35d1.c line 325
Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10
-0+ + chip->ops = &nuvoton_pwm_ops; + chip->atomic = true; + + ret =devm_pwmchip_add (dev, chip); +if (ret) +return dev_err_probe (dev, ret,"Unable to add PWM chip\n" ); + +return 0 ; +}
Best regards
Uwe
Uwe Kleine-König 於 2026/7/16 下午 11:29 寫道:
Quoted 66 lines
Hello,
drivers/pwm/pwm-ma35d1.c line 38
Quoted 5 lines · Chi-Wen Weng
drivers/pwm/pwm-ma35d1.c new file +5
-0+#include <linux/bits.h> +#include <linux/clk.h> +#include <linux/io.h> +#include <linux/math64.h> +#include <linux/mod_devicetable.h> Please don't include that file, <
linux/platform_device.h> should pull in the things you need from that file.Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10
-0+#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> + +#define MA35D1_REG_PWM_CTL0 0x00 +#define MA35D1_REG_PWM_CTL1 0x04 +#define MA35D1_REG_PWM_CNTEN 0x20 +#define MA35D1_REG_PWM_PERIOD (ch) (0x30 + 4 * (ch)) +#define MA35D1_REG_PWM_CMPDAT (ch) (0x50 + 4 * (ch)) +#define MA35D1_REG_PWM_WGCTL0 0xb0drivers/pwm/pwm-ma35d1.c line 58
Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10
-0+#define MA35D1_REG_PWM_WGCTL1 0xb4 +#define MA35D1_REG_PWM_POLCTL 0xd4 +#define MA35D1_REG_PWM_POEN 0xd8 + +#define MA35D1_PWM_CTL1_CNTMODE_MASK (ch) BIT(16 + (ch)) +#define MA35D1_PWM_CTL1_OUTMODE_MASK (ch) BIT(24 + ((ch) / 2)) + +#define MA35D1_PWM_WGCTL_ACTION_MASK 0x3 +#define MA35D1_PWM_WGCTL_ACTION_LOW 1 +#define MA35D1_PWM_WGCTL_ACTION_HIGH 2If you make this:
#define MA35D1_PWM_WGCTL_ACTION(ch) GENMASK(2 * (ch) + 2, 2 * (ch)) #define MA35D1_PWM_WGCTL_ACTION_LOW 1 #define MA35D1_PWM_WGCTL_ACTION_HIGH 2you can drop the static inlines below.
drivers/pwm/pwm-ma35d1.c line 71
Quoted 13 lines
drivers/pwm/pwm-ma35d1.c new file +13
-0+ +#define MA35D1_PWM_WGCTL_ZERO_HIGH (ch) \ + (MA35D1_PWM_WGCTL_ACTION_HIGH << (2 * (ch))) +#define MA35D1_PWM_WGCTL_CMP_UP_LOW (ch) \ + (MA35D1_PWM_WGCTL_ACTION_LOW << (2 * (ch))) + +#define MA35D1_PWM_CNTEN_EN (ch) BIT(ch) +#define MA35D1_PWM_POEN_EN (ch) BIT(ch) +#define MA35D1_PWM_POLCTL_INV (ch) BIT(ch) + +#define MA35D1_PWM_MAX_CMPDAT 0xffff +#define MA35D1_PWM_MAX_PERIOD 0xfffe +#define MA35D1_PWM_MAX_PERIOD_CYCLES (MA35D1_PWM_MAX_PERIOD + 1)This is irritating with similar names and different values/semantic.
drivers/pwm/pwm-ma35d1.c line 315
Quoted 7 lines
drivers/pwm/pwm-ma35d1.c new file +5
-0+#define MA35D1_PWM_NUM_CHANNELS 6 + +static int nuvoton_pwm_probe (struct platform_device *pdev) +{ +nuvoton_pwm_init (nvtpwm);This clobbers what the hardware is doing. The idea here is to not modify the hardware settings at probe time to keep e.g. a backlight configured as it was setup by the bootloader and only modify on explicit calls to .apply().
Quoted 12 lines
drivers/pwm/pwm-ma35d1.c line 325
Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10
-0+ + chip->ops = &nuvoton_pwm_ops; + chip->atomic = true; + + ret =devm_pwmchip_add (dev, chip); +if (ret) +return dev_err_probe (dev, ret,"Unable to add PWM chip\n" ); + +return 0 ; +}Best regards
Uwe
Hi Uwe,
Thanks for the review.
On the include, I will drop <linux/mod_devicetable.h> as <linux/platform_device.h> already provides what is needed here.
For the WGCTL helpers, I will rework this to avoid the extra static inline helpers and compute the 2-bit action field mask/value locally when configuring a channel. The WGCTL action field is 2 bits wide, so I will use the bit range [2 * ch + 1 : 2 * ch] for the mask.
I will also rename the maximum value definitions to make the semantics clearer. The intent is to keep PERIOD below the 16-bit register field maximum so that CMPDAT can be programmed greater than PERIOD to generate a 100% duty cycle.
Most importantly, you are right about the probe-time initialization. The driver should not reconfigure CTL1/WGCTL for all channels during probe, as that can disturb a PWM output already configured and enabled by firmware, such as a backlight.
In the next version I will remove the controller initialization from probe(). Instead, the driver will configure only the channel being changed from the .apply() callback. The disable path will only clear POENn and CNTENn and will not touch the waveform/control configuration.
Best regards,
Chi-Wen