[PATCH v7 0/4] Introduce Allwinner H616 PWM controller

[PATCH v7 0/4] Introduce Allwinner H616 PWM controller

[Richard Genoud]

Allwinner H616 PWM controller is quite different from the A10 one.

It can drive 6 PWM channels, and like for the A10, each channel has a
bypass that permits to output a clock, bypassing the PWM logic, when
enabled.

But, the channels are paired 2 by 2, sharing a first set of
MUX/prescaler/gate.
Then, for each channel, there's another prescaler (that will be bypassed
if the bypass is enabled for this channel).

It looks like that:
            _____      ______      ________
OSC24M --->|     |    |      |    |        |
APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> PWM_clock_src_xy
           |_____|    |______|    |________|
                          ________
                         |        |
                      +->| /div_k |---> PWM_clock_x
                      |  |________|
                      |    ______
                      |   |      |
                      +-->| Gate |----> PWM_bypass_clock_x
                      |   |______|
PWM_clock_src_xy -----+   ________
                      |  |        |
                      +->| /div_k |---> PWM_clock_y
                      |  |________|
                      |    ______
                      |   |      |
                      +-->| Gate |----> PWM_bypass_clock_y
                          |______|

Where xy can be 0/1, 2/3, 4/5

As this is quite complex for a PWM, the Common Clock Framework is
leverage to deal with that and not reinvent the wheel.

PWM_clock_x/y serve for the PWM purpose.
PWM_bypass_clock_x/y serve for the clock-provider purpose.
The common clock framework has been used to manage those clocks.

This PWM driver serves as a clock-provider for PWM_bypass_clocks.
This is needed for example by the embedded AC300 PHY which clock comes
from PMW5 pin (PB12).

Usually, to get a clock from a PWM driver, we use the pwm-clock driver
so that the PWM driver doesn't need to be a clk-provider itself.
While this works in most cases, here it just doesn't.
That's because the pwm-clock request a period from the PWM driver,
without any clue that it actually wants a clock at a specific frequency,
and not a PWM signal with duty cycle capability.
So, the PWM driver doesn't know if it can use the bypass or not, it
doesn't even have the real accurate frequency information (23809524 Hz
instead of 24MHz) because PWM drivers only deal with periods.

With pwm-clock, we loose a precious information along the way (that we
actually want a clock and not a PWM signal).
That's ok with simple PWM drivers that don't have multiple input clocks,
but in this case, without this information, we can't know for sure which
clock to use.
And here, for instance, if we ask for a 24MHz clock, pwm-clock will
requests 42ns (assigned-clocks doesn't help for that matter). The logic
is to select the highest clock (100MHz) with no prescaler and a duty
cycle value of 2/4 => we have 25MHz instead of 24MHz.
And that's a perfectly fine choice for a PMW, because we still can
change the duty cycle in the range [0-4]/4.
But obviously for a clock, we don't care about the duty cycle, but more
about the clock accuracy.

And actually, this PWM is really a PWM AND a real clock when the bypass
is set.

This series is based onto v7.2-rc1

NB: checkpatch is not happy with patch 2, but it's a false positive.
It doesn't detect that PWM_XY_SRC_MUX/GATE/DIV are structures, but as
it's more readable like that, I prefer keeping it that way.

Changes since v6:
- rebase on v7.2-rc1
- Cc common clock framework maintainers, suggested by Uwe
- add missing static before SUN8I_PWM_X_BYPASS_GATE
- reorder code in probe to fix potential life cycle issues
- set pwmcc_data[i].parent_names to NULL in sun8i_pwm_unregister_clk

Changes since v5:
- remove trailing junk after commit message in patch 4
- remove Tested-by when it doesn't make sense.
(sorry for the noise)

Changes since v4:
- Fix a bug on bypass for channels greater than 1
- add colons to clarify 2 debug messages
- switch from H616 to sun8i prefix (in code, filename, module name)
- fix consistency issues in macro parameters
- rename some macros with a confusing naming
- rebase on v7.0

Changes since v3:
- gather Acked-by/Tested-by
- fix cast from pointer to integer of different size (kernel test robot
  with arc platform)
- add devm_action for clk_hw_unregister_composite as suggested by Philipp
- remove now unused pwm_remove as suggested by Philipp

Changes since v2:
- use U32_MAX instead of defining UINT32_MAX
- add a comment on U32_MAX usage in clk_round_rate()
- change clk_table_div_m (use macros)
- fix formatting (double space, superfluous comma, extra line feed)
- fix the parent clock order
- simplify code by using scoped_guard()
- add missing const in to_h616_pwm_chip() and rename to
h616_pwm_from_chip()
- add/remove missing/superfluous error messages
- rename cnt->period_ticks, duty_cnt->duty_ticks
- fix PWM_PERIOD_MAX
- add .remove() callback
- fix DIV_ROUND_CLOSEST_ULL->DIV_ROUND_UP_ULL
- add H616_ prefix
- protect _reg in macros
- switch to waveforms instead of apply/get_state
- shrink struct h616_pwm_channel
- rebase on v6.19-rc4

Changes since v1:
- rebase onto v6.19-rc1
- add missing headers
- remove MODULE_ALIAS (suggested by Krzysztof)
- use sun4i-pwm binding instead of creating a new one (suggested by Krzysztof)
- retrieve the parent clocks from the devicetree
- switch num_parents to unsigned int

Richard Genoud (4):
  dt-bindings: pwm: allwinner: add h616 pwm compatible
  pwm: sun8i: Add H616 PWM support
  arm64: dts: allwinner: h616: add PWM controller
  MAINTAINERS: Add entry on Allwinner sun8i/H616 PWM driver

 .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml |  19 +-
 MAINTAINERS                                   |   5 +
 .../arm64/boot/dts/allwinner/sun50i-h616.dtsi |  47 +
 drivers/pwm/Kconfig                           |  12 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-sun8i.c                       | 938 ++++++++++++++++++
 6 files changed, 1021 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pwm/pwm-sun8i.c

-- 
2.47.3

[PATCH v7 1/4] dt-bindings: pwm: allwinner: add h616 pwm compatible

[Richard Genoud]

Allwinner H616 PWM block is quite different from the A10 or H6, but at
the end, it uses the same clocks as the H6; so the sun4i pwm binding can
be used.
It has 6 channels than can generate PWM waveforms.
If the bypass is enabled (one bypass per channel) the output is no more
a PWM waveform, but a clock that can (and is) used as input for other
devices, like the AC300 PHY.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
index 1197858e431f..4f58110ec98f 100644
--- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
@@ -14,6 +14,9 @@ properties:
   "#pwm-cells":
     const: 3
 
+  "#clock-cells":
+    const: 1
+
   compatible:
     oneOf:
       - const: allwinner,sun4i-a10-pwm
@@ -36,6 +39,7 @@ properties:
           - const: allwinner,sun50i-h5-pwm
           - const: allwinner,sun5i-a13-pwm
       - const: allwinner,sun50i-h6-pwm
+      - const: allwinner,sun50i-h616-pwm
 
   reg:
     maxItems: 1
@@ -62,7 +66,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: allwinner,sun50i-h6-pwm
+            enum:
+              - allwinner,sun50i-h6-pwm
+              - allwinner,sun50i-h616-pwm
 
     then:
       properties:
@@ -83,6 +89,17 @@ allOf:
         clocks:
           maxItems: 1
 
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: allwinner,sun50i-h616-pwm
+
+    then:
+      properties:
+        "#clock-cells": false
+
 required:
   - compatible
   - reg
-- 
2.47.3

[PATCH v7 2/4] pwm: sun8i: Add H616 PWM support

[Richard Genoud]

Add driver for Allwinner H616 PWM controller, supporting up to 6
channels.
Those channels output can be either a PWM signal output or a clock
output, thanks to the bypass.

The channels are paired (0/1, 2/3 and 4/5) and each pair has a
prescaler/mux/gate.
Moreover, each channel has its own prescaler and bypass.

The clock provider part of this driver is needed not only because the
H616 PWM controller provides also clocks when bypass is enabled, but
really because pwm-clock isn't fit to handle all cases here.
pwm-clock would work if the 100MHz clock is requested, but if a lower
clock is requested (like 24MHz), it will request a 42ns period to the
PWM driver which will happily serve, with the 100MHz clock as input a
25MHz frequency and a duty cycle adjustable in the range [0-4]/4,
because that is a sane thing to do for a PWM.
The information missing is that a real clock is resquested, not a PWM.

Tested-by: John Stultz <jstultz@google.com>
Tested-by: Joao Schim <joao@schimsalabim.eu>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 drivers/pwm/Kconfig     |  12 +
 drivers/pwm/Makefile    |   1 +
 drivers/pwm/pwm-sun8i.c | 938 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 951 insertions(+)
 create mode 100644 drivers/pwm/pwm-sun8i.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index e8886a9b64d9..6e055a3a748c 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -748,6 +748,18 @@ config PWM_SUN4I
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-sun4i.
 
+config PWM_SUN8I
+	tristate "Allwinner sun8i/sun50i PWM support"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	depends on HAS_IOMEM && COMMON_CLK
+	help
+	  Generic PWM framework driver for Allwinner H616 SoCs.
+	  It supports generic PWM, but can also provides a plain clock.
+	  The AC300 PHY integrated in H616 SoC needs such a clock.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-sun8i.
+
 config PWM_SUNPLUS
 	tristate "Sunplus PWM support"
 	depends on ARCH_SUNPLUS || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 5630a521a7cf..9c922b1fd0b4 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -68,6 +68,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_SUN8I)		+= pwm-sun8i.o
 obj-$(CONFIG_PWM_SUNPLUS)	+= pwm-sunplus.o
 obj-$(CONFIG_PWM_TEGRA)		+= pwm-tegra.o
 obj-$(CONFIG_PWM_TH1520)	+= pwm_th1520.o
