[PATCH v2 0/6] pwm: Update PWM_PXA driver for SpacemiT K1

[PATCH v2 0/6] pwm: Update PWM_PXA driver for SpacemiT K1

[Guodong Xu]

This patchset adds support for the SpacemiT K1 SoC in the PWM_PXA driver
and updates related device tree bindings. The changes enable PWM
functionality on the K1 platform through driver enhancements,
configuration updates, and device tree additions.

Functionality has been verified on the Banana Pi BPI-F3 board using PWM14,
configured as a pwm-backlight. Per community feedback, the actual
pwm-backlight node is not included in this patchset but can be found in
patch 7 of the v1 series.

This patchset is based on the following dependencies:
1. Clock controller driver, posted by Heylen Chu (v8), with
   most of it has been accepted:
https://lore.kernel.org/all/20250416135406.16284-1-heylenay@4d2.org/
2. Reset controller driver, posted by Alex Elder (v5):
https://lore.kernel.org/all/20250418145401.2603648-1-elder@riscstar.com/

Major differences between v2 and v1:
 - Dropped the addition of spacemit,k1-pwm as a compatible string in the
   PWM_PXA driver; instead, it now falls back to marvell,pxa910-pwm.
 - Removed pinctrl settings for all PWM nodes (pwm0-pwm14); only the
   pwm14_1 configuration is included in this version.
 - Changed PWM_PXA from built-in to a loadable module (=m) in the
   riscv defconfig.

V2 consists of the following patches:
Patch 1: Add spacemit,k1-pwm compatible string (with fallback to
           marvell,pxa910-pwm) and support optional resets property.
Patch 2: Add reset controller support to the PWM_PXA driver.
Patch 3: Add device tree nodes for all 20 PWM instances on K1.
Patch 4: Add pinctrl settings for PWM14.
Patch 5: Add ARCH_SPACEMIT dependency to the PWM_PXA Kconfig entry.
Patch 6: Enable PWM and PWM_PXA in riscv defconfig for SpacemiT K1.

Best regards,
Guodong Xu

v1:
https://lore.kernel.org/all/20250411131423.3802611-1-guodong@riscstar.com/

Guodong Xu (6):
  dt-bindings: pwm: marvell,pxa-pwm: Add SpacemiT K1 PWM support
  pwm: pxa: add optional reset control
  riscv: dts: spacemit: add PWM support for K1 SoC
  riscv: dts: spacemit: add pwm14_1 pinctrl setting
  pwm: Kconfig: add depends on ARCH_SPACEMIT to PWM_PXA
  riscv: defconfig: Enable PWM support for SpacemiT K1 SoC

 .../bindings/pwm/marvell,pxa-pwm.yaml         |  17 +-
 arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi  |   7 +
 arch/riscv/boot/dts/spacemit/k1.dtsi          | 180 ++++++++++++++++++
 arch/riscv/configs/defconfig                  |   2 +
 drivers/pwm/Kconfig                           |   2 +-
 drivers/pwm/pwm-pxa.c                         |  14 +-
 6 files changed, 212 insertions(+), 10 deletions(-)

-- 
2.43.0

[PATCH v2 1/6] dt-bindings: pwm: marvell,pxa-pwm: Add SpacemiT K1 PWM support

[Guodong Xu]

The SpacemiT K1 SoC reuses the Marvell PXA910-compatible PWM controller
with one notable difference: the addition of a resets property. To make
the device tree pass schema validation (make dtbs_check W=3), this patch
updates the binding to accept spacemit,k1-pwm as a compatible string, when
used in conjunction with the fallback marvell,pxa910-pwm.

Support for the optional resets property is also added, as it is required
by the K1 integration but was not present in the original Marvell bindings.

Since the PWM reset line may be deasserted during the early bootloader
stage, making the resets property optional avoids potential
double-deassertion, which could otherwise cause flickering on displays
that use PWM for backlight control.

Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v2: Accept spacemit,k1-pwm as a compatible string, when used in conjunction
    with the fallback marvell,pxa910-pwm

.../bindings/pwm/marvell,pxa-pwm.yaml           | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml
index 9ee1946dc2e1..0d97333c7fee 100644
--- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml
@@ -14,11 +14,15 @@ allOf:
 
 properties:
   compatible:
