[PATCH v6 0/3] Add support for Allwinner PWM on D1/T113s/R329 SoCs

[PATCH v6 0/3] Add support for Allwinner PWM on D1/T113s/R329 SoCs

[Aleksandr Shubin]

Hi,

This series adds support for PWM controller on new
Allwinner's SoCs, such as D1, T113s and R329. The implemented driver
provides basic functionality for control PWM channels.

v2:
 - fix dt-bindings
 - fix a remark in the driver

v3:
 - fix dt-bindings
 - fix sunxi-d1s-t113.dtsi

v4:
 - fix a remark in the driver

v5:
 - dropped unused varibale in the driver
 - fix dt-bindings

v6:
 - add apb0 clock

Aleksandr Shubin (3):
  dt-bindings: pwm: Add binding for Allwinner D1/T113-S3/R329 PWM
    controller
  pwm: Add Allwinner's D1/T113-S3/R329 SoCs PWM support
  riscv: dts: allwinner: d1: Add pwm node

 .../bindings/pwm/allwinner,sun20i-pwm.yaml    |  87 +++++
 .../boot/dts/allwinner/sunxi-d1s-t113.dtsi    |  12 +
 drivers/pwm/Kconfig                           |  10 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-sun20i.c                      | 328 ++++++++++++++++++
 5 files changed, 438 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml
 create mode 100644 drivers/pwm/pwm-sun20i.c

-- 
2.25.1

[PATCH v6 1/3] dt-bindings: pwm: Add binding for Allwinner D1/T113-S3/R329 PWM controller

[Aleksandr Shubin]

Allwinner's D1, T113-S3 and R329 SoCs have a new pwm
controller witch is different from the previous pwm-sun4i.

The D1 and T113 are identical in terms of peripherals,
they differ only in the architecture of the CPU core, and
even share the majority of their DT. Because of that,
using the same compatible makes sense.
The R329 is a different SoC though, and should have
a different compatible string added, especially as there
is a difference in the number of channels.

D1 and T113s SoCs have one PWM controller with 8 channels.
R329 SoC has two PWM controllers in both power domains, one of
them has 9 channels (CPUX one) and the other has 6 (CPUS one).

Add a device tree binding for them.

Signed-off-by: Aleksandr Shubin <privatesub2@gmail.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/pwm/allwinner,sun20i-pwm.yaml    | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml

diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml
new file mode 100644
index 000000000000..a65324e9b138
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/allwinner,sun20i-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1, T113-S3 and R329 PWM
+
+maintainers:
+  - Aleksandr Shubin <privatesub2@gmail.com>
+
+properties:
+  compatible:
+    oneOf:
+      - const: allwinner,sun20i-d1-pwm
+      - items:
+          - const: allwinner,sun20i-r329-pwm
+          - const: allwinner,sun20i-d1-pwm
+
+  reg:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 3
+
+  clocks:
+    items:
+      - description: Bus clock
+      - description: 24 MHz oscillator
+      - description: APB0 clock
+
+  clock-names:
+    items:
+      - const: bus
+      - const: hosc
+      - const: apb0
+
+  resets:
+    maxItems: 1
+
+  allwinner,pwm-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The number of PWM channels configured for this instance
+    enum: [6, 9]
+
+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: allwinner,sun20i-r329-pwm
+
+    then:
+      required:
+        - allwinner,pwm-channels
+
+    else:
+      properties:
+        allwinner,pwm-channels: false
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#pwm-cells"
+  - clocks
+  - clock-names
+  - resets
+
+examples:
+  - |
+    #include <dt-bindings/clock/sun20i-d1-ccu.h>
+    #include <dt-bindings/reset/sun20i-d1-ccu.h>
+
+    pwm: pwm@2000c00 {
+      compatible = "allwinner,sun20i-d1-pwm";
+      reg = <0x02000c00 0x400>;
+      clocks = <&ccu CLK_BUS_PWM>, <&dcxo>, <&ccu CLK_APB0>;
+      clock-names = "bus", "hosc", "apb0";
+      resets = <&ccu RST_BUS_PWM>;
+      #pwm-cells = <0x3>;
+    };
+
+...
-- 
2.25.1

[PATCH v6 2/3] pwm: Add Allwinner's D1/T113-S3/R329 SoCs PWM support

[Aleksandr Shubin]

Allwinner's D1, T113-S3 and R329 SoCs have a quite different PWM
controllers with ones supported by pwm-sun4i driver.

This patch adds a PWM controller driver for Allwinner's D1,
T113-S3 and R329 SoCs. The main difference between these SoCs
is the number of channels defined by the DT property.

Signed-off-by: Aleksandr Shubin <privatesub2@gmail.com>
---
 drivers/pwm/Kconfig      |  10 ++
 drivers/pwm/Makefile     |   1 +
 drivers/pwm/pwm-sun20i.c | 328 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 339 insertions(+)
 create mode 100644 drivers/pwm/pwm-sun20i.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 8df861b1f4a3..05c48a36969e 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -594,6 +594,16 @@ config PWM_SUN4I
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-sun4i.
 
