Skip to contentSkip to conversation

Add support for nuvoton ma35d1 pwm controller

PATCH RESEND Superseded Chi-Wen Weng 4 participants 8 messages Oct 24, 2024 – Dec 6, 2024

Subsystems (3)
Device tree bindings PWM Arm/nuvoton MA35 architecture

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Chi-Wen Weng

author
#0 lore raw

v2 cover Add support for nuvoton ma35d1 pwm controller

This patch series adds pwm driver for the nuvoton ma35d1 ARMv8 SoC. It includes DT binding documentation and the ma35d1 pwm driver.

v2 resend:
- Remove wrong 'Reviewed-by' tags.

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: nuvoton: Add MA35D1 pwm
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                      | 169 ++++++++++++++++++
 4 files changed, 224 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
author
#1 lore raw

PATCH RESEND v2 1/2 dt-bindings: pwm: nuvoton: Add MA35D1 pwm

Add dt-bindings for Nuvoton MA35D1 SoC PWM controller.

Signed-off-by Chi-Wen Weng <[email protected]>
---
 .../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
new file mode 100644
index 000000000000..ed32fc573a24
@@ -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: 2
+
+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 = <2>;
+    };
Signature · 1 line
2.25.1
Reviewed-by on v2 1/2
#2 lore raw

Re: [PATCH RESEND v2 1/2] dt-bindings: pwm: nuvoton: Add MA35D1 pwm

Quoted 4 lines · Chi-Wen Weng

Add dt-bindings for Nuvoton MA35D1 SoC PWM controller.

Signed-off-by Chi-Wen Weng <[email protected]>

---

Reviewed-by Krzysztof Kozlowski <[email protected]>

---

<form letter>
Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions, under or above your Signed-off-by tag. Tag is "received", when provided in a message replied to you on the mailing list. Tools like b4 can help here. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for tags received on the version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
</form letter>

Best regards,
Krzysztof

author
#3 lore raw

PATCH RESEND v2 2/2 pwm: Add Nuvoton MA35D1 PWM controller support

In reply to Chi-Wen Weng

This commit adds a generic PWM framework driver for Nuvoton MA35D1 PWM controller.

Signed-off-by Chi-Wen Weng <[email protected]>
---
 drivers/pwm/Kconfig      |   9 +++
 drivers/pwm/Makefile     |   1 +
 drivers/pwm/pwm-ma35d1.c | 169 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 drivers/pwm/pwm-ma35d1.c
drivers/pwm/Kconfig +9 -0
index 0915c1e7df16..97b9e83af020 100644
@@ -411,6 +411,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_MESON
 	tristate "Amlogic Meson PWM driver"
 	depends on ARCH_MESON || COMPILE_TEST
drivers/pwm/Makefile +1 -0
index 9081e0c0e9e0..c1d3a1d8add0 100644
@@ -36,6 +36,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_MESON)		+= pwm-meson.o
 obj-$(CONFIG_PWM_MEDIATEK)	+= pwm-mediatek.o
 obj-$(CONFIG_PWM_MICROCHIP_CORE)	+= pwm-microchip-core.o
drivers/pwm/pwm-ma35d1.c new file +169 -0

169 lines not on this page · raw message

Signature · 1 line
2.25.1
#4 lore raw

Re: [PATCH RESEND v2 2/2] pwm: Add Nuvoton MA35D1 PWM controller support

Hello,

Quoted 10 lines · Chi-Wen Weng

This commit adds a generic PWM framework driver for Nuvoton MA35D1 PWM controller.

Signed-off-by Chi-Wen Weng <[email protected]>
---
  drivers/pwm/Kconfig      |   9 +++
  drivers/pwm/Makefile     |   1 +
  drivers/pwm/pwm-ma35d1.c | 169 +++++++++++++++++++++++++++++++++++++++
  3 files changed, 179 insertions(+)

create mode 100644 drivers/pwm/pwm-ma35d1.c