-    enum:
-      - marvell,pxa250-pwm
-      - marvell,pxa270-pwm
-      - marvell,pxa168-pwm
-      - marvell,pxa910-pwm
+    oneOf:
+      - enum:
+          - marvell,pxa250-pwm
+          - marvell,pxa270-pwm
+          - marvell,pxa168-pwm
+          - marvell,pxa910-pwm
+      - items:
+          - const: spacemit,k1-pwm
+          - const: marvell,pxa910-pwm
 
   reg:
     # Length should be 0x10
@@ -31,6 +35,9 @@ properties:
   clocks:
     maxItems: 1
 
+  resets:
+    maxItems: 1
+
 required:
   - compatible
   - reg
-- 
2.43.0

[PATCH v2 2/6] pwm: pxa: add optional reset control

[Guodong Xu]

Support optional reset control for the PWM PXA driver.

During the probe, it acquires the reset controller using
devm_reset_control_get_optional_exclusive_deasserted() to get and deassert
the reset controller to enable the PWM channel.

Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v2: No change

drivers/pwm/pwm-pxa.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index 430bd6a709e9..dd9c6af0f672 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -25,6 +25,7 @@
 #include <linux/io.h>
 #include <linux/pwm.h>
 #include <linux/of.h>
+#include <linux/reset.h>
 
 #include <asm/div64.h>
 
@@ -49,10 +50,10 @@ MODULE_DEVICE_TABLE(platform, pwm_id_table);
 #define PWMDCR_FD	(1 << 10)
 
 struct pxa_pwm_chip {
-	struct device	*dev;
-
-	struct clk	*clk;
-	void __iomem	*mmio_base;
+	struct device		*dev;
+	struct clk		*clk;
+	void __iomem		*mmio_base;
+	struct reset_control	*reset;
 };
 
 static inline struct pxa_pwm_chip *to_pxa_pwm_chip(struct pwm_chip *chip)
@@ -179,6 +180,11 @@ static int pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(pc->clk))
 		return PTR_ERR(pc->clk);
 
+	pc->reset = devm_reset_control_get_optional_exclusive_deasserted(
+			&pdev->dev, NULL);
+	if (IS_ERR(pc->reset))
+		return PTR_ERR(pc->reset);
+
 	chip->ops = &pxa_pwm_ops;
 
 	if (IS_ENABLED(CONFIG_OF))
-- 
2.43.0

[PATCH v2 3/6] riscv: dts: spacemit: add PWM support for K1 SoC

[Guodong Xu]

The SpacemiT K1 SoC features a PWM controller with 20 independent
channels. Add the corresponding 20 PWM nodes to the device tree.

Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v2: Changed compatible string with the fallback marvell,pxa910-pwm

arch/riscv/boot/dts/spacemit/k1.dtsi | 180 +++++++++++++++++++++++++++
 1 file changed, 180 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index c0cc4b99c935..e7dba623e877 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -556,5 +556,185 @@ sec_uart1: serial@f0612000 {
 			reg-io-width = <4>;
 			status = "reserved"; /* for TEE usage */
 		};