+config PWM_SUN20I
+	tristate "Allwinner D1/T113s/R329 PWM support"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	depends on COMMON_CLK
+	help
+	  Generic PWM framework driver for Allwinner D1/T113s/R329 SoCs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-sun20i.
+
 config PWM_SUNPLUS
 	tristate "Sunplus PWM support"
 	depends on ARCH_SUNPLUS || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 19899b912e00..cea872e22c78 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_PWM_STM32)		+= pwm-stm32.o
 obj-$(CONFIG_PWM_STM32_LP)	+= pwm-stm32-lp.o
 obj-$(CONFIG_PWM_STMPE)		+= pwm-stmpe.o
 obj-$(CONFIG_PWM_SUN4I)		+= pwm-sun4i.o
+obj-$(CONFIG_PWM_SUN20I)	+= pwm-sun20i.o
 obj-$(CONFIG_PWM_SUNPLUS)	+= pwm-sunplus.o
 obj-$(CONFIG_PWM_TEGRA)		+= pwm-tegra.o
 obj-$(CONFIG_PWM_TIECAP)	+= pwm-tiecap.o
diff --git a/drivers/pwm/pwm-sun20i.c b/drivers/pwm/pwm-sun20i.c
new file mode 100644
index 000000000000..20e6b7b5b62e
--- /dev/null
+++ b/drivers/pwm/pwm-sun20i.c
@@ -0,0 +1,328 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PWM Controller Driver for sunxi platforms (D1, T113-S3 and R329)
+ *
+ * Limitations:
+ * - When the parameters change, current running period will not be completed
+ *   and run new settings immediately.
+ * - It output HIGH-Z state when PWM channel disabled.
+ *
+ * Copyright (c) 2023 Aleksandr Shubin <privatesub2@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+
+#define PWM_CLK_CFG(chan)		(0x20 + (((chan) >> 1) * 0x4))
+#define PWM_CLK_CFG_SRC			GENMASK(8, 7)
+#define PWM_CLK_CFG_DIV_M		GENMASK(3, 0)
+
+#define PWM_CLK_GATE			0x40
+#define PWM_CLK_GATE_BYPASS(chan)	BIT((chan) - 16)
+#define PWM_CLK_GATE_GATING(chan)	BIT(chan)
+
+#define PWM_ENABLE			0x80
+#define PWM_ENABLE_EN(chan)		BIT(chan)
+
+#define PWM_CTL(chan)			(0x100 + (chan) * 0x20)
+#define PWM_CTL_ACT_STA			BIT(8)
+#define PWM_CTL_PRESCAL_K		GENMASK(7, 0)
+
+#define PWM_PERIOD(chan)		(0x104 + (chan) * 0x20)
+#define PWM_PERIOD_ENTIRE_CYCLE		GENMASK(31, 16)
+#define PWM_PERIOD_ACT_CYCLE		GENMASK(15, 0)
+
+#define PWM_MAGIC			(255 * 65535 + 2 * 65534 + 1)
+
+struct sun20i_pwm_chip {
+	struct clk *clk_bus, *clk_hosc, *clk_apb0;
+	struct reset_control *rst;
+	struct pwm_chip chip;
+	void __iomem *base;
+	/* Mutex to protect pwm apply state */
+	struct mutex mutex;
+};
+
+static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip(struct pwm_chip *chip)
+{
+	return container_of(chip, struct sun20i_pwm_chip, chip);
+}
+
+static inline u32 sun20i_pwm_readl(struct sun20i_pwm_chip *chip,
+				   unsigned long offset)
+{
+	return readl(chip->base + offset);
+}
+
+static inline void sun20i_pwm_writel(struct sun20i_pwm_chip *chip,
+				     u32 val, unsigned long offset)
+{
+	writel(val, chip->base + offset);
+}
+
+static int sun20i_pwm_get_state(struct pwm_chip *chip,
+				struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip);
+	u16 ent_cycle, act_cycle, prescal;
+	u64 clk_rate, tmp;
+	u8 div_id;
+	u32 val;
+
+	mutex_lock(&sun20i_chip->mutex);
+
+	val = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG(pwm->hwpwm));
+	div_id = FIELD_GET(PWM_CLK_CFG_DIV_M, val);
+	if (FIELD_GET(PWM_CLK_CFG_SRC, val) == 0)
+		clk_rate = clk_get_rate(sun20i_chip->clk_hosc);
+	else
+		clk_rate = clk_get_rate(sun20i_chip->clk_apb0);
+
+	val = sun20i_pwm_readl(sun20i_chip, PWM_CTL(pwm->hwpwm));
+	state->polarity = (PWM_CTL_ACT_STA & val) ? PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED;
+
+	prescal = FIELD_GET(PWM_CTL_PRESCAL_K, val) + 1;
+
+	val = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE);
+	state->enabled = (PWM_ENABLE_EN(pwm->hwpwm) & val) ? true : false;
+
+	val = sun20i_pwm_readl(sun20i_chip, PWM_PERIOD(pwm->hwpwm));
+	act_cycle = FIELD_GET(PWM_PERIOD_ACT_CYCLE, val);
+	ent_cycle = FIELD_GET(PWM_PERIOD_ENTIRE_CYCLE, val);
+
+	/*
+	 * The duration of the active phase should not be longer
+	 * than the duration of the period
+	 */
+	if (act_cycle > ent_cycle)
+		act_cycle = ent_cycle;
+
+	tmp = ((u64)(act_cycle) * prescal << div_id) * NSEC_PER_SEC;
+	state->duty_cycle = DIV_ROUND_UP_ULL(tmp, clk_rate);
+	tmp = ((u64)(ent_cycle) * prescal << div_id) * NSEC_PER_SEC;
+	state->period = DIV_ROUND_UP_ULL(tmp, clk_rate);
+	mutex_unlock(&sun20i_chip->mutex);
+
+	return 0;
+}
+
+static int sun20i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			    const struct pwm_state *state)
+{
+	struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip);
+	u32 clk_gate, clk_cfg, pwm_en, ctl, period;
+	u64 bus_rate, hosc_rate, clk_div, val;
+	u32 prescaler, div_m;
+	bool use_bus_clk;
+	int ret = 0;
+
+	mutex_lock(&sun20i_chip->mutex);
+
+	pwm_en = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE);
+
+	if (state->enabled != pwm->state.enabled)
+		clk_gate = sun20i_pwm_readl(sun20i_chip, PWM_CLK_GATE);
+
+	if (state->enabled != pwm->state.enabled && !state->enabled) {
+		clk_gate &= ~PWM_CLK_GATE_GATING(pwm->hwpwm);
+		pwm_en &= ~PWM_ENABLE_EN(pwm->hwpwm);
+		sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE);
+		sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE);
+	}
+
+	if (state->polarity != pwm->state.polarity ||
+	    state->duty_cycle != pwm->state.duty_cycle ||
+	    state->period != pwm->state.period) {
+		ctl = sun20i_pwm_readl(sun20i_chip, PWM_CTL(pwm->hwpwm));
+		clk_cfg = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG(pwm->hwpwm));
+		hosc_rate = clk_get_rate(sun20i_chip->clk_hosc);
+		bus_rate = clk_get_rate(sun20i_chip->clk_apb0);
+		if (pwm_en & PWM_ENABLE_EN(pwm->hwpwm ^ 1)) {
+			/* if the neighbor channel is enable, check period only */
+			use_bus_clk = FIELD_GET(PWM_CLK_CFG_SRC, clk_cfg) != 0;
+			val = state->period * (use_bus_clk ? bus_rate : hosc_rate);
+			do_div(val, NSEC_PER_SEC);
+
+			div_m = FIELD_GET(PWM_CLK_CFG_DIV_M, clk_cfg);
+		} else {
+			/* check period and select clock source */
+			use_bus_clk = false;
+			val = state->period * hosc_rate;
+			do_div(val, NSEC_PER_SEC);
+			if (val <= 1) {
+				use_bus_clk = true;
+				val = state->period * bus_rate;
+				do_div(val, NSEC_PER_SEC);
+				if (val <= 1) {
+					ret = -EINVAL;
+					goto unlock_mutex;
+				}
+			}
+			div_m = fls(DIV_ROUND_DOWN_ULL(val, PWM_MAGIC));
+			if (div_m >= 9) {
+				ret = -EINVAL;
+				goto unlock_mutex;
+			}
+
+			/* set up the CLK_DIV_M and clock CLK_SRC */
+			clk_cfg = FIELD_PREP(PWM_CLK_CFG_DIV_M, div_m);
+			clk_cfg |= FIELD_PREP(PWM_CLK_CFG_SRC, use_bus_clk);
+
+			sun20i_pwm_writel(sun20i_chip, clk_cfg, PWM_CLK_CFG(pwm->hwpwm));
+		}
+
+		/* calculate prescaler, PWM entire cycle */
+		clk_div = val >> div_m;
+		if (clk_div <= 65534) {
+			prescaler = 0;
+		} else {
+			prescaler = DIV_ROUND_UP_ULL(clk_div - 65534, 65535);
+			if (prescaler >= 256) {
+				ret = -EINVAL;
+				goto unlock_mutex;
+			}
+			do_div(clk_div, prescaler + 1);
+		}
+
+		period = FIELD_PREP(PWM_PERIOD_ENTIRE_CYCLE, clk_div);
+
+		/* set duty cycle */
+		val = state->duty_cycle * (use_bus_clk ? bus_rate : hosc_rate);
+		do_div(val, NSEC_PER_SEC);
+		clk_div = val >> div_m;
+		do_div(clk_div, prescaler + 1);
+
+		/*
+		 * The formula of the output period and the duty-cycle for PWM are as follows.
+		 * T period = (PWM01_CLK / PWM0_PRESCALE_K)^-1 * (PPR0.PWM_ENTIRE_CYCLE + 1)
+		 * T high-level = (PWM01_CLK / PWM0_PRESCALE_K)^-1 * PPR0.PWM_ACT_CYCLE
+		 * Duty-cycle = T high-level / T period
+		 * In accordance with this formula, in order to set the duty-cycle to 100%,
+		 * it is necessary that PWM_ACT_CYCLE >= PWM_ENTIRE_CYCLE + 1
+		 */
+		if (state->duty_cycle == state->period)
+			clk_div++;
+		period |= FIELD_PREP(PWM_PERIOD_ACT_CYCLE, clk_div);
+		sun20i_pwm_writel(sun20i_chip, period, PWM_PERIOD(pwm->hwpwm));
+
+		ctl = FIELD_PREP(PWM_CTL_PRESCAL_K, prescaler);
+		if (state->polarity == PWM_POLARITY_NORMAL)
+			ctl |= PWM_CTL_ACT_STA;
+
+		sun20i_pwm_writel(sun20i_chip, ctl, PWM_CTL(pwm->hwpwm));
+	}
+
+	if (state->enabled != pwm->state.enabled && state->enabled) {
+		clk_gate &= ~PWM_CLK_GATE_BYPASS(pwm->hwpwm);
+		clk_gate |= PWM_CLK_GATE_GATING(pwm->hwpwm);
+		pwm_en |= PWM_ENABLE_EN(pwm->hwpwm);
+		sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE);
+		sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE);
+	}
+
+unlock_mutex:
+	mutex_unlock(&sun20i_chip->mutex);
+
+	return ret;
+}
+
+static const struct pwm_ops sun20i_pwm_ops = {
+	.get_state = sun20i_pwm_get_state,
+	.apply = sun20i_pwm_apply,
+	.owner = THIS_MODULE,
+};
+
+static const struct of_device_id sun20i_pwm_dt_ids[] = {
+	{ .compatible = "allwinner,sun20i-d1-pwm" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun20i_pwm_dt_ids);
+
+static int sun20i_pwm_probe(struct platform_device *pdev)
+{
+	struct sun20i_pwm_chip *sun20i_chip;
+	int ret;
+
+	sun20i_chip = devm_kzalloc(&pdev->dev, sizeof(*sun20i_chip), GFP_KERNEL);
+	if (!sun20i_chip)
+		return -ENOMEM;
+
+	sun20i_chip->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(sun20i_chip->base))
+		return PTR_ERR(sun20i_chip->base);
+
+	sun20i_chip->clk_bus = devm_clk_get_enabled(&pdev->dev, "bus");
+	if (IS_ERR(sun20i_chip->clk_bus))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_bus),
+				     "failed to get bus clock\n");
+
+	sun20i_chip->clk_hosc = devm_clk_get_enabled(&pdev->dev, "hosc");
+	if (IS_ERR(sun20i_chip->clk_hosc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_hosc),
+				     "failed to get hosc clock\n");
+
+	sun20i_chip->clk_apb0 = devm_clk_get_enabled(&pdev->dev, "apb0");
+	if (IS_ERR(sun20i_chip->clk_apb0))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_apb0),
+				     "failed to get apb0 clock\n");
+
+	sun20i_chip->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(sun20i_chip->rst))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->rst),
+				     "failed to get bus reset\n");
+
+	ret = of_property_read_u32(pdev->dev.of_node, "allwinner,pwm-channels",
+				   &sun20i_chip->chip.npwm);
+	if (ret)
+		sun20i_chip->chip.npwm = 8;
+
+	/* Deassert reset */
+	ret = reset_control_deassert(sun20i_chip->rst);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to deassert reset\n");
+
+	sun20i_chip->chip.dev = &pdev->dev;
+	sun20i_chip->chip.ops = &sun20i_pwm_ops;
+
+	mutex_init(&sun20i_chip->mutex);
+
+	ret = pwmchip_add(&sun20i_chip->chip);
+	if (ret < 0) {
+		reset_control_assert(sun20i_chip->rst);
+		return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");
+	}
+
+	platform_set_drvdata(pdev, sun20i_chip);
+
+	return 0;
+}
+
+static void sun20i_pwm_remove(struct platform_device *pdev)
+{
+	struct sun20i_pwm_chip *sun20i_chip = platform_get_drvdata(pdev);
+
+	pwmchip_remove(&sun20i_chip->chip);
+
+	reset_control_assert(sun20i_chip->rst);
+}
+
+static struct platform_driver sun20i_pwm_driver = {
+	.driver = {
+		.name = "sun20i-pwm",
+		.of_match_table = sun20i_pwm_dt_ids,
+	},
+	.probe = sun20i_pwm_probe,
+	.remove_new = sun20i_pwm_remove,
+};
+module_platform_driver(sun20i_pwm_driver);
+
+MODULE_AUTHOR("Aleksandr Shubin <privatesub2@gmail.com>");
+MODULE_DESCRIPTION("Allwinner sun20i PWM driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v6 2/3] pwm: Add Allwinner's D1/T113-S3/R329 SoCs PWM support

[Uwe Kleine-König]

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

Hello Aleksandr,

On Thu, Aug 24, 2023 at 02:40:26PM +0300, Aleksandr Shubin wrote:
> Allwinner's D1, T113-S3 and R329 SoCs have a quite different PWM
> controllers with ones supported by pwm-sun4i driver.
> 
> This patch adds a PWM controller driver for Allwinner's D1,
> T113-S3 and R329 SoCs. The main difference between these SoCs
> is the number of channels defined by the DT property.
> 
> Signed-off-by: Aleksandr Shubin <privatesub2@gmail.com>
> ---
>  drivers/pwm/Kconfig      |  10 ++
>  drivers/pwm/Makefile     |   1 +
>  drivers/pwm/pwm-sun20i.c | 328 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 339 insertions(+)
>  create mode 100644 drivers/pwm/pwm-sun20i.c
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 8df861b1f4a3..05c48a36969e 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -594,6 +594,16 @@ config PWM_SUN4I
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pwm-sun4i.
>  
> +config PWM_SUN20I
> +	tristate "Allwinner D1/T113s/R329 PWM support"
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	depends on COMMON_CLK
> +	help
> +	  Generic PWM framework driver for Allwinner D1/T113s/R329 SoCs.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called pwm-sun20i.
> +
>  config PWM_SUNPLUS
>  	tristate "Sunplus PWM support"
>  	depends on ARCH_SUNPLUS || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 19899b912e00..cea872e22c78 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_PWM_STM32)		+= pwm-stm32.o
>  obj-$(CONFIG_PWM_STM32_LP)	+= pwm-stm32-lp.o
>  obj-$(CONFIG_PWM_STMPE)		+= pwm-stmpe.o
>  obj-$(CONFIG_PWM_SUN4I)		+= pwm-sun4i.o
> +obj-$(CONFIG_PWM_SUN20I)	+= pwm-sun20i.o
>  obj-$(CONFIG_PWM_SUNPLUS)	+= pwm-sunplus.o
>  obj-$(CONFIG_PWM_TEGRA)		+= pwm-tegra.o
>  obj-$(CONFIG_PWM_TIECAP)	+= pwm-tiecap.o
> diff --git a/drivers/pwm/pwm-sun20i.c b/drivers/pwm/pwm-sun20i.c
> new file mode 100644
> index 000000000000..20e6b7b5b62e
> --- /dev/null
> +++ b/drivers/pwm/pwm-sun20i.c
> @@ -0,0 +1,328 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PWM Controller Driver for sunxi platforms (D1, T113-S3 and R329)
> + *
> + * Limitations:
> + * - When the parameters change, current running period will not be completed
> + *   and run new settings immediately.
> + * - It output HIGH-Z state when PWM channel disabled.
> + *
> + * Copyright (c) 2023 Aleksandr Shubin <privatesub2@gmail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pwm.h>
> +#include <linux/reset.h>
> +
> +#define PWM_CLK_CFG(chan)		(0x20 + (((chan) >> 1) * 0x4))
> +#define PWM_CLK_CFG_SRC			GENMASK(8, 7)
> +#define PWM_CLK_CFG_DIV_M		GENMASK(3, 0)
> +
> +#define PWM_CLK_GATE			0x40
> +#define PWM_CLK_GATE_BYPASS(chan)	BIT((chan) - 16)

Really? With chan == 0 this gives you BIT(-16).

> +#define PWM_CLK_GATE_GATING(chan)	BIT(chan)
> +
> +#define PWM_ENABLE			0x80
> +#define PWM_ENABLE_EN(chan)		BIT(chan)
> +
> +#define PWM_CTL(chan)			(0x100 + (chan) * 0x20)
> +#define PWM_CTL_ACT_STA			BIT(8)
> +#define PWM_CTL_PRESCAL_K		GENMASK(7, 0)
> +
> +#define PWM_PERIOD(chan)		(0x104 + (chan) * 0x20)
> +#define PWM_PERIOD_ENTIRE_CYCLE		GENMASK(31, 16)
> +#define PWM_PERIOD_ACT_CYCLE		GENMASK(15, 0)
> +
> +#define PWM_MAGIC			(255 * 65535 + 2 * 65534 + 1)

A comment about PWM_MAGIC would be nice.

I'd like to have these register defines prefixed with (say) SUN20I_,
otherwise the names are too generic and likely overlap with other
defines.

> +struct sun20i_pwm_chip {
> +	struct clk *clk_bus, *clk_hosc, *clk_apb0;
> +	struct reset_control *rst;
> +	struct pwm_chip chip;
> +	void __iomem *base;
> +	/* Mutex to protect pwm apply state */
> +	struct mutex mutex;
> +};
> +
> +static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip(struct pwm_chip *chip)
> +{
> +	return container_of(chip, struct sun20i_pwm_chip, chip);
> +}
> +
> +static inline u32 sun20i_pwm_readl(struct sun20i_pwm_chip *chip,
> +				   unsigned long offset)
> +{
> +	return readl(chip->base + offset);
> +}
> +
> +static inline void sun20i_pwm_writel(struct sun20i_pwm_chip *chip,
> +				     u32 val, unsigned long offset)
> +{
> +	writel(val, chip->base + offset);
> +}
> +
> +static int sun20i_pwm_get_state(struct pwm_chip *chip,
> +				struct pwm_device *pwm,
> +				struct pwm_state *state)
> +{
> +	struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip);
> +	u16 ent_cycle, act_cycle, prescal;
> +	u64 clk_rate, tmp;
> +	u8 div_id;
> +	u32 val;
> +
> +	mutex_lock(&sun20i_chip->mutex);
> +
> +	val = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG(pwm->hwpwm));
> +	div_id = FIELD_GET(PWM_CLK_CFG_DIV_M, val);
> +	if (FIELD_GET(PWM_CLK_CFG_SRC, val) == 0)
> +		clk_rate = clk_get_rate(sun20i_chip->clk_hosc);
> +	else
> +		clk_rate = clk_get_rate(sun20i_chip->clk_apb0);
> +
> +	val = sun20i_pwm_readl(sun20i_chip, PWM_CTL(pwm->hwpwm));
> +	state->polarity = (PWM_CTL_ACT_STA & val) ? PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED;
> +
> +	prescal = FIELD_GET(PWM_CTL_PRESCAL_K, val) + 1;
> +
> +	val = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE);
> +	state->enabled = (PWM_ENABLE_EN(pwm->hwpwm) & val) ? true : false;
> +
> +	val = sun20i_pwm_readl(sun20i_chip, PWM_PERIOD(pwm->hwpwm));