diff --git a/drivers/pwm/pwm-sun8i.c b/drivers/pwm/pwm-sun8i.c
new file mode 100644
index 000000000000..8f1023e3a2e5
--- /dev/null
+++ b/drivers/pwm/pwm-sun8i.c
@@ -0,0 +1,938 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Allwinner sun8i Pulse Width Modulation Controller
+ *
+ * (C) Copyright 2025 Richard Genoud, Bootlin <richard.genoud@bootlin.com>
+ *
+ * Based on drivers/pwm/pwm-sun4i.c with Copyright:
+ *
+ * Copyright (C) 2014 Alexandre Belloni <alexandre.belloni@bootlin.com>
+ *
+ * Limitations:
+ * - As the channels are paired (0/1, 2/3, 4/5), they share the same clock
+ *   source and prescaler(div_m), but they also have their own prescaler(div_k)
+ *   and bypass.
+ *
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/limits.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+#include <linux/time.h>
+
+/* PWM IRQ Enable Register */
+#define SUN8I_PWM_PIER			0x0
+
+/* PWM IRQ Status Register */
+#define SUN8I_PWM_PISR			0x4
+
+/* PWM Capture IRQ Enable Register */
+#define SUN8I_PWM_CIER			0x10
+
+/* PWM Capture IRQ Status Register */
+#define SUN8I_PWM_CISR			0x14
+
+/* PWMCC Pairs Clock Configuration Registers */
+#define SUN8I_PWM_PCCR(pair)		(0x20 + ((pair) * 0x4))
+#define SUN8I_PWM_PCCR_SRC_SHIFT	7
+#define SUN8I_PWM_PCCR_SRC_MASK		1
+#define SUN8I_PWM_PCCR_GATE_BIT		4
+#define SUN8I_PWM_PCCR_BYPASS_BIT(chan)	((chan) % 2 + 5)
+#define SUN8I_PWM_PCCR_DIV_M_SHIFT	0
+
+/* PWMCC Pairs Dead Zone Control Registers */
+#define SUN8I_PWM_PDZCR(pair)		(0x30 + ((pair) * 0x4))
+
+/* PWM Enable Register */
+#define SUN8I_PWM_PER			0x40
+#define SUN8I_PWM_ENABLE(chan)		BIT(chan)
+
+/* PWM Capture Enable Register */
+#define SUN8I_PWM_CER			0x44
+
+/* PWM Control Register */
+#define SUN8I_PWM_PCR(chan)		(0x60 + (chan) * 0x20)
+#define SUN8I_PWM_PCR_PRESCAL_K_SHIFT	0
+#define SUN8I_PWM_PCR_PRESCAL_K_WIDTH	8
+#define SUN8I_PWM_PCR_ACTIVE_STATE	BIT(8)
+
+/* PWM Period Register */
+#define SUN8I_PWM_PPR(chan)		(0x64 + (chan) * 0x20)
+#define SUN8I_PWM_PPR_PERIOD_MASK	GENMASK(31, 16)
+#define SUN8I_PWM_PPR_DUTY_MASK		GENMASK(15, 0)
+#define SUN8I_PWM_PPR_PERIOD_VALUE(reg)	(FIELD_GET(SUN8I_PWM_PPR_PERIOD_MASK, reg) + 1)
+#define SUN8I_PWM_PPR_DUTY_VALUE(reg)	FIELD_GET(SUN8I_PWM_PPR_DUTY_MASK, reg)
+#define SUN8I_PWM_PPR_PERIOD(prd)	FIELD_PREP(SUN8I_PWM_PPR_PERIOD_MASK, (prd) - 1)
+#define SUN8I_PWM_DUTY(dty)		FIELD_PREP(SUN8I_PWM_PPR_DUTY_MASK, dty)
+#define SUN8I_PWM_PPR_PERIOD_MAX	(FIELD_MAX(SUN8I_PWM_PPR_PERIOD_MASK) + 1)
+
+/* PWM Count Register */
+#define SUN8I_PWM_PCNTR(chan)		(0x68 + (chan) * 0x20)
+
+/* PWM Capture Control Register */
+#define SUN8I_PWM_CCR(chan)		(0x6c + (chan) * 0x20)
+
+/* PWM Capture Rise Lock Register */
+#define SUN8I_PWM_CRLR(chan)		(0x70 + (chan) * 0x20)
+
+/* PWM Capture Fall Lock Register */
+#define SUN8I_PWM_CFLR(chan)		(0x74 + (chan) * 0x20)
+
+#define SUN8I_PWM_PAIR_IDX(chan)	((chan) >> 1)
+
+/*
+ * Block diagram of the PWM clock controller:
+ *
+ *             _____      ______      ________
+ * OSC24M --->|     |    |      |    |        |
+ * APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> SUN8I_PWM_clock_src_xy
+ *            |_____|    |______|    |________|
+ *                               ________
+ *                              |        |
+ *                           +->| /div_k |---> SUN8I_PWM_clock_x
+ *                           |  |________|
+ *                           |    ______
+ *                           |   |      |
+ *                           +-->| Gate |----> SUN8I_PWM_bypass_clock_x
+ *                           |   |______|
+ * SUN8I_PWM_clock_src_xy ---+   ________
+ *                           |  |        |
+ *                           +->| /div_k |---> SUN8I_PWM_clock_y
+ *                           |  |________|
+ *                           |    ______
+ *                           |   |      |
+ *                           +-->| Gate |----> SUN8I_PWM_bypass_clock_y
+ *                               |______|
+ *
+ * NB: when the bypass is set, all the PWM logic is bypassed.
+ * So, the duty cycle and polarity can't be modified (we just have a clock).
+ * The bypass in PWM mode is used to achieve a 1/2 relative duty cycle with the
+ * fastest clock.
+ *
+ * SUN8I_PWM_clock_x/y serve for the PWM purpose.
+ * SUN8I_PWM_bypass_clock_x/y serve for the clock-provider purpose.
+ *
+ */
+
+/*
+ * Table used for /div_m (diviser before obtaining SUN8I_PWM_clock_src_xy)
+ * It's actually CLK_DIVIDER_POWER_OF_TWO, but limited to /256
+ */
+#define CLK_TABLE_DIV_M_ENTRY(i) { \
+	.val = (i), .div = 1 << (i) \
+}
+
+static const struct clk_div_table clk_table_div_m[] = {
+	CLK_TABLE_DIV_M_ENTRY(0),
+	CLK_TABLE_DIV_M_ENTRY(1),
+	CLK_TABLE_DIV_M_ENTRY(2),
+	CLK_TABLE_DIV_M_ENTRY(3),
+	CLK_TABLE_DIV_M_ENTRY(4),
+	CLK_TABLE_DIV_M_ENTRY(5),
+	CLK_TABLE_DIV_M_ENTRY(6),
+	CLK_TABLE_DIV_M_ENTRY(7),
+	CLK_TABLE_DIV_M_ENTRY(8),
+	{ /* sentinel */ }
+};
+
+#define SUN8I_PWM_XY_SRC_GATE(_pair, _reg)		\
+struct clk_gate gate_xy_##_pair = {			\
+	.reg = (void *)(_reg),				\
+	.bit_idx = SUN8I_PWM_PCCR_GATE_BIT,		\
+	.hw.init = &(struct clk_init_data){		\
+		.ops = &clk_gate_ops,			\
+	}						\
+}
+
+#define SUN8I_PWM_XY_SRC_MUX(_pair, _reg)		\
+struct clk_mux mux_xy_##_pair = {			\
+	.reg = (void *)(_reg),				\
+	.shift = SUN8I_PWM_PCCR_SRC_SHIFT,		\
+	.mask = SUN8I_PWM_PCCR_SRC_MASK,		\
+	.flags = CLK_MUX_ROUND_CLOSEST,			\
+	.hw.init = &(struct clk_init_data){		\
+		.ops = &clk_mux_ops,			\
+	}						\
+}
+
+#define SUN8I_PWM_XY_SRC_DIV(_pair, _reg)		\
+struct clk_divider rate_xy_##_pair = {			\
+	.reg = (void *)(_reg),				\
+	.shift = SUN8I_PWM_PCCR_DIV_M_SHIFT,		\
+	.table = clk_table_div_m,			\
+	.hw.init = &(struct clk_init_data){		\
+		.ops = &clk_divider_ops,		\
+	}						\
+}
+
+#define SUN8I_PWM_X_DIV(_idx, _reg)			\
+struct clk_divider rate_x_##_idx = {			\
+	.reg = (void *)(_reg),				\
+	.shift = SUN8I_PWM_PCR_PRESCAL_K_SHIFT,		\
+	.width = SUN8I_PWM_PCR_PRESCAL_K_WIDTH,		\
+	.hw.init = &(struct clk_init_data){		\
+		.ops = &clk_divider_ops,		\
+	}						\
+}
+
+#define SUN8I_PWM_X_BYPASS_GATE(_idx)			\
+struct clk_gate gate_x_bypass_##_idx = {		\
+	.reg = (void *)SUN8I_PWM_PER,			\
+	.bit_idx = _idx,				\
+	.hw.init = &(struct clk_init_data){		\
+		.ops = &clk_gate_ops,			\
+	}						\
+}
+
+#define SUN8I_PWM_XY_CLK_SRC(_pair, _reg)			\
+	static SUN8I_PWM_XY_SRC_MUX(_pair, _reg);		\
+	static SUN8I_PWM_XY_SRC_GATE(_pair, _reg);		\
+	static SUN8I_PWM_XY_SRC_DIV(_pair, _reg)
+
+#define SUN8I_PWM_X_CLK(_idx)					\
+	static SUN8I_PWM_X_DIV(_idx, SUN8I_PWM_PCR(_idx))
+
+#define SUN8I_PWM_X_BYPASS_CLK(_idx)				\
+	SUN8I_PWM_X_BYPASS_GATE(_idx)
+
+#define REF_CLK_XY_SRC(_pair)						\
+	{								\
+		.name = "pwm-clk-src" #_pair,				\
+		.mux_hw = &mux_xy_##_pair.hw,				\
+		.gate_hw = &gate_xy_##_pair.hw,				\
+		.rate_hw = &rate_xy_##_pair.hw,				\
+	}
+
+#define REF_CLK_X(_idx, _pair)						\
+	{								\
+		.name = "pwm-clk" #_idx,				\
+		.parent_names = (const char *[]){ "pwm-clk-src" #_pair }, \
+		.num_parents = 1,					\
+		.rate_hw = &rate_x_##_idx.hw,				\
+		.flags = CLK_SET_RATE_PARENT,				\
+	}
+
+#define REF_CLK_BYPASS(_idx, _pair)					\
+	{								\
+		.name = "pwm-clk-bypass" #_idx,				\
+		.parent_names = (const char *[]){ "pwm-clk-src" #_pair }, \
+		.num_parents = 1,					\
+		.gate_hw = &gate_x_bypass_##_idx.hw,			\
+		.flags = CLK_SET_RATE_PARENT,				\
+	}
+
+/*
+ * SUN8I_PWM_clock_src_xy generation:
+ *             _____      ______      ________
+ * OSC24M --->|     |    |      |    |        |
+ * APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> SUN8I_PWM_clock_src_xy
+ *            |_____|    |______|    |________|
+ */
+SUN8I_PWM_XY_CLK_SRC(01, SUN8I_PWM_PCCR(0));
+SUN8I_PWM_XY_CLK_SRC(23, SUN8I_PWM_PCCR(1));
+SUN8I_PWM_XY_CLK_SRC(45, SUN8I_PWM_PCCR(2));
+
+/*
+ * SUN8I_PWM_clock_x_div generation:
+ *                            ________
+ *                           |        | SUN8I_PWM_clock_x/y
+ * SUN8I_PWM_clock_src_xy -->| /div_k |--------------->
+ *                           |________|
+ */
+SUN8I_PWM_X_CLK(0);
+SUN8I_PWM_X_CLK(1);
+SUN8I_PWM_X_CLK(2);
+SUN8I_PWM_X_CLK(3);
+SUN8I_PWM_X_CLK(4);
+SUN8I_PWM_X_CLK(5);
+
+/*
+ * SUN8I_PWM_bypass_clock_xy generation:
+ *                             ______
+ *                            |      |
+ * SUN8I_PWM_clock_src_xy --->| Gate |-------> SUN8I_PWM_bypass_clock_x
+ *                            |______|
+ *
+ * The gate is actually SUN8I_PWM_PER register.
+ */
+SUN8I_PWM_X_BYPASS_CLK(0);
+SUN8I_PWM_X_BYPASS_CLK(1);
+SUN8I_PWM_X_BYPASS_CLK(2);
+SUN8I_PWM_X_BYPASS_CLK(3);
+SUN8I_PWM_X_BYPASS_CLK(4);
+SUN8I_PWM_X_BYPASS_CLK(5);
+
+struct clk_pwm_data {
+	const char *name;
+	const char **parent_names;
+	unsigned int num_parents;
+	struct clk_hw *mux_hw;
+	struct clk_hw *rate_hw;
+	struct clk_hw *gate_hw;
+	unsigned long flags;
+};
+
+/* Indexes of REF_CLK_BYPASS and REF_CLK_XY_SRC in the array */
+#define CLK_BYPASS_IDX(sun8i_chip, chan) ((sun8i_chip)->data->npwm + (chan))
+#define CLK_XY_SRC_IDX(sun8i_chip, chan) \
+	((sun8i_chip)->data->npwm * 2 + SUN8I_PWM_PAIR_IDX(chan))
+static struct clk_pwm_data pwmcc_data[] = {
+	REF_CLK_X(0, 01),
+	REF_CLK_X(1, 01),
+	REF_CLK_X(2, 23),
+	REF_CLK_X(3, 23),
+	REF_CLK_X(4, 45),
+	REF_CLK_X(5, 45),
+	REF_CLK_BYPASS(0, 01),
+	REF_CLK_BYPASS(1, 01),
+	REF_CLK_BYPASS(2, 23),
+	REF_CLK_BYPASS(3, 23),
+	REF_CLK_BYPASS(4, 45),
+	REF_CLK_BYPASS(5, 45),
+	REF_CLK_XY_SRC(01),
+	REF_CLK_XY_SRC(23),
+	REF_CLK_XY_SRC(45),
+	{ /* sentinel */ }
+};
+
+enum sun8i_pwm_mode {
+	SUN8I_PWM_MODE_NONE,
+	SUN8I_PWM_MODE_PWM,
+	SUN8I_PWM_MODE_CLK,
+};
+
+struct sun8i_pwm_data {
+	unsigned int npwm;
+};
+
+struct sun8i_pwm_channel {
+	struct clk *pwm_clk;
+	enum sun8i_pwm_mode mode;
+};
+
+struct clk_pwm_pdata {
+	struct clk_hw_onecell_data *hw_data;
+	spinlock_t lock;
+	void __iomem *reg;
+};
+
+struct sun8i_pwm_chip {
+	struct clk_pwm_pdata *clk_pdata;
+	struct sun8i_pwm_channel *channels;
+	struct clk *bus_clk;
+	struct reset_control *rst;
+	void __iomem *base;
+	const struct sun8i_pwm_data *data;
+};
+
+struct sun8i_pwm_waveform {
+	u8 enabled:1;
+	u8 active_state:1;
+	u8 bypass_en:1;
+	u16 duty_ticks;
+	u32 period_ticks;
+	unsigned long clk_rate;
+};
+
+static inline struct sun8i_pwm_chip *sun8i_pwm_from_chip(const struct pwm_chip *chip)
+{
+	return pwmchip_get_drvdata(chip);
+}
+
+static inline u32 sun8i_pwm_readl(struct sun8i_pwm_chip *sun8i_chip,
+				  unsigned long offset)
+{
+	return readl(sun8i_chip->base + offset);
+}
+
+static inline void sun8i_pwm_writel(struct sun8i_pwm_chip *sun8i_chip,
+				    u32 val, unsigned long offset)
+{
+	writel(val, sun8i_chip->base + offset);
+}
+
+static void sun8i_pwm_set_bypass(struct sun8i_pwm_chip *sun8i_chip,
+				 unsigned int idx, bool en_bypass)
+{
+	unsigned long flags, reg_offset;
+	u32 val;
+
+	spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
+
+	reg_offset = SUN8I_PWM_PCCR(SUN8I_PWM_PAIR_IDX(idx));
+	val = sun8i_pwm_readl(sun8i_chip, reg_offset);
+	if (en_bypass)
+		val |= BIT(SUN8I_PWM_PCCR_BYPASS_BIT(idx));
+	else
+		val &= ~BIT(SUN8I_PWM_PCCR_BYPASS_BIT(idx));
+
+	sun8i_pwm_writel(sun8i_chip, val, reg_offset);
+
+	spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);
+}
+
+static int sun8i_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
+	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
+
+	scoped_guard(spinlock_irqsave, &sun8i_chip->clk_pdata->lock) {
+		if (chan->mode == SUN8I_PWM_MODE_CLK)
+			return -EBUSY;
+		chan->mode = SUN8I_PWM_MODE_PWM;
+	}
+
+	return clk_prepare_enable(chan->pwm_clk);
+}
+
+static void sun8i_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
+	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
+
+	clk_disable_unprepare(chan->pwm_clk);
+	chan->mode = SUN8I_PWM_MODE_NONE;
+}
+
+static int sun8i_pwm_read_waveform(struct pwm_chip *chip,
+				   struct pwm_device *pwm,
+				   void *_wfhw)
+{
+	struct sun8i_pwm_waveform *wfhw = _wfhw;
+	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
+	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
+	u32 val;
+
+	wfhw->clk_rate = clk_get_rate(chan->pwm_clk);
+
+	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PER);
+	wfhw->enabled = !!(SUN8I_PWM_ENABLE(pwm->hwpwm) & val);
+
+	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCCR(SUN8I_PWM_PAIR_IDX(pwm->hwpwm)));
+	wfhw->bypass_en = !!(val & BIT(SUN8I_PWM_PCCR_BYPASS_BIT(pwm->hwpwm)));
+
+	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCR(pwm->hwpwm));
+	wfhw->active_state = !!(val & SUN8I_PWM_PCR_ACTIVE_STATE);
+
+	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PPR(pwm->hwpwm));
+	wfhw->duty_ticks = SUN8I_PWM_PPR_DUTY_VALUE(val);
+	wfhw->period_ticks = SUN8I_PWM_PPR_PERIOD_VALUE(val);
+
+	dev_dbg(pwmchip_parent(chip),
+		"pwm%u: %s, bypass: %s, polarity: %s, clk_rate=%lu period_ticks=%u duty_ticks=%u\n",
+		pwm->hwpwm,
+		wfhw->enabled ? "enabled" : "disabled",
+		wfhw->bypass_en ? "enabled" : "disabled",
+		wfhw->active_state ? "normal" : "inversed",
+		wfhw->clk_rate, wfhw->period_ticks, wfhw->duty_ticks);
+
+	return 0;
+}
+
+static int sun8i_pwm_round_waveform_fromhw(struct pwm_chip *chip,
+					   struct pwm_device *pwm,
+					   const void *_wfhw,
+					   struct pwm_waveform *wf)
+{
+	const struct sun8i_pwm_waveform *wfhw = _wfhw;
+	u64 tmp, resolution;
+
+	dev_dbg(pwmchip_parent(chip),
+		"pwm%u: %s, bypass: %s, polarity: %s, clk_rate=%lu period_ticks=%u duty_ticks=%u\n",
+		pwm->hwpwm,
+		wfhw->enabled ? "enabled" : "disabled",
+		wfhw->bypass_en ? "enabled" : "disabled",
+		wfhw->active_state ? "normal" : "inversed",
+		wfhw->clk_rate, wfhw->period_ticks, wfhw->duty_ticks);
+
+	wf->duty_offset_ns = 0;
+
+	if (!wfhw->enabled || !wfhw->clk_rate) {
+		wf->period_length_ns = 0;
+		wf->duty_length_ns = 0;
+		return 0;
+	}
+
+	if (wfhw->bypass_en) {
+		wf->period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC,
+							wfhw->clk_rate);
+		wf->duty_length_ns = DIV_ROUND_UP_ULL(wf->period_length_ns, 2);
+		return 0;
+	}
+
+	tmp = NSEC_PER_SEC * (u64)wfhw->period_ticks;
+	wf->period_length_ns = DIV_ROUND_UP_ULL(tmp, wfhw->clk_rate);
+
+	tmp = NSEC_PER_SEC * (u64)wfhw->duty_ticks;
+	wf->duty_length_ns = DIV_ROUND_UP_ULL(tmp, wfhw->clk_rate);
+	if (!wfhw->active_state) {
+		/*
+		 * For inverted polarity, we have to fix cases where
+		 * computed duty_length_ns > requested duty_length_ns
+		 * For that, we subtract the actual resolution of the PWM
+		 * registers
+		 */
+		wf->duty_offset_ns = wf->duty_length_ns;
+		wf->duty_length_ns = wf->period_length_ns - wf->duty_length_ns;
+
+		resolution = DIV_ROUND_UP_ULL(NSEC_PER_SEC, wfhw->clk_rate);
+
+		if (wf->duty_offset_ns >= resolution)
+			wf->duty_offset_ns -= resolution;
+	}
+
+	dev_dbg(pwmchip_parent(chip),
+		"pwm%u period_length_ns=%llu duty_length_ns=%llu duty_offset_ns=%llu\n",
+		pwm->hwpwm, wf->period_length_ns, wf->duty_length_ns,
+		wf->duty_offset_ns);
+
+	return 0;
+}
+
+static int sun8i_pwm_write_waveform(struct pwm_chip *chip,
+				    struct pwm_device *pwm, const void *_wfhw)
+{
+	const struct sun8i_pwm_waveform *wfhw = _wfhw;
+	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
+	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
+	unsigned long flags;
+	u32 val;
+	int ret;
+
+	ret = clk_set_rate(chan->pwm_clk, wfhw->clk_rate);
+	if (ret)
+		return ret;
+
+	sun8i_pwm_set_bypass(sun8i_chip, pwm->hwpwm, wfhw->bypass_en);
+
+	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCR(pwm->hwpwm));
+	if (wfhw->active_state)
+		val |= SUN8I_PWM_PCR_ACTIVE_STATE;
+	else
+		val &= ~SUN8I_PWM_PCR_ACTIVE_STATE;
+	sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PCR(pwm->hwpwm));
+
+	val = SUN8I_PWM_DUTY(wfhw->duty_ticks);
+	val |= SUN8I_PWM_PPR_PERIOD(wfhw->period_ticks);
+	sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PPR(pwm->hwpwm));
+
+	spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
+
+	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PER);
+	if (wfhw->enabled)
+		val |= SUN8I_PWM_ENABLE(pwm->hwpwm);
+	else
+		val &= ~SUN8I_PWM_ENABLE(pwm->hwpwm);
+	sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PER);
+
+	spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);
+
+	return 0;
+}
+
+static int sun8i_pwm_round_waveform_tohw(struct pwm_chip *chip,
+					 struct pwm_device *pwm,
+					 const struct pwm_waveform *wf,
+					 void *_wfhw)
+{
+	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
+	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
+	struct sun8i_pwm_waveform *wfhw = _wfhw;
+	unsigned long max_rate;
+	long calc_rate;
+	u64 period_ratio, double_duty_ratio, freq, duty_cycle;
+
+	dev_dbg(pwmchip_parent(chip),
+		"pwm%u period_length_ns=%llu duty_length_ns=%llu duty_offset_ns=%llu\n",
+		pwm->hwpwm, wf->period_length_ns, wf->duty_length_ns,
+		wf->duty_offset_ns);
+
+	if (wf->period_length_ns == 0) {
+		wfhw->enabled = 0;
+		return 0;
+	}
+
+	wfhw->enabled = 1;
+
+	duty_cycle = wf->duty_length_ns;
+	if (wf->duty_length_ns + wf->duty_offset_ns < wf->period_length_ns)
+		wfhw->active_state = 1;
+	else
+		wfhw->active_state = 0;
+
+	dev_dbg(pwmchip_parent(chip), "polarity: %s\n",
+		wfhw->active_state ? "normal" : "inversed");
+
+	/*
+	 * Lowest possible period case:
+	 * Without bypass, the lowest possible period is when:
+	 * duty cycle = 1 and period cycle = 2 (0x10001 in period register)
+	 * E.g. if the input clock is 100MHz, we have a lowest period of 20ns.
+	 * Now, with the bypass, the period register is ignored and we directly
+	 * have the 100MHz clock as PWM output, that can act as a 10ns period
+	 * with 5ns duty.
+	 * So, to detect this lowest period case, just get the maximum possible
+	 * rate from chan->pwm_clk and compare it with requested period and
+	 * duty_cycle.
+	 *
+	 * But, to get the maximum possible rate, we have to use U32_MAX instead
+	 * of (unsigned long)-1.
+	 * This is because clk_round_rate() uses ultimately DIV_ROUND_UP_ULL()
+	 * that in turn do_div(n,base). And base is uint32_t divisor.
+	 */
+	max_rate = clk_round_rate(chan->pwm_clk, U32_MAX);
+
+	dev_dbg(pwmchip_parent(chip), "max_rate: %ld Hz\n", max_rate);
+
+	period_ratio = mul_u64_u64_div_u64(wf->period_length_ns,
+					   max_rate, NSEC_PER_SEC);
+	double_duty_ratio = mul_u64_u64_div_u64(duty_cycle, (u64)max_rate * 2,
+						NSEC_PER_SEC);
+	if (period_ratio == 1) {
+		if (double_duty_ratio == 0)
+			/* requested period and duty are too small */
+			return -EINVAL;
+		/*
+		 * If the requested period is to small to be generated by the
+		 * PWM, but matches the highest clock with a
+		 * duty_cycle >= period*2, just bypass the PWM logic
+		 */
+		freq = div64_u64(NSEC_PER_SEC, wf->period_length_ns);
+		wfhw->bypass_en = true;
+	} else {
+		wfhw->bypass_en = false;
+		freq = div64_u64(NSEC_PER_SEC * (u64)SUN8I_PWM_PPR_PERIOD_MAX,
+				 wf->period_length_ns);
+		/*
+		 * Same remark as above, this is to prevent a value to big for
+		 * clk_round_rate() to handle
+		 */
+		if (freq > U32_MAX)
+			freq = U32_MAX;
+	}
+
+	dev_dbg(pwmchip_parent(chip), "bypass: %s\n",
+		wfhw->bypass_en ? "enabled" : "disabled");
+
+	calc_rate = clk_round_rate(chan->pwm_clk, freq);
+	if (calc_rate <= 0)
+		return calc_rate ? calc_rate : -EINVAL;
+
+	dev_dbg(pwmchip_parent(chip), "calc_rate: %ld Hz\n", calc_rate);
+
+	wfhw->period_ticks = mul_u64_u64_div_u64(calc_rate,
+						 wf->period_length_ns,
+						 NSEC_PER_SEC);
+	if (wfhw->period_ticks > SUN8I_PWM_PPR_PERIOD_MAX)
+		wfhw->period_ticks = SUN8I_PWM_PPR_PERIOD_MAX;
+
+	/* min value in period register is 1 */
+	if (wfhw->period_ticks == 0)
+		return -EINVAL;
+
+	wfhw->duty_ticks = mul_u64_u64_div_u64(calc_rate, duty_cycle,
+					       NSEC_PER_SEC);
+
+	if (wfhw->duty_ticks > wfhw->period_ticks)
+		wfhw->duty_ticks = wfhw->period_ticks;
+
+	if (!wfhw->active_state)
+		wfhw->duty_ticks = wfhw->period_ticks - wfhw->duty_ticks;
+
+	dev_dbg(pwmchip_parent(chip),
+		"pwm%u period_ticks=%u duty_cycle=%llu duty_ticks=%u\n",
+		pwm->hwpwm, wfhw->period_ticks, duty_cycle, wfhw->duty_ticks);
+
+	wfhw->clk_rate = calc_rate;
+
+	return 0;
+}
+
+static const struct pwm_ops sun8i_pwm_ops = {
+	.request = sun8i_pwm_request,
+	.free = sun8i_pwm_free,
+	.sizeof_wfhw = sizeof(struct sun8i_pwm_waveform),
+	.round_waveform_tohw = sun8i_pwm_round_waveform_tohw,
+	.round_waveform_fromhw = sun8i_pwm_round_waveform_fromhw,
+	.read_waveform = sun8i_pwm_read_waveform,
+	.write_waveform = sun8i_pwm_write_waveform,
+};
+
+static struct clk_hw *sun8i_pwm_of_clk_get(struct of_phandle_args *clkspec,
+					   void *data)
+{
+	struct sun8i_pwm_chip *sun8i_chip = data;
+	struct clk_hw_onecell_data *hw_data = sun8i_chip->clk_pdata->hw_data;
+	unsigned int idx = clkspec->args[0];
+	struct sun8i_pwm_channel *chan;
+	struct clk_hw *ret_clk = NULL;
+	unsigned long flags;
+
+	if (idx >= sun8i_chip->data->npwm)
+		return ERR_PTR(-EINVAL);
+
+	chan = &sun8i_chip->channels[idx];
+
+	spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
+
+	if (chan->mode == SUN8I_PWM_MODE_PWM) {
+		ret_clk = ERR_PTR(-EBUSY);
+	} else {
+		chan->mode = SUN8I_PWM_MODE_CLK;
+		ret_clk = hw_data->hws[CLK_BYPASS_IDX(sun8i_chip, idx)];
+	}
+	spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);
+
+	if (IS_ERR(ret_clk))
+		goto out;
+
+	sun8i_pwm_set_bypass(sun8i_chip, idx, true);
+out:
+	return ret_clk;
+}
+
+static int sun8i_add_composite_clk(struct clk_pwm_data *data,
+				   void __iomem *reg, spinlock_t *lock,
+				   struct device *dev, struct clk_hw **hw)
+{
+	const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, *rate_ops = NULL;
+	struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *rate_hw = NULL;
+	struct device_node *node = dev->of_node;
+
+	if (data->mux_hw) {
+		struct clk_mux *mux;
+
+		mux_hw = data->mux_hw;
+		mux = to_clk_mux(mux_hw);
+		mux->lock = lock;
+		mux_ops = mux_hw->init->ops;
+		mux->reg = (uintptr_t)mux->reg + reg;
+	}
+
+	if (data->gate_hw) {
+		struct clk_gate *gate;
+
+		gate_hw = data->gate_hw;
+		gate = to_clk_gate(gate_hw);
+		gate->lock = lock;
+		gate_ops = gate_hw->init->ops;
+		gate->reg = (uintptr_t)gate->reg + reg;
+	}
+
+	if (data->rate_hw) {
+		struct clk_divider *rate;
+
+		rate_hw = data->rate_hw;
+		rate = to_clk_divider(rate_hw);
+		rate_ops = rate_hw->init->ops;
+		rate->lock = lock;
+		rate->reg = (uintptr_t)rate->reg + reg;
+
+		if (rate->table) {
+			const struct clk_div_table *clkt;
+			int table_size = 0;
+
+			for (clkt = rate->table; clkt->div; clkt++)
+				table_size++;
+			rate->width = order_base_2(table_size);
+		}
+	}
+
+	/*
+	 * Retrieve the parent clock names from DTS for pwm-clk-srcxy
+	 */
+	if (!data->parent_names) {
+		data->num_parents = of_clk_get_parent_count(node);
+		if (data->num_parents == 0)
+			return -ENOENT;
+
+		data->parent_names = devm_kzalloc(dev,
+						  sizeof(*data->parent_names),
+						  GFP_KERNEL);
+		for (unsigned int i = 0; i < data->num_parents; i++)
+			data->parent_names[i] = of_clk_get_parent_name(node, i);
+	}
+
+	*hw = clk_hw_register_composite(dev, data->name, data->parent_names,
+					data->num_parents, mux_hw,
+					mux_ops, rate_hw, rate_ops,
+					gate_hw, gate_ops, data->flags);
+
+	return PTR_ERR_OR_ZERO(*hw);
+}
+
+static int sun8i_pwm_init_clocks(struct platform_device *pdev,
+				 struct sun8i_pwm_chip *sun8i_chip)
+{
+	struct clk_pwm_pdata *pdata;
+	struct device *dev = &pdev->dev;
+	int num_clocks = 0;
+	int ret;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return dev_err_probe(dev, -ENOMEM,
+				     "Failed to allocate clk_pwm_pdata\n");
+
+	while (pwmcc_data[num_clocks].name)
+		num_clocks++;
+
+	pdata->hw_data = devm_kzalloc(dev, struct_size(pdata->hw_data, hws, num_clocks),
+				      GFP_KERNEL);
+	if (!pdata->hw_data)
+		return dev_err_probe(dev, -ENOMEM,
+				     "Failed to allocate hw clocks\n");
+
+	pdata->hw_data->num = num_clocks;
+	pdata->reg = sun8i_chip->base;
+
+	spin_lock_init(&pdata->lock);
+
+	for (int i = 0; i < num_clocks; i++) {
+		struct clk_hw **hw = &pdata->hw_data->hws[i];
+
+		ret = sun8i_add_composite_clk(&pwmcc_data[i], pdata->reg,
+					      &pdata->lock, dev, hw);
+		if (ret) {
+			dev_err_probe(dev, ret,
+				      "Failed to register hw clock %s\n",
+				      pwmcc_data[i].name);
+			for (i--; i >= 0; i--)
+				clk_hw_unregister_composite(pdata->hw_data->hws[i]);
+			return ret;
+		}
+	}
+
+	sun8i_chip->clk_pdata = pdata;
+
+	return 0;
+}
+
+static void sun8i_pwm_unregister_clk(void *data)
+{
+	struct clk_hw_onecell_data *hw_data = data;
+
+	for (unsigned int i = 0; i < hw_data->num; i++)
+		clk_hw_unregister_composite(hw_data->hws[i]);
+}
+
+static int sun8i_pwm_probe(struct platform_device *pdev)
+{
+	const struct sun8i_pwm_data *data;
+	struct device *dev = &pdev->dev;
+	struct sun8i_pwm_chip *sun8i_chip;
+	struct pwm_chip *chip;
+	int ret;
+
+	data = of_device_get_match_data(dev);
+	if (!data)
+		return dev_err_probe(dev, -ENODEV,
+				     "Missing specific data structure\n");
+
+	chip = devm_pwmchip_alloc(dev, data->npwm, sizeof(*sun8i_chip));
+	if (IS_ERR(chip))
+		return dev_err_probe(dev, PTR_ERR(chip),
+				     "Failed to allocate pwmchip\n");
+
+	sun8i_chip = sun8i_pwm_from_chip(chip);
+	sun8i_chip->data = data;
+	sun8i_chip->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(sun8i_chip->base))
+		return dev_err_probe(dev, PTR_ERR(sun8i_chip->base),
+				     "Failed to get PWM base address\n");
+
+	sun8i_chip->bus_clk = devm_clk_get_enabled(dev, "bus");
+	if (IS_ERR(sun8i_chip->bus_clk))
+		return dev_err_probe(dev, PTR_ERR(sun8i_chip->bus_clk),
+				     "Failed to get bus clock\n");
+
+	sun8i_chip->channels = devm_kmalloc_array(dev, data->npwm,
+						  sizeof(*(sun8i_chip->channels)),
+						  GFP_KERNEL);
+	if (!sun8i_chip->channels)
+		return dev_err_probe(dev, -ENOMEM,
+				     "Failed to allocate %d channels array\n",
+				     data->npwm);
+
+	chip->ops = &sun8i_pwm_ops;
+
+	ret = sun8i_pwm_init_clocks(pdev, sun8i_chip);
+	if (ret)
+		return ret;
+
+	for (unsigned int i = 0; i < data->npwm; i++) {
+		struct sun8i_pwm_channel *chan = &sun8i_chip->channels[i];
+		struct clk_hw **hw = &sun8i_chip->clk_pdata->hw_data->hws[i];
+
+		chan->pwm_clk = devm_clk_hw_get_clk(dev, *hw, NULL);
+		if (IS_ERR(chan->pwm_clk)) {
+			ret = dev_err_probe(dev, PTR_ERR(chan->pwm_clk),
+					    "Failed to register PWM clock %d\n", i);
+			return ret;
+		}
+		chan->mode = SUN8I_PWM_MODE_NONE;
+	}
+
+	ret = devm_of_clk_add_hw_provider(dev, sun8i_pwm_of_clk_get, sun8i_chip);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add HW clock provider\n");
+
+	ret = devm_add_action_or_reset(dev, sun8i_pwm_unregister_clk,
+				       sun8i_chip->clk_pdata->hw_data);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add devm action\n");
+
+	/* Deassert reset */
+	sun8i_chip->rst = devm_reset_control_get_shared_deasserted(dev, NULL);
+	if (IS_ERR(sun8i_chip->rst))
+		return dev_err_probe(dev, PTR_ERR(sun8i_chip->rst),
+				     "Failed to get reset control\n");
+
+	ret = devm_pwmchip_add(dev, chip);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to add PWM chip\n");
+
+	platform_set_drvdata(pdev, chip);
+
+	return 0;
+}
+
+static const struct sun8i_pwm_data sun50i_h616_pwm_data = {
+	.npwm = 6,
+};
+
+static const struct of_device_id sun8i_pwm_dt_ids[] = {
+	{
+		.compatible = "allwinner,sun50i-h616-pwm",
+		.data = &sun50i_h616_pwm_data,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, sun8i_pwm_dt_ids);
+
+static struct platform_driver sun8i_pwm_driver = {
+	.driver = {
+		.name = "sun8i-pwm",
+		.of_match_table = sun8i_pwm_dt_ids,
+	},
+	.probe = sun8i_pwm_probe,
+};
+module_platform_driver(sun8i_pwm_driver);
+
+MODULE_AUTHOR("Richard Genoud <richard.genoud@bootlin.com>");
+MODULE_DESCRIPTION("Allwinner sun8i PWM driver");
+MODULE_LICENSE("GPL");
-- 
2.47.3

Re: [PATCH v7 2/4] pwm: sun8i: Add H616 PWM support

[Philipp Zabel]

On Fr, 2026-07-03 at 17:22 +0200, Richard Genoud wrote:
> Add driver for Allwinner H616 PWM controller, supporting up to 6
> channels.
> Those channels output can be either a PWM signal output or a clock
> output, thanks to the bypass.
> 
> The channels are paired (0/1, 2/3 and 4/5) and each pair has a
> prescaler/mux/gate.
> Moreover, each channel has its own prescaler and bypass.
> 
> The clock provider part of this driver is needed not only because the
> H616 PWM controller provides also clocks when bypass is enabled, but
> really because pwm-clock isn't fit to handle all cases here.
> pwm-clock would work if the 100MHz clock is requested, but if a lower
> clock is requested (like 24MHz), it will request a 42ns period to the
> PWM driver which will happily serve, with the 100MHz clock as input a
> 25MHz frequency and a duty cycle adjustable in the range [0-4]/4,
> because that is a sane thing to do for a PWM.
> The information missing is that a real clock is resquested, not a PWM.
> 
> Tested-by: John Stultz <jstultz@google.com>
> Tested-by: Joao Schim <joao@schimsalabim.eu>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> ---
>  drivers/pwm/Kconfig     |  12 +
>  drivers/pwm/Makefile    |   1 +
>  drivers/pwm/pwm-sun8i.c | 938 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 951 insertions(+)
>  create mode 100644 drivers/pwm/pwm-sun8i.c
> 
[...]
> diff --git a/drivers/pwm/pwm-sun8i.c b/drivers/pwm/pwm-sun8i.c
> new file mode 100644
> index 000000000000..8f1023e3a2e5
> --- /dev/null
> +++ b/drivers/pwm/pwm-sun8i.c
> @@ -0,0 +1,938 @@
[...]
> +struct sun8i_pwm_chip {
> +	struct clk_pwm_pdata *clk_pdata;
> +	struct sun8i_pwm_channel *channels;
> +	struct clk *bus_clk;
> +	struct reset_control *rst;

The rst field is unused, I suggest you remove it.

> +	void __iomem *base;
> +	const struct sun8i_pwm_data *data;
> +};
[...]
> +static int sun8i_pwm_probe(struct platform_device *pdev)
> +{
[...]
> +	ret = sun8i_pwm_init_clocks(pdev, sun8i_chip);
> +	if (ret)
> +		return ret;
> +
> +	for (unsigned int i = 0; i < data->npwm; i++) {
> +		struct sun8i_pwm_channel *chan = &sun8i_chip->channels[i];
> +		struct clk_hw **hw = &sun8i_chip->clk_pdata->hw_data->hws[i];
> +
> +		chan->pwm_clk = devm_clk_hw_get_clk(dev, *hw, NULL);
> +		if (IS_ERR(chan->pwm_clk)) {
> +			ret = dev_err_probe(dev, PTR_ERR(chan->pwm_clk),
> +					    "Failed to register PWM clock %d\n", i);
> +			return ret;

If this returns ...

> +		}
> +		chan->mode = SUN8I_PWM_MODE_NONE;
> +	}
> +
> +	ret = devm_of_clk_add_hw_provider(dev, sun8i_pwm_of_clk_get, sun8i_chip);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add HW clock provider\n");

... or this returns ...

> +
> +	ret = devm_add_action_or_reset(dev, sun8i_pwm_unregister_clk,
> +				       sun8i_chip->clk_pdata->hw_data);

... the clk_hw registered in sun8i_pwm_init_clocks() are never cleaned
up, so this devres action should be set up right after
sun8i_pwm_init_clocks().

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add devm action\n");
> +
> +	/* Deassert reset */
> +	sun8i_chip->rst = devm_reset_control_get_shared_deasserted(dev, NULL);

rst is never used again. It should be a local variable.

> +	if (IS_ERR(sun8i_chip->rst))
> +		return dev_err_probe(dev, PTR_ERR(sun8i_chip->rst),
> +				     "Failed to get reset control\n");
> +
> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to add PWM chip\n");
> +
> +	platform_set_drvdata(pdev, chip);
> +
> +	return 0;
> +}

regards
Philipp

Re: [PATCH v7 2/4] pwm: sun8i: Add H616 PWM support

[Uwe Kleine-König]

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

Hello,

On Fri, Jul 03, 2026 at 05:22:13PM +0200, Richard Genoud wrote:
> Add driver for Allwinner H616 PWM controller, supporting up to 6
> channels.
> Those channels output can be either a PWM signal output or a clock
> output, thanks to the bypass.
> 
> The channels are paired (0/1, 2/3 and 4/5) and each pair has a
> prescaler/mux/gate.
> Moreover, each channel has its own prescaler and bypass.
> 
> The clock provider part of this driver is needed not only because the
> H616 PWM controller provides also clocks when bypass is enabled, but
> really because pwm-clock isn't fit to handle all cases here.
> pwm-clock would work if the 100MHz clock is requested, but if a lower
> clock is requested (like 24MHz), it will request a 42ns period to the
> PWM driver which will happily serve, with the 100MHz clock as input a
> 25MHz frequency and a duty cycle adjustable in the range [0-4]/4,
> because that is a sane thing to do for a PWM.
> The information missing is that a real clock is resquested, not a PWM.
> 
> Tested-by: John Stultz <jstultz@google.com>
> Tested-by: Joao Schim <joao@schimsalabim.eu>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> ---
>  drivers/pwm/Kconfig     |  12 +
>  drivers/pwm/Makefile    |   1 +
>  drivers/pwm/pwm-sun8i.c | 938 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 951 insertions(+)
>  create mode 100644 drivers/pwm/pwm-sun8i.c
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index e8886a9b64d9..6e055a3a748c 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -748,6 +748,18 @@ config PWM_SUN4I
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pwm-sun4i.
>  
> +config PWM_SUN8I
> +	tristate "Allwinner sun8i/sun50i PWM support"
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	depends on HAS_IOMEM && COMMON_CLK
> +	help
> +	  Generic PWM framework driver for Allwinner H616 SoCs.
> +	  It supports generic PWM, but can also provides a plain clock.
> +	  The AC300 PHY integrated in H616 SoC needs such a clock.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called pwm-sun8i.
> +
>  config PWM_SUNPLUS
>  	tristate "Sunplus PWM support"
>  	depends on ARCH_SUNPLUS || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 5630a521a7cf..9c922b1fd0b4 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -68,6 +68,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_SUN8I)		+= pwm-sun8i.o
>  obj-$(CONFIG_PWM_SUNPLUS)	+= pwm-sunplus.o
>  obj-$(CONFIG_PWM_TEGRA)		+= pwm-tegra.o
>  obj-$(CONFIG_PWM_TH1520)	+= pwm_th1520.o
> diff --git a/drivers/pwm/pwm-sun8i.c b/drivers/pwm/pwm-sun8i.c
> new file mode 100644
> index 000000000000..8f1023e3a2e5
> --- /dev/null
> +++ b/drivers/pwm/pwm-sun8i.c
> @@ -0,0 +1,938 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for Allwinner sun8i Pulse Width Modulation Controller
> + *
> + * (C) Copyright 2025 Richard Genoud, Bootlin <richard.genoud@bootlin.com>
> + *
> + * Based on drivers/pwm/pwm-sun4i.c with Copyright:
> + *
> + * Copyright (C) 2014 Alexandre Belloni <alexandre.belloni@bootlin.com>
> + *
> + * Limitations:
> + * - As the channels are paired (0/1, 2/3, 4/5), they share the same clock
> + *   source and prescaler(div_m), but they also have their own prescaler(div_k)
> + *   and bypass.
> + *
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/limits.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +#include <linux/time.h>
> +
> +/* PWM IRQ Enable Register */
> +#define SUN8I_PWM_PIER			0x0
> +
> +/* PWM IRQ Status Register */
> +#define SUN8I_PWM_PISR			0x4
> +
> +/* PWM Capture IRQ Enable Register */
> +#define SUN8I_PWM_CIER			0x10
> +
> +/* PWM Capture IRQ Status Register */
> +#define SUN8I_PWM_CISR			0x14
> +
> +/* PWMCC Pairs Clock Configuration Registers */
> +#define SUN8I_PWM_PCCR(pair)		(0x20 + ((pair) * 0x4))
> +#define SUN8I_PWM_PCCR_SRC_SHIFT	7
> +#define SUN8I_PWM_PCCR_SRC_MASK		1
> +#define SUN8I_PWM_PCCR_GATE_BIT		4
> +#define SUN8I_PWM_PCCR_BYPASS_BIT(chan)	((chan) % 2 + 5)

