[PATCH v8 0/3] Update designware pwm driver

[PATCH v8 0/3] Update designware pwm driver

[dongxuyang]

From: Xuyang Dong <dongxuyang@eswincomputing.com>

There is already a patch [1] for the DesignWare PWM driver,
which is posted by Ben and still under review.
Based on this patch, this series is a continuation of [1]
to add support for IP versions 2.11a and later, which
includes support for "Pulse Width Modulation with 0%
and 100% Duty Cycle".

Supported chips:
ESWIN EIC7700 series SoC.

Test:
Tested this patch on the Sifive HiFive Premier P550 (which uses the EIC7700
SoC).

[1] https://lore.kernel.org/lkml/20230907161242.67190-1-ben.dooks@codethink.co.uk/

Updates:
  Changes in v8:
  - YAML:
    - Split the v7 binding into two patches.
      Patch 1 explains why to add the resets property.
      Patch 2 adds the eswin compatible string and specified reset.
  - Driver:
    - Use mul_u64_u64_div_u64() to safely scale the values and avoid
      64-bit multiplication overflow in __dwc_pwm_configure_timer()
      and dwc_pwm_get_state().
      Add the include for linux/math64.h (Sashiko review of v7).
    - Keep the current usage of pwm->args.polarity until a better solution
      is available.
    - Use pm_runtime_resume_and_get() in dwc_pwm_get_state() instead of
      pm_runtime_get_sync(), so that register access is skipped if the
      device fails to resume (Sashiko review of v7).
    - Replace devm_pwmchip_add() with pwmchip_add() and move it after
      pm_runtime_enable(), so that the PWM chip is registered only after
      runtime PM has been fully initialized (Sashiko review of v7).
    - Remove the reset_assert label and reset_control_assert()
      (Sashiko review of v7).
    - Remove the pm_runtime_status_suspended() check and unconditionally
      use pm_runtime_get_sync() instead (Sashiko review of v7).
    - Remove  the pwm_en flag, but keep the pm_runtime_put_noidle() call
      (Sashiko review of v7; see email for explanation)
    - Use pm_runtime_status_suspended() to check the runtime PM status.
      If the device is not suspended (i.e., active), call
      clk_disable_unprepare(). If it is suspended, skip this block
      (Sashiko review of v7).
    - Use an explicit pwmchip_remove() as the first step of .remove(),
      instead of relying on devm_pwmchip_add() to unregister the chip
      after .remove() returns. This prevents the hardware teardown that
      follows from racing against a still-registered chip
      (Sashiko review of v7).
    - Add a check for dwc->rst before asserting reset in the remove path
      (Sashiko review of v7).
    - Drop the return value check from pm_runtime_put_sync()
      (Sashiko review of v7).

  - Link to v7: https://lore.kernel.org/all/20260605082242.1541-1-dongxuyang@eswincomputing.com/

  Changes in v7:
  - YAML:
    - Dropped Conor's Acked-by due to significant schema changes.
    - Rename patch 1 from "dt-bindings: pwm: dwc: add optional reset" to
      "dt-bindings: pwm: dwc: Add eswin compatible and resets property".
    - Update the commit message to explain why the EIC7700 supports only
      one reset.
    - Add constraints 'minItems: 1' and 'maxItems: 1' for the 'resets'
      property of eswin,eic7700-pwm.
    - Add an example for eswin,eic7700-pwm.

  - Link to v6: https://lore.kernel.org/all/20260424094529.1691-1-dongxuyang@eswincomputing.com/

  Changes in v6:
  - YAML:
    - Drop properties resets and its items description for eswin,eic7700-pwm.

  - Link to v5: https://lore.kernel.org/all/20260423083644.1168-1-dongxuyang@eswincomputing.com/

  Changes in v5:
  - YAML:
    - Add 'eswin,eic7700-pwm' compatible string.
    - Add the items description for the resets property and set minItems to 1.
    - Require resets property with exactly 1 reset for eswin,eic7700-pwm compatible.
  - Driver:
    - Add support for 'eswin,eic7700-pwm' compatible.
    - Add structure dwc_pwm_plat_data to manage the API for obtaining resets.

  - Link to v4: https://lore.kernel.org/all/20260415094908.1539-1-dongxuyang@eswincomputing.com/

  Changes in v4:
  - YAML:
    - Change maxItems from 1 to 2. As there is a corresponding reset signal
      for each clock domain, the effective maxItems of the resets property
      is set to 2.
    - Update the YAML commit message to describe the hardware.
  - Driver:
    - Replace devm_reset_control_get_optional_exclusive() with
      devm_reset_control_array_get_optional_exclusive(). Since the number
      of reset signals has increased from one to two, we need to use the
      array API to acquire them.

  - Link to v3: https://lore.kernel.org/all/20260402091718.1608-1-dongxuyang@eswincomputing.com/

  Changes in v3:
  - YAML:
    - Added a clear justification for the optional resets property. It is
      required to support proper controller initialization when no PWM
      channel is active at boot time, while allowing the driver to skip
      reset deassertion if any channel is already enabled.
  - Driver:
    - Update the boundary value check of tmp in __dwc_pwm_configure_timer()
      for DWC_TIM_CTRL_0N100PWM_EN.
    - Replace 'sizeof(struct dwc_pwm_drvdata)' with
      'struct_size(data, chips, 1)'.
    - Drop devm_clk_get_enabled() in favor of devm_clk_get() with explicit
      clk_prepare_enable() and clk_disable_unprepare() allowing runtime PM
      to manage clock state.
    - Replace devm_reset_control_get_optional_exclusive_deasserted() with
      devm_reset_control_get_optional_exclusive() and issue a full reset via
      reset_control_reset() only when no PWM channel is active at probe time.
    - Detect bootloader-enabled PWM channels by reading the enable bit, and
      initialize runtime PM as active for those channels by calling
      pm_runtime_set_active() and pm_runtime_get_noresume().
    - Remove autosuspend as it is not required for this driver.
    - Use explicit pm_runtime_enable() and pm_runtime_disable() instead of
      the managed devm_pm_runtime_enable() variant to ensure correct cleanup.
    - On device removal, recheck the channel enable status. If any channel
      remains active, call pm_runtime_put_noidle() before disabling clocks
      via clk_disable_unprepare().
      Resume device before register access during removal if it is runtime
      suspended, and re-suspend it afterward.
    - If device is suspended, resume it before register access during system
      resume/suspend.
    - Use pm_ptr() instead of pm_sleep_ptr() for correct PM operation.

  - Link to v2: https://lore.kernel.org/all/20260306093000.2065-1-dongxuyang@eswincomputing.com/

  Changes in v2:
  - YAML:
    - Remove eswin,eic7700-pwm.yaml. Use snps,dw-apb-timers-pwm2.yaml.
      The description in snps,dw-apb-timers-pwm2.yaml is better.
    - Add the resets property as optional, as defined in the databook.
    - Remove snps,pwm-full-range-enable as no additional property is needed.
  - Driver:
    - Change the file from pwm-dwc-eic7700.c to pwm-dwc-of.c from [1].
    - Define DWC_TIM_VERSION_ID_2_11A 2.11a as the baseline version.
    - Enable the 0% and 100% duty cycle mode by setting dwc->feature if
      the version read from the TIMERS_COMP_VERSION register is later
      than or equal to DWC_TIM_VERSION_ID_2_11A.
    - Use the DIV_ROUND_UP_ULL() to calculate width in the .apply and
      .get_state.
    - Additionally, Power Management (PM) support has been added to the
      pwm-dwc-of.c driver.
    - Drop the headers that are not used.
    - Use devm_clk_get_enabled() instead of devm_clk_get().
    - Drop of_match_ptr.
    - Fix build error with 1ULL << 32.
      Reported-by: kernel test robot <lkp@intel.com>
      Closes: https://lore.kernel.org/oe-kbuild-all/202512061720.j31AsgM7-lkp@intel.com/

  - Link to v1: https://lore.kernel.org/all/20251205090411.1388-1-dongxuyang@eswincomputing.com/
  - Link to v9: https://lore.kernel.org/lkml/20230907161242.67190-1-ben.dooks@codethink.co.uk/