You can release the lock already here (or even after PWM_ENABLE is
read?)

> +	act_cycle = FIELD_GET(PWM_PERIOD_ACT_CYCLE, val);
> +	ent_cycle = FIELD_GET(PWM_PERIOD_ENTIRE_CYCLE, val);
> +
> +	/*
> +	 * The duration of the active phase should not be longer
> +	 * than the duration of the period
> +	 */
> +	if (act_cycle > ent_cycle)
> +		act_cycle = ent_cycle;
> +
> +	tmp = ((u64)(act_cycle) * prescal << div_id) * NSEC_PER_SEC;

act_cycle is a 16 bit value, prescal is <= 256 and div_id is <= 15. So
the maximal value tmp has to hold is 0x1dcd47329b00000000. This doesn't
fit into an u64. You need something like mul_u64_u64_div64_roundup here.

> +	state->duty_cycle = DIV_ROUND_UP_ULL(tmp, clk_rate);
> +	tmp = ((u64)(ent_cycle) * prescal << div_id) * NSEC_PER_SEC;
> +	state->period = DIV_ROUND_UP_ULL(tmp, clk_rate);
> +	mutex_unlock(&sun20i_chip->mutex);
> +
> +	return 0;
> +}
> +
> +static int sun20i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			    const struct pwm_state *state)
> +{
> +	struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip);
> +	u32 clk_gate, clk_cfg, pwm_en, ctl, period;
> +	u64 bus_rate, hosc_rate, clk_div, val;
> +	u32 prescaler, div_m;
> +	bool use_bus_clk;
> +	int ret = 0;
> +
> +	mutex_lock(&sun20i_chip->mutex);
> +
> +	pwm_en = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE);
> +
> +	if (state->enabled != pwm->state.enabled)
> +		clk_gate = sun20i_pwm_readl(sun20i_chip, PWM_CLK_GATE);
> +
> +	if (state->enabled != pwm->state.enabled && !state->enabled) {
> +		clk_gate &= ~PWM_CLK_GATE_GATING(pwm->hwpwm);
> +		pwm_en &= ~PWM_ENABLE_EN(pwm->hwpwm);
> +		sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE);
> +		sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE);