I'd prefer:

#define SUN8I_PWM_PCCR_GATE		BIT(4)
#define SUN8I_PWM_PCCR_BYPASS(chan)	BIT((chan) % 2 + 5)
#define SUN8I_PWM_PCCR_SRC		BIT(7)

> +#define SUN8I_PWM_PCCR_DIV_M_SHIFT	0
> +
> +/* PWMCC Pairs Dead Zone Control Registers */
> +#define SUN8I_PWM_PDZCR(pair)		(0x30 + ((pair) * 0x4))
> +
> +/* PWM Enable Register */
> +#define SUN8I_PWM_PER			0x40
> +#define SUN8I_PWM_ENABLE(chan)		BIT(chan)
> +
> +/* PWM Capture Enable Register */
> +#define SUN8I_PWM_CER			0x44
> +
> +/* PWM Control Register */
> +#define SUN8I_PWM_PCR(chan)		(0x60 + (chan) * 0x20)
> +#define SUN8I_PWM_PCR_PRESCAL_K_SHIFT	0
> +#define SUN8I_PWM_PCR_PRESCAL_K_WIDTH	8
> +#define SUN8I_PWM_PCR_ACTIVE_STATE	BIT(8)
> +
> +/* PWM Period Register */
> +#define SUN8I_PWM_PPR(chan)		(0x64 + (chan) * 0x20)
> +#define SUN8I_PWM_PPR_PERIOD_MASK	GENMASK(31, 16)
> +#define SUN8I_PWM_PPR_DUTY_MASK		GENMASK(15, 0)
> +#define SUN8I_PWM_PPR_PERIOD_VALUE(reg)	(FIELD_GET(SUN8I_PWM_PPR_PERIOD_MASK, reg) + 1)
> +#define SUN8I_PWM_PPR_DUTY_VALUE(reg)	FIELD_GET(SUN8I_PWM_PPR_DUTY_MASK, reg)
> +#define SUN8I_PWM_PPR_PERIOD(prd)	FIELD_PREP(SUN8I_PWM_PPR_PERIOD_MASK, (prd) - 1)
> +#define SUN8I_PWM_DUTY(dty)		FIELD_PREP(SUN8I_PWM_PPR_DUTY_MASK, dty)
> +#define SUN8I_PWM_PPR_PERIOD_MAX	(FIELD_MAX(SUN8I_PWM_PPR_PERIOD_MASK) + 1)
> +
> +/* PWM Count Register */
> +#define SUN8I_PWM_PCNTR(chan)		(0x68 + (chan) * 0x20)
> +
> +/* PWM Capture Control Register */
> +#define SUN8I_PWM_CCR(chan)		(0x6c + (chan) * 0x20)
> +
> +/* PWM Capture Rise Lock Register */
> +#define SUN8I_PWM_CRLR(chan)		(0x70 + (chan) * 0x20)
> +
> +/* PWM Capture Fall Lock Register */
> +#define SUN8I_PWM_CFLR(chan)		(0x74 + (chan) * 0x20)
> +
> +#define SUN8I_PWM_PAIR_IDX(chan)	((chan) >> 1)
> +
> +/*
> + * Block diagram of the PWM clock controller:
> + *
> + *             _____      ______      ________
> + * OSC24M --->|     |    |      |    |        |
> + * APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> SUN8I_PWM_clock_src_xy
> + *            |_____|    |______|    |________|
> + *                               ________
> + *                              |        |
> + *                           +->| /div_k |---> SUN8I_PWM_clock_x
> + *                           |  |________|
> + *                           |    ______
> + *                           |   |      |
> + *                           +-->| Gate |----> SUN8I_PWM_bypass_clock_x
> + *                           |   |______|
> + * SUN8I_PWM_clock_src_xy ---+   ________
> + *                           |  |        |
> + *                           +->| /div_k |---> SUN8I_PWM_clock_y
> + *                           |  |________|
> + *                           |    ______
> + *                           |   |      |
> + *                           +-->| Gate |----> SUN8I_PWM_bypass_clock_y
> + *                               |______|
> + *
> + * NB: when the bypass is set, all the PWM logic is bypassed.
> + * So, the duty cycle and polarity can't be modified (we just have a clock).
> + * The bypass in PWM mode is used to achieve a 1/2 relative duty cycle with the
> + * fastest clock.
> + *
> + * SUN8I_PWM_clock_x/y serve for the PWM purpose.
> + * SUN8I_PWM_bypass_clock_x/y serve for the clock-provider purpose.
> + *
> + */
> +
> +/*
> + * Table used for /div_m (diviser before obtaining SUN8I_PWM_clock_src_xy)
> + * It's actually CLK_DIVIDER_POWER_OF_TWO, but limited to /256
> + */
> +#define CLK_TABLE_DIV_M_ENTRY(i) { \
> +	.val = (i), .div = 1 << (i) \
> +}
> +
> +static const struct clk_div_table clk_table_div_m[] = {
> +	CLK_TABLE_DIV_M_ENTRY(0),
> +	CLK_TABLE_DIV_M_ENTRY(1),
> +	CLK_TABLE_DIV_M_ENTRY(2),
> +	CLK_TABLE_DIV_M_ENTRY(3),
> +	CLK_TABLE_DIV_M_ENTRY(4),
> +	CLK_TABLE_DIV_M_ENTRY(5),
> +	CLK_TABLE_DIV_M_ENTRY(6),
> +	CLK_TABLE_DIV_M_ENTRY(7),
> +	CLK_TABLE_DIV_M_ENTRY(8),
> +	{ /* sentinel */ }
> +};
> +
> +#define SUN8I_PWM_XY_SRC_GATE(_pair, _reg)		\
> +struct clk_gate gate_xy_##_pair = {			\

Could these be const?

> +	.reg = (void *)(_reg),				\
> +	.bit_idx = SUN8I_PWM_PCCR_GATE_BIT,		\
> +	.hw.init = &(struct clk_init_data){		\
> +		.ops = &clk_gate_ops,			\
> +	}						\
> +}
> +
> +#define SUN8I_PWM_XY_SRC_MUX(_pair, _reg)		\
> +struct clk_mux mux_xy_##_pair = {			\
> +	.reg = (void *)(_reg),				\
> +	.shift = SUN8I_PWM_PCCR_SRC_SHIFT,		\
> +	.mask = SUN8I_PWM_PCCR_SRC_MASK,		\
> +	.flags = CLK_MUX_ROUND_CLOSEST,			\
> +	.hw.init = &(struct clk_init_data){		\
> +		.ops = &clk_mux_ops,			\
> +	}						\
> +}
> +
> +#define SUN8I_PWM_XY_SRC_DIV(_pair, _reg)		\
> +struct clk_divider rate_xy_##_pair = {			\
> +	.reg = (void *)(_reg),				\
> +	.shift = SUN8I_PWM_PCCR_DIV_M_SHIFT,		\
> +	.table = clk_table_div_m,			\
> +	.hw.init = &(struct clk_init_data){		\
> +		.ops = &clk_divider_ops,		\
> +	}						\
> +}
> +
> +#define SUN8I_PWM_X_DIV(_idx, _reg)			\
> +struct clk_divider rate_x_##_idx = {			\
> +	.reg = (void *)(_reg),				\
> +	.shift = SUN8I_PWM_PCR_PRESCAL_K_SHIFT,		\
> +	.width = SUN8I_PWM_PCR_PRESCAL_K_WIDTH,		\
> +	.hw.init = &(struct clk_init_data){		\
> +		.ops = &clk_divider_ops,		\
> +	}						\
> +}
> +
> +#define SUN8I_PWM_X_BYPASS_GATE(_idx)			\
> +struct clk_gate gate_x_bypass_##_idx = {		\
> +	.reg = (void *)SUN8I_PWM_PER,			\
> +	.bit_idx = _idx,				\
> +	.hw.init = &(struct clk_init_data){		\
> +		.ops = &clk_gate_ops,			\
> +	}						\
> +}
> +
> +#define SUN8I_PWM_XY_CLK_SRC(_pair, _reg)			\
> +	static SUN8I_PWM_XY_SRC_MUX(_pair, _reg);		\
> +	static SUN8I_PWM_XY_SRC_GATE(_pair, _reg);		\
> +	static SUN8I_PWM_XY_SRC_DIV(_pair, _reg)
> +
> +#define SUN8I_PWM_X_CLK(_idx)					\
> +	static SUN8I_PWM_X_DIV(_idx, SUN8I_PWM_PCR(_idx))
> +
> +#define SUN8I_PWM_X_BYPASS_CLK(_idx)				\
> +	SUN8I_PWM_X_BYPASS_GATE(_idx)
> +
> +#define REF_CLK_XY_SRC(_pair)						\
> +	{								\
> +		.name = "pwm-clk-src" #_pair,				\
> +		.mux_hw = &mux_xy_##_pair.hw,				\
> +		.gate_hw = &gate_xy_##_pair.hw,				\
> +		.rate_hw = &rate_xy_##_pair.hw,				\

No hard requirement, but I'd like spaces around ##

> +	}
> +
> +#define REF_CLK_X(_idx, _pair)						\
> +	{								\
> +		.name = "pwm-clk" #_idx,				\
> +		.parent_names = (const char *[]){ "pwm-clk-src" #_pair }, \
> +		.num_parents = 1,					\
> +		.rate_hw = &rate_x_##_idx.hw,				\
> +		.flags = CLK_SET_RATE_PARENT,				\
> +	}
> +
> +#define REF_CLK_BYPASS(_idx, _pair)					\
> +	{								\
> +		.name = "pwm-clk-bypass" #_idx,				\
> +		.parent_names = (const char *[]){ "pwm-clk-src" #_pair }, \
> +		.num_parents = 1,					\
> +		.gate_hw = &gate_x_bypass_##_idx.hw,			\
> +		.flags = CLK_SET_RATE_PARENT,				\
> +	}
> +
> +/*
> + * SUN8I_PWM_clock_src_xy generation:
> + *             _____      ______      ________
> + * OSC24M --->|     |    |      |    |        |
> + * APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> SUN8I_PWM_clock_src_xy
> + *            |_____|    |______|    |________|
> + */
> +SUN8I_PWM_XY_CLK_SRC(01, SUN8I_PWM_PCCR(0));
> +SUN8I_PWM_XY_CLK_SRC(23, SUN8I_PWM_PCCR(1));
> +SUN8I_PWM_XY_CLK_SRC(45, SUN8I_PWM_PCCR(2));
> +
> +/*
> + * SUN8I_PWM_clock_x_div generation:
> + *                            ________
> + *                           |        | SUN8I_PWM_clock_x/y
> + * SUN8I_PWM_clock_src_xy -->| /div_k |--------------->
> + *                           |________|
> + */
> +SUN8I_PWM_X_CLK(0);
> +SUN8I_PWM_X_CLK(1);
> +SUN8I_PWM_X_CLK(2);
> +SUN8I_PWM_X_CLK(3);
> +SUN8I_PWM_X_CLK(4);
> +SUN8I_PWM_X_CLK(5);
> +
> +/*
> + * SUN8I_PWM_bypass_clock_xy generation:
> + *                             ______
> + *                            |      |
> + * SUN8I_PWM_clock_src_xy --->| Gate |-------> SUN8I_PWM_bypass_clock_x
> + *                            |______|
> + *
> + * The gate is actually SUN8I_PWM_PER register.
> + */
> +SUN8I_PWM_X_BYPASS_CLK(0);
> +SUN8I_PWM_X_BYPASS_CLK(1);
> +SUN8I_PWM_X_BYPASS_CLK(2);
> +SUN8I_PWM_X_BYPASS_CLK(3);
> +SUN8I_PWM_X_BYPASS_CLK(4);
> +SUN8I_PWM_X_BYPASS_CLK(5);
> +
> +struct clk_pwm_data {
> +	const char *name;
> +	const char **parent_names;
> +	unsigned int num_parents;
> +	struct clk_hw *mux_hw;
> +	struct clk_hw *rate_hw;
> +	struct clk_hw *gate_hw;
> +	unsigned long flags;
> +};
> +
> +/* Indexes of REF_CLK_BYPASS and REF_CLK_XY_SRC in the array */
> +#define CLK_BYPASS_IDX(sun8i_chip, chan) ((sun8i_chip)->data->npwm + (chan))
> +#define CLK_XY_SRC_IDX(sun8i_chip, chan) \
> +	((sun8i_chip)->data->npwm * 2 + SUN8I_PWM_PAIR_IDX(chan))
> +static struct clk_pwm_data pwmcc_data[] = {
> +	REF_CLK_X(0, 01),
> +	REF_CLK_X(1, 01),
> +	REF_CLK_X(2, 23),
> +	REF_CLK_X(3, 23),
> +	REF_CLK_X(4, 45),
> +	REF_CLK_X(5, 45),
> +	REF_CLK_BYPASS(0, 01),
> +	REF_CLK_BYPASS(1, 01),
> +	REF_CLK_BYPASS(2, 23),
> +	REF_CLK_BYPASS(3, 23),
> +	REF_CLK_BYPASS(4, 45),
> +	REF_CLK_BYPASS(5, 45),
> +	REF_CLK_XY_SRC(01),
> +	REF_CLK_XY_SRC(23),
> +	REF_CLK_XY_SRC(45),
> +	{ /* sentinel */ }
> +};

For the massive clk-specific part it would be great to get feedback from
someone knowledgeable about clk.

> +enum sun8i_pwm_mode {
> +	SUN8I_PWM_MODE_NONE,
> +	SUN8I_PWM_MODE_PWM,
> +	SUN8I_PWM_MODE_CLK,
> +};
> +
> +struct sun8i_pwm_data {
> +	unsigned int npwm;
> +};
> +
> +struct sun8i_pwm_channel {
> +	struct clk *pwm_clk;
> +	enum sun8i_pwm_mode mode;
> +};
> +
> +struct clk_pwm_pdata {
> +	struct clk_hw_onecell_data *hw_data;
> +	spinlock_t lock;
> +	void __iomem *reg;
> +};
> +
> +struct sun8i_pwm_chip {
> +	struct clk_pwm_pdata *clk_pdata;
> +	struct sun8i_pwm_channel *channels;
> +	struct clk *bus_clk;
> +	struct reset_control *rst;
> +	void __iomem *base;
> +	const struct sun8i_pwm_data *data;
> +};
> +
> +struct sun8i_pwm_waveform {
> +	u8 enabled:1;
> +	u8 active_state:1;
> +	u8 bypass_en:1;
> +	u16 duty_ticks;
> +	u32 period_ticks;
> +	unsigned long clk_rate;
> +};
> +
> +static inline struct sun8i_pwm_chip *sun8i_pwm_from_chip(const struct pwm_chip *chip)
> +{
> +	return pwmchip_get_drvdata(chip);
> +}
> +
> +static inline u32 sun8i_pwm_readl(struct sun8i_pwm_chip *sun8i_chip,
> +				  unsigned long offset)
> +{
> +	return readl(sun8i_chip->base + offset);
> +}
> +
> +static inline void sun8i_pwm_writel(struct sun8i_pwm_chip *sun8i_chip,
> +				    u32 val, unsigned long offset)
> +{
> +	writel(val, sun8i_chip->base + offset);
> +}
> +
> +static void sun8i_pwm_set_bypass(struct sun8i_pwm_chip *sun8i_chip,
> +				 unsigned int idx, bool en_bypass)
> +{
> +	unsigned long flags, reg_offset;
> +	u32 val;
> +
> +	spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
> +
> +	reg_offset = SUN8I_PWM_PCCR(SUN8I_PWM_PAIR_IDX(idx));
> +	val = sun8i_pwm_readl(sun8i_chip, reg_offset);
> +	if (en_bypass)
> +		val |= BIT(SUN8I_PWM_PCCR_BYPASS_BIT(idx));
> +	else
> +		val &= ~BIT(SUN8I_PWM_PCCR_BYPASS_BIT(idx));
> +
> +	sun8i_pwm_writel(sun8i_chip, val, reg_offset);
> +
> +	spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);

Use guard() here as you did in other functions?

> +}
> +
> +static int sun8i_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> +	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> +
> +	scoped_guard(spinlock_irqsave, &sun8i_chip->clk_pdata->lock) {
> +		if (chan->mode == SUN8I_PWM_MODE_CLK)
> +			return -EBUSY;
> +		chan->mode = SUN8I_PWM_MODE_PWM;
> +	}
> +
> +	return clk_prepare_enable(chan->pwm_clk);
> +}
> +
> +static void sun8i_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> +	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> +
> +	clk_disable_unprepare(chan->pwm_clk);
> +	chan->mode = SUN8I_PWM_MODE_NONE;
> +}
> +
> +static int sun8i_pwm_read_waveform(struct pwm_chip *chip,
> +				   struct pwm_device *pwm,
> +				   void *_wfhw)
> +{
> +	struct sun8i_pwm_waveform *wfhw = _wfhw;
> +	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> +	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> +	u32 val;
> +
> +	wfhw->clk_rate = clk_get_rate(chan->pwm_clk);

clk_get_rate() is a sleeping call. Unless I miss something this is the
only sleeping call and if you query the clk rate in .request() after
calling clk_rate_exclusive_get() (which you should do anyhow to prevent
changes to the running PWM) you can mark the chip atomic.

> +	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PER);
> +	wfhw->enabled = !!(SUN8I_PWM_ENABLE(pwm->hwpwm) & val);
> +
> +	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCCR(SUN8I_PWM_PAIR_IDX(pwm->hwpwm)));
> +	wfhw->bypass_en = !!(val & BIT(SUN8I_PWM_PCCR_BYPASS_BIT(pwm->hwpwm)));
> +
> +	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCR(pwm->hwpwm));
> +	wfhw->active_state = !!(val & SUN8I_PWM_PCR_ACTIVE_STATE);
> +
> +	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PPR(pwm->hwpwm));
> +	wfhw->duty_ticks = SUN8I_PWM_PPR_DUTY_VALUE(val);
> +	wfhw->period_ticks = SUN8I_PWM_PPR_PERIOD_VALUE(val);
> +
> +	dev_dbg(pwmchip_parent(chip),
> +		"pwm%u: %s, bypass: %s, polarity: %s, clk_rate=%lu period_ticks=%u duty_ticks=%u\n",
> +		pwm->hwpwm,
> +		wfhw->enabled ? "enabled" : "disabled",
> +		wfhw->bypass_en ? "enabled" : "disabled",
> +		wfhw->active_state ? "normal" : "inversed",
> +		wfhw->clk_rate, wfhw->period_ticks, wfhw->duty_ticks);
> +
> +	return 0;
> +}
> +
> +static int sun8i_pwm_round_waveform_fromhw(struct pwm_chip *chip,
> +					   struct pwm_device *pwm,
> +					   const void *_wfhw,
> +					   struct pwm_waveform *wf)
> +{
> +	const struct sun8i_pwm_waveform *wfhw = _wfhw;
> +	u64 tmp, resolution;
> +
> +	dev_dbg(pwmchip_parent(chip),
> +		"pwm%u: %s, bypass: %s, polarity: %s, clk_rate=%lu period_ticks=%u duty_ticks=%u\n",
> +		pwm->hwpwm,
> +		wfhw->enabled ? "enabled" : "disabled",
> +		wfhw->bypass_en ? "enabled" : "disabled",
> +		wfhw->active_state ? "normal" : "inversed",
> +		wfhw->clk_rate, wfhw->period_ticks, wfhw->duty_ticks);
> +
> +	wf->duty_offset_ns = 0;
> +
> +	if (!wfhw->enabled || !wfhw->clk_rate) {
> +		wf->period_length_ns = 0;
> +		wf->duty_length_ns = 0;
> +		return 0;
> +	}
> +
> +	if (wfhw->bypass_en) {
> +		wf->period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC,
> +							wfhw->clk_rate);
> +		wf->duty_length_ns = DIV_ROUND_UP_ULL(wf->period_length_ns, 2);

It would be great to have a dev_dbg covering this branch, too.

> +		return 0;
> +	}
> +
> +	tmp = NSEC_PER_SEC * (u64)wfhw->period_ticks;
> +	wf->period_length_ns = DIV_ROUND_UP_ULL(tmp, wfhw->clk_rate);
> +
> +	tmp = NSEC_PER_SEC * (u64)wfhw->duty_ticks;
> +	wf->duty_length_ns = DIV_ROUND_UP_ULL(tmp, wfhw->clk_rate);
> +	if (!wfhw->active_state) {
> +		/*
> +		 * For inverted polarity, we have to fix cases where
> +		 * computed duty_length_ns > requested duty_length_ns
> +		 * For that, we subtract the actual resolution of the PWM
> +		 * registers
> +		 */
> +		wf->duty_offset_ns = wf->duty_length_ns;
> +		wf->duty_length_ns = wf->period_length_ns - wf->duty_length_ns;
> +
> +		resolution = DIV_ROUND_UP_ULL(NSEC_PER_SEC, wfhw->clk_rate);
> +
> +		if (wf->duty_offset_ns >= resolution)
> +			wf->duty_offset_ns -= resolution;

This is broken. Consider clk_rate = 1234567, period_ticks = 8000 and
duty_ticks = 7000 Then you calculate:

	period_length_ns = 6480005
	duty_length_ns = 6480005 - 5670005 = 810000

but the actual duty_length is 810000.5913004316 and thus 810001 should
be reported. Look at how the stm32 driver does it, that applies here,
too.

> +	}
> +
> +	dev_dbg(pwmchip_parent(chip),
> +		"pwm%u period_length_ns=%llu duty_length_ns=%llu duty_offset_ns=%llu\n",
> +		pwm->hwpwm, wf->period_length_ns, wf->duty_length_ns,
> +		wf->duty_offset_ns);