Xuyang Dong (3):
  dt-bindings: pwm: dwc: Document optional resets property
  dt-bindings: pwm: dwc: Add eswin compatible
  pwm: dwc: add of/platform support

 .../bindings/pwm/snps,dw-apb-timers-pwm2.yaml |  37 +-
 drivers/pwm/Kconfig                           |  10 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-dwc-core.c                    | 103 ++++--
 drivers/pwm/pwm-dwc-of.c                      | 346 ++++++++++++++++++
 drivers/pwm/pwm-dwc.h                         |  25 +-
 6 files changed, 488 insertions(+), 34 deletions(-)
 create mode 100644 drivers/pwm/pwm-dwc-of.c

--
2.34.1

[PATCH v8 1/3] dt-bindings: pwm: dwc: Document optional resets property

[dongxuyang]

From: Xuyang Dong <dongxuyang@eswincomputing.com>

The DesignWare PWM IP has two active-low reset inputs: presetn resets
the register interface logic in the pclk (bus) domain, and
timer_N_resetn resets the counter/timer logic in the timer_N_clk
domain. The existing snps,dw-apb-timers-pwm2 binding does not
describe either of these lines.

Add the resets property and describe the function of each reset to
support future use of resets.

Signed-off-by: Xuyang Dong <dongxuyang@eswincomputing.com>
---
 .../devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml     | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
index 7523a89a1773..213fdaef25d9 100644
--- a/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
+++ b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
@@ -43,6 +43,11 @@ properties:
       - const: bus
       - const: timer
 
+  resets:
+    items:
+      - description: Interface bus (presetn) reset
+      - description: PWM timer logic (timer_N_resetn) reset
+
   snps,pwm-number:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: The number of PWM channels configured for this instance
-- 
2.34.1

[PATCH v8 2/3] dt-bindings: pwm: dwc: Add eswin compatible

[dongxuyang]

From: Xuyang Dong <dongxuyang@eswincomputing.com>

EIC7700 integrates the DesignWare PWM IP described by the generic
snps,dw-apb-timers-pwm2 binding. On this SoC, the presetn and
timer_N_resetn inputs are physically tied together to a single reset
line, so exactly one reset is both required and sufficient, unlike
the generic binding where up to two independent lines are optional.

Add the eswin,eic7700-pwm compatible string and constrain its resets
property to exactly one entry.

Signed-off-by: Xuyang Dong <dongxuyang@eswincomputing.com>
---
 .../bindings/pwm/snps,dw-apb-timers-pwm2.yaml | 32 ++++++++++++++++---
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
index 213fdaef25d9..9a9a24ddc61b 100644
--- a/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
+++ b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
@@ -20,12 +20,11 @@ description:
   instead of having to encode the IP version number in the device tree
   compatible.
 