I don't see a MAINTAINERS entry? That needs to be added in the bindings patch first, and then it should be updated to list this driver file.

Quoted 44 lines
drivers/pwm/Kconfig +9 -0
index 0915c1e7df16..97b9e83af020 100644
@@ -411,6 +411,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_MESON
  	tristate "Amlogic Meson PWM driver"
  	depends on ARCH_MESON || COMPILE_TEST
drivers/pwm/Makefile +1 -0
index 9081e0c0e9e0..c1d3a1d8add0 100644
@@ -36,6 +36,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_MESON)		+= pwm-meson.o
  obj-$(CONFIG_PWM_MEDIATEK)	+= pwm-mediatek.o
  obj-$(CONFIG_PWM_MICROCHIP_CORE)	+= pwm-microchip-core.o
drivers/pwm/pwm-ma35d1.c new file +5 -0
new file mode 100644
index 000000000000..0c4eec4a0b07
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Nuvoton MA35D1 PWM controller
+ *
+ * Copyright (C) 2024 Nuvoton Corporation

drivers/pwm/pwm-ma35d1.c line 15

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+ *               Chi-Wen Weng <[email protected]>
+ */
+
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/math64.h>

These should be organized alphabetically.

drivers/pwm/pwm-ma35d1.c line 24

Quoted 9 lines
drivers/pwm/pwm-ma35d1.c new file +9 -0
+
+/* The following are registers for PWM controller */
+#define REG_PWM_CTL0            (0x00)
+#define REG_PWM_CNTEN           (0x20)
+#define REG_PWM_PERIOD0         (0x30)
+#define REG_PWM_CMPDAT0         (0x50)
+#define REG_PWM_WGCTL0          (0xB0)
+#define REG_PWM_POLCTL          (0xD4)
+#define REG_PWM_POEN            (0xD8)

These too, I think - it will make it more readable for others.

You should also prefix all of your macros to be more explicit about their use, e.g. MA35D1_REG_PWM_CTL0. That way it's clearer that they're specific to this driver and not from elsewhere.

drivers/pwm/pwm-ma35d1.c line 27

Quoted 3 lines
drivers/pwm/pwm-ma35d1.c new file +3 -0
+
+#define PWM_TOTAL_CHANNELS      6
+#define PWM_CH_REG_SIZE         4

And these.

Quoted 16 lines
drivers/pwm/pwm-ma35d1.c new file +16 -0
+
+struct nuvoton_pwm {
+	void __iomem *base;
+	u64 clkrate;
+};
+
+static inline struct nuvoton_pwm *to_nuvoton_pwm(struct pwm_chip *chip)
+{
+	return pwmchip_get_drvdata(chip);
+}
+
+static int nuvoton_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			     const struct pwm_state *state)
+{
+	struct nuvoton_pwm *nvtpwm;
+	unsigned int ch = pwm->hwpwm;

drivers/pwm/pwm-ma35d1.c line 53

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+
+	nvtpwm = to_nuvoton_pwm(chip);
+	if (state->enabled) {
+		u64 duty_cycles, period_cycles;
+
+		/* Calculate the duty and period cycles */
+		duty_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+						  state->duty_cycle, NSEC_PER_SEC);
+		if (duty_cycles > 0xFFFF)
+			duty_cycles = 0xFFFF;

It would be good to create a macro for this value 0xFFFF, e.g. MA35D1_MAX_PWM_RATE.

drivers/pwm/pwm-ma35d1.c line 62

Quoted 9 lines
drivers/pwm/pwm-ma35d1.c new file +9 -0
+
+		period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+						    state->period, NSEC_PER_SEC);
+		if (period_cycles > 0xFFFF)
+			period_cycles = 0xFFFF;
+
+		/* Write the duty and period cycles to registers */
+		writel(duty_cycles, nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE));
+		writel(period_cycles, nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE));

Since you are using things like

nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE)

and similar so frequently, I suggest creating more macros for these sorts of accesses to improve readability, e.g.

#define MA35D1_PWM_CMPDAT0_ADDR(base, ch) base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE));

or even

#define MA35D1_PWM_CMPDAT0_ADDR(nvtpwm, ch) nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE));

and then use those instead.

drivers/pwm/pwm-ma35d1.c line 65

Quoted 3 lines
drivers/pwm/pwm-ma35d1.c new file +3 -0
+		/* Enable counter */
+		writel(readl(nvtpwm->base + REG_PWM_CNTEN) | BIT(ch),
+		       nvtpwm->base + REG_PWM_CNTEN);

Same for cases like this.

Quoted 75 lines
drivers/pwm/pwm-ma35d1.c new file +75 -0
+		/* Enable output */
+		writel(readl(nvtpwm->base + REG_PWM_POEN) | BIT(ch),
+		       nvtpwm->base + REG_PWM_POEN);
+	} else {
+		/* Disable counter */
+		writel(readl(nvtpwm->base + REG_PWM_CNTEN) & ~BIT(ch),
+		       nvtpwm->base + REG_PWM_CNTEN);
+		/* Disable output */
+		writel(readl(nvtpwm->base + REG_PWM_POEN) & ~BIT(ch),
+		       nvtpwm->base + REG_PWM_POEN);
+	}
+
+	/* Set polarity state to register */
+	if (state->polarity == PWM_POLARITY_NORMAL)
+		writel(readl(nvtpwm->base + REG_PWM_POLCTL) & ~BIT(ch),
+		       nvtpwm->base + REG_PWM_POLCTL);
+	else
+		writel(readl(nvtpwm->base + REG_PWM_POLCTL) | BIT(ch),
+		       nvtpwm->base + REG_PWM_POLCTL);
+
+	return 0;
+}
+
+static int nuvoton_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+				 struct pwm_state *state)
+{
+	struct nuvoton_pwm *nvtpwm;
+	unsigned int duty_cycles, period_cycles, cnten, outen, polarity;
+	unsigned int ch = pwm->hwpwm;
+
+	nvtpwm = to_nuvoton_pwm(chip);
+
+	cnten = readl(nvtpwm->base + REG_PWM_CNTEN);
+	outen = readl(nvtpwm->base + REG_PWM_POEN);
+	duty_cycles = readl(nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE));
+	period_cycles = readl(nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE));
+	polarity = readl(nvtpwm->base + REG_PWM_POLCTL) & BIT(ch);
+
+	state->enabled = (cnten & BIT(ch)) && (outen & BIT(ch));
+	state->polarity = polarity ? PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL;
+	state->duty_cycle = DIV64_U64_ROUND_UP((u64)duty_cycles * NSEC_PER_SEC, nvtpwm->clkrate);
+	state->period = DIV64_U64_ROUND_UP((u64)period_cycles * NSEC_PER_SEC, nvtpwm->clkrate);
+
+	return 0;
+}
+
+static const struct pwm_ops nuvoton_pwm_ops = {
+	.apply = nuvoton_pwm_apply,
+	.get_state = nuvoton_pwm_get_state,
+};
+
+static int nuvoton_pwm_probe(struct platform_device *pdev)
+{
+	struct pwm_chip *chip;
+	struct nuvoton_pwm *nvtpwm;
+	struct clk *clk;
+	int ret;
+
+	chip = devm_pwmchip_alloc(&pdev->dev, PWM_TOTAL_CHANNELS, sizeof(*nvtpwm));
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	nvtpwm = to_nuvoton_pwm(chip);
+
+	nvtpwm->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(nvtpwm->base))
+		return PTR_ERR(nvtpwm->base);
+
+	clk = devm_clk_get_enabled(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(clk), "unable to get the clock");
+
+	nvtpwm->clkrate = clk_get_rate(clk);
+	if (nvtpwm->clkrate > NSEC_PER_SEC)
+		return dev_err_probe(&pdev->dev, -EINVAL, "pwm clock out of range");