A single dev_dbg() mentioning both the hardware specfic parts and *wf
please. Also use

	"... %lld/%lld [+%lld] ...", ...,
	wf->duty_length_ns, wf->period_length_ns, wf->duty_offset_ns,

to emit *wf.

> +
> +	return 0;
> +}
> +
> +static int sun8i_pwm_write_waveform(struct pwm_chip *chip,
> +				    struct pwm_device *pwm, const void *_wfhw)
> +{
> +	const struct sun8i_pwm_waveform *wfhw = _wfhw;
> +	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> +	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> +	unsigned long flags;
> +	u32 val;
> +	int ret;
> +
> +	ret = clk_set_rate(chan->pwm_clk, wfhw->clk_rate);
> +	if (ret)
> +		return ret;

You must check that this resulted indeed in setting the rate to exactly
wfhw->clk_rate.

Also IIUC this might affect channel pwm->hwpwm ^ 1, so there needs to be
some guard to not modify that if it's running.

> +	sun8i_pwm_set_bypass(sun8i_chip, pwm->hwpwm, wfhw->bypass_en);
> +
> +	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PCR(pwm->hwpwm));
> +	if (wfhw->active_state)
> +		val |= SUN8I_PWM_PCR_ACTIVE_STATE;
> +	else
> +		val &= ~SUN8I_PWM_PCR_ACTIVE_STATE;
> +	sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PCR(pwm->hwpwm));
> +
> +	val = SUN8I_PWM_DUTY(wfhw->duty_ticks);
> +	val |= SUN8I_PWM_PPR_PERIOD(wfhw->period_ticks);
> +	sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PPR(pwm->hwpwm));
> +
> +	spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
> +
> +	val = sun8i_pwm_readl(sun8i_chip, SUN8I_PWM_PER);
> +	if (wfhw->enabled)
> +		val |= SUN8I_PWM_ENABLE(pwm->hwpwm);
> +	else
> +		val &= ~SUN8I_PWM_ENABLE(pwm->hwpwm);
> +	sun8i_pwm_writel(sun8i_chip, val, SUN8I_PWM_PER);
> +
> +	spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int sun8i_pwm_round_waveform_tohw(struct pwm_chip *chip,
> +					 struct pwm_device *pwm,
> +					 const struct pwm_waveform *wf,
> +					 void *_wfhw)
> +{
> +	struct sun8i_pwm_chip *sun8i_chip = sun8i_pwm_from_chip(chip);
> +	struct sun8i_pwm_channel *chan = &sun8i_chip->channels[pwm->hwpwm];
> +	struct sun8i_pwm_waveform *wfhw = _wfhw;
> +	unsigned long max_rate;
> +	long calc_rate;
> +	u64 period_ratio, double_duty_ratio, freq, duty_cycle;
> +
> +	dev_dbg(pwmchip_parent(chip),
> +		"pwm%u period_length_ns=%llu duty_length_ns=%llu duty_offset_ns=%llu\n",
> +		pwm->hwpwm, wf->period_length_ns, wf->duty_length_ns,
> +		wf->duty_offset_ns);
> +
> +	if (wf->period_length_ns == 0) {
> +		wfhw->enabled = 0;
> +		return 0;
> +	}
> +
> +	wfhw->enabled = 1;
> +
> +	duty_cycle = wf->duty_length_ns;
> +	if (wf->duty_length_ns + wf->duty_offset_ns < wf->period_length_ns)
> +		wfhw->active_state = 1;
> +	else
> +		wfhw->active_state = 0;

That's wrong. The decision about polarity must be done in the tick
domain. See commit 5d087c485b6ecf200a9ebb2a032bf8571d330250 for a
similar issue in the stm32 driver.

> +	dev_dbg(pwmchip_parent(chip), "polarity: %s\n",
> +		wfhw->active_state ? "normal" : "inversed");
> +
> +	/*
> +	 * Lowest possible period case:
> +	 * Without bypass, the lowest possible period is when:
> +	 * duty cycle = 1 and period cycle = 2 (0x10001 in period register)

period cycle = 1 doesn't work?

> +	 * E.g. if the input clock is 100MHz, we have a lowest period of 20ns.
> +	 * Now, with the bypass, the period register is ignored and we directly
> +	 * have the 100MHz clock as PWM output, that can act as a 10ns period
> +	 * with 5ns duty.
> +	 * So, to detect this lowest period case, just get the maximum possible
> +	 * rate from chan->pwm_clk and compare it with requested period and
> +	 * duty_cycle.
> +	 *
> +	 * But, to get the maximum possible rate, we have to use U32_MAX instead
> +	 * of (unsigned long)-1.
> +	 * This is because clk_round_rate() uses ultimately DIV_ROUND_UP_ULL()
> +	 * that in turn do_div(n,base). And base is uint32_t divisor.

Sounds like a bug in the clk subsystem.

> +	 */
> +	max_rate = clk_round_rate(chan->pwm_clk, U32_MAX);
> +
> +	dev_dbg(pwmchip_parent(chip), "max_rate: %ld Hz\n", max_rate);
> +
> +	period_ratio = mul_u64_u64_div_u64(wf->period_length_ns,
> +					   max_rate, NSEC_PER_SEC);

This might overflow for max_rate > 1 GHz.

> +	double_duty_ratio = mul_u64_u64_div_u64(duty_cycle, (u64)max_rate * 2,
> +						NSEC_PER_SEC);
> +	if (period_ratio == 1) {
> +		if (double_duty_ratio == 0)
> +			/* requested period and duty are too small */
> +			return -EINVAL;

period_ratio == 0 is ok? Also I fail to see the relevance of
double_duty_ratio, so this likely needs a comment.

Also you're supposed to pick the smallest period if the requested period
is too small.

> +		/*
> +		 * If the requested period is to small to be generated by the
> +		 * PWM, but matches the highest clock with a
> +		 * duty_cycle >= period*2, just bypass the PWM logic
> +		 */
> +		freq = div64_u64(NSEC_PER_SEC, wf->period_length_ns);
> +		wfhw->bypass_en = true;
> +	} else {
> +		wfhw->bypass_en = false;
> +		freq = div64_u64(NSEC_PER_SEC * (u64)SUN8I_PWM_PPR_PERIOD_MAX,
> +				 wf->period_length_ns);
> +		/*
> +		 * Same remark as above, this is to prevent a value to big for
> +		 * clk_round_rate() to handle
> +		 */
> +		if (freq > U32_MAX)
> +			freq = U32_MAX;
> +	}
> +
> +	dev_dbg(pwmchip_parent(chip), "bypass: %s\n",
> +		wfhw->bypass_en ? "enabled" : "disabled");
> +
> +	calc_rate = clk_round_rate(chan->pwm_clk, freq);

It's a pity that there are no guarantees in the clk subsystem about how
clk_round_rate() works. If you ask for say freq=1000000 and the clock
can only do 950000 or 1050000, it's unspecified which of these
clk_round_rate() returns. And you might want to check the one that
wasn't returned as it might give better approximations. (And note it
depends on the purpose which of these is better. freq might be an upper
bound or a lower bound, and depending on the usecase you want to
minimise abs(freq - calc_rate) or abs(1 - freq/calc_rate) (or abs(1 -
calc_rate/freq)?), so a single function with the prototype of
clk_round_rate() cannot cover all use cases.)