+
+		pwm0: pwm@d401a000 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd401a000 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM0>;
+			resets = <&syscon_apbc RESET_PWM0>;
+			status = "disabled";
+		};
+
+		pwm1: pwm@d401a400 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd401a400 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM1>;
+			resets = <&syscon_apbc RESET_PWM1>;
+			status = "disabled";
+		};
+
+		pwm2: pwm@d401a800 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd401a800 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM2>;
+			resets = <&syscon_apbc RESET_PWM2>;
+			status = "disabled";
+		};
+
+		pwm3: pwm@d401ac00 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd401ac00 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM3>;
+			resets = <&syscon_apbc RESET_PWM3>;
+			status = "disabled";
+		};
+
+		pwm4: pwm@d401b000 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd401b000 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM4>;
+			resets = <&syscon_apbc RESET_PWM4>;
+			status = "disabled";
+		};
+
+		pwm5: pwm@d401b400 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd401b400 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM5>;
+			resets = <&syscon_apbc RESET_PWM5>;
+			status = "disabled";
+		};
+
+		pwm6: pwm@d401b800 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd401b800 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM6>;
+			resets = <&syscon_apbc RESET_PWM6>;
+			status = "disabled";
+		};
+
+		pwm7: pwm@d401bc00 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd401bc00 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM7>;
+			resets = <&syscon_apbc RESET_PWM7>;
+			status = "disabled";
+		};
+
+		pwm8: pwm@d4020000 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4020000 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM8>;
+			resets = <&syscon_apbc RESET_PWM8>;
+			status = "disabled";
+		};
+
+		pwm9: pwm@d4020400 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4020400 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM9>;
+			resets = <&syscon_apbc RESET_PWM9>;
+			status = "disabled";
+		};
+
+		pwm10: pwm@d4020800 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4020800 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM10>;
+			resets = <&syscon_apbc RESET_PWM10>;
+			status = "disabled";
+		};
+
+		pwm11: pwm@d4020c00 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4020c00 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM11>;
+			resets = <&syscon_apbc RESET_PWM11>;
+			status = "disabled";
+		};
+
+		pwm12: pwm@d4021000 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4021000 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM12>;
+			resets = <&syscon_apbc RESET_PWM12>;
+			status = "disabled";
+		};
+
+		pwm13: pwm@d4021400 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4021400 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM13>;
+			resets = <&syscon_apbc RESET_PWM13>;
+			status = "disabled";
+		};
+
+		pwm14: pwm@d4021800 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4021800 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM14>;
+			resets = <&syscon_apbc RESET_PWM14>;
+			status = "disabled";
+		};
+
+		pwm15: pwm@d4021c00 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4021c00 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM15>;
+			resets = <&syscon_apbc RESET_PWM15>;
+			status = "disabled";
+		};
+
+		pwm16: pwm@d4022000 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4022000 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM16>;
+			resets = <&syscon_apbc RESET_PWM16>;
+			status = "disabled";
+		};
+
+		pwm17: pwm@d4022400 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4022400 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM17>;
+			resets = <&syscon_apbc RESET_PWM17>;
+			status = "disabled";
+		};
+
+		pwm18: pwm@d4022800 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4022800 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM18>;
+			resets = <&syscon_apbc RESET_PWM18>;
+			status = "disabled";
+		};
+
+		pwm19: pwm@d4022c00 {
+			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
+			reg = <0x0 0xd4022c00 0x0 0x10>;
+			#pwm-cells = <1>;
+			clocks = <&syscon_apbc CLK_PWM19>;
+			resets = <&syscon_apbc RESET_PWM19>;
+			status = "disabled";
+		};
 	};
 };
-- 
2.43.0

[PATCH v2 4/6] riscv: dts: spacemit: add pwm14_1 pinctrl setting

[Guodong Xu]

This patch adds the option 1 (hence the name pwm14_1) pinctrl
configuration for PWM14 on the SpacemiT K1 SoC.

PWM14 option 1 is used for PWM-based backlight control on MIPI displays.
This configuration is present on all existing K1 development boards,
such as the Banana Pi BPI-F3 and the Milk-V Jupiter.

For reference, a more complete list of PWM0-PWM19 pinctrl configurations
including all options can be found in [1].

Note: Since the corresponding functionality for other pins is not yet
in use or ready for upstreaming, this patch includes only the pwm14_1
setting.

[1] https://lore.kernel.org/all/20250411131423.3802611-7-guodong@riscstar.com/

Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v2: Discard pwm0-pwm19 pinctrl configurations, and adds only pwm14_1_cfg

arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
index 283663647a86..195eb8874f3c 100644
--- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
@@ -20,4 +20,11 @@ uart0-2-pins {
 			drive-strength = <32>;
 		};
 	};
+	pwm14_1_cfg: pwm14-1-cfg {
+		pwm14-1-pins {
+			pinmux = <K1_PADCONF(44, 4)>;
+			bias-pull-up = <0>;
+			drive-strength = <32>;
+		};
+	};
 };
-- 
2.43.0

[PATCH v2 5/6] pwm: Kconfig: add depends on ARCH_SPACEMIT to PWM_PXA

[Guodong Xu]

The SpacemiT K1 SoC uses PWM_PXA driver.

Update the Kconfig file for the PWM_PXA driver to allow
the SpacemiT K1 SoC to use it.

Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v2: No change

 drivers/pwm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 4731d5b90d7e..6731669e724e 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -493,7 +493,7 @@ config PWM_PCA9685
 
 config PWM_PXA
 	tristate "PXA PWM support"
-	depends on ARCH_PXA || ARCH_MMP || COMPILE_TEST
+	depends on ARCH_PXA || ARCH_MMP || ARCH_SPACEMIT || COMPILE_TEST
 	depends on HAS_IOMEM
 	help
 	  Generic PWM framework driver for PXA.
-- 
2.43.0

[PATCH v2 6/6] riscv: defconfig: Enable PWM support for SpacemiT K1 SoC

[Guodong Xu]

Enable CONFIG_PWM and CONFIG_PWM_PXA in the defconfig
to support the PWM controller used on the SpacemiT K1 SoC.

Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v2: Changed PWM_PXA from built-in to a loadable module (=m)

arch/riscv/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 4888529df1d8..8a8b77a0ac5a 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -259,6 +259,8 @@ CONFIG_RPMSG_CTRL=y
 CONFIG_RPMSG_VIRTIO=y
 CONFIG_PM_DEVFREQ=y
 CONFIG_IIO=y
+CONFIG_PWM=y
+CONFIG_PWM_PXA=m
 CONFIG_THEAD_C900_ACLINT_SSWI=y
 CONFIG_PHY_SUN4I_USB=m
 CONFIG_PHY_STARFIVE_JH7110_DPHY_RX=m
-- 
2.43.0

Re: [PATCH v2 3/6] riscv: dts: spacemit: add PWM support for K1 SoC

[kernel test robot]

Hi Guodong,

kernel test robot noticed the following build errors:

[auto build test ERROR on spacemit/for-next]
[also build test ERROR on spacemit/fixes robh/for-next linus/master v6.15-rc2 next-20250417]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Guodong-Xu/dt-bindings-pwm-marvell-pxa-pwm-Add-SpacemiT-K1-PWM-support/20250420-150635
base:   https://github.com/spacemit-com/linux for-next
patch link:    https://lore.kernel.org/r/20250420070251.378950-4-guodong%40riscstar.com
patch subject: [PATCH v2 3/6] riscv: dts: spacemit: add PWM support for K1 SoC
config: riscv-randconfig-001-20250420 (https://download.01.org/0day-ci/archive/20250420/202504201920.P8LBocr2-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250420/202504201920.P8LBocr2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504201920.P8LBocr2-lkp@intel.com/

All errors (new ones prefixed by >>):

>> Error: arch/riscv/boot/dts/spacemit/k1.dtsi:532.27-28 syntax error
   FATAL ERROR: Unable to parse input tree

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/6] dt-bindings: pwm: marvell,pxa-pwm: Add SpacemiT K1 PWM support

[Rob Herring (Arm)]

On Sun, 20 Apr 2025 15:02:46 +0800, Guodong Xu wrote:
> The SpacemiT K1 SoC reuses the Marvell PXA910-compatible PWM controller
> with one notable difference: the addition of a resets property. To make
> the device tree pass schema validation (make dtbs_check W=3), this patch
> updates the binding to accept spacemit,k1-pwm as a compatible string, when
> used in conjunction with the fallback marvell,pxa910-pwm.
> 
> Support for the optional resets property is also added, as it is required
> by the K1 integration but was not present in the original Marvell bindings.
> 
> Since the PWM reset line may be deasserted during the early bootloader
> stage, making the resets property optional avoids potential
> double-deassertion, which could otherwise cause flickering on displays
> that use PWM for backlight control.
> 
> Signed-off-by: Guodong Xu <guodong@riscstar.com>
> ---
> v2: Accept spacemit,k1-pwm as a compatible string, when used in conjunction
>     with the fallback marvell,pxa910-pwm
> 
> .../bindings/pwm/marvell,pxa-pwm.yaml           | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v2 2/6] pwm: pxa: add optional reset control

[Uwe Kleine-König]

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

Hello,

On Sun, Apr 20, 2025 at 03:02:47PM +0800, Guodong Xu wrote:
> @@ -49,10 +50,10 @@ MODULE_DEVICE_TABLE(platform, pwm_id_table);
>  #define PWMDCR_FD	(1 << 10)
>  
>  struct pxa_pwm_chip {
> -	struct device	*dev;
> -
> -	struct clk	*clk;
> -	void __iomem	*mmio_base;
> +	struct device		*dev;
> +	struct clk		*clk;
> +	void __iomem		*mmio_base;
> +	struct reset_control	*reset;

Changes like these are the reason I prefer to not align the member
names. Luckily reset is a write only variable and you can just drop this
hunk if you use a local variable for it in pwm_probe() below.

>  };
>  
>  static inline struct pxa_pwm_chip *to_pxa_pwm_chip(struct pwm_chip *chip)
> @@ -179,6 +180,11 @@ static int pwm_probe(struct platform_device *pdev)
>  	if (IS_ERR(pc->clk))
>  		return PTR_ERR(pc->clk);
>  
> +	pc->reset = devm_reset_control_get_optional_exclusive_deasserted(
> +			&pdev->dev, NULL);
> +	if (IS_ERR(pc->reset))
> +		return PTR_ERR(pc->reset);
> +
>  	chip->ops = &pxa_pwm_ops;
>  
>  	if (IS_ENABLED(CONFIG_OF))