Does ENABLE configure if the output pin is driven?

> +	}

You can these to like:

	if (state->enabled != pwm->state.enabled) {
		clk_gate = ...
		if (!state->enabled) {
			...
		}
	}

> +
> +	if (state->polarity != pwm->state.polarity ||
> +	    state->duty_cycle != pwm->state.duty_cycle ||
> +	    state->period != pwm->state.period) {
> +		ctl = sun20i_pwm_readl(sun20i_chip, PWM_CTL(pwm->hwpwm));
> +		clk_cfg = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG(pwm->hwpwm));
> +		hosc_rate = clk_get_rate(sun20i_chip->clk_hosc);
> +		bus_rate = clk_get_rate(sun20i_chip->clk_apb0);
> +		if (pwm_en & PWM_ENABLE_EN(pwm->hwpwm ^ 1)) {
> +			/* if the neighbor channel is enable, check period only */
> +			use_bus_clk = FIELD_GET(PWM_CLK_CFG_SRC, clk_cfg) != 0;
> +			val = state->period * (use_bus_clk ? bus_rate : hosc_rate);

This can overflow.

> +			do_div(val, NSEC_PER_SEC);
> +
> +			div_m = FIELD_GET(PWM_CLK_CFG_DIV_M, clk_cfg);
> +		} else {
> +			/* check period and select clock source */
> +			use_bus_clk = false;
> +			val = state->period * hosc_rate;
> +			do_div(val, NSEC_PER_SEC);
> +			if (val <= 1) {
> +				use_bus_clk = true;
> +				val = state->period * bus_rate;
> +				do_div(val, NSEC_PER_SEC);
> +				if (val <= 1) {
> +					ret = -EINVAL;
> +					goto unlock_mutex;
> +				}
> +			}
> +			div_m = fls(DIV_ROUND_DOWN_ULL(val, PWM_MAGIC));
> +			if (div_m >= 9) {

What is 9 here? Something like DIV_M_MAX? There are a few other
constants that might deserve a name.

> +				ret = -EINVAL;
> +				goto unlock_mutex;
> +			}
> +
> +			/* set up the CLK_DIV_M and clock CLK_SRC */
> +			clk_cfg = FIELD_PREP(PWM_CLK_CFG_DIV_M, div_m);
> +			clk_cfg |= FIELD_PREP(PWM_CLK_CFG_SRC, use_bus_clk);
> +
> +			sun20i_pwm_writel(sun20i_chip, clk_cfg, PWM_CLK_CFG(pwm->hwpwm));
> +		}
> +
> +		/* calculate prescaler, PWM entire cycle */
> +		clk_div = val >> div_m;
> +		if (clk_div <= 65534) {
> +			prescaler = 0;
> +		} else {
> +			prescaler = DIV_ROUND_UP_ULL(clk_div - 65534, 65535);

This looks strange. The result is the same as
DIV_ROUND_DOWN_ULL(clk_div, 65535) which by the way also does the right
thing for clk_div <= 65534.

> +			if (prescaler >= 256) {
> +				ret = -EINVAL;
> +				goto unlock_mutex;

If this happens the requested period is too big, right? Please use

	prescaler = 255;

then and proceed. (The idea is to configure the biggest period that is
not bigger than the requested period.)

> +			}
> +			do_div(clk_div, prescaler + 1);
> +		}
> +
> +		period = FIELD_PREP(PWM_PERIOD_ENTIRE_CYCLE, clk_div);
> +
> +		/* set duty cycle */
> +		val = state->duty_cycle * (use_bus_clk ? bus_rate : hosc_rate);
> +		do_div(val, NSEC_PER_SEC);
> +		clk_div = val >> div_m;
> +		do_div(clk_div, prescaler + 1);
> +
> +		/*
> +		 * The formula of the output period and the duty-cycle for PWM are as follows.
> +		 * T period = (PWM01_CLK / PWM0_PRESCALE_K)^-1 * (PPR0.PWM_ENTIRE_CYCLE + 1)
> +		 * T high-level = (PWM01_CLK / PWM0_PRESCALE_K)^-1 * PPR0.PWM_ACT_CYCLE
> +		 * Duty-cycle = T high-level / T period
> +		 * In accordance with this formula, in order to set the duty-cycle to 100%,
> +		 * it is necessary that PWM_ACT_CYCLE >= PWM_ENTIRE_CYCLE + 1
> +		 */
> +		if (state->duty_cycle == state->period)
> +			clk_div++;