In general it's really hard to choose the optimal parent rate, even if
the clk framework gave you an easy way to explore the possibilities.
So it's unclear that NSEC_PER_SEC * SUN8I_PWM_PPR_PERIOD_MAX /
wf->period_length_ns is a good canditate at all.

So I think it's impossible to get the tohw callback correct without
further guarantees by the clk framework (or implicit knowledge about
chan->pwm_clk) and even then it's hard.

> +	if (calc_rate <= 0)
> +		return calc_rate ? calc_rate : -EINVAL;

calc_rate ?: -EINVAL;

> +
> +	dev_dbg(pwmchip_parent(chip), "calc_rate: %ld Hz\n", calc_rate);
> +
> +	wfhw->period_ticks = mul_u64_u64_div_u64(calc_rate,
> +						 wf->period_length_ns,
> +						 NSEC_PER_SEC);
> +	if (wfhw->period_ticks > SUN8I_PWM_PPR_PERIOD_MAX)
> +		wfhw->period_ticks = SUN8I_PWM_PPR_PERIOD_MAX;
> +
> +	/* min value in period register is 1 */
> +	if (wfhw->period_ticks == 0)
> +		return -EINVAL;

As above, configure the smallest possible period in this case.

> +	wfhw->duty_ticks = mul_u64_u64_div_u64(calc_rate, duty_cycle,
> +					       NSEC_PER_SEC);
> +
> +	if (wfhw->duty_ticks > wfhw->period_ticks)
> +		wfhw->duty_ticks = wfhw->period_ticks;
> +
> +	if (!wfhw->active_state)
> +		wfhw->duty_ticks = wfhw->period_ticks - wfhw->duty_ticks;
> +
> +	dev_dbg(pwmchip_parent(chip),
> +		"pwm%u period_ticks=%u duty_cycle=%llu duty_ticks=%u\n",
> +		pwm->hwpwm, wfhw->period_ticks, duty_cycle, wfhw->duty_ticks);
> +
> +	wfhw->clk_rate = calc_rate;
> +
> +	return 0;
> +}
> +
> +static const struct pwm_ops sun8i_pwm_ops = {
> +	.request = sun8i_pwm_request,
> +	.free = sun8i_pwm_free,
> +	.sizeof_wfhw = sizeof(struct sun8i_pwm_waveform),
> +	.round_waveform_tohw = sun8i_pwm_round_waveform_tohw,
> +	.round_waveform_fromhw = sun8i_pwm_round_waveform_fromhw,
> +	.read_waveform = sun8i_pwm_read_waveform,
> +	.write_waveform = sun8i_pwm_write_waveform,
> +};
> +
> +static struct clk_hw *sun8i_pwm_of_clk_get(struct of_phandle_args *clkspec,
> +					   void *data)
> +{
> +	struct sun8i_pwm_chip *sun8i_chip = data;
> +	struct clk_hw_onecell_data *hw_data = sun8i_chip->clk_pdata->hw_data;
> +	unsigned int idx = clkspec->args[0];
> +	struct sun8i_pwm_channel *chan;
> +	struct clk_hw *ret_clk = NULL;
> +	unsigned long flags;
> +
> +	if (idx >= sun8i_chip->data->npwm)
> +		return ERR_PTR(-EINVAL);
> +
> +	chan = &sun8i_chip->channels[idx];
> +
> +	spin_lock_irqsave(&sun8i_chip->clk_pdata->lock, flags);
> +
> +	if (chan->mode == SUN8I_PWM_MODE_PWM) {
> +		ret_clk = ERR_PTR(-EBUSY);
> +	} else {
> +		chan->mode = SUN8I_PWM_MODE_CLK;
> +		ret_clk = hw_data->hws[CLK_BYPASS_IDX(sun8i_chip, idx)];
> +	}
> +	spin_unlock_irqrestore(&sun8i_chip->clk_pdata->lock, flags);
> +
> +	if (IS_ERR(ret_clk))
> +		goto out;
> +
> +	sun8i_pwm_set_bypass(sun8i_chip, idx, true);
> +out:

If you switch to guard you can drop the goto.

> +	return ret_clk;
> +}
> +
> +static int sun8i_add_composite_clk(struct clk_pwm_data *data,
> +				   void __iomem *reg, spinlock_t *lock,
> +				   struct device *dev, struct clk_hw **hw)
> +{
> +	const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, *rate_ops = NULL;
> +	struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *rate_hw = NULL;
> +	struct device_node *node = dev->of_node;
> +
> +	if (data->mux_hw) {
> +		struct clk_mux *mux;
> +
> +		mux_hw = data->mux_hw;
> +		mux = to_clk_mux(mux_hw);
> +		mux->lock = lock;
> +		mux_ops = mux_hw->init->ops;
> +		mux->reg = (uintptr_t)mux->reg + reg;
> +	}
> +
> +	if (data->gate_hw) {
> +		struct clk_gate *gate;
> +
> +		gate_hw = data->gate_hw;
> +		gate = to_clk_gate(gate_hw);
> +		gate->lock = lock;
> +		gate_ops = gate_hw->init->ops;
> +		gate->reg = (uintptr_t)gate->reg + reg;
> +	}
> +
> +	if (data->rate_hw) {
> +		struct clk_divider *rate;
> +
> +		rate_hw = data->rate_hw;
> +		rate = to_clk_divider(rate_hw);
> +		rate_ops = rate_hw->init->ops;
> +		rate->lock = lock;
> +		rate->reg = (uintptr_t)rate->reg + reg;
> +
> +		if (rate->table) {
> +			const struct clk_div_table *clkt;
> +			int table_size = 0;
> +
> +			for (clkt = rate->table; clkt->div; clkt++)
> +				table_size++;
> +			rate->width = order_base_2(table_size);
> +		}
> +	}
> +
> +	/*
> +	 * Retrieve the parent clock names from DTS for pwm-clk-srcxy
> +	 */
> +	if (!data->parent_names) {
> +		data->num_parents = of_clk_get_parent_count(node);
> +		if (data->num_parents == 0)
> +			return -ENOENT;
> +
> +		data->parent_names = devm_kzalloc(dev,
> +						  sizeof(*data->parent_names),
> +						  GFP_KERNEL);
> +		for (unsigned int i = 0; i < data->num_parents; i++)
> +			data->parent_names[i] = of_clk_get_parent_name(node, i);
> +	}
> +
> +	*hw = clk_hw_register_composite(dev, data->name, data->parent_names,
> +					data->num_parents, mux_hw,
> +					mux_ops, rate_hw, rate_ops,
> +					gate_hw, gate_ops, data->flags);
> +
> +	return PTR_ERR_OR_ZERO(*hw);
> +}
> +
> +static int sun8i_pwm_init_clocks(struct platform_device *pdev,
> +				 struct sun8i_pwm_chip *sun8i_chip)
> +{
> +	struct clk_pwm_pdata *pdata;
> +	struct device *dev = &pdev->dev;
> +	int num_clocks = 0;
> +	int ret;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return dev_err_probe(dev, -ENOMEM,
> +				     "Failed to allocate clk_pwm_pdata\n");
> +
> +	while (pwmcc_data[num_clocks].name)
> +		num_clocks++;
> +
> +	pdata->hw_data = devm_kzalloc(dev, struct_size(pdata->hw_data, hws, num_clocks),
> +				      GFP_KERNEL);
> +	if (!pdata->hw_data)
> +		return dev_err_probe(dev, -ENOMEM,
> +				     "Failed to allocate hw clocks\n");
> +
> +	pdata->hw_data->num = num_clocks;
> +	pdata->reg = sun8i_chip->base;
> +
> +	spin_lock_init(&pdata->lock);
> +
> +	for (int i = 0; i < num_clocks; i++) {
> +		struct clk_hw **hw = &pdata->hw_data->hws[i];
> +
> +		ret = sun8i_add_composite_clk(&pwmcc_data[i], pdata->reg,
> +					      &pdata->lock, dev, hw);
> +		if (ret) {
> +			dev_err_probe(dev, ret,
> +				      "Failed to register hw clock %s\n",
> +				      pwmcc_data[i].name);
> +			for (i--; i >= 0; i--)
> +				clk_hw_unregister_composite(pdata->hw_data->hws[i]);
> +			return ret;
> +		}
> +	}
> +
> +	sun8i_chip->clk_pdata = pdata;
> +
> +	return 0;
> +}
> +
> +static void sun8i_pwm_unregister_clk(void *data)
> +{
> +	struct clk_hw_onecell_data *hw_data = data;
> +
> +	for (unsigned int i = 0; i < hw_data->num; i++)
> +		clk_hw_unregister_composite(hw_data->hws[i]);
> +}
> +
> +static int sun8i_pwm_probe(struct platform_device *pdev)
> +{
> +	const struct sun8i_pwm_data *data;
> +	struct device *dev = &pdev->dev;
> +	struct sun8i_pwm_chip *sun8i_chip;
> +	struct pwm_chip *chip;
> +	int ret;
> +
> +	data = of_device_get_match_data(dev);
> +	if (!data)
> +		return dev_err_probe(dev, -ENODEV,
> +				     "Missing specific data structure\n");
> +
> +	chip = devm_pwmchip_alloc(dev, data->npwm, sizeof(*sun8i_chip));
> +	if (IS_ERR(chip))
> +		return dev_err_probe(dev, PTR_ERR(chip),
> +				     "Failed to allocate pwmchip\n");
> +
> +	sun8i_chip = sun8i_pwm_from_chip(chip);
> +	sun8i_chip->data = data;
> +	sun8i_chip->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(sun8i_chip->base))
> +		return dev_err_probe(dev, PTR_ERR(sun8i_chip->base),
> +				     "Failed to get PWM base address\n");
> +
> +	sun8i_chip->bus_clk = devm_clk_get_enabled(dev, "bus");
> +	if (IS_ERR(sun8i_chip->bus_clk))
> +		return dev_err_probe(dev, PTR_ERR(sun8i_chip->bus_clk),
> +				     "Failed to get bus clock\n");
> +
> +	sun8i_chip->channels = devm_kmalloc_array(dev, data->npwm,
> +						  sizeof(*(sun8i_chip->channels)),
> +						  GFP_KERNEL);
> +	if (!sun8i_chip->channels)
> +		return dev_err_probe(dev, -ENOMEM,
> +				     "Failed to allocate %d channels array\n",
> +				     data->npwm);