Best regards
Uwe

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

Re: [PATCH v2 3/6] riscv: dts: spacemit: add PWM support for K1 SoC

[Uwe Kleine-König]

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

Hello,

On Sun, Apr 20, 2025 at 03:02:48PM +0800, Guodong Xu wrote:
> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index c0cc4b99c935..e7dba623e877 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -556,5 +556,185 @@ sec_uart1: serial@f0612000 {
>  			reg-io-width = <4>;
>  			status = "reserved"; /* for TEE usage */
>  		};
> +
> +		pwm0: pwm@d401a000 {
> +			compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
> +			reg = <0x0 0xd401a000 0x0 0x10>;
> +			#pwm-cells = <1>;

I want to make all pwms use #pwm-cells = <3> in the long run. Can you
please use that for the new binding? (Of course this needs adaption in
the binding doc, the code should already be prepared for that.)

> +			clocks = <&syscon_apbc CLK_PWM0>;
> +			resets = <&syscon_apbc RESET_PWM0>;
> +			status = "disabled";
> +		};

The error that the build bot reports happens (I think) because CLK_PWM0
isn't known.

Best regards
Uwe

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

Re: [PATCH v2 2/6] pwm: pxa: add optional reset control

[Guodong Xu]

On Thu, Apr 24, 2025 at 3:30 PM Uwe Kleine-König <ukleinek@kernel.org> wrote:
>
> Hello,
>
> On Sun, Apr 20, 2025 at 03:02:47PM +0800, Guodong Xu wrote:
> > @@ -49,10 +50,10 @@ MODULE_DEVICE_TABLE(platform, pwm_id_table);
> >  #define PWMDCR_FD    (1 << 10)
> >
> >  struct pxa_pwm_chip {
> > -     struct device   *dev;
> > -
> > -     struct clk      *clk;
> > -     void __iomem    *mmio_base;
> > +     struct device           *dev;
> > +     struct clk              *clk;
> > +     void __iomem            *mmio_base;
> > +     struct reset_control    *reset;
>
> Changes like these are the reason I prefer to not align the member
> names. Luckily reset is a write only variable and you can just drop this
> hunk if you use a local variable for it in pwm_probe() below.
>

Thanks Uwe for the suggestion, I agree.
I will update it in v3 by dropping this hunk and using a local variable for
reset in pwm_probe() as you recommended.

BR,
Guodong

> >  };
> >
> >  static inline struct pxa_pwm_chip *to_pxa_pwm_chip(struct pwm_chip *chip)
> > @@ -179,6 +180,11 @@ static int pwm_probe(struct platform_device *pdev)
> >       if (IS_ERR(pc->clk))
> >               return PTR_ERR(pc->clk);
> >
> > +     pc->reset = devm_reset_control_get_optional_exclusive_deasserted(
> > +                     &pdev->dev, NULL);
> > +     if (IS_ERR(pc->reset))
> > +             return PTR_ERR(pc->reset);
> > +
> >       chip->ops = &pxa_pwm_ops;
> >
> >       if (IS_ENABLED(CONFIG_OF))
>
> Best regards
> Uwe

Re: [PATCH v2 3/6] riscv: dts: spacemit: add PWM support for K1 SoC

[Guodong Xu]

