Re: [PATCH v14 3/5] pwm: dwc: add of/platform support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dwc_pwm_runtime_resume() writes the EN bit and timer configuration simultaneously, violating the hardware specification.
--

--- Patch [3]: [PATCH v14 3/5] pwm: dwc: add of/platform support ---
commit b441231de2fcc89e4187b672bde1919622ac7347
Author: Xuyang Dong <dongxuyang@eswincomputing.com>

pwm: dwc: add of/platform support

This commit adds platform and OF device tree support for the DesignWare PWM
controller, enabling its use in non-PCI systems. It also implements polarity
control using dual load registers and adds runtime PM callbacks to save and
restore hardware state.

> diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
> new file mode 100644
> index 0000000000000..e96e5569eb81e
> --- /dev/null
> +++ b/drivers/pwm/pwm-dwc-of.c
[ ... ]
> +static int dwc_pwm_runtime_resume(struct device *dev)
> +{
[ ... ]
> +	for (idx = 0; idx < chip->npwm; idx++) {
> +		dwc_pwm_writel(dwc, dwc->ctx[idx].cnt, DWC_TIM_LD_CNT(idx));
> +		dwc_pwm_writel(dwc, dwc->ctx[idx].cnt2, DWC_TIM_LD_CNT2(idx));
> +		dwc_pwm_writel(dwc, dwc->ctx[idx].ctrl, DWC_TIM_CTRL(idx));

[Severity: Medium]
Does writing the saved control register directly in dwc_pwm_runtime_resume()
violate the hardware initialization sequence?

The driver notes in __dwc_pwm_configure_timer() that the specification
requires disabling the timer, programming it, and then enabling it.

Since ctx[idx].ctrl may have the DWC_TIM_CTRL_EN bit set from when the device
was active, does this single write attempt to configure and enable the timer
simultaneously? Could this cause unpredictable behavior upon resume?

> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730111425.1680-1-dongxuyang@eswincomputing.com?part=3