Can you use a flexible array for ->channels and allocate it together
with sun8i_chip to reduce memory fragmentation?

> +	chip->ops = &sun8i_pwm_ops;
> +
> +	ret = sun8i_pwm_init_clocks(pdev, sun8i_chip);
> +	if (ret)
> +		return ret;
> +
> +	for (unsigned int i = 0; i < data->npwm; i++) {
> +		struct sun8i_pwm_channel *chan = &sun8i_chip->channels[i];
> +		struct clk_hw **hw = &sun8i_chip->clk_pdata->hw_data->hws[i];
> +
> +		chan->pwm_clk = devm_clk_hw_get_clk(dev, *hw, NULL);
> +		if (IS_ERR(chan->pwm_clk)) {
> +			ret = dev_err_probe(dev, PTR_ERR(chan->pwm_clk),
> +					    "Failed to register PWM clock %d\n", i);
> +			return ret;
> +		}
> +		chan->mode = SUN8I_PWM_MODE_NONE;
> +	}
> +
> +	ret = devm_of_clk_add_hw_provider(dev, sun8i_pwm_of_clk_get, sun8i_chip);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add HW clock provider\n");
> +
> +	ret = devm_add_action_or_reset(dev, sun8i_pwm_unregister_clk,
> +				       sun8i_chip->clk_pdata->hw_data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add devm action\n");

This undoes sun8i_pwm_init_clocks(), right? So this should be moved to
directly after the call to that function, otherwise it's skipped on the
error returns between the two calls?

> +	/* Deassert reset */
> +	sun8i_chip->rst = devm_reset_control_get_shared_deasserted(dev, NULL);
> +	if (IS_ERR(sun8i_chip->rst))
> +		return dev_err_probe(dev, PTR_ERR(sun8i_chip->rst),
> +				     "Failed to get reset control\n");

Does this reset affect the clks registered above and thus should be
deasserted before?

> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to add PWM chip\n");
> +
> +	platform_set_drvdata(pdev, chip);

This is unused I think?

> +	return 0;
> +}