-allOf:
-  - $ref: pwm.yaml#
-
 properties:
   compatible:
-    const: snps,dw-apb-timers-pwm2
+    enum:
+      - snps,dw-apb-timers-pwm2
+      - eswin,eic7700-pwm
 
   reg:
     maxItems: 1
@@ -44,6 +43,7 @@ properties:
       - const: timer
 
   resets:
+    minItems: 1
     items:
       - description: Interface bus (presetn) reset
       - description: PWM timer logic (timer_N_resetn) reset
@@ -59,6 +59,21 @@ required:
   - clocks
   - clock-names
 
+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: eswin,eic7700-pwm
+    then:
+      properties:
+        resets:
+          maxItems: 1
+      required:
+        - resets
+
 additionalProperties: false
 
 examples:
@@ -70,3 +85,12 @@ examples:
       clocks = <&bus>, <&timer>;
       clock-names = "bus", "timer";
     };
+  - |
+    pwm@50818000 {
+      compatible = "eswin,eic7700-pwm";
+      reg = <0x50818000 0x4000>;
+      #pwm-cells = <3>;
+      clocks = <&bus>, <&timer>;
+      clock-names = "bus", "timer";
+      resets = <&reset>;
+    };
-- 
2.34.1

[PATCH v8 3/3] pwm: dwc: add of/platform support

[dongxuyang]

From: Xuyang Dong <dongxuyang@eswincomputing.com>

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.

Co-developed-by: Ben Dooks <ben.dooks@codethink.co.uk>
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Signed-off-by: Xiang Xu <xuxiang@eswincomputing.com>
Signed-off-by: Guosheng Wang <wangguosheng@eswincomputing.com>
Signed-off-by: Xuyang Dong <dongxuyang@eswincomputing.com>
---
 drivers/pwm/Kconfig        |  10 ++
 drivers/pwm/Makefile       |   1 +
 drivers/pwm/pwm-dwc-core.c | 103 ++++++++---
 drivers/pwm/pwm-dwc-of.c   | 346 +++++++++++++++++++++++++++++++++++++
 drivers/pwm/pwm-dwc.h      |  25 ++-
 5 files changed, 455 insertions(+), 30 deletions(-)
 create mode 100644 drivers/pwm/pwm-dwc-of.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index e8886a9b64d9..fd1d68beab67 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -249,6 +249,16 @@ config PWM_DWC
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-dwc.
 
+config PWM_DWC_OF
+	tristate "DesignWare PWM Controller (OF bus)"
+	depends on HAS_IOMEM && (OF || COMPILE_TEST)
+	select PWM_DWC_CORE
+	help
+	  PWM driver for Synopsys DWC PWM Controller on an OF bus or
+	  a platform bus.
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-dwc-of.
+
 config PWM_EP93XX
 	tristate "Cirrus Logic EP93xx PWM support"
 	depends on ARCH_EP93XX || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 5630a521a7cf..acd7dfe98dff 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_PWM_CRC)		+= pwm-crc.o
 obj-$(CONFIG_PWM_CROS_EC)	+= pwm-cros-ec.o
 obj-$(CONFIG_PWM_DWC_CORE)	+= pwm-dwc-core.o
 obj-$(CONFIG_PWM_DWC)		+= pwm-dwc.o
+obj-$(CONFIG_PWM_DWC_OF)	+= pwm-dwc-of.o
 obj-$(CONFIG_PWM_EP93XX)	+= pwm-ep93xx.o
 obj-$(CONFIG_PWM_FSL_FTM)	+= pwm-fsl-ftm.o
 obj-$(CONFIG_PWM_GPIO)		+= pwm-gpio.o
diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
index 6dabec93a3c6..b9a9d6ae0b45 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -12,8 +12,10 @@
 #define DEFAULT_SYMBOL_NAMESPACE "dwc_pwm"
 
 #include <linux/bitops.h>
+#include <linux/clk.h>
 #include <linux/export.h>
 #include <linux/kernel.h>
+#include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>
@@ -44,21 +46,55 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
 	u32 high;
 	u32 low;
 