drivers/pwm/pwm-ma35d1.c line 150

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+
+	chip->ops = &nuvoton_pwm_ops;
+	chip->atomic = true;
+
+	ret = devm_pwmchip_add(&pdev->dev, chip);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "unable to add pwm chip");
+
+	return 0;
+}

As a final note, please be aware that there is a change to the PWM subsystem coming in 6.13. You don't need to do anything now, but it's worth considering how the driver might change once that hits mainline.

You can find the changes here: https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git/log/?h=pwm/duty_offset .

Quoted 19 lines
drivers/pwm/pwm-ma35d1.c new file +19 -0
+
+static const struct of_device_id nuvoton_pwm_of_match[] = {
+	{ .compatible = "nuvoton,ma35d1-pwm" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match);
+
+static struct platform_driver nuvoton_pwm_driver = {
+	.probe = nuvoton_pwm_probe,
+	.driver = {
+		.name = "nuvoton-pwm",
+		.of_match_table = nuvoton_pwm_of_match,
+	},
+};
+module_platform_driver(nuvoton_pwm_driver);
+
+MODULE_AUTHOR("Chi-Wen Weng <[email protected]>");
+MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver");
+MODULE_LICENSE("GPL");
maintainer
#5 lore raw

Re: [PATCH RESEND v2 2/2] pwm: Add Nuvoton MA35D1 PWM controller support

hello,

Quoted 9 lines · Trevor Gamblin

drivers/pwm/pwm-ma35d1.c line 15

Quoted 7 lines · Chi-Wen Weng
drivers/pwm/pwm-ma35d1.c new file +7 -0
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/math64.h>

These should be organized alphabetically.

Quoted 11 lines

drivers/pwm/pwm-ma35d1.c line 24

Quoted 9 lines
drivers/pwm/pwm-ma35d1.c new file +9 -0
+
+/* The following are registers for PWM controller */
+#define REG_PWM_CTL0            (0x00)
+#define REG_PWM_CNTEN           (0x20)
+#define REG_PWM_PERIOD0         (0x30)
+#define REG_PWM_CMPDAT0         (0x50)
+#define REG_PWM_WGCTL0          (0xB0)
+#define REG_PWM_POLCTL          (0xD4)
+#define REG_PWM_POEN            (0xD8)

These too, I think - it will make it more readable for others.

Keeping the registers in address order is the usual thing to do, so please keep the order of these.

Otherwise I agree to Trevor's comments. Thanks for taking the time to look at this patch.

Best regards
Uwe

author
#6 lore raw

Re: [PATCH RESEND v2 2/2] pwm: Add Nuvoton MA35D1 PWM controller support

Hi Uwe,

Thanks for your comments.

Quoted 22 lines · Uwe Kleine-König

hello,

Quoted 9 lines · Trevor Gamblin

drivers/pwm/pwm-ma35d1.c line 15

Quoted 7 lines · Chi-Wen Weng
drivers/pwm/pwm-ma35d1.c new file +7 -0
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/math64.h>

These should be organized alphabetically.

Quoted 10 lines

drivers/pwm/pwm-ma35d1.c line 24

Quoted 9 lines
drivers/pwm/pwm-ma35d1.c new file +9 -0
+
+/* The following are registers for PWM controller */
+#define REG_PWM_CTL0            (0x00)
+#define REG_PWM_CNTEN           (0x20)
+#define REG_PWM_PERIOD0         (0x30)
+#define REG_PWM_CMPDAT0         (0x50)
+#define REG_PWM_WGCTL0          (0xB0)
+#define REG_PWM_POLCTL          (0xD4)
+#define REG_PWM_POEN            (0xD8)

These too, I think - it will make it more readable for others.

Quoted 8 lines

Keeping the registers in address order is the usual thing to do, so please keep the order of these.

Otherwise I agree to Trevor's comments. Thanks for taking the time to look at this patch.

Best regards
Uwe

I will keep the registers in address order.

Otherwise I will modify per Trevor's suggestions .

Thanks.

Chi-Wen Weng

author
#7 lore raw

Re: [PATCH RESEND v2 2/2] pwm: Add Nuvoton MA35D1 PWM controller support

In reply to Trevor Gamblin

Hi Trevor,

Thanks for your suggestions.

Quoted 255 lines · Trevor Gamblin

Hello,

Quoted 10 lines · Chi-Wen Weng

This commit adds a generic PWM framework driver for Nuvoton MA35D1 PWM controller.

Signed-off-by Chi-Wen Weng <[email protected]>

---
  drivers/pwm/Kconfig      |   9 +++
  drivers/pwm/Makefile     |   1 +
  drivers/pwm/pwm-ma35d1.c | 169 +++++++++++++++++++++++++++++++++++++++

  3 files changed, 179 insertions(+)

  create mode 100644 drivers/pwm/pwm-ma35d1.c

I don't see a MAINTAINERS entry? That needs to be added in the bindings patch first, and then it should be updated to list this driver file.

Quoted 43 lines
drivers/pwm/Kconfig +0 -0
index 0915c1e7df16..97b9e83af020 100644
@@ -411,6 +411,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.

drivers/pwm/Kconfig +4 -0
+
+      To compile this driver as a module, choose M here: the module
+      will be called pwm-ma35d1.
+

  config PWM_MESON
      tristate "Amlogic Meson PWM driver"
      depends on ARCH_MESON || COMPILE_TEST

drivers/pwm/Makefile +0 -0
index 9081e0c0e9e0..c1d3a1d8add0 100644
@@ -36,6 +36,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_MESON)        += pwm-meson.o obj-$(CONFIG_PWM_MEDIATEK)    += pwm-mediatek.o obj-$(CONFIG_PWM_MICROCHIP_CORE)    += pwm-microchip-core.o

drivers/pwm/pwm-ma35d1.c new file +5 -0
new file mode 100644
index 000000000000..0c4eec4a0b07
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Nuvoton MA35D1 PWM controller
+ *
+ * Copyright (C) 2024 Nuvoton Corporation

drivers/pwm/pwm-ma35d1.c line 15

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+ *               Chi-Wen Weng <[email protected]>
+ */
+
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/math64.h>

These should be organized alphabetically.

drivers/pwm/pwm-ma35d1.c line 24

Quoted 9 lines
drivers/pwm/pwm-ma35d1.c new file +9 -0
+
+/* The following are registers for PWM controller */
+#define REG_PWM_CTL0            (0x00)
+#define REG_PWM_CNTEN           (0x20)
+#define REG_PWM_PERIOD0         (0x30)
+#define REG_PWM_CMPDAT0         (0x50)
+#define REG_PWM_WGCTL0          (0xB0)
+#define REG_PWM_POLCTL          (0xD4)
+#define REG_PWM_POEN            (0xD8)

These too, I think - it will make it more readable for others.

You should also prefix all of your macros to be more explicit about their use, e.g. MA35D1_REG_PWM_CTL0. That way it's clearer that they're specific to this driver and not from elsewhere.

drivers/pwm/pwm-ma35d1.c line 27

Quoted 3 lines
drivers/pwm/pwm-ma35d1.c new file +3 -0
+
+#define PWM_TOTAL_CHANNELS      6
+#define PWM_CH_REG_SIZE         4

And these.

Quoted 17 lines
drivers/pwm/pwm-ma35d1.c new file +12 -0
+
+struct nuvoton_pwm {
+    void __iomem *base;
+    u64 clkrate;
+};
+
+static inline struct nuvoton_pwm *to_nuvoton_pwm(struct pwm_chip *chip)
+{
+    return pwmchip_get_drvdata(chip);
+}
+
+static int nuvoton_pwm_apply(struct pwm_chip *chip, struct

pwm_device *pwm,
+                 const struct pwm_state *state)