On Thu, Apr 24, 2025 at 4:18 PM Uwe Kleine-König <ukleinek@kernel.org> wrote:
>
> Hello,
>
> On Sun, Apr 20, 2025 at 03:02:48PM +0800, Guodong Xu wrote:
> > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > index c0cc4b99c935..e7dba623e877 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > @@ -556,5 +556,185 @@ sec_uart1: serial@f0612000 {
> >                       reg-io-width = <4>;
> >                       status = "reserved"; /* for TEE usage */
> >               };
> > +
> > +             pwm0: pwm@d401a000 {
> > +                     compatible = "spacemit,k1-pwm", "marvell,pxa910-pwm";
> > +                     reg = <0x0 0xd401a000 0x0 0x10>;
> > +                     #pwm-cells = <1>;
>
> I want to make all pwms use #pwm-cells = <3> in the long run. Can you

Sure. I can do this.

> please use that for the new binding? (Of course this needs adaption in
> the binding doc, the code should already be prepared for that.)
>

I got what you mean. The code change for that is already integrated into
v6.15-rc1.
Commit 895fe4537cc8 ("pwm: Add upgrade path to #pwm-cells = <3> for users of
of_pwm_single_xlate()")

Now, if I change this #pwm-cells from <1> to <3>, without the dt-binding doc
changes, I would expect to see warnings (" #pwm-cells: 1 was expected") during
  make dtbs_check W=3

Any suggestions when the dt-binding changes will be merged?
or I can add your patch as a dependency.
https://lore.kernel.org/all/cb799d8a5bb284cd861785a691b8d5e329300d99.1738842938.git.u.kleine-koenig@baylibre.com/

> > +                     clocks = <&syscon_apbc CLK_PWM0>;
> > +                     resets = <&syscon_apbc RESET_PWM0>;
> > +                     status = "disabled";
> > +             };
>
> The error that the build bot reports happens (I think) because CLK_PWM0
> isn't known.
>

Yes, thanks for checking. This patchset depends on the clk and reset.
I will add them as prerequisite.

-Guodong


> Best regards
> Uwe

Re: [PATCH v2 3/6] riscv: dts: spacemit: add PWM support for K1 SoC

[Uwe Kleine-König]

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

Hello,

On Mon, Apr 28, 2025 at 08:46:50PM +0800, Guodong Xu wrote:
> On Thu, Apr 24, 2025 at 4:18 PM Uwe Kleine-König <ukleinek@kernel.org> wrote:
> > I want to make all pwms use #pwm-cells = <3> in the long run. Can you
> 
> Sure. I can do this.
> 
> > please use that for the new binding? (Of course this needs adaption in
> > the binding doc, the code should already be prepared for that.)
> >
> 
> I got what you mean. The code change for that is already integrated into
> v6.15-rc1.
> Commit 895fe4537cc8 ("pwm: Add upgrade path to #pwm-cells = <3> for users of
> of_pwm_single_xlate()")
> 
> Now, if I change this #pwm-cells from <1> to <3>, without the dt-binding doc
> changes, I would expect to see warnings (" #pwm-cells: 1 was expected") during
>   make dtbs_check W=3
> 
> Any suggestions when the dt-binding changes will be merged?
> or I can add your patch as a dependency.
> https://lore.kernel.org/all/cb799d8a5bb284cd861785a691b8d5e329300d99.1738842938.git.u.kleine-koenig@baylibre.com/

I don't want to merge this very soon given that 895fe4537cc8 isn't that
old yet. But I suggest you adapt patch #1 to require #pwm-cells = <3>
for the newly added compatible.

Best regards
Uwe

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

Re: [PATCH v2 3/6] riscv: dts: spacemit: add PWM support for K1 SoC

[Guodong Xu]

On Tue, Apr 29, 2025 at 12:32 AM Uwe Kleine-König <ukleinek@kernel.org> wrote:
>
> Hello,
>
> On Mon, Apr 28, 2025 at 08:46:50PM +0800, Guodong Xu wrote:
> > On Thu, Apr 24, 2025 at 4:18 PM Uwe Kleine-König <ukleinek@kernel.org> wrote:
> > > I want to make all pwms use #pwm-cells = <3> in the long run. Can you
> >
> > Sure. I can do this.
> >
> > > please use that for the new binding? (Of course this needs adaption in
> > > the binding doc, the code should already be prepared for that.)
> > >
> >
> > I got what you mean. The code change for that is already integrated into
> > v6.15-rc1.
> > Commit 895fe4537cc8 ("pwm: Add upgrade path to #pwm-cells = <3> for users of
> > of_pwm_single_xlate()")
> >
> > Now, if I change this #pwm-cells from <1> to <3>, without the dt-binding doc
> > changes, I would expect to see warnings (" #pwm-cells: 1 was expected") during
> >   make dtbs_check W=3
> >
> > Any suggestions when the dt-binding changes will be merged?
> > or I can add your patch as a dependency.
> > https://lore.kernel.org/all/cb799d8a5bb284cd861785a691b8d5e329300d99.1738842938.git.u.kleine-koenig@baylibre.com/
>
> I don't want to merge this very soon given that 895fe4537cc8 isn't that
> old yet. But I suggest you adapt patch #1 to require #pwm-cells = <3>
> for the newly added compatible.

ok, I will add a conditional check of #pwm-cells based on the compatible.
spacemit,k1-pwm must use 3; other compatibles use 1.
 (if:then:else in the bingding yaml)

Thanks.
Guodong

>
> Best regards
> Uwe