I didn't look at Sashiko's feedback
(https://lore.kernel.org/all/20260703153337.B5AFF1F000E9@smtp.kernel.org)
but I would expect that not all 9 high-severity issues identified are
slop. Please look through these for the next revision.

Best regards
Uwe

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

[PATCH v7 3/4] arm64: dts: allwinner: h616: add PWM controller

[Richard Genoud]

The H616 has a PWM controller that can provide PWM signals, but also
plain clocks.

Add the PWM controller node and pins in the device tree.

Tested-by: John Stultz <jstultz@google.com>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 .../arm64/boot/dts/allwinner/sun50i-h616.dtsi | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
index 1598e86259ab..90fe56a1d3cd 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
@@ -281,6 +281,17 @@ watchdog: watchdog@30090a0 {
 			clocks = <&osc24M>;
 		};
 
+		pwm: pwm@300a000 {
+			compatible = "allwinner,sun50i-h616-pwm";
+			reg = <0x0300a000 0x400>;
+			clocks = <&osc24M>, <&ccu CLK_BUS_PWM>;
+			clock-names = "mod", "bus";
+			resets = <&ccu RST_BUS_PWM>;
+			#pwm-cells = <3>;
+			#clock-cells = <1>;
+			status = "disabled";
+		};
+
 		pio: pinctrl@300b000 {
 			compatible = "allwinner,sun50i-h616-pinctrl";
 			reg = <0x0300b000 0x400>;
@@ -385,6 +396,42 @@ nand_rb1_pin: nand-rb1-pin {
 				bias-pull-up;
 			};
 
+			/omit-if-no-ref/
+			pwm0_pin: pwm0-pin {
+				pins = "PD28";
+				function = "pwm0";
+			};
+
+			/omit-if-no-ref/
+			pwm1_pin: pwm1-pin {
+				pins = "PG19";
+				function = "pwm1";
+			};
+
+			/omit-if-no-ref/
+			pwm2_pin: pwm2-pin {
+				pins = "PH2";
+				function = "pwm2";
+			};
+
+			/omit-if-no-ref/
+			pwm3_pin: pwm3-pin {
+				pins = "PH0";
+				function = "pwm3";
+			};
+
+			/omit-if-no-ref/
+			pwm4_pin: pwm4-pin {
+				pins = "PI14";
+				function = "pwm4";
+			};
+
+			/omit-if-no-ref/
+			pwm5_pin: pwm5-pin {
+				pins = "PA12";
+				function = "pwm5";
+			};
+
 			/omit-if-no-ref/
 			spi0_pins: spi0-pins {
 				pins = "PC0", "PC2", "PC4";
-- 
2.47.3

[PATCH v7 4/4] MAINTAINERS: Add entry on Allwinner sun8i/H616 PWM driver

[Richard Genoud]

Add myself as maintainer of Allwinner SUN8I PWM driver.

Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 15011f5752a9..51ca159bcaf1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -919,6 +919,11 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/hwlock/allwinner,sun6i-a31-hwspinlock.yaml
 F:	drivers/hwspinlock/sun6i_hwspinlock.c
 
+ALLWINNER SUN8I PWM DRIVER
+M:	Richard Genoud <richard.genoud@bootlin.com>
+S:	Maintained
+F:	drivers/pwm/pwm-sun8i.c
+
 ALLWINNER THERMAL DRIVER
 M:	Vasily Khoruzhick <anarsoul@gmail.com>
 M:	Yangtao Li <tiny.windzz@gmail.com>
-- 
2.47.3