Devicetree
 help / color / mirror / Atom feed
* [PATCH v18 0/3] Add OpenCores PTC PWM support
@ 2026-05-15  5:47 Hal Feng
  2026-05-15  5:47 ` [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers Hal Feng
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Hal Feng @ 2026-05-15  5:47 UTC (permalink / raw)
  To: Uwe Kleine-König, Philipp Zabel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Emil Renner Berthing,
	Palmer Dabbelt, Paul Walmsley, Albert Ou
  Cc: Changhuang Liang, Hal Feng, linux-pwm, devicetree, linux-riscv,
	linux-kernel

Add OpenCores PTC PWM driver which is used in StarFive
JH7100/JH7110/JHB100 SoC.

I will maintain this pwm module in place of William.

Changes since v17:
- Simplify the code. Make it more readable.
- Restructure the driver to register the pwm chip for one pwm channel,
  because each OpenCores PTC IP core only supports one PWM channel.
  Drop starfive compatibles.
  Add patches to fix the dt-bindings and device tree.
- Support runtime pm and system sleep pm.
- Disable the pwm module and reset the pwm counter before updating the
  period and duty cycle.
- Improve the descriptions.
- Update the dt-bindings maintainer to Hal Feng.

History:
v17: https://lore.kernel.org/all/20250106103540.10079-1-william.qiu@starfivetech.com/

Hal Feng (3):
  dt-bindings: pwm: opencores: Drop starfive compatibles and update
    maintainers
  riscv: dts: starfive: Correct pwm nodes
  pwm: Add OpenCores PTC PWM driver

 .../bindings/pwm/opencores,pwm.yaml           |  10 +-
 MAINTAINERS                                   |   6 +
 .../boot/dts/starfive/jh7100-common.dtsi      |  28 +-
 arch/riscv/boot/dts/starfive/jh7100.dtsi      |  69 ++++-
 .../boot/dts/starfive/jh7110-common.dtsi      |  27 +-
 .../boot/dts/starfive/jh7110-milkv-mars.dts   |   6 +-
 .../dts/starfive/jh7110-milkv-marscm.dtsi     |   6 +-
 .../dts/starfive/jh7110-pine64-star64.dts     |   6 +-
 .../jh7110-starfive-visionfive-2-lite.dtsi    |   6 +-
 .../jh7110-starfive-visionfive-2.dtsi         |   6 +-
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  69 ++++-
 drivers/pwm/Kconfig                           |  12 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-ocores.c                      | 249 ++++++++++++++++++
 14 files changed, 471 insertions(+), 30 deletions(-)
 create mode 100644 drivers/pwm/pwm-ocores.c


base-commit: 66182ca873a4e87b3496eca79d57f86b76d7f52d
-- 
2.43.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers
  2026-05-15  5:47 [PATCH v18 0/3] Add OpenCores PTC PWM support Hal Feng
@ 2026-05-15  5:47 ` Hal Feng
  2026-05-15  6:02   ` sashiko-bot
  2026-05-15 13:06   ` Conor Dooley
  2026-05-15  5:47 ` [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes Hal Feng
  2026-05-15  5:47 ` [PATCH v18 3/3] pwm: Add OpenCores PTC PWM driver Hal Feng
  2 siblings, 2 replies; 9+ messages in thread
From: Hal Feng @ 2026-05-15  5:47 UTC (permalink / raw)
  To: Uwe Kleine-König, Philipp Zabel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Emil Renner Berthing,
	Palmer Dabbelt, Paul Walmsley, Albert Ou
  Cc: Changhuang Liang, Hal Feng, linux-pwm, devicetree, linux-riscv,
	linux-kernel

Each of the StarFive JH7100/JH7110/JH8100 SoCs has 8 OpenCores PTC IP
cores. One OpenCores PTC IP core can output one PWM channel. The only
difference among them is the register base address. There is no need
to add starfive compatibles to distinguish them.

I will maintain the pwm module in place of William.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../devicetree/bindings/pwm/opencores,pwm.yaml         | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
index 52a59d245cdb..834fb17ec595 100644
--- a/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 title: OpenCores PWM controller
 
 maintainers:
-  - William Qiu <william.qiu@starfivetech.com>
+  - Hal Feng <hal.feng@starfivetech.com>
 
 description:
   The OpenCores PTC ip core contains a PWM controller. When operating in PWM
@@ -20,10 +20,6 @@ allOf:
 properties:
   compatible:
     items:
-      - enum:
-          - starfive,jh7100-pwm
-          - starfive,jh7110-pwm
-          - starfive,jh8100-pwm
       - const: opencores,pwm-v1
 
   reg:
