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

Mon May 25 14:15:26 PDT 2026

On Thu, 16 Apr 2026 15:40:33 +0200
Richard Genoud <richard.genoud at bootlin.com> wrote:

Hi Richard,

> 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
> 
> 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.

Didn't we discuss this choice of "highest clock" before? I dimly
remember asking whether we cannot use a least-error approach, so
considering all clocks and choosing the one which matches the target
best?

> 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.

Thanks for your research into this and summarising this up! I see your
point, and always found the choice to use nanoseconds in PWM somewhat
problematic, especially when looking at the rounding errors.
And while turning the PWM into a clock provider looks like a clever
solution, this is somewhat arbitrary - why do we have this for those
SoCs and not the other (older) ones? The 24 MHz != 1/42ns problem is
the same there.
What also bugs me a bit is also that this is actually a decision
affecting the generic hardware devicetree description of the system,
but its rooted in a Linux implementation detail (ns resolution periods).

So before we consider going this route:
- Can we change the internal interface in Linux, maybe introducing
  some special interface from pwm-clock to the pwm drivers, to convey
  frequencies directly, instead of period lengths?
- Can we add an optional interface for pwm drivers in general, to use
  a frequency / duty-cycle pair to describe a PWM setup? I would naively
  think those to be some kind of natural PWM parameters.
- Can we at least add a picosecond precision interface? That doesn't
  solve the rounding issue for those number not only divisible by
  2 or 5 (like 24), but at least it seems to mitigate it:
  24 MHz => 41666 ps => 24.000384 MHz

I see that having a clock provider seems like a more sustainable and
fitting choice, but as mentioned it's something that affects the DT
description, so I don't want to change that lightly.

Cheers,
Andre

> And actually, this PWM is really a PWM AND a real clock when the bypass
> is set.
> 
> This series is based onto v7.0
> 
> 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 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
> 
> 
> base-commit: 028ef9c96e96197026887c0f092424679298aae8
> 