Skip to contentSkip to conversation

Add support for nuvoton ma35d1 pwm controller

PATCH Superseded Chi-Wen Weng 4 participants 11 messages Oct 18, 2024

Subsystems (3)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover Add support for nuvoton ma35d1 pwm controller

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

Chi-Wen Weng (2):
dt-bindings: pwm: Add dt-bindings for Nuvoton MA35D1 SoC PWM
Controller
pwm: Add Nuvoton 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
#1 lore raw

PATCH 1/2 dt-bindings: pwm: Add dt-bindings for Nuvoton MA35D1 SoC PWM Controller

Add documentation to describe nuvoton ma35d1 PWM controller.

---
 .../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..95f0a0819f53
@@ -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: 2
+
+  clocks:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+    pwm0: pwm@40580000 {
+      compatible = "nuvoton,ma35d1-pwm";
+      reg = <0 0x40580000 0 0x400>;
+      clocks = <&clk EPWM0_GATE>;
+      #pwm-cells = <2>;
+    };
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH 1/2] dt-bindings: pwm: Add dt-bindings for Nuvoton MA35D1 SoC PWM Controller

Quoted 1 line · Chi-Wen Weng

Add documentation to describe nuvoton ma35d1 PWM controller.

A nit, subject: drop second/last, redundant "dt-bindings for". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Quoted 24 lines
---
 .../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 +11 -0
new file mode 100644
index 000000000000..95f0a0819f53
@@ -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]>
+

Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml line 21

Quoted 10 lines
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +10 -0
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,ma35d1-pwm
+
+  reg:
+    maxItems: 2

Instead list and describe the items.

Quoted 9 lines
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +9 -0
+
+  clocks:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 2
+
+required:
+  - compatible

Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml line 40