Can it happen, that clk_div gets too big here?

> +		period |= FIELD_PREP(PWM_PERIOD_ACT_CYCLE, clk_div);

It's a bit irritating (to me at least) that the variable "period" holds
the configuration for the duty_cycle. Maybe call it "reg_period" or
similar to not confuse it with state->period?

> +		sun20i_pwm_writel(sun20i_chip, period, PWM_PERIOD(pwm->hwpwm));
> +
> +		ctl = FIELD_PREP(PWM_CTL_PRESCAL_K, prescaler);
> +		if (state->polarity == PWM_POLARITY_NORMAL)
> +			ctl |= PWM_CTL_ACT_STA;
> +
> +		sun20i_pwm_writel(sun20i_chip, ctl, PWM_CTL(pwm->hwpwm));

Is there a glitch if you switch polarity? i.e. after period is written a
few lines above, a new period starts already and if you then invert
PWM_CTL_ACT_STA, the output immediately switches polarity? If so that's
something to mention in the Limitations section.

> +	}
> +[...]
> +static int sun20i_pwm_probe(struct platform_device *pdev)
> +{
> +	struct sun20i_pwm_chip *sun20i_chip;
> +	int ret;
> +
> +	sun20i_chip = devm_kzalloc(&pdev->dev, sizeof(*sun20i_chip), GFP_KERNEL);
> +	if (!sun20i_chip)
> +		return -ENOMEM;
> +
> +	sun20i_chip->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(sun20i_chip->base))
> +		return PTR_ERR(sun20i_chip->base);
> +
> +	sun20i_chip->clk_bus = devm_clk_get_enabled(&pdev->dev, "bus");
> +	if (IS_ERR(sun20i_chip->clk_bus))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_bus),
> +				     "failed to get bus clock\n");
> +
> +	sun20i_chip->clk_hosc = devm_clk_get_enabled(&pdev->dev, "hosc");
> +	if (IS_ERR(sun20i_chip->clk_hosc))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_hosc),
> +				     "failed to get hosc clock\n");
> +
> +	sun20i_chip->clk_apb0 = devm_clk_get_enabled(&pdev->dev, "apb0");
> +	if (IS_ERR(sun20i_chip->clk_apb0))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->clk_apb0),
> +				     "failed to get apb0 clock\n");
> +
> +	sun20i_chip->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(sun20i_chip->rst))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(sun20i_chip->rst),
> +				     "failed to get bus reset\n");
> +
> +	ret = of_property_read_u32(pdev->dev.of_node, "allwinner,pwm-channels",
> +				   &sun20i_chip->chip.npwm);
> +	if (ret)
> +		sun20i_chip->chip.npwm = 8;