@@ -48,8 +44,8 @@ additionalProperties: false
 examples:
   - |
     pwm@12490000 {
-        compatible = "starfive,jh7110-pwm", "opencores,pwm-v1";
-        reg = <0x12490000 0x10000>;
+        compatible = "opencores,pwm-v1";
+        reg = <0x12490000 0x10>;
         clocks = <&clkgen 181>;
         resets = <&rstgen 109>;
         #pwm-cells = <3>;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes
  2026-05-15  5:47 [PATCH v18 0/3] Add OpenCores PTC PWM support Hal Feng
  2026-05-15  5:47 ` [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers Hal Feng
@ 2026-05-15  5:47 ` Hal Feng
  2026-05-15  6:34   ` sashiko-bot
  2026-05-15 13:09   ` Conor Dooley
  2026-05-15  5:47 ` [PATCH v18 3/3] pwm: Add OpenCores PTC PWM driver Hal Feng
  2 siblings, 2 replies; 9+ messages in thread
From: Hal Feng @ 2026-05-15  5:47 UTC (permalink / raw)
  To: Uwe Kleine-König, Philipp Zabel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Emil Renner Berthing,
	Palmer Dabbelt, Paul Walmsley, Albert Ou
  Cc: Changhuang Liang, Hal Feng, linux-pwm, devicetree, linux-riscv,
	linux-kernel

Each of the StarFive JH7100/JH7110 SoCs has 8 OpenCores PTC IP
cores. One OpenCores PTC IP core can output one PWM channel.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 .../boot/dts/starfive/jh7100-common.dtsi      | 28 ++++++--
 arch/riscv/boot/dts/starfive/jh7100.dtsi      | 69 ++++++++++++++++++-
 .../boot/dts/starfive/jh7110-common.dtsi      | 27 ++++++--
 .../boot/dts/starfive/jh7110-milkv-mars.dts   |  6 +-
 .../dts/starfive/jh7110-milkv-marscm.dtsi     |  6 +-
 .../dts/starfive/jh7110-pine64-star64.dts     |  6 +-
 .../jh7110-starfive-visionfive-2-lite.dtsi    |  6 +-
 .../jh7110-starfive-visionfive-2.dtsi         |  6 +-
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 69 ++++++++++++++++++-
 9 files changed, 200 insertions(+), 23 deletions(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
index ae1a6aeb0aea..85106545090e 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
@@ -199,13 +199,23 @@ GPO_I2C2_PAD_SDA_OEN,
 		};
 	};
 
-	pwm_pins: pwm-0 {
-		pwm-pins {
+	pwm0_pins: pwm0-0 {
+		pwm0-pins {
 			pinmux = <GPIOMUX(7,
 				  GPO_PWM_PAD_OUT_BIT0,
 				  GPO_PWM_PAD_OE_N_BIT0,
-				  GPI_NONE)>,
-				 <GPIOMUX(5,
+				  GPI_NONE)>;
+			bias-disable;
+			drive-strength = <35>;
+			input-disable;
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+	};
+
+	pwm1_pins: pwm1-0 {
+		pwm1-pins {
+			pinmux =  <GPIOMUX(5,
 				  GPO_PWM_PAD_OUT_BIT1,
 				  GPO_PWM_PAD_OE_N_BIT1,
 				  GPI_NONE)>;
@@ -359,9 +369,15 @@ &osc_aud {
 	clock-frequency = <27000000>;
 };
 
-&pwm {
+&pwm0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm0_pins>;
+	status = "okay";
+};
+
+&pwm1 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pwm_pins>;
+	pinctrl-0 = <&pwm1_pins>;
 	status = "okay";
 };
 
diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index 7de0732b8eab..4629e9747307 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -360,9 +360,72 @@ watchdog@12480000 {
 				 <&rstgen JH7100_RSTN_WDT>;
 		};
 
-		pwm: pwm@12490000 {
-			compatible = "starfive,jh7100-pwm", "opencores,pwm-v1";
-			reg = <0x0 0x12490000 0x0 0x10000>;
+		pwm0: pwm@12490000 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x12490000 0x0 0x10>;
+			clocks = <&clkgen JH7100_CLK_PWM_APB>;
+			resets = <&rstgen JH7100_RSTN_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm1: pwm@12490010 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x12490010 0x0 0x10>;
+			clocks = <&clkgen JH7100_CLK_PWM_APB>;
+			resets = <&rstgen JH7100_RSTN_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm2: pwm@12490020 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x12490020 0x0 0x10>;
+			clocks = <&clkgen JH7100_CLK_PWM_APB>;
+			resets = <&rstgen JH7100_RSTN_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm3: pwm@12490030 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x12490030 0x0 0x10>;
+			clocks = <&clkgen JH7100_CLK_PWM_APB>;
+			resets = <&rstgen JH7100_RSTN_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm4: pwm@12498000 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x12498000 0x0 0x10>;
+			clocks = <&clkgen JH7100_CLK_PWM_APB>;
+			resets = <&rstgen JH7100_RSTN_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm5: pwm@12498010 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x12498010 0x0 0x10>;
+			clocks = <&clkgen JH7100_CLK_PWM_APB>;
+			resets = <&rstgen JH7100_RSTN_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm6: pwm@12498020 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x12498020 0x0 0x10>;
+			clocks = <&clkgen JH7100_CLK_PWM_APB>;
+			resets = <&rstgen JH7100_RSTN_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm7: pwm@12498030 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x12498030 0x0 0x10>;
 			clocks = <&clkgen JH7100_CLK_PWM_APB>;
 			resets = <&rstgen JH7100_RSTN_PWM_APB>;
 			#pwm-cells = <3>;
diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
index 8cfe8033305d..5aa225b8bca8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi
@@ -351,9 +351,14 @@ uboot@100000 {
 	};
 };
 
-&pwm {
+&pwm0 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pwm_pins>;
+	pinctrl-0 = <&pwm0_pins>;
+};
+
+&pwm1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm1_pins>;
 };
 
 &spi0 {
@@ -553,12 +558,22 @@ GPOEN_ENABLE,
 		};
 	};
 
-	pwm_pins: pwm-0 {
-		pwm-pins {
+	pwm0_pins: pwm0-0 {
+		pwm0-pins {
 			pinmux = <GPIOMUX(46, GPOUT_SYS_PWM_CHANNEL0,
 					      GPOEN_SYS_PWM0_CHANNEL0,
-					      GPI_NONE)>,
-				 <GPIOMUX(59, GPOUT_SYS_PWM_CHANNEL1,
+					      GPI_NONE)>;
+			bias-disable;
+			drive-strength = <12>;
+			input-disable;
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+	};
+
+	pwm1_pins: pwm1-0 {
+		pwm1-pins {
+			pinmux = <GPIOMUX(59, GPOUT_SYS_PWM_CHANNEL1,
 					      GPOEN_SYS_PWM0_CHANNEL1,
 					      GPI_NONE)>;
 			bias-disable;
diff --git a/arch/riscv/boot/dts/starfive/jh7110-milkv-mars.dts b/arch/riscv/boot/dts/starfive/jh7110-milkv-mars.dts
index 21873612d993..54013c70f4b4 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-milkv-mars.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-milkv-mars.dts
@@ -68,7 +68,11 @@ &phy0 {
 	motorcomm,tx-clk-adj-enabled;
 };
 
-&pwm {
+&pwm0 {
+	status = "okay";
+};
+
+&pwm1 {
 	status = "okay";
 };
 
diff --git a/arch/riscv/boot/dts/starfive/jh7110-milkv-marscm.dtsi b/arch/riscv/boot/dts/starfive/jh7110-milkv-marscm.dtsi
index 025471061d43..31afac27b86d 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-milkv-marscm.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-milkv-marscm.dtsi
@@ -87,7 +87,11 @@ &phy0 {
 	motorcomm,tx-clk-adj-enabled;
 };
 
-&pwm {
+&pwm0 {
+	status = "okay";
+};
+
+&pwm1 {
 	status = "okay";
 };
 
diff --git a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
index aec7ae3d1f5b..a9e82f25efde 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
@@ -95,7 +95,11 @@ &phy1 {
 	motorcomm,tx-clk-100-inverted;
 };
 
-&pwm {
+&pwm0 {
+	status = "okay";
+};
+
+&pwm1 {
 	status = "okay";
 };
 
diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-lite.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-lite.dtsi
index f8797a666dbf..85b56a72dff7 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-lite.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-lite.dtsi
@@ -74,7 +74,11 @@ &phy0 {
 	tx-internal-delay-ps = <1500>;
 };
 
-&pwm {
+&pwm0 {
+	status = "okay";
+};
+
+&pwm1 {
 	status = "okay";
 };
 
diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index edc8f4588133..35208f95cd3d 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -73,7 +73,11 @@ &pcie1 {
 	status = "okay";
 };
 
-&pwm {
+&pwm0 {
+	status = "okay";
+};
+
+&pwm1 {
 	status = "okay";
 };
 
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 6e56e9d20bb0..e6b9b02bf8b2 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -846,9 +846,72 @@ i2stx1: i2s@120c0000 {
 			status = "disabled";
 		};
 
-		pwm: pwm@120d0000 {
-			compatible = "starfive,jh7110-pwm", "opencores,pwm-v1";
-			reg = <0x0 0x120d0000 0x0 0x10000>;
+		pwm0: pwm@120d0000 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x120d0000 0x0 0x10>;
+			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
+			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm1: pwm@120d0010 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x120d0010 0x0 0x10>;
+			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
+			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm2: pwm@120d0020 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x120d0020 0x0 0x10>;
+			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
+			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm3: pwm@120d0030 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x120d0030 0x0 0x10>;
+			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
+			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm4: pwm@120d8000 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x120d8000 0x0 0x10>;
+			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
+			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm5: pwm@120d8010 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x120d8010 0x0 0x10>;
+			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
+			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm6: pwm@120d8020 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x120d8020 0x0 0x10>;
+			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
+			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm7: pwm@120d8030 {
+			compatible = "opencores,pwm-v1";
+			reg = <0x0 0x120d8030 0x0 0x10>;
 			clocks = <&syscrg JH7110_SYSCLK_PWM_APB>;
 			resets = <&syscrg JH7110_SYSRST_PWM_APB>;
 			#pwm-cells = <3>;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v18 3/3] pwm: Add OpenCores PTC PWM driver
  2026-05-15  5:47 [PATCH v18 0/3] Add OpenCores PTC PWM support Hal Feng
  2026-05-15  5:47 ` [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers Hal Feng
  2026-05-15  5:47 ` [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes Hal Feng
@ 2026-05-15  5:47 ` Hal Feng
  2026-05-15  7:02   ` sashiko-bot
  2 siblings, 1 reply; 9+ messages in thread
From: Hal Feng @ 2026-05-15  5:47 UTC (permalink / raw)
  To: Uwe Kleine-König, Philipp Zabel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Emil Renner Berthing,
	Palmer Dabbelt, Paul Walmsley, Albert Ou
  Cc: Changhuang Liang, Hal Feng, linux-pwm, devicetree, linux-riscv,
	linux-kernel

Add PWM driver for OpenCores PTC IP core.

Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
---
 MAINTAINERS              |   6 +
 drivers/pwm/Kconfig      |  12 ++
 drivers/pwm/Makefile     |   1 +
 drivers/pwm/pwm-ocores.c | 249 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 268 insertions(+)
 create mode 100644 drivers/pwm/pwm-ocores.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6aa3fe2ee1bb..14af609f4ada 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20027,6 +20027,12 @@ F:	Documentation/i2c/busses/i2c-ocores.rst
 F:	drivers/i2c/busses/i2c-ocores.c
 F:	include/linux/platform_data/i2c-ocores.h
 
+OPENCORES PWM DRIVER
+M:	Hal Feng <hal.feng@starfivetech.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
+F:	drivers/pwm/pwm-ocores.c
+
 OPENRISC ARCHITECTURE
 M:	Jonas Bonn <jonas@southpole.se>
 M:	Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 6f3147518376..dd7f3bf5c3eb 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -534,6 +534,18 @@ config PWM_NTXEC
 	  controller found in certain e-book readers designed by the original
 	  design manufacturer Netronix.
 
+config PWM_OCORES
+	tristate "OpenCores PTC PWM support"
+	depends on HAS_IOMEM && OF
+	depends on COMMON_CLK
+	depends on ARCH_STARFIVE || COMPILE_TEST
+	help
+	  PWM driver for OpenCores PTC IP core.
+	  For details see https://opencores.org/projects/ptc.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-ocores.
+
 config PWM_OMAP_DMTIMER
 	tristate "OMAP Dual-Mode Timer PWM support"
 	depends on OF
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 0dc0d2b69025..2d47bad7bd74 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_PWM_MICROCHIP_CORE)	+= pwm-microchip-core.o
 obj-$(CONFIG_PWM_MTK_DISP)	+= pwm-mtk-disp.o
 obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
 obj-$(CONFIG_PWM_NTXEC)		+= pwm-ntxec.o
+obj-$(CONFIG_PWM_OCORES)	+= pwm-ocores.o
 obj-$(CONFIG_PWM_OMAP_DMTIMER)	+= pwm-omap-dmtimer.o
 obj-$(CONFIG_PWM_PCA9685)	+= pwm-pca9685.o
 obj-$(CONFIG_PWM_PXA)		+= pwm-pxa.o
diff --git a/drivers/pwm/pwm-ocores.c b/drivers/pwm/pwm-ocores.c
new file mode 100644
index 000000000000..fa6a34117cde
--- /dev/null
+++ b/drivers/pwm/pwm-ocores.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * OpenCores PTC PWM Driver
+ *
+ * https://opencores.org/projects/ptc
+ *
+ * Copyright (C) 2018-2026 StarFive Technology Co., Ltd.
+ *
+ * Limitations:
+ * - The hardware only supports inverted polarity.
+ * - The hardware minimum period / duty_cycle of PWM is (1 / pwm_apb clock frequency).
+ * - The hardware maximum period / duty_cycle of PWM is (U32_MAX / pwm_apb clock frequency).
+ * - The output is immediately set to low when the module is disabled.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+
+#define OCPWM_HRC	0x4
+#define OCPWM_LRC	0x8
+#define OCPWM_CTRL	0xC
+
+#define OCPWM_CTRL_EN	BIT(0)
+#define OCPWM_CTRL_OE	BIT(3)
+#define OCPWM_CTRL_RST	BIT(7)
+
+struct ocores_pwm_device {
+	void __iomem *base;
+	struct clk *clk;
+	unsigned long clk_rate;
+	struct reset_control *rst;
+};
+
+static int ocores_pwm_get_state(struct pwm_chip *chip,
+				struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct ocores_pwm_device *ddata = pwmchip_get_drvdata(chip);
+	u32 period_data, duty_data, ctrl_data;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(pwmchip_parent(chip));
+	if (ret < 0)
+		return ret;
+
+	period_data = readl(ddata->base + OCPWM_LRC);
+	duty_data = readl(ddata->base + OCPWM_HRC);
+	ctrl_data = readl(ddata->base + OCPWM_CTRL);
+
+	state->period = DIV_ROUND_UP_ULL((u64)period_data * NSEC_PER_SEC, ddata->clk_rate);
+	state->duty_cycle = DIV_ROUND_UP_ULL((u64)duty_data * NSEC_PER_SEC, ddata->clk_rate);
+	if (state->duty_cycle > state->period)
+		state->duty_cycle = state->period;
+
+	state->polarity = PWM_POLARITY_INVERSED;
+	state->enabled = (ctrl_data & OCPWM_CTRL_EN) ? true : false;
+
+	pm_runtime_put(pwmchip_parent(chip));
+
+	return 0;
+}
+
+static int ocores_pwm_apply(struct pwm_chip *chip,
+			    struct pwm_device *pwm,
+			    const struct pwm_state *state)
+{
+	struct ocores_pwm_device *ddata = pwmchip_get_drvdata(chip);
+	u64 period_data, duty_data;
+	int ret;
+
+	if (state->polarity != PWM_POLARITY_INVERSED)
+		return -EINVAL;
+
+	if (state->enabled) {
+		if (!pwm_is_enabled(pwm)) {
+			ret = pm_runtime_resume_and_get(pwmchip_parent(chip));
+			if (ret < 0)
+				return ret;
+		}
+	} else {
+		if (pwm_is_enabled(pwm)) {
+			writel(0, ddata->base + OCPWM_CTRL);
+			pm_runtime_put(pwmchip_parent(chip));
+		}
+		return 0;
+	}
+
+	writel(0, ddata->base + OCPWM_CTRL);
+	writel(OCPWM_CTRL_RST, ddata->base + OCPWM_CTRL);
+
+	period_data = mul_u64_u32_div(state->period, ddata->clk_rate, NSEC_PER_SEC);
+	if (period_data > U32_MAX)
+		period_data = U32_MAX;
+
+	duty_data = mul_u64_u32_div(state->duty_cycle, ddata->clk_rate, NSEC_PER_SEC);
+	if (duty_data > U32_MAX)
+		duty_data = U32_MAX;
+
+	writel(period_data, ddata->base + OCPWM_LRC);
+	writel(duty_data, ddata->base + OCPWM_HRC);
+	writel(OCPWM_CTRL_OE | OCPWM_CTRL_EN, ddata->base + OCPWM_CTRL);
+
+	return 0;
+}
+
+static const struct pwm_ops ocores_pwm_ops = {
+	.get_state = ocores_pwm_get_state,
+	.apply = ocores_pwm_apply,
+};
+
+static int ocores_pwm_runtime_suspend(struct device *dev)
+{
+	struct ocores_pwm_device *ddata = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(ddata->clk);
+
+	return 0;
+}
+
+static int ocores_pwm_runtime_resume(struct device *dev)
+{
+	struct ocores_pwm_device *ddata = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(ddata->clk);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable clock\n");
+
+	return 0;
+}
+
+static const struct dev_pm_ops ocores_pwm_pm_ops = {
+	RUNTIME_PM_OPS(ocores_pwm_runtime_suspend,
+		       ocores_pwm_runtime_resume, NULL)
+	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+			    pm_runtime_force_resume)
+};
+
+static void ocores_pwm_pm_disable(void *data)
+{
+	struct device *dev = data;
+	struct ocores_pwm_device *ddata = dev_get_drvdata(dev);
+
+	pm_runtime_disable(dev);
+
+	if (!pm_runtime_status_suspended(dev))
+		ocores_pwm_runtime_suspend(dev);
+
+	reset_control_assert(ddata->rst);
+}
+
+static int ocores_pwm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ocores_pwm_device *ddata;
+	struct pwm_chip *chip;
+	int ret;
+
+	chip = devm_pwmchip_alloc(dev, 1, sizeof(*ddata));
+	if (IS_ERR(chip))
+		return -ENOMEM;
+
+	chip->ops = &ocores_pwm_ops;
+	ddata = pwmchip_get_drvdata(chip);
+
+	ddata->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ddata->base))
+		return dev_err_probe(dev, PTR_ERR(ddata->base),
+				     "Failed to map IO resources\n");
+
+	ddata->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(ddata->clk))
+		return dev_err_probe(dev, PTR_ERR(ddata->clk),
+				     "Failed to get clock\n");
+
+	ddata->clk_rate = clk_get_rate(ddata->clk);
+	if (!ddata->clk_rate || ddata->clk_rate > NSEC_PER_SEC)
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid clock rate: %lu\n", ddata->clk_rate);
+
+	ddata->rst = devm_reset_control_get_optional_shared(dev, NULL);
+	if (IS_ERR(ddata->rst))
+		return dev_err_probe(dev, PTR_ERR(ddata->rst),
+				     "Failed to get reset\n");
+
+	platform_set_drvdata(pdev, ddata);
+
+	ret = ocores_pwm_runtime_resume(dev);
+	if (ret)
+		return ret;
+
+	ret = reset_control_deassert(ddata->rst);
+	if (ret)
+		goto err_clk_disable;
+
+	ret = pm_runtime_set_active(dev);
+	if (ret)
+		goto err_reset_assert;
+
+	pm_runtime_enable(dev);
+
+	ret = devm_add_action_or_reset(dev, ocores_pwm_pm_disable, dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add pm disable action\n");
+
+	pm_runtime_get_noresume(dev);
+
+	writel(0, ddata->base + OCPWM_CTRL);
+
+	pm_runtime_put(dev);
+
+	ret = devm_pwmchip_add(dev, chip);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Could not register PWM chip\n");
+
+	return 0;
+
+err_reset_assert:
+	reset_control_assert(ddata->rst);
+err_clk_disable:
+	ocores_pwm_runtime_suspend(dev);
+	return dev_err_probe(dev, ret, "Failed to init pwm power\n");
+}
+
+static const struct of_device_id ocores_pwm_of_match[] = {
+	{ .compatible = "opencores,pwm-v1" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ocores_pwm_of_match);
+
+static struct platform_driver ocores_pwm_driver = {
+	.probe = ocores_pwm_probe,
+	.driver = {
+		.name = "ocores-pwm",
+		.of_match_table = ocores_pwm_of_match,
+		.pm = pm_ptr(&ocores_pwm_pm_ops),
+	},
+};
+module_platform_driver(ocores_pwm_driver);
+
+MODULE_AUTHOR("Jieqin Chen");
+MODULE_AUTHOR("Hal Feng <hal.feng@starfivetech.com>");
+MODULE_DESCRIPTION("OpenCores PTC PWM driver");
+MODULE_LICENSE("GPL");
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers
  2026-05-15  5:47 ` [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers Hal Feng
@ 2026-05-15  6:02   ` sashiko-bot
  2026-05-15 13:06   ` Conor Dooley
  1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-15  6:02 UTC (permalink / raw)
  To: Hal Feng; +Cc: robh, devicetree, krzk+dt, conor+dt

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Removing existing StarFive compatible strings breaks DT schema validation for existing device trees.
- [Medium] The commit silently modifies the `reg` space size in the schema example.
- [Low] The `compatible` schema contains an awkward single-item list structure after modification.
--

commit c3152a2a38b5f4d3a25df0696646b840e0d04b60
Author: Hal Feng <hal.feng@starfivetech.com>

dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers

Each of the StarFive SoCs has 8 OpenCores PTC IP cores that only differ
by their register base address. There is no need for starfive compatibles to
distinguish them, so they are dropped.

> diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
> index 52a59d245cdb1..834fb17ec595e 100644
> --- a/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml

[ ... ]

> @@ -20,10 +20,6 @@ allOf:
>  properties:
>    compatible:
>      items:
> -      - enum:
> -          - starfive,jh7100-pwm
> -          - starfive,jh7110-pwm
> -          - starfive,jh8100-pwm
>        - const: opencores,pwm-v1

Does removing the legacy compatible strings break dtbs_check for existing
device trees? Older device trees likely still use the two-string format.

Would it be better to use a oneOf construct to allow both the legacy
two-string format and the new single-string format to maintain backward
compatibility?

This isn't a bug, but leaving a single element under items is an anti-pattern
in dt-schema. Since only one string is allowed now, could this be simplified
to just a direct const property instead of an items array?

[ ... ]

> @@ -48,8 +44,8 @@ additionalProperties: false
>  examples:
>    - |
>      pwm@12490000 {
> -        compatible = "starfive,jh7110-pwm", "opencores,pwm-v1";
> -        reg = <0x12490000 0x10000>;
> +        compatible = "opencores,pwm-v1";
> +        reg = <0x12490000 0x10>;

The commit message mentions dropping the starfive compatibles and updating
maintainers, but does not document changing the register space size from
0x10000 to 0x10.

Could the commit message be updated to explicitly explain and justify this
register size change?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515054723.25024-1-hal.feng@starfivetech.com?part=1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes
  2026-05-15  5:47 ` [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes Hal Feng
@ 2026-05-15  6:34   ` sashiko-bot
  2026-05-15 13:09   ` Conor Dooley
  1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-15  6:34 UTC (permalink / raw)
  To: Hal Feng; +Cc: conor+dt, devicetree, robh, krzk+dt

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Pinmux configuration properties were erroneously omitted from `pwm1_pins` in `jh7100-common.dtsi`.
- [Medium] Removal of SoC-specific compatible strings violates Device Tree ABI best practices.
--

commit e05af83398a49d79a54da1bad8e64c7c1e5f98eb
Author: Hal Feng <hal.feng@starfivetech.com>

riscv: dts: starfive: Correct pwm nodes

This commit updates the PWM nodes for StarFive JH7100 and JH7110 SoCs,
splitting the original single PWM node into 8 individual nodes representing
the 8 OpenCores PTC IP cores.

> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi

[ ... ]

> +	pwm1_pins: pwm1-0 {
> +		pwm1-pins {
> +			pinmux =  <GPIOMUX(5,
>  				  GPO_PWM_PAD_OUT_BIT1,
>  				  GPO_PWM_PAD_OE_N_BIT1,
>  				  GPI_NONE)>;
>  		};
>  	};

In the original code, the single pwm_pins node applied electrical
properties like bias-disable, drive-strength = <35>, input-disable,
input-schmitt-disable, and slew-rate = <0> to both PWM0 and PWM1 pins. When
splitting the node, these properties were copied to pwm0_pins but omitted
here.

Will omitting these properties cause the pwm1 output to operate with
undefined or default hardware pin states instead of the intended parameters?

> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi

[ ... ]

> -		pwm: pwm@12490000 {
> -			compatible = "starfive,jh7100-pwm", "opencores,pwm-v1";
> -			reg = <0x0 0x12490000 0x0 0x10000>;
> +		pwm0: pwm@12490000 {
> +			compatible = "opencores,pwm-v1";

Is it intentional to drop the SoC-specific "starfive,jh7100-pwm" compatible
string here, as well as "starfive,jh7110-pwm" in jh7110.dtsi?

Device Tree best practices generally recommend retaining SoC-specific
strings alongside generic IP strings. This ensures that if SoC-specific
integration quirks are discovered later, workarounds can be applied without
requiring users to update deployed device trees, and it maintains ABI
compatibility for older kernels that might match against the removed
strings.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515054723.25024-1-hal.feng@starfivetech.com?part=2

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v18 3/3] pwm: Add OpenCores PTC PWM driver
  2026-05-15  5:47 ` [PATCH v18 3/3] pwm: Add OpenCores PTC PWM driver Hal Feng
@ 2026-05-15  7:02   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-15  7:02 UTC (permalink / raw)
  To: Hal Feng; +Cc: conor+dt, robh, devicetree, krzk+dt

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] Probe fails unconditionally when CONFIG_PM is disabled.
- [High] Teardown symmetry violation in `ocores_pwm_pm_disable()` leads to reset assertion without an active clock.
- [High] Race condition between `pm_runtime_enable()` and `pm_runtime_get_noresume()` allows premature hardware access without a clock.
- [High] Missing context restore on system resume leads to silently broken PWM output.
- [High] Runtime PM usage count leak permanently blocks PM suspend if the driver is unbound while the PWM is enabled.
- [Medium] Subsystem rule violation: Using asynchronous `pm_runtime_put()` before `pm_runtime_disable()` causes race conditions.
- [Medium] Hardware minimum period and duty cycle limitations are not enforced, allowing invalid configuration.
- [Medium] Unconditional hardware disable in probe() breaks bootloader handover.
- [Low] Masking of true error codes from devm_pwmchip_alloc().
--

commit 2a9909cb21b11044209c8ae47fbb6d140786ac45
Author: Hal Feng <hal.feng@starfivetech.com>
Subject: pwm: Add OpenCores PTC PWM driver

This commit adds a new PWM driver for the OpenCores PTC IP core.

> --- /dev/null
> +++ b/drivers/pwm/pwm-ocores.c
[ ... ]
> +static int ocores_pwm_apply(struct pwm_chip *chip,
> +			    struct pwm_device *pwm,
> +			    const struct pwm_state *state)
> +{
[ ... ]
> +	if (state->enabled) {
> +		if (!pwm_is_enabled(pwm)) {
> +			ret = pm_runtime_resume_and_get(pwmchip_parent(chip));
> +			if (ret < 0)
> +				return ret;
> +		}

Does this leak the runtime PM usage count if the driver is unbound while the
PWM is enabled?

If the driver is unbound via sysfs while the PWM is actively running, the PWM
core marks the chip non-operational but does not disable the running PWM
hardware. The devres teardown action ocores_pwm_pm_disable() will run and
forcefully turn off the hardware, but it doesn't decrement the elevated PM
usage counter.

Since the struct device object persists across bind/unbind cycles, could this
leaked usage count permanently prevent the device from entering runtime suspend
on subsequent binds?

> +	} else {
> +		if (pwm_is_enabled(pwm)) {
> +			writel(0, ddata->base + OCPWM_CTRL);
> +			pm_runtime_put(pwmchip_parent(chip));
> +		}
> +		return 0;
> +	}
> +
> +	writel(0, ddata->base + OCPWM_CTRL);
> +	writel(OCPWM_CTRL_RST, ddata->base + OCPWM_CTRL);
> +
> +	period_data = mul_u64_u32_div(state->period, ddata->clk_rate, NSEC_PER_SEC);

The header comment notes the hardware minimum period and duty cycle is 1 clock
cycle. If the requested state->period is smaller than one clock cycle, wouldn't
mul_u64_u32_div() silently truncate the result to 0?

Writing 0 to the OCPWM_LRC and OCPWM_HRC registers violates this hardware
constraint and might cause undefined IP behavior. Should the driver validate
that period_data >= 1 and return -EINVAL if the request cannot be satisfied?

> +	if (period_data > U32_MAX)
> +		period_data = U32_MAX;
[ ... ]
> +static int ocores_pwm_runtime_resume(struct device *dev)
> +{
> +	struct ocores_pwm_device *ddata = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(ddata->clk);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable clock\n");
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops ocores_pwm_pm_ops = {
> +	RUNTIME_PM_OPS(ocores_pwm_runtime_suspend,
> +		       ocores_pwm_runtime_resume, NULL)
> +	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +			    pm_runtime_force_resume)
> +};

Will this silently break PWM output upon resuming from system sleep?

During system suspend, if the device's power domain is powered off, the
hardware register contents are typically lost.

The ocores_pwm_runtime_resume() callback only re-enables the clock; it doesn't
save and restore the PWM configuration registers. If a PWM was actively
outputting a signal before system suspend, will it fail to output the correct
signal after the system wakes up?

> +static void ocores_pwm_pm_disable(void *data)
> +{
> +	struct device *dev = data;
> +	struct ocores_pwm_device *ddata = dev_get_drvdata(dev);
> +
> +	pm_runtime_disable(dev);
> +
> +	if (!pm_runtime_status_suspended(dev))
> +		ocores_pwm_runtime_suspend(dev);
> +
> +	reset_control_assert(ddata->rst);
> +}

Could asserting the reset while the clock is turned off cause issues?

In ocores_pwm_probe(), the hardware initialization order is strictly clock
enable followed by reset deassertion. The explicit probe error path also
respects this by reversing the order, asserting reset before disabling the
clock.

However, ocores_pwm_pm_disable() calls ocores_pwm_runtime_suspend()
(which turns off the clock) before calling reset_control_assert(). If the IP
requires an active clock to propagate the reset signal, asserting the reset
while the clock is turned off might fail or hang the bus.

> +static int ocores_pwm_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ocores_pwm_device *ddata;
> +	struct pwm_chip *chip;
> +	int ret;
> +
> +	chip = devm_pwmchip_alloc(dev, 1, sizeof(*ddata));
> +	if (IS_ERR(chip))
> +		return -ENOMEM;

Should this return PTR_ERR(chip) instead of hardcoding -ENOMEM?

devm_pwmchip_alloc() returns an ERR_PTR that can propagate various error codes,
and returning -ENOMEM masks the true failure reason.

> +	chip->ops = &ocores_pwm_ops;
> +	ddata = pwmchip_get_drvdata(chip);
[ ... ]
> +	ret = reset_control_deassert(ddata->rst);
> +	if (ret)
> +		goto err_clk_disable;
> +
> +	ret = pm_runtime_set_active(dev);
> +	if (ret)
> +		goto err_reset_assert;

Does this fail probe unconditionally when CONFIG_PM is disabled?

When CONFIG_PM is disabled, the dummy implementation of
pm_runtime_set_active() returns 1. The if (ret) check evaluates to true
when ret = 1, causing the probe function to jump to the error path.

Should this check if (ret < 0) to distinguish PM framework errors from
successful status returns?

> +	pm_runtime_enable(dev);
> +
> +	ret = devm_add_action_or_reset(dev, ocores_pwm_pm_disable, dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add pm disable action\n");
> +
> +	pm_runtime_get_noresume(dev);
> +
> +	writel(0, ddata->base + OCPWM_CTRL);

Are there a couple of issues with this sequence?

First, pm_runtime_enable() is called while the runtime PM usage counter is 0.
This opens a window where the PM core can asynchronously suspend the device.
The driver then calls pm_runtime_get_noresume(), which increments the usage
counter but does not resume the device if it was already suspended.

If the device was suspended during this window, the subsequent writel()
would access hardware registers while the clock is turned off.
Should pm_runtime_get_noresume() be called before pm_runtime_enable()?

Second, does this unconditional writel(0, ...) break bootloader handover?
If the bootloader previously configured and enabled the PWM, this write will
instantly power it down. Could this cause hardware state disruption until
the consumer driver re-enables it?

> +	pm_runtime_put(dev);
> +
> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Could not register PWM chip\n");

Is it safe to use the asynchronous pm_runtime_put() here?

If devm_pwmchip_add() fails, the devres teardown action ocores_pwm_pm_disable()
runs immediately and invokes pm_runtime_disable(). This forcefully cancels
the pending asynchronous suspend work from pm_runtime_put(), leaving the device
in an undefined PM state.

The Power Management subsystem guidelines dictate that pm_runtime_put_sync()
must be used instead of pm_runtime_put() when pm_runtime_disable() might follow
immediately.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515054723.25024-1-hal.feng@starfivetech.com?part=3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers
  2026-05-15  5:47 ` [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers Hal Feng
  2026-05-15  6:02   ` sashiko-bot
@ 2026-05-15 13:06   ` Conor Dooley
  1 sibling, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2026-05-15 13:06 UTC (permalink / raw)
  To: Hal Feng
  Cc: Uwe Kleine-König, Philipp Zabel, Rob Herring,
	Krzysztof Kozlowski, Emil Renner Berthing, Palmer Dabbelt,
	Paul Walmsley, Albert Ou, Changhuang Liang, linux-pwm, devicetree,
	linux-riscv, linux-kernel

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

On Fri, May 15, 2026 at 01:47:20PM +0800, Hal Feng wrote:
> Each of the StarFive JH7100/JH7110/JH8100 SoCs has 8 OpenCores PTC IP
> cores. One OpenCores PTC IP core can output one PWM channel. The only
> difference among them is the register base address. There is no need
> to add starfive compatibles to distinguish them.

NAK, that's not an acceptable reason to remove soc-specific compatibles.

pw-bot: changes-requested

> 
> I will maintain the pwm module in place of William.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  .../devicetree/bindings/pwm/opencores,pwm.yaml         | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
> index 52a59d245cdb..834fb17ec595 100644
> --- a/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm.yaml
> @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
>  title: OpenCores PWM controller
>  
>  maintainers:
> -  - William Qiu <william.qiu@starfivetech.com>
> +  - Hal Feng <hal.feng@starfivetech.com>
>  
>  description:
>    The OpenCores PTC ip core contains a PWM controller. When operating in PWM
> @@ -20,10 +20,6 @@ allOf:
>  properties:
>    compatible:
>      items:
> -      - enum:
> -          - starfive,jh7100-pwm
> -          - starfive,jh7110-pwm
> -          - starfive,jh8100-pwm
>        - const: opencores,pwm-v1
>  
>    reg:
> @@ -48,8 +44,8 @@ additionalProperties: false
>  examples:
>    - |
>      pwm@12490000 {
> -        compatible = "starfive,jh7110-pwm", "opencores,pwm-v1";
> -        reg = <0x12490000 0x10000>;
> +        compatible = "opencores,pwm-v1";
> +        reg = <0x12490000 0x10>;
>          clocks = <&clkgen 181>;
>          resets = <&rstgen 109>;
>          #pwm-cells = <3>;
> -- 
> 2.43.2
> 

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes
  2026-05-15  5:47 ` [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes Hal Feng
  2026-05-15  6:34   ` sashiko-bot
@ 2026-05-15 13:09   ` Conor Dooley
  1 sibling, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2026-05-15 13:09 UTC (permalink / raw)
  To: Hal Feng
  Cc: Uwe Kleine-König, Philipp Zabel, Rob Herring,
	Krzysztof Kozlowski, Emil Renner Berthing, Palmer Dabbelt,
	Paul Walmsley, Albert Ou, Changhuang Liang, linux-pwm, devicetree,
	linux-riscv, linux-kernel

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

On Fri, May 15, 2026 at 01:47:21PM +0800, Hal Feng wrote:
> Each of the StarFive JH7100/JH7110 SoCs has 8 OpenCores PTC IP
> cores. One OpenCores PTC IP core can output one PWM channel.
> 
> Signed-off-by: Hal Feng <hal.feng@starfivetech.com>
> ---
>  .../boot/dts/starfive/jh7100-common.dtsi      | 28 ++++++--
>  arch/riscv/boot/dts/starfive/jh7100.dtsi      | 69 ++++++++++++++++++-
>  .../boot/dts/starfive/jh7110-common.dtsi      | 27 ++++++--
>  .../boot/dts/starfive/jh7110-milkv-mars.dts   |  6 +-
>  .../dts/starfive/jh7110-milkv-marscm.dtsi     |  6 +-
>  .../dts/starfive/jh7110-pine64-star64.dts     |  6 +-
>  .../jh7110-starfive-visionfive-2-lite.dtsi    |  6 +-
>  .../jh7110-starfive-visionfive-2.dtsi         |  6 +-
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 69 ++++++++++++++++++-
>  9 files changed, 200 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> index ae1a6aeb0aea..85106545090e 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> @@ -199,13 +199,23 @@ GPO_I2C2_PAD_SDA_OEN,
>  		};
>  	};
>  
> -	pwm_pins: pwm-0 {
> -		pwm-pins {
> +	pwm0_pins: pwm0-0 {
> +		pwm0-pins {
>  			pinmux = <GPIOMUX(7,
>  				  GPO_PWM_PAD_OUT_BIT0,
>  				  GPO_PWM_PAD_OE_N_BIT0,
> -				  GPI_NONE)>,
> -				 <GPIOMUX(5,
> +				  GPI_NONE)>;
> +			bias-disable;
> +			drive-strength = <35>;
> +			input-disable;
> +			input-schmitt-disable;
> +			slew-rate = <0>;
> +		};
> +	};
> +
> +	pwm1_pins: pwm1-0 {
> +		pwm1-pins {
> +			pinmux =  <GPIOMUX(5,
>  				  GPO_PWM_PAD_OUT_BIT1,
>  				  GPO_PWM_PAD_OE_N_BIT1,
>  				  GPI_NONE)>;
> @@ -359,9 +369,15 @@ &osc_aud {
>  	clock-frequency = <27000000>;
>  };
>  
> -&pwm {
> +&pwm0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pwm0_pins>;
> +	status = "okay";
> +};
> +
> +&pwm1 {
>  	pinctrl-names = "default";
> -	pinctrl-0 = <&pwm_pins>;
> +	pinctrl-0 = <&pwm1_pins>;
>  	status = "okay";
>  };
>  
> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> index 7de0732b8eab..4629e9747307 100644
> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
> @@ -360,9 +360,72 @@ watchdog@12480000 {
>  				 <&rstgen JH7100_RSTN_WDT>;
>  		};
>  
> -		pwm: pwm@12490000 {
> -			compatible = "starfive,jh7100-pwm", "opencores,pwm-v1";
> -			reg = <0x0 0x12490000 0x0 0x10000>;
> +		pwm0: pwm@12490000 {
> +			compatible = "opencores,pwm-v1";
> +			reg = <0x0 0x12490000 0x0 0x10>;

NAK on the compatibles front, but this also looks very suspect, given
the size of the register regions, but I think it is actually correct.
You need to explain why it is correct in the commit message.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-05-15 13:09 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-15  5:47 [PATCH v18 0/3] Add OpenCores PTC PWM support Hal Feng
2026-05-15  5:47 ` [PATCH v18 1/3] dt-bindings: pwm: opencores: Drop starfive compatibles and update maintainers Hal Feng
2026-05-15  6:02   ` sashiko-bot
2026-05-15 13:06   ` Conor Dooley
2026-05-15  5:47 ` [PATCH v18 2/3] riscv: dts: starfive: Correct pwm nodes Hal Feng
2026-05-15  6:34   ` sashiko-bot
2026-05-15 13:09   ` Conor Dooley
2026-05-15  5:47 ` [PATCH v18 3/3] pwm: Add OpenCores PTC PWM driver Hal Feng
2026-05-15  7:02   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox