Re: [PATCH v12 3/4] pwm: dwc: add of/platform support

[Philipp Zabel]

On Di, 2026-07-21 at 20:09 +0800, dongxuyang@xxxxxxxxxxxxxxxxxx wrote:
> From: Xuyang Dong <dongxuyang@xxxxxxxxxxxxxxxxxx>
> 
> The dwc pwm controller can be used in non-PCI systems, so allow
> either platform or OF based probing.
> 
> The controller is reset only when no PWM channel is enabled.
> Otherwise, clocks are enabled and the runtime PM state is updated
> to reflect the active hardware configuration.
> 
> The DWC PWM controller does not provide a hardware polarity bit.
> Currently, the driver only supports active-low output, which is
> incompatible with devices requiring active-high waveforms (e.g.,
> backlight controllers, fan speed regulators).
> 
> Implement polarity control by exploiting the timer's dual load
> registers. The hardware uses:
> - LD_CNT:  LOW period count
> - LD_CNT2: HIGH period count
> 
> The total period is defined as (LD_CNT + LD_CNT2). By swapping the
> duty cycle between these registers, we invert the polarity while
> keeping the period unchanged:
> - PWM_POLARITY_NORMAL:  write duty_cycle to LD_CNT2 (HIGH period)
> - PWM_POLARITY_INVERSED: write duty_cycle to LD_CNT  (LOW period)
> 
> Implementation:
> Update both apply() and get_state() to handle state->polarity
> consistently. Since the hardware does not store polarity, get_state()
> returns the last successfully applied software state, ensuring that
> read-back matches what was originally set.
> 
> Co-developed-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
> Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
> Signed-off-by: Xiang Xu <xuxiang@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Guosheng Wang <wangguosheng@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Xuyang Dong <dongxuyang@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/pwm/Kconfig        |  10 ++
>  drivers/pwm/Makefile       |   1 +
>  drivers/pwm/pwm-dwc-core.c | 172 ++++++++++++++++----
>  drivers/pwm/pwm-dwc-of.c   | 312 +++++++++++++++++++++++++++++++++++++
>  drivers/pwm/pwm-dwc.h      |  25 ++-
>  5 files changed, 481 insertions(+), 39 deletions(-)
>  create mode 100644 drivers/pwm/pwm-dwc-of.c
> 
[...]
> diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
> new file mode 100644
> index 000000000000..460863549274
> --- /dev/null
> +++ b/drivers/pwm/pwm-dwc-of.c
> @@ -0,0 +1,312 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DesignWare PWM Controller driver OF
> + *
> + * Copyright (C) 2026 SiFive, Inc.
> + */
> +
> +#define DEFAULT_SYMBOL_NAMESPACE "dwc_pwm_of"
> +
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pwm.h>
> +#include <linux/reset.h>
> +
> +#include "pwm-dwc.h"
> +
> +struct dwc_pwm_plat_data {
> +	bool reset_required;

This is not needed.

> +};
> +
> +static int dwc_pwm_plat_probe(struct platform_device *pdev)
> +{
[...]
> +	pdata = device_get_match_data(dev);
> +	if (pdata && pdata->reset_required)
> +		dwc->rst = devm_reset_control_get_exclusive(dev, NULL);
> +	else
> +		dwc->rst = devm_reset_control_array_get_optional_exclusive(dev);

Please drop reset_required from pdata and just use

	dwc->rst = devm_reset_control_array_get_optional_exclusive(dev);

That works for a single reset as well.

Device tree validation already makes sure required reset controls are
present. There is no need handle this in the driver as a special case.

> +
> +	if (IS_ERR(dwc->rst))
> +		return dev_err_probe(dev, PTR_ERR(dwc->rst),
> +				     "failed to get reset control\n");
> +
> +	ret = clk_prepare_enable(dwc->bus_clk);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to enable bus clock\n");
> +
> +	ret = clk_prepare_enable(dwc->clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable timer clock\n");

		dev_err_probe(dev, "failed to enable timer clock\n");

prints the actual error. Same for other dev_err()s in the probe
function.

regards
Philipp