The register layout allows npwm <= 16 only, right? I suggest to add a
check for that.

> +	/* Deassert reset */
> +	ret = reset_control_deassert(sun20i_chip->rst);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "failed to deassert reset\n");

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

[PATCH v6 3/3] riscv: dts: allwinner: d1: Add pwm node

[Aleksandr Shubin]

D1 and T113s contain a pwm controller with 8 channels.
This controller is supported by the sun20i-pwm driver.

Add a device tree node for it.

Signed-off-by: Aleksandr Shubin <privatesub2@gmail.com>
---
 arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
index 922e8e0e2c09..c4ce13ab9512 100644
--- a/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
+++ b/arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi
@@ -127,6 +127,18 @@ uart3_pb_pins: uart3-pb-pins {
 			};
 		};
 
+		pwm: pwm@2000c00 {
+			compatible = "allwinner,sun20i-d1-pwm";
+			reg = <0x02000c00 0x400>;
+			clocks = <&ccu CLK_BUS_PWM>,
+				 <&dcxo>,
+				 <&ccu CLK_APB0>;
+			clock-names = "bus", "hosc", "apb0";
+			resets = <&ccu RST_BUS_PWM>;
+			status = "disabled";
+			#pwm-cells = <0x3>;
+		};
+
 		ccu: clock-controller@2001000 {
 			compatible = "allwinner,sun20i-d1-ccu";
 			reg = <0x2001000 0x1000>;
-- 
2.25.1

Re: [PATCH v6 0/3] Add support for Allwinner PWM on D1/T113s/R329 SoCs

[John Watts]

On Thu, Aug 24, 2023 at 02:40:24PM +0300, Aleksandr Shubin wrote:
> Hi,
> 
> This series adds support for PWM controller on new
> Allwinner's SoCs, such as D1, T113s and R329. The implemented driver
> provides basic functionality for control PWM channels.

I have tested this patch and earlier versions successfully on a Mango Pi MQ
Dual and verified it outputs a PWM signal.

John.

Re: [PATCH v6 0/3] Add support for Allwinner PWM on D1/T113s/R329 SoCs

[Uwe Kleine-König]

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

Hi John,

On Wed, Sep 06, 2023 at 12:46:19PM +1000, John Watts wrote:
> On Thu, Aug 24, 2023 at 02:40:24PM +0300, Aleksandr Shubin wrote:
> > This series adds support for PWM controller on new
> > Allwinner's SoCs, such as D1, T113s and R329. The implemented driver
> > provides basic functionality for control PWM channels.
> 
> I have tested this patch and earlier versions successfully on a Mango Pi MQ
> Dual and verified it outputs a PWM signal.

If you want that documented, the usual thing to do is to include a
Tested-by: tag in your reply.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH v6 0/3] Add support for Allwinner PWM on D1/T113s/R329 SoCs