drivers/pwm/pwm-ma35d1.c new file +3 -0
+{
+    struct nuvoton_pwm *nvtpwm;
+    unsigned int ch = pwm->hwpwm;

drivers/pwm/pwm-ma35d1.c line 53

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+
+    nvtpwm = to_nuvoton_pwm(chip);
+    if (state->enabled) {
+        u64 duty_cycles, period_cycles;
+
+        /* Calculate the duty and period cycles */
+        duty_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+                          state->duty_cycle, NSEC_PER_SEC);
+        if (duty_cycles > 0xFFFF)
+            duty_cycles = 0xFFFF;

It would be good to create a macro for this value 0xFFFF, e.g. MA35D1_MAX_PWM_RATE.

Quoted 11 lines
drivers/pwm/pwm-ma35d1.c new file +8 -0
+
+        period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+                            state->period, NSEC_PER_SEC);
+        if (period_cycles > 0xFFFF)
+            period_cycles = 0xFFFF;
+
+        /* Write the duty and period cycles to registers */
+        writel(duty_cycles, nvtpwm->base + REG_PWM_CMPDAT0 + (ch *

PWM_CH_REG_SIZE));
+        writel(period_cycles, nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE));

Since you are using things like

nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE)

and similar so frequently, I suggest creating more macros for these sorts of accesses to improve readability, e.g.

#define MA35D1_PWM_CMPDAT0_ADDR(base, ch)     base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE));

or even

#define MA35D1_PWM_CMPDAT0_ADDR(nvtpwm, ch)     nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE));

and then use those instead.

Quoted 3 lines

+        /* Enable counter */
+        writel(readl(nvtpwm->base + REG_PWM_CNTEN) | BIT(ch), +               nvtpwm->base + REG_PWM_CNTEN);

Same for cases like this.

Quoted 92 lines

+        /* Enable output */
+        writel(readl(nvtpwm->base + REG_PWM_POEN) | BIT(ch), +               nvtpwm->base + REG_PWM_POEN);
+    } else {
+        /* Disable counter */
+        writel(readl(nvtpwm->base + REG_PWM_CNTEN) & ~BIT(ch), +               nvtpwm->base + REG_PWM_CNTEN);
+        /* Disable output */
+        writel(readl(nvtpwm->base + REG_PWM_POEN) & ~BIT(ch), +               nvtpwm->base + REG_PWM_POEN);
+    }

drivers/pwm/pwm-ma35d1.c new file +13 -0
+
+    /* Set polarity state to register */
+    if (state->polarity == PWM_POLARITY_NORMAL)
+        writel(readl(nvtpwm->base + REG_PWM_POLCTL) & ~BIT(ch),
+               nvtpwm->base + REG_PWM_POLCTL);
+    else
+        writel(readl(nvtpwm->base + REG_PWM_POLCTL) | BIT(ch),
+               nvtpwm->base + REG_PWM_POLCTL);
+
+    return 0;
+}
+
+static int nuvoton_pwm_get_state(struct pwm_chip *chip, struct

pwm_device *pwm,
+                 struct pwm_state *state)

drivers/pwm/pwm-ma35d1.c new file +10 -0
+{
+    struct nuvoton_pwm *nvtpwm;
+    unsigned int duty_cycles, period_cycles, cnten, outen, polarity;
+    unsigned int ch = pwm->hwpwm;
+
+    nvtpwm = to_nuvoton_pwm(chip);
+
+    cnten = readl(nvtpwm->base + REG_PWM_CNTEN);
+    outen = readl(nvtpwm->base + REG_PWM_POEN);
+    duty_cycles = readl(nvtpwm->base + REG_PWM_CMPDAT0 + (ch *

PWM_CH_REG_SIZE));
+    period_cycles = readl(nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE));
+    polarity = readl(nvtpwm->base + REG_PWM_POLCTL) & BIT(ch); +
+    state->enabled = (cnten & BIT(ch)) && (outen & BIT(ch)); +    state->polarity = polarity ? PWM_POLARITY_INVERSED :
PWM_POLARITY_NORMAL;
+    state->duty_cycle = DIV64_U64_ROUND_UP((u64)duty_cycles * NSEC_PER_SEC, nvtpwm->clkrate);
+    state->period = DIV64_U64_ROUND_UP((u64)period_cycles * NSEC_PER_SEC, nvtpwm->clkrate);