Quoted 10 lines
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +10 -0
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+    pwm0: pwm@40580000 {

Drop unused label.

Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml line 42

Quoted 2 lines
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +2 -0
+      compatible = "nuvoton,ma35d1-pwm";
+      reg = <0 0x40580000 0 0x400>;

But you have only one item here? No, that's just incorrect.

Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml line 43

Quoted 1 line
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +1 -0
+      clocks = <&clk EPWM0_GATE>;

Best regards,
Krzysztof

#3 lore raw

Re: [PATCH 1/2] dt-bindings: pwm: Add dt-bindings for Nuvoton MA35D1 SoC PWM Controller

Hi Krzysztof,

Thank you for your reply.

Quoted 6 lines · Krzysztof Kozlowski
Quoted 1 line · Chi-Wen Weng

Add documentation to describe nuvoton ma35d1 PWM controller.

A nit, subject: drop second/last, redundant "dt-bindings for". The "dt-bindings" prefix is already stating that these are bindings. See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Ok. I will fix it.

Quoted 26 lines
Quoted 26 lines
---
  .../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 +14 -0
new file mode 100644
index 000000000000..95f0a0819f53
@@ -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#
+
Quoted 8 lines

Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml line 21

Quoted 7 lines
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +7 -0
+properties:
+  compatible:
+    enum:
+      - nuvoton,ma35d1-pwm
+
+  reg:
+    maxItems: 2

Instead list and describe the items.

Sorry, it should be 1. I will fix it.

Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +12 -0
+
+  clocks:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - clocks
+
Quoted 8 lines

Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml line 40

Quoted 7 lines
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +7 -0
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+    pwm0: pwm@40580000 {

Drop unused label.

Ok. I will drop it.

Quoted 4 lines

Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml line 42

Quoted 2 lines
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +2 -0
+      compatible = "nuvoton,ma35d1-pwm";
+      reg = <0 0x40580000 0 0x400>;

But you have only one item here? No, that's just incorrect.

Ok. I will fix it.

Quoted 3 lines

Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml line 43

Quoted 1 line
Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml new file +1 -0
+      clocks = <&clk EPWM0_GATE>;

Best regards,
Krzysztof

Thanks.

Chi-Wen Weng

#4 lore raw

PATCH 2/2 pwm: Add Nuvoton PWM controller support

In reply to Chi-Wen Weng

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

---
 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
#5 lore raw

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

Quoted 4 lines · Chi-Wen Weng

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

...

Quoted 7 lines
drivers/pwm/pwm-ma35d1.c new file +7 -0
+
+static const struct of_device_id nuvoton_pwm_of_match[] = {
+	{ .compatible = "nuvoton,ma35d1-pwm" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match);
+

drivers/pwm/pwm-ma35d1.c line 166

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+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_ALIAS("platform:nuvoton-pwm");

Drop, not needed and not correct either. If you need platform alias for non-OF binds, this is supposed to match OF ID table.

drivers/pwm/pwm-ma35d1.c line 169

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

Best regards,
Krzysztof

#6 lore raw

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

Hi Krzysztof,

Thank you for your reply.

Quoted 20 lines · Krzysztof Kozlowski
Quoted 4 lines · Chi-Wen Weng

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

...

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 166

Quoted 6 lines
drivers/pwm/pwm-ma35d1.c new file +6 -0
+		.of_match_table = nuvoton_pwm_of_match,
+	},
+};
+module_platform_driver(nuvoton_pwm_driver);
+
+MODULE_ALIAS("platform:nuvoton-pwm");

Drop, not needed and not correct either. If you need platform alias for non-OF binds, this is supposed to match OF ID table.

Ok. I will drop it.

Quoted 5 lines

drivers/pwm/pwm-ma35d1.c line 169

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

Best regards,
Krzysztof

Thanks.

Chi-Wen Weng

#7 lore raw

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

In reply to Chi-Wen Weng
Quoted 97 lines · Chi-Wen Weng

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

---
 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 +48 -0
new file mode 100644
index 000000000000..dc2f1f494a91
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Nuvoton MA35D1 PWM controller
+ *
+ * Copyright (C) 2024 Nuvoton Corporation
+ *               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>
+
+/* 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)
+
+#define PWM_TOTAL_CHANNELS      6
+#define PWM_CH_REG_SIZE         4
+
+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;
+
+	nvtpwm = to_nuvoton_pwm(chip);
+	if (state->enabled) {
+		u64 duty_cycles, period_cycles;
+

drivers/pwm/pwm-ma35d1.c line 58

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+		/* 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;
+
+		period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+						    state->period, NSEC_PER_SEC);
+		if (period_cycles > 0xFFFF)
+			period_cycles = 0xFFFF;

If a period is not supported, return -EINVAL - maybe even do a dev_err(). Same for the duty cycle above. It might make sense to calculate the period first, so that the error is more likely to be about the period than the duty cycle.

Then again I don't know if all the drivers do this, but at least some of them do.

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

drivers/pwm/pwm-ma35d1.c line 142

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+
+	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");
+
+	chip->ops = &nuvoton_pwm_ops;

I think you can add chip->atomic = true; here

Sean

Quoted 30 lines
drivers/pwm/pwm-ma35d1.c new file +27 -0
+
+	ret = devm_pwmchip_add(&pdev->dev, chip);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "unable to add pwm chip");
+
+	return 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_ALIAS("platform:nuvoton-pwm");
+MODULE_AUTHOR("Chi-Wen Weng <[email protected]>");
+MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.25.1
#8 lore raw

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

Hello Sean,

Quoted 17 lines · Sean Young
Quoted 17 lines
drivers/pwm/pwm-ma35d1.c new file +17 -0
+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;
+
+	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;
+
+		period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
Quoted 8 lines

drivers/pwm/pwm-ma35d1.c line 58

Quoted 3 lines
drivers/pwm/pwm-ma35d1.c new file +3 -0
+						    state->period, NSEC_PER_SEC);
+		if (period_cycles > 0xFFFF)
+			period_cycles = 0xFFFF;

If a period is not supported, return -EINVAL - maybe even do a dev_err(). Same for the duty cycle above. It might make sense to calculate the period first, so that the error is more likely to be about the period than the duty cycle.

That's a wrong advice. Drivers are supposed to implement the highest period possible that is not bigger than the requested one. So clamping the value to 0xFFFF looks right.

However I wonder what happens in hardware if period_cycles == 0. If that disables the hardware that is something to catch and return an error for.

Quoted 2 lines

Then again I don't know if all the drivers do this, but at least some of them do.

Yeah, and I hesitate to align them because their behaviour might be relied on. But for new drivers the above rule applies.

(And with the new waveform stuff, consumers can rely on the rounding rule and even query the resulting waveform before calling the equivalent of pwm_apply_might_sleep().

Quoted 3 lines

drivers/pwm/pwm-ma35d1.c line 142

Quoted 1 line
drivers/pwm/pwm-ma35d1.c new file +1 -0
+	chip->ops = &nuvoton_pwm_ops;

I think you can add chip->atomic = true; here

ack.

Best regards
Uwe

#9 lore raw

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

Hi Uwe,

Thank you for your reply.

Quoted 27 lines · Uwe Kleine-König

Hello Sean,

Quoted 16 lines · Sean Young
Quoted 16 lines
drivers/pwm/pwm-ma35d1.c new file +16 -0
+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;
+
+	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;
+
Quoted 8 lines

drivers/pwm/pwm-ma35d1.c line 58

Quoted 4 lines
drivers/pwm/pwm-ma35d1.c new file +4 -0
+		period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+						    state->period, NSEC_PER_SEC);
+		if (period_cycles > 0xFFFF)
+			period_cycles = 0xFFFF;

If a period is not supported, return -EINVAL - maybe even do a dev_err(). Same for the duty cycle above. It might make sense to calculate the period first, so that the error is more likely to be about the period than the duty cycle.

Quoted 3 lines

That's a wrong advice. Drivers are supposed to implement the highest period possible that is not bigger than the requested one. So clamping the value to 0xFFFF looks right.

Thanks for your explanation.

Quoted 3 lines

However I wonder what happens in hardware if period_cycles == 0. If that disables the hardware that is something to catch and return an error for.

If period_cycles = 0, the waveform output will be always low.

Quoted 10 lines
Quoted 2 lines

Then again I don't know if all the drivers do this, but at least some of them do.

Yeah, and I hesitate to align them because their behaviour might be relied on. But for new drivers the above rule applies.

(And with the new waveform stuff, consumers can rely on the rounding rule and even query the resulting waveform before calling the equivalent of pwm_apply_might_sleep().

Quoted 6 lines
Quoted 2 lines

drivers/pwm/pwm-ma35d1.c line 142

Quoted 1 line
drivers/pwm/pwm-ma35d1.c new file +1 -0
+	chip->ops = &nuvoton_pwm_ops;

I think you can add chip->atomic = true; here

ack.

Best regards
Uwe

Thanks.

Chi-Wen Weng

#10 lore raw

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

In reply to Sean Young

Hi Sean,

Thank you for your reply.

Quoted 113 lines · Sean Young
Quoted 97 lines · Chi-Wen Weng

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

---
  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 +48 -0
new file mode 100644
index 000000000000..dc2f1f494a91
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Nuvoton MA35D1 PWM controller
+ *
+ * Copyright (C) 2024 Nuvoton Corporation
+ *               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>
+
+/* 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)
+
+#define PWM_TOTAL_CHANNELS      6
+#define PWM_CH_REG_SIZE         4
+
+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;
+
+	nvtpwm = to_nuvoton_pwm(chip);
+	if (state->enabled) {
+		u64 duty_cycles, period_cycles;
+

drivers/pwm/pwm-ma35d1.c line 58

Quoted 10 lines
drivers/pwm/pwm-ma35d1.c new file +10 -0
+		/* 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;
+
+		period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+						    state->period, NSEC_PER_SEC);
+		if (period_cycles > 0xFFFF)
+			period_cycles = 0xFFFF;

If a period is not supported, return -EINVAL - maybe even do a dev_err(). Same for the duty cycle above. It might make sense to calculate the period first, so that the error is more likely to be about the period than the duty cycle.

Quoted 2 lines

Then again I don't know if all the drivers do this, but at least some of them do.

Uwe has explained it. Thanks Uwe.

Quoted 77 lines
Quoted 77 lines
drivers/pwm/pwm-ma35d1.c new file +77 -0
+
+		/* 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));
+		/* Enable counter */
+		writel(readl(nvtpwm->base + REG_PWM_CNTEN) | BIT(ch),
+		       nvtpwm->base + REG_PWM_CNTEN);
+		/* 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))
Quoted 8 lines

drivers/pwm/pwm-ma35d1.c line 142

Quoted 7 lines
drivers/pwm/pwm-ma35d1.c new file +7 -0
+		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");
+
+	chip->ops = &nuvoton_pwm_ops;

I think you can add chip->atomic = true; here

Ok.  I will add it.  Thanks.

Quoted 24 lines

Sean

Quoted 21 lines
drivers/pwm/pwm-ma35d1.c new file +21 -0
+
+	ret = devm_pwmchip_add(&pdev->dev, chip);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "unable to add pwm chip");
+
+	return 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,
+	},
+};
Quoted 8 lines
Quoted 8 lines
drivers/pwm/pwm-ma35d1.c new file +6 -0
+module_platform_driver(nuvoton_pwm_driver);
+
+MODULE_ALIAS("platform:nuvoton-pwm");
+MODULE_AUTHOR("Chi-Wen Weng <[email protected]>");
+MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.25.1

Thanks.

Chi-Wen Weng