[John Watts]

On Thu, Sep 21, 2023 at 11:27:36PM +0200, Uwe Kleine-König wrote:
> Hi John,
> 
> On Wed, Sep 06, 2023 at 12:46:19PM +1000, John Watts wrote:
> > On Thu, Aug 24, 2023 at 02:40:24PM +0300, Aleksandr Shubin wrote:
> > > This series adds support for PWM controller on new
> > > Allwinner's SoCs, such as D1, T113s and R329. The implemented driver
> > > provides basic functionality for control PWM channels.
> > 
> > I have tested this patch and earlier versions successfully on a Mango Pi MQ
> > Dual and verified it outputs a PWM signal.
> 
> If you want that documented, the usual thing to do is to include a
> Tested-by: tag in your reply.

That's a good idea! I'll take a closer look under a scope and against the data
sheet and give another tested-by then when I can. I'm not sure how close that
gets to a reviewed-by but I don't have the experience in driver development to
know if something is off in the design.

> 
> Best regards
> Uwe

John.

Re: [PATCH v6 0/3] Add support for Allwinner PWM on D1/T113s/R329 SoCs

[Uwe Kleine-König]

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

Hey John,

On Fri, Sep 22, 2023 at 07:35:19AM +1000, John Watts wrote:
> On Thu, Sep 21, 2023 at 11:27:36PM +0200, Uwe Kleine-König wrote:
> > Hi John,
> > 
> > On Wed, Sep 06, 2023 at 12:46:19PM +1000, John Watts wrote:
> > > On Thu, Aug 24, 2023 at 02:40:24PM +0300, Aleksandr Shubin wrote:
> > > > This series adds support for PWM controller on new
> > > > Allwinner's SoCs, such as D1, T113s and R329. The implemented driver
> > > > provides basic functionality for control PWM channels.
> > > 
> > > I have tested this patch and earlier versions successfully on a Mango Pi MQ
> > > Dual and verified it outputs a PWM signal.
> > 
> > If you want that documented, the usual thing to do is to include a
> > Tested-by: tag in your reply.
> 
> That's a good idea! I'll take a closer look under a scope and against the data
> sheet and give another tested-by then when I can. I'm not sure how close that
> gets to a reviewed-by but I don't have the experience in driver development to
> know if something is off in the design.

Tested-by has little to nothing to do with development or review. If you
tested that patch and it worked for you, that's enough to give such a
tag.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

Re: [PATCH v6 0/3] Add support for Allwinner PWM on D1/T113s/R329 SoCs

[John Watts]

On Thu, Aug 24, 2023 at 02:40:24PM +0300, Aleksandr Shubin wrote:
> Hi,
> 
> This series adds support for PWM controller on new
> Allwinner's SoCs, such as D1, T113s and R329. The implemented driver
> provides basic functionality for control PWM channels.

I did some more testing of this and it seems to work on my T113, outputting
correct periods and duty cycles.

Tested-by: John Watts <contact@jookia.org>

John.