drivers/pwm/pwm-ma35d1.c new file +17 -0
+
+    return 0;
+}
+
+static const struct pwm_ops nuvoton_pwm_ops = {
+    .apply = nuvoton_pwm_apply,
+    .get_state = nuvoton_pwm_get_state,
+};
+
+static int nuvoton_pwm_probe(struct platform_device *pdev)
+{
+    struct pwm_chip *chip;
+    struct nuvoton_pwm *nvtpwm;
+    struct clk *clk;
+    int ret;
+
+    chip = devm_pwmchip_alloc(&pdev->dev, PWM_TOTAL_CHANNELS,

sizeof(*nvtpwm));
+    if (IS_ERR(chip))
+        return PTR_ERR(chip);

drivers/pwm/pwm-ma35d1.c new file +10 -0
+
+    nvtpwm = to_nuvoton_pwm(chip);
+
+    nvtpwm->base = devm_platform_ioremap_resource(pdev, 0);
+    if (IS_ERR(nvtpwm->base))
+        return PTR_ERR(nvtpwm->base);
+
+    clk = devm_clk_get_enabled(&pdev->dev, NULL);
+    if (IS_ERR(clk))
+        return dev_err_probe(&pdev->dev, PTR_ERR(clk), "unable to

get the clock");
+
+    nvtpwm->clkrate = clk_get_rate(clk);
+    if (nvtpwm->clkrate > NSEC_PER_SEC)
+        return dev_err_probe(&pdev->dev, -EINVAL, "pwm clock out of range");

drivers/pwm/pwm-ma35d1.c new file +7 -0
+
+    chip->ops = &nuvoton_pwm_ops;
+    chip->atomic = true;
+
+    ret = devm_pwmchip_add(&pdev->dev, chip);
+    if (ret < 0)
+        return dev_err_probe(&pdev->dev, ret, "unable to add pwm

chip");

drivers/pwm/pwm-ma35d1.c line 87

Quoted 3 lines
drivers/pwm/pwm-ma35d1.c new file +3 -0
+
+    return 0;
+}

As a final note, please be aware that there is a change to the PWM subsystem coming in 6.13. You don't need to do anything now, but it's worth considering how the driver might change once that hits mainline.

You can find the changes here: https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git/log/?h=pwm/duty_offset .

Quoted 11 lines
drivers/pwm/pwm-ma35d1.c new file +11 -0
+
+static const struct of_device_id nuvoton_pwm_of_match[] = {
+    { .compatible = "nuvoton,ma35d1-pwm" },
+    {}
+};
+MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match);
+
+static struct platform_driver nuvoton_pwm_driver = {
+    .probe = nuvoton_pwm_probe,
+    .driver = {
+        .name = "nuvoton-pwm",
Quoted 8 lines

drivers/pwm/pwm-ma35d1.c line 169

Quoted 8 lines

+        .of_match_table = nuvoton_pwm_of_match,
+    },

drivers/pwm/pwm-ma35d1.c new file +6 -0
+};
+module_platform_driver(nuvoton_pwm_driver);
+
+MODULE_AUTHOR("Chi-Wen Weng <[email protected]>");
+MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver");
+MODULE_LICENSE("GPL");

Per Uwe's commet, I will keep the registers in address order.

Otherwise I will modify in next version.

Thanks.

Chi-Wen Weng