-	/*
-	 * Calculate width of low and high period in terms of input clock
-	 * periods and check are the result within HW limits between 1 and
-	 * 2^32 periods.
-	 */
-	tmp = DIV_ROUND_CLOSEST_ULL(state->duty_cycle, dwc->clk_ns);
-	if (tmp < 1 || tmp > (1ULL << 32))
-		return -ERANGE;
-	low = tmp - 1;
-
-	tmp = DIV_ROUND_CLOSEST_ULL(state->period - state->duty_cycle,
-				    dwc->clk_ns);
-	if (tmp < 1 || tmp > (1ULL << 32))
-		return -ERANGE;
-	high = tmp - 1;
+	if (dwc->clk)
+		dwc->clk_rate = clk_get_rate(dwc->clk);
+
+	if (dwc->features & DWC_TIM_CTRL_0N100PWM_EN) {
+		/*
+		 * Calculate width of low and high period in terms of input
+		 * clock periods and check are the result within HW limits
+		 * between 0 and 2^32 periods.
+		 * Use mul_u64_u64_div_u64() to avoid overflowing the 64-bit
+		 * intermediate result and to round down to the nearest
+		 * achievable hardware value, as required by the PWM core.
+		 */
+		tmp = mul_u64_u64_div_u64(state->duty_cycle, dwc->clk_rate,
+					  NSEC_PER_SEC);
+		if (tmp >= (1ULL << 32))
+			return -ERANGE;
+
+		if (pwm->args.polarity == PWM_POLARITY_INVERSED)
+			high = tmp;
+		else
+			low = tmp;
+
+		tmp = mul_u64_u64_div_u64(state->period - state->duty_cycle,
+					  dwc->clk_rate, NSEC_PER_SEC);
+		if (tmp >= (1ULL << 32))
+			return -ERANGE;
+
+		if (pwm->args.polarity == PWM_POLARITY_INVERSED)
+			low = tmp;
+		else
+			high = tmp;
+	} else {
+		/*
+		 * Calculate width of low and high period in terms of input
+		 * clock periods and check are the result within HW limits
+		 * between 1 and 2^32 periods.
+		 */
+		tmp = mul_u64_u64_div_u64(state->duty_cycle, dwc->clk_rate,
+					  NSEC_PER_SEC);
+		if (tmp < 1 || tmp > (1ULL << 32))
+			return -ERANGE;
+		low = tmp - 1;
+
+		tmp = mul_u64_u64_div_u64(state->period - state->duty_cycle,
+					  dwc->clk_rate, NSEC_PER_SEC);
+		if (tmp < 1 || tmp > (1ULL << 32))
+			return -ERANGE;
+		high = tmp - 1;
+	}
 
 	/*
 	 * Specification says timer usage flow is to disable timer, then
@@ -74,6 +110,7 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
 	 * width of low period and latter the width of high period in terms
 	 * multiple of input clock periods:
 	 * Width = ((Count + 1) * input clock period).
+	 * Width = (Count * input clock period) : supported 0% and 100%.
 	 */
 	dwc_pwm_writel(dwc, low, DWC_TIM_LD_CNT(pwm->hwpwm));
 	dwc_pwm_writel(dwc, high, DWC_TIM_LD_CNT2(pwm->hwpwm));
@@ -85,6 +122,9 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
 	 * periods are set by Load Count registers.
 	 */
 	ctrl = DWC_TIM_CTRL_MODE_USER | DWC_TIM_CTRL_PWM;
+	if (dwc->features & DWC_TIM_CTRL_0N100PWM_EN)
+		ctrl |= DWC_TIM_CTRL_0N100PWM_EN;
+
 	dwc_pwm_writel(dwc, ctrl, DWC_TIM_CTRL(pwm->hwpwm));
 
 	/*
@@ -121,10 +161,19 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 			     struct pwm_state *state)
 {
 	struct dwc_pwm *dwc = to_dwc_pwm(chip);
+	unsigned long clk_rate;
 	u64 duty, period;
 	u32 ctrl, ld, ld2;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(pwmchip_parent(chip));
+	if (ret)
+		return ret;
 
-	pm_runtime_get_sync(pwmchip_parent(chip));
+	if (dwc->clk)
+		dwc->clk_rate = clk_get_rate(dwc->clk);
+
+	clk_rate = dwc->clk_rate;
 
 	ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm->hwpwm));
 	ld = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
@@ -137,17 +186,25 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * based on the timer load-count only.
 	 */
 	if (ctrl & DWC_TIM_CTRL_PWM) {
-		duty = (ld + 1) * dwc->clk_ns;
-		period = (ld2 + 1)  * dwc->clk_ns;
-		period += duty;
+		if (dwc->features & DWC_TIM_CTRL_0N100PWM_EN) {
+			if (pwm->args.polarity == PWM_POLARITY_INVERSED)
+				duty = ld2;
+			else
+				duty = ld;
+			period = (u64)ld + ld2;
+		} else {
+			duty = ld + 1;
+			period = ld2 + 1;
+			period += duty;
+		}
 	} else {
-		duty = (ld + 1) * dwc->clk_ns;
+		duty = ld + 1;
 		period = duty * 2;
 	}
 
 	state->polarity = PWM_POLARITY_INVERSED;
-	state->period = period;
-	state->duty_cycle = duty;
+	state->period = mul_u64_u64_div_u64(period, NSEC_PER_SEC, clk_rate);
+	state->duty_cycle = mul_u64_u64_div_u64(duty, NSEC_PER_SEC, clk_rate);
 
 	pm_runtime_put_sync(pwmchip_parent(chip));
 
@@ -169,7 +226,7 @@ struct pwm_chip *dwc_pwm_alloc(struct device *dev)
 		return chip;
 	dwc = to_dwc_pwm(chip);
 
-	dwc->clk_ns = 10;
+	dwc->clk_rate = NSEC_PER_SEC / 10;
 	chip->ops = &dwc_pwm_ops;
 
 	return chip;
diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
new file mode 100644
index 000000000000..34713cb7299e
--- /dev/null
+++ b/drivers/pwm/pwm-dwc-of.c
@@ -0,0 +1,346 @@
+// 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;
+};
+
+static int dwc_pwm_plat_probe(struct platform_device *pdev)
+{
+	const struct dwc_pwm_plat_data *pdata;
+	struct device *dev = &pdev->dev;
+	struct dwc_pwm_drvdata *data;
+	u32 ctrl[DWC_TIMERS_TOTAL];
+	struct pwm_chip *chip;
+	struct dwc_pwm *dwc;
+	bool pwm_en = false;
+	u32 nr_pwm, tim_id;
+	unsigned int i;
+	int ret;
+
+	data = devm_kzalloc(dev, struct_size(data, chips, 1), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	chip = dwc_pwm_alloc(dev);
+	if (IS_ERR(chip))
+		return dev_err_probe(dev, PTR_ERR(chip),
+				     "failed to alloc pwm\n");
+
+	dwc = to_dwc_pwm(chip);
+
+	dwc->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(dwc->base))
+		return PTR_ERR(dwc->base);
+
+	if (!device_property_read_u32(dev, "snps,pwm-number", &nr_pwm)) {
+		if (nr_pwm > DWC_TIMERS_TOTAL)
+			dev_warn(dev, "too many PWMs (%d), capping at %d\n",
+				 nr_pwm, chip->npwm);
+		else
+			chip->npwm = nr_pwm;
+	}
+
+	dwc->bus_clk = devm_clk_get(dev, "bus");
+	if (IS_ERR(dwc->bus_clk))
+		return dev_err_probe(dev, PTR_ERR(dwc->bus_clk),
+				     "failed to get bus clock\n");
+
+	dwc->clk = devm_clk_get(dev, "timer");
+	if (IS_ERR(dwc->clk))
+		return dev_err_probe(dev, PTR_ERR(dwc->clk),
+				     "failed to get timer clock\n");
+
+	ret = devm_clk_rate_exclusive_get(dev, dwc->clk);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to get exclusive rate\n");
+
+	dwc->clk_rate = clk_get_rate(dwc->clk);
+
+	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);
+
+	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");
+		goto disable_busclk;
+	}
+
+	/*
+	 * Check all channels to see if any channel is enabled.
+	 * Read the control register of each channel and extract the enable bit
+	 */
+	for (i = 0; i < chip->npwm; i++) {
+		ctrl[i] = dwc_pwm_readl(dwc, DWC_TIM_CTRL(i)) & DWC_TIM_CTRL_EN;
+		if (ctrl[i])
+			pwm_en = true;
+	}
+
+	/*
+	 * Only issue a reset pulse when all channels are disabled, so a PWM
+	 * channel already running (e.g. configured by firmware before Linux
+	 * took over) is left undisturbed.
+	 */
+	if (!pwm_en) {
+		ret = reset_control_reset(dwc->rst);
+		if (ret) {
+			dev_err(dev, "failed to reset\n");
+			goto disable_clk;
+		}
+	}
+
+	/* init PWM feature */
+	dwc->features = 0;
+	/*
+	 * Support for 0% and 100% duty cycle mode was added in version 2.11a
+	 * and later.
+	 */
+	tim_id = dwc_pwm_readl(dwc, DWC_TIMERS_COMP_VERSION);
+	if (tim_id >= DWC_TIM_VERSION_ID_2_11A)
+		dwc->features |= DWC_TIM_CTRL_0N100PWM_EN;
+
+	data->chips[0] = chip;
+	dev_set_drvdata(dev, data);
+
+	/*
+	 * If any PWM channel is enabled, mark device active and hold runtime PM
+	 * references for each enabled channel. Otherwise, gate the clocks.
+	 */
+	if (pwm_en) {
+		pm_runtime_set_active(dev);
+		for (i = 0; i < chip->npwm; i++) {
+			if (ctrl[i])
+				pm_runtime_get_noresume(dev);
+		}
+	} else {
+		clk_disable_unprepare(dwc->clk);
+		clk_disable_unprepare(dwc->bus_clk);
+	}
+
+	pm_runtime_enable(dev);
+
+	ret = pwmchip_add(chip);
+	if (ret) {
+		dev_err(dev, "failed to add pwm chip\n");
+		goto pm_disable;
+	}
+
+	return 0;
+
+pm_disable:
+	pm_runtime_disable(dev);
+	if (pwm_en) {
+		for (i = 0; i < chip->npwm; i++) {
+			if (ctrl[i])
+				pm_runtime_put_noidle(dev);
+		}
+	}
+disable_clk:
+	clk_disable_unprepare(dwc->clk);
+disable_busclk:
+	clk_disable_unprepare(dwc->bus_clk);
+
+	return ret;
+}
+
+static void dwc_pwm_plat_remove(struct platform_device *pdev)
+{
+	struct dwc_pwm_drvdata *data = platform_get_drvdata(pdev);
+	struct pwm_chip *chip = data->chips[0];
+	struct dwc_pwm *dwc = to_dwc_pwm(chip);
+	unsigned int idx;
+	int ret;
+
+	pm_runtime_get_sync(&pdev->dev);
+	pwmchip_remove(chip);
+
+	for (idx = 0; idx < chip->npwm; idx++) {
+		if (dwc_pwm_readl(dwc, DWC_TIM_CTRL(idx)) & DWC_TIM_CTRL_EN)
+			pm_runtime_put_noidle(&pdev->dev);
+	}
+	pm_runtime_put_sync(&pdev->dev);
+
+	if (!pm_runtime_status_suspended(&pdev->dev)) {
+		clk_disable_unprepare(dwc->clk);
+		clk_disable_unprepare(dwc->bus_clk);
+	}
+	pm_runtime_disable(&pdev->dev);
+
+	if (dwc->rst) {
+		ret = reset_control_assert(dwc->rst);
+		if (ret)
+			dev_warn(&pdev->dev, "failed to assert reset: %d\n",
+				 ret);
+	}
+}
+
+static int dwc_pwm_runtime_suspend(struct device *dev)
+{
+	struct dwc_pwm_drvdata *data = dev_get_drvdata(dev);
+	struct pwm_chip *chip = data->chips[0];
+	struct dwc_pwm *dwc = to_dwc_pwm(chip);
+
+	clk_disable_unprepare(dwc->clk);
+	clk_disable_unprepare(dwc->bus_clk);
+
+	return 0;
+}
+
+static int dwc_pwm_runtime_resume(struct device *dev)
+{
+	struct dwc_pwm_drvdata *data = dev_get_drvdata(dev);
+	struct pwm_chip *chip = data->chips[0];
+	struct dwc_pwm *dwc = to_dwc_pwm(chip);
+	int ret;
+
+	ret = clk_prepare_enable(dwc->bus_clk);
+	if (ret) {
+		dev_err(dev, "failed to enable bus clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(dwc->clk);
+	if (ret) {
+		dev_err(dev, "failed to enable timer clock: %d\n", ret);
+		clk_disable_unprepare(dwc->bus_clk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dwc_pwm_suspend(struct device *dev)
+{
+	struct dwc_pwm_drvdata *data = dev_get_drvdata(dev);
+	struct pwm_chip *chip = data->chips[0];
+	struct dwc_pwm *dwc = to_dwc_pwm(chip);
+	unsigned int idx;
+	int ret;
+
+	if (pm_runtime_status_suspended(dev)) {
+		ret = dwc_pwm_runtime_resume(dev);
+		if (ret)
+			return ret;
+	}
+
+	for (idx = 0; idx < chip->npwm; idx++) {
+		if (chip->pwms[idx].state.enabled)
+			return -EBUSY;
+
+		dwc->ctx[idx].cnt = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(idx));
+		dwc->ctx[idx].cnt2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(idx));
+		dwc->ctx[idx].ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(idx));
+	}
+
+	ret = dwc_pwm_runtime_suspend(dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int dwc_pwm_resume(struct device *dev)
+{
+	struct dwc_pwm_drvdata *data = dev_get_drvdata(dev);
+	struct pwm_chip *chip = data->chips[0];
+	struct dwc_pwm *dwc = to_dwc_pwm(chip);
+	unsigned int idx;
+	bool pm_flags;
+	int ret;
+
+	/* Check if device was runtime suspended before system resume */
+	pm_flags = pm_runtime_status_suspended(dev);
+	if (pm_flags) {
+		/*
+		 * Use PM framework to resume device
+		 * (calls dwc_pwm_runtime_resume)
+		 */
+		ret = pm_runtime_get_sync(dev);
+		if (ret < 0)
+			return ret;
+	} else {
+		/*
+		 * Device was active, but clocks might be off after system sleep
+		 * Call runtime_resume directly to restore hardware state
+		 */
+		ret = dwc_pwm_runtime_resume(dev);
+		if (ret)
+			return ret;
+	}
+
+	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));
+	}
+
+	if (pm_flags) {
+		/*
+		 * Balance the refcount taken by pm_runtime_get_sync
+		 * if it was used
+		 */
+		pm_runtime_put_sync(dev);
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops dwc_pwm_pm_ops = {
+	RUNTIME_PM_OPS(dwc_pwm_runtime_suspend, dwc_pwm_runtime_resume, NULL)
+	SYSTEM_SLEEP_PM_OPS(dwc_pwm_suspend, dwc_pwm_resume)
+};
+
+static const struct dwc_pwm_plat_data pwm_eic7700_pdata = {
+	.reset_required = true,
+};
+
+static const struct of_device_id dwc_pwm_dt_ids[] = {
+	{ .compatible = "snps,dw-apb-timers-pwm2" },
+	{ .compatible = "eswin,eic7700-pwm", .data = &pwm_eic7700_pdata },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dwc_pwm_dt_ids);
+
+static struct platform_driver dwc_pwm_plat_driver = {
+	.driver = {
+		.name = "dwc-pwm",
+		.pm = pm_ptr(&dwc_pwm_pm_ops),
+		.of_match_table = dwc_pwm_dt_ids,
+	},
+	.probe = dwc_pwm_plat_probe,
+	.remove = dwc_pwm_plat_remove,
+};
+
+module_platform_driver(dwc_pwm_plat_driver);
+
+MODULE_ALIAS("platform:dwc-pwm-of");
+MODULE_AUTHOR("Ben Dooks <ben.dooks@codethink.co.uk>");
+MODULE_DESCRIPTION("DesignWare PWM Controller");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
index 1562594e7f85..75f7c2d031c4 100644
--- a/drivers/pwm/pwm-dwc.h
+++ b/drivers/pwm/pwm-dwc.h
@@ -26,12 +26,19 @@ MODULE_IMPORT_NS("dwc_pwm");
 #define DWC_TIMERS_TOTAL	8
 
 /* Timer Control Register */
-#define DWC_TIM_CTRL_EN		BIT(0)
-#define DWC_TIM_CTRL_MODE	BIT(1)
-#define DWC_TIM_CTRL_MODE_FREE	(0 << 1)
-#define DWC_TIM_CTRL_MODE_USER	(1 << 1)
-#define DWC_TIM_CTRL_INT_MASK	BIT(2)
-#define DWC_TIM_CTRL_PWM	BIT(3)
+#define DWC_TIM_CTRL_EN			BIT(0)
+#define DWC_TIM_CTRL_MODE		BIT(1)
+#define DWC_TIM_CTRL_MODE_FREE		(0 << 1)
+#define DWC_TIM_CTRL_MODE_USER		BIT(1)
+#define DWC_TIM_CTRL_INT_MASK		BIT(2)
+#define DWC_TIM_CTRL_PWM		BIT(3)
+#define DWC_TIM_CTRL_0N100PWM_EN	BIT(4)
+
+/*
+ * The version 2.11a and later add "Pulse Width Modulation with
+ * 0% and 100% Duty Cycle".
+ */
+#define DWC_TIM_VERSION_ID_2_11A	0x3231312a
 
 struct dwc_pwm_info {
 	unsigned int nr;
@@ -52,8 +59,12 @@ struct dwc_pwm_ctx {
 
 struct dwc_pwm {
 	void __iomem *base;
-	unsigned int clk_ns;
+	struct clk *bus_clk;
+	struct clk *clk;
+	unsigned long clk_rate;
+	struct reset_control *rst;
 	struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL];
+	u32 features;
 };
 
 static inline struct dwc_pwm *to_dwc_pwm(struct pwm_chip *chip)
-- 
2.34.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [High] Incorrect PWM polarity mapping and violation of the atomic PWM API by using pwm->args instead of state for 0N100PWM-capable hardware.
- [High] System suspend unconditionally fails if any PWM channel is enabled, breaking system-wide sleep transitions.
- [High] PM runtime usage counter leak in dwc_pwm_resume when system resume fails.
- [High] The function dwc_pwm_get_state() divides by the clock rate without verifying if it is zero, leading to a kernel panic.
- [High] The driver ignores the return value of pm_runtime_get_sync() in dwc_pwm_plat_remove(), leading to potential bus faults.
- [Medium] The error handling path in dwc_pwm_plat_probe() double-disables the device clocks if no PWM channels are initially enabled.

Pre-existing issues:
- [High] Unchecked pm_runtime_get_sync failure and leaked reference in dwc_pwm_apply.
--

commit 061974fbba26ee19760c596a4145184402b90275
Author: Xuyang Dong <dongxuyang@eswincomputing.com>

pwm: dwc: add of/platform support

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.

> diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
> index 6dabec93a3c64..b9a9d6ae0b45b 100644
> --- a/drivers/pwm/pwm-dwc-core.c
> +++ b/drivers/pwm/pwm-dwc-core.c
> @@ -44,21 +46,55 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,

[ ... ]

> +	if (dwc->features & DWC_TIM_CTRL_0N100PWM_EN) {
> +		/*
> +		 * Calculate width of low and high period in terms of input
> +		 * clock periods and check are the result within HW limits
> +		 * between 0 and 2^32 periods.
> +		 * Use mul_u64_u64_div_u64() to avoid overflowing the 64-bit
> +		 * intermediate result and to round down to the nearest
> +		 * achievable hardware value, as required by the PWM core.
> +		 */
> +		tmp = mul_u64_u64_div_u64(state->duty_cycle, dwc->clk_rate,
> +					  NSEC_PER_SEC);
> +		if (tmp >= (1ULL << 32))
> +			return -ERANGE;
> +
> +		if (pwm->args.polarity == PWM_POLARITY_INVERSED)
> +			high = tmp;
> +		else
> +			low = tmp;

[Severity: High]
Is it correct to evaluate the static default pwm->args.polarity here
instead of state->polarity?

If the args polarity is INVERSED, this logic maps the duty_cycle to the
HIGH period. This causes the signal to be active-high, directly violating
the requested INVERSED state from the PWM core.

[ ... ]

> @@ -121,10 +161,19 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>  			     struct pwm_state *state)
>  {
>  	struct dwc_pwm *dwc = to_dwc_pwm(chip);
> +	unsigned long clk_rate;
>  	u64 duty, period;
>  	u32 ctrl, ld, ld2;
> +	int ret;
> +
> +	ret = pm_runtime_resume_and_get(pwmchip_parent(chip));
> +	if (ret)
> +		return ret;

[Severity: High]
This is a pre-existing issue, but while looking at the runtime PM handling
here, I noticed that dwc_pwm_apply() (not modified by this patch) calls
pm_runtime_get_sync(pwmchip_parent(chip)) without checking its return value.

If resuming the parent device fails, dwc_pwm_apply() proceeds to call
__dwc_pwm_configure_timer(), which attempts to access hardware registers
while clocks might be disabled, potentially leading to a synchronous external
abort. It also permanently leaks the PM usage reference on failure. Should
dwc_pwm_apply() also be updated to check for failure?

> -	pm_runtime_get_sync(pwmchip_parent(chip));
> +	if (dwc->clk)
> +		dwc->clk_rate = clk_get_rate(dwc->clk);
> +
> +	clk_rate = dwc->clk_rate;

[ ... ]

>  	if (ctrl & DWC_TIM_CTRL_PWM) {
> -		duty = (ld + 1) * dwc->clk_ns;
> -		period = (ld2 + 1)  * dwc->clk_ns;
> -		period += duty;
> +		if (dwc->features & DWC_TIM_CTRL_0N100PWM_EN) {
> +			if (pwm->args.polarity == PWM_POLARITY_INVERSED)
> +				duty = ld2;

[Severity: High]
Similar to the issue above in __dwc_pwm_configure_timer(), should this
check state->polarity instead of pwm->args.polarity to correctly report
the active duty cycle?

> +			else
> +				duty = ld;
> +			period = (u64)ld + ld2;
> +		} else {
> +			duty = ld + 1;
> +			period = ld2 + 1;
> +			period += duty;
> +		}
>  	} else {
> -		duty = (ld + 1) * dwc->clk_ns;
> +		duty = ld + 1;
>  		period = duty * 2;
>  	}
>  
>  	state->polarity = PWM_POLARITY_INVERSED;
> -	state->period = period;
> -	state->duty_cycle = duty;
> +	state->period = mul_u64_u64_div_u64(period, NSEC_PER_SEC, clk_rate);
> +	state->duty_cycle = mul_u64_u64_div_u64(duty, NSEC_PER_SEC, clk_rate);

[Severity: High]
Could clk_rate be zero here if the device tree provides an uninitialized or
dummy clock?

dwc->clk_rate is initialized using clk_get_rate() during probe without
verifying if it is non-zero. If it is 0, these divisions will cause a kernel
panic when the PWM core calls get_state() during pwmchip_add().

> diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
> new file mode 100644
> index 0000000000000..34713cb7299e6
> --- /dev/null
> +++ b/drivers/pwm/pwm-dwc-of.c

[ ... ]

> +	/*
> +	 * If any PWM channel is enabled, mark device active and hold runtime PM
> +	 * references for each enabled channel. Otherwise, gate the clocks.
> +	 */
> +	if (pwm_en) {
> +		pm_runtime_set_active(dev);
> +		for (i = 0; i < chip->npwm; i++) {
> +			if (ctrl[i])
> +				pm_runtime_get_noresume(dev);
> +		}
> +	} else {
> +		clk_disable_unprepare(dwc->clk);
> +		clk_disable_unprepare(dwc->bus_clk);
> +	}
> +
> +	pm_runtime_enable(dev);
> +
> +	ret = pwmchip_add(chip);
> +	if (ret) {
> +		dev_err(dev, "failed to add pwm chip\n");
> +		goto pm_disable;
> +	}
> +
> +	return 0;
> +
> +pm_disable:
> +	pm_runtime_disable(dev);
> +	if (pwm_en) {
> +		for (i = 0; i < chip->npwm; i++) {
> +			if (ctrl[i])
> +				pm_runtime_put_noidle(dev);
> +		}
> +	}
> +disable_clk:
> +	clk_disable_unprepare(dwc->clk);
> +disable_busclk:
> +	clk_disable_unprepare(dwc->bus_clk);

[Severity: Medium]
If pwmchip_add() fails when pwm_en is false, will this error path
double-disable the clocks?

During probe, if no PWM channels are enabled, dwc->clk and dwc->bus_clk
are explicitly disabled in the else branch above. If pwmchip_add() then
fails, the execution jumps to pm_disable and falls through to disable_clk
and disable_busclk, executing clk_disable_unprepare() on both clocks a
second time. This causes a clock reference count underflow.

[ ... ]

> +static void dwc_pwm_plat_remove(struct platform_device *pdev)
> +{
> +	struct dwc_pwm_drvdata *data = platform_get_drvdata(pdev);
> +	struct pwm_chip *chip = data->chips[0];
> +	struct dwc_pwm *dwc = to_dwc_pwm(chip);
> +	unsigned int idx;
> +	int ret;
> +
> +	pm_runtime_get_sync(&pdev->dev);

[Severity: High]
Does pm_runtime_get_sync() need its return value checked here?

If the PM framework fails to resume the device during removal (e.g., due to a
power domain failure), the device's clocks or power will remain off.

> +	pwmchip_remove(chip);
> +
> +	for (idx = 0; idx < chip->npwm; idx++) {
> +		if (dwc_pwm_readl(dwc, DWC_TIM_CTRL(idx)) & DWC_TIM_CTRL_EN)

[Severity: High]
This isn't a bug introduced in this exact line, but as a continuation of the
previous concern: if pm_runtime_get_sync() failed, this dwc_pwm_readl()
will access unclocked memory-mapped I/O, which triggers a bus fault.

[ ... ]

> +static int dwc_pwm_suspend(struct device *dev)
> +{
> +	struct dwc_pwm_drvdata *data = dev_get_drvdata(dev);
> +	struct pwm_chip *chip = data->chips[0];
> +	struct dwc_pwm *dwc = to_dwc_pwm(chip);
> +	unsigned int idx;
> +	int ret;
> +
> +	if (pm_runtime_status_suspended(dev)) {
> +		ret = dwc_pwm_runtime_resume(dev);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (idx = 0; idx < chip->npwm; idx++) {
> +		if (chip->pwms[idx].state.enabled)
> +			return -EBUSY;

[Severity: High]
Does returning -EBUSY when a PWM channel is enabled unintentionally break
system-wide sleep transitions?

If a userspace sysfs client or a consumer device leaves a PWM running, this
aborts the entire system suspend sequence.

[ ... ]

> +static int dwc_pwm_resume(struct device *dev)
> +{
> +	struct dwc_pwm_drvdata *data = dev_get_drvdata(dev);
> +	struct pwm_chip *chip = data->chips[0];
> +	struct dwc_pwm *dwc = to_dwc_pwm(chip);
> +	unsigned int idx;
> +	bool pm_flags;
> +	int ret;
> +
> +	/* Check if device was runtime suspended before system resume */
> +	pm_flags = pm_runtime_status_suspended(dev);
> +	if (pm_flags) {
> +		/*
> +		 * Use PM framework to resume device
> +		 * (calls dwc_pwm_runtime_resume)
> +		 */
> +		ret = pm_runtime_get_sync(dev);
> +		if (ret < 0)
> +			return ret;

[Severity: High]
Can this leak the PM usage counter on failure?

pm_runtime_get_sync() increments the PM usage counter even if it returns an
error. If this call fails, propagating the error early leaves the device's
usage counter incremented, preventing it from ever entering runtime suspend
again. Should pm_runtime_put_noidle() be called before returning?

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