[PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Chukun Pan]

Add pwm nodes for RK3528. Most rk3528 boards use pwm-regulator to
supply to CPU, add node to enable them. The PWM core on RK3528 is
the same as RK3328, but the driver doesn't support interrupts yet.

Unlike other SoCs, pinctrl-names need to be in "active" state,
I'm not sure about this, but otherwise the pwm-regulator will
not work properly.

---
vdd_arm: regulator-vdd-arm {
	compatible = "pwm-regulator";
	pwms = <&pwm1 0 5000 1>;
	pwm-supply = <&vcc5v0_sys>;
	regulator-name = "vdd_arm";
	regulator-min-microvolt = <746000>;
	regulator-max-microvolt = <1201000>;
	regulator-always-on;
	regulator-boot-on;
	regulator-settling-time-up-us = <250>;
};

&cpu0 {
	cpu-supply = <&vdd_arm>;
};

&pwm1 {
	status = "okay";
};
---

Chukun Pan (2):
  dt-bindings: pwm: rockchip: Add rockchip,rk3528-pwm
  arm64: dts: rockchip: Add pwm nodes for RK3528

 .../devicetree/bindings/pwm/pwm-rockchip.yaml |  1 +
 arch/arm64/boot/dts/rockchip/rk3528.dtsi      | 88 +++++++++++++++++++
 2 files changed, 89 insertions(+)

-- 
2.25.1

[PATCH 1/2] dt-bindings: pwm: rockchip: Add rockchip,rk3528-pwm

[Chukun Pan]

Document pwm compatible for rk3528 which is fallback compatible
of rk3328-pwm group.

Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
---
 Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
index e4e1976c542d..c8cdfb723336 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
@@ -30,6 +30,7 @@ properties:
           - enum:
               - rockchip,px30-pwm
               - rockchip,rk3308-pwm
+              - rockchip,rk3528-pwm
               - rockchip,rk3562-pwm
               - rockchip,rk3568-pwm
               - rockchip,rk3588-pwm
-- 
2.25.1

Re: [PATCH 1/2] dt-bindings: pwm: rockchip: Add rockchip,rk3528-pwm

[Conor Dooley]

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

On Fri, Mar 07, 2025 at 08:00:03PM +0800, Chukun Pan wrote:
> Document pwm compatible for rk3528 which is fallback compatible
> of rk3328-pwm group.
> 
> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

> ---
>  Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
> index e4e1976c542d..c8cdfb723336 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
> +++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.yaml
> @@ -30,6 +30,7 @@ properties:
>            - enum:
>                - rockchip,px30-pwm
>                - rockchip,rk3308-pwm
> +              - rockchip,rk3528-pwm
>                - rockchip,rk3562-pwm
>                - rockchip,rk3568-pwm
>                - rockchip,rk3588-pwm
> -- 
> 2.25.1
> 

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

Re: [PATCH 1/2] dt-bindings: pwm: rockchip: Add rockchip,rk3528-pwm

[Uwe Kleine-König]

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

Hello,

On Fri, Mar 07, 2025 at 08:00:03PM +0800, Chukun Pan wrote:
> Document pwm compatible for rk3528 which is fallback compatible
> of rk3328-pwm group.
> 
> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>

to prevent binding warnings it's probably sensible to let both patches
go in via the same tree at the same time. Feel free to take the binding
patch via rockchip/arm-soc.

Acked-by: Uwe Kleine-König <ukleinek@kernel.org>

Thanks
Uwe

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

Re: [PATCH 1/2] dt-bindings: pwm: rockchip: Add rockchip,rk3528-pwm

[Heiko Stuebner]

Hi Uwe,

Am Freitag, 7. März 2025, 18:07:47 MEZ schrieb Uwe Kleine-König:
> On Fri, Mar 07, 2025 at 08:00:03PM +0800, Chukun Pan wrote:
> > Document pwm compatible for rk3528 which is fallback compatible
> > of rk3328-pwm group.
> > 
> > Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
> 
> to prevent binding warnings it's probably sensible to let both patches
> go in via the same tree at the same time. Feel free to take the binding
> patch via rockchip/arm-soc.
> 
> Acked-by: Uwe Kleine-König <ukleinek@kernel.org>

You already have the rk3562 binding in your for-next branch [0], which
could create merge-conflicts later on.

This patch already contains it, so should fit neatly onto your branch.

And binding warnings only come from linux-next, so won't trigger as the
pwm will feed the binding there too :-)

So I guess it might be better to take the binding through the pwm tree
and me then picking up the dts patch.

For the binding
Reviewed-by: Heiko Stuebner <heiko@sntech.de>


[0] https://web.git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git/commit/?h=pwm/for-next&id=058210e84b48dbb670a6bf72afaed6fbd8043a37

Re: [PATCH 1/2] dt-bindings: pwm: rockchip: Add rockchip,rk3528-pwm

[Uwe Kleine-König]

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

Hey Heiko,

On Sat, Mar 08, 2025 at 05:25:09PM +0100, Heiko Stuebner wrote:
> Am Freitag, 7. März 2025, 18:07:47 MEZ schrieb Uwe Kleine-König:
> > On Fri, Mar 07, 2025 at 08:00:03PM +0800, Chukun Pan wrote:
> > > Document pwm compatible for rk3528 which is fallback compatible
> > > of rk3328-pwm group.
> > > 
> > > Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
> > 
> > to prevent binding warnings it's probably sensible to let both patches
> > go in via the same tree at the same time. Feel free to take the binding
> > patch via rockchip/arm-soc.
> > 
> > Acked-by: Uwe Kleine-König <ukleinek@kernel.org>
> 
> You already have the rk3562 binding in your for-next branch [0], which
> could create merge-conflicts later on.

Oh indeed.

> This patch already contains it, so should fit neatly onto your branch.
> 
> And binding warnings only come from linux-next, so won't trigger as the
> pwm will feed the binding there too :-)
> 
> So I guess it might be better to take the binding through the pwm tree
> and me then picking up the dts patch.

Fine for me, so I applied the first patch to

https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next

.

Best regards
Uwe

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

[PATCH 2/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Chukun Pan]

Add pwm nodes for RK3528. The PWM core on RK3528 is the same as
RK3328, but the driver does not support interrupts yet.

Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
---
 arch/arm64/boot/dts/rockchip/rk3528.dtsi | 88 ++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
index b1713ed4d7e2..ab1ac3273611 100644
--- a/arch/arm64/boot/dts/rockchip/rk3528.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
@@ -264,6 +264,94 @@ uart7: serial@ffa28000 {
 			status = "disabled";
 		};
 
+		pwm0: pwm@ffa90000 {
+			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";
+			reg = <0x0 0xffa90000 0x0 0x10>;
+			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
+			clock-names = "pwm", "pclk";
+			pinctrl-0 = <&pwm0m0_pins>;
+			pinctrl-names = "active";
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm1: pwm@ffa90010 {
+			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";
+			reg = <0x0 0xffa90010 0x0 0x10>;
+			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
+			clock-names = "pwm", "pclk";
+			pinctrl-0 = <&pwm1m0_pins>;
+			pinctrl-names = "active";
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm2: pwm@ffa90020 {
+			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";
+			reg = <0x0 0xffa90020 0x0 0x10>;
+			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
+			clock-names = "pwm", "pclk";
+			pinctrl-0 = <&pwm2m0_pins>;
+			pinctrl-names = "active";
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm3: pwm@ffa90030 {
+			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";
+			reg = <0x0 0xffa90030 0x0 0x10>;
+			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
+			clock-names = "pwm", "pclk";
+			pinctrl-0 = <&pwm3m0_pins>;
+			pinctrl-names = "active";
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm4: pwm@ffa98000 {
+			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";
+			reg = <0x0 0xffa98000 0x0 0x10>;
+			clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
+			clock-names = "pwm", "pclk";
+			pinctrl-0 = <&pwm4m0_pins>;
+			pinctrl-names = "active";
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm5: pwm@ffa98010 {
+			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";
+			reg = <0x0 0xffa98010 0x0 0x10>;
+			clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
+			clock-names = "pwm", "pclk";
+			pinctrl-0 = <&pwm5m0_pins>;
+			pinctrl-names = "active";
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm6: pwm@ffa98020 {
+			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";
+			reg = <0x0 0xffa98020 0x0 0x10>;
+			clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
+			clock-names = "pwm", "pclk";
+			pinctrl-0 = <&pwm6m0_pins>;
+			pinctrl-names = "active";
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm7: pwm@ffa98030 {
+			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";
+			reg = <0x0 0xffa98030 0x0 0x10>;
+			clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
+			clock-names = "pwm", "pclk";
+			pinctrl-0 = <&pwm7m0_pins>;
+			pinctrl-names = "active";
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		pinctrl: pinctrl {
 			compatible = "rockchip,rk3528-pinctrl";
 			rockchip,grf = <&ioc_grf>;
-- 
2.25.1

Re: [PATCH 2/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Jonas Karlman]

Hi Chukun,

On 2025-03-07 13:00, Chukun Pan wrote:
> Add pwm nodes for RK3528. The PWM core on RK3528 is the same as
> RK3328, but the driver does not support interrupts yet.
> 
> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
> ---
>  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 88 ++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> index b1713ed4d7e2..ab1ac3273611 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> @@ -264,6 +264,94 @@ uart7: serial@ffa28000 {
>  			status = "disabled";
>  		};
>  
> +		pwm0: pwm@ffa90000 {
> +			compatible = "rockchip,rk3528-pwm", "rockchip,rk3328-pwm";

nitpick: This could be split in two lines to match similar >80 chars
long lines in this file:

			compatible = "rockchip,rk3528-pwm",
				     "rockchip,rk3328-pwm";

And similar for the remaining pwmX.

Regards,
Jonas

> +			reg = <0x0 0xffa90000 0x0 0x10>;
> +			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
> +			clock-names = "pwm", "pclk";
> +			pinctrl-0 = <&pwm0m0_pins>;
> +			pinctrl-names = "active";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +

[snip]

Re: (subset) [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Heiko Stuebner]

On Fri, 07 Mar 2025 20:00:02 +0800, Chukun Pan wrote:
> Add pwm nodes for RK3528. Most rk3528 boards use pwm-regulator to
> supply to CPU, add node to enable them. The PWM core on RK3528 is
> the same as RK3328, but the driver doesn't support interrupts yet.
> 
> Unlike other SoCs, pinctrl-names need to be in "active" state,
> I'm not sure about this, but otherwise the pwm-regulator will
> not work properly.
> 
> [...]

Applied, thanks!

[2/2] arm64: dts: rockchip: Add pwm nodes for RK3528
      commit: 2973d077aedfc114affab96c3b2c7286163cc8c9

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: (subset) [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Jonas Karlman]

Hi Heiko,

On 2025-03-12 08:45, Heiko Stuebner wrote:
> 
> On Fri, 07 Mar 2025 20:00:02 +0800, Chukun Pan wrote:
>> Add pwm nodes for RK3528. Most rk3528 boards use pwm-regulator to
>> supply to CPU, add node to enable them. The PWM core on RK3528 is
>> the same as RK3328, but the driver doesn't support interrupts yet.
>>
>> Unlike other SoCs, pinctrl-names need to be in "active" state,
>> I'm not sure about this, but otherwise the pwm-regulator will
>> not work properly.
>>
>> [...]
> 
> Applied, thanks!

The pinctrl-names should be changed to "default" and not "active",
something you can fixup or do you want a patch?

In commit 96d8d3253246 ("arm64: dts: rockchip: Fix PWM pinctrl names")
similar issue was corrected for current rockchip boards.

Regards,
Jonas

> 
> [2/2] arm64: dts: rockchip: Add pwm nodes for RK3528
>       commit: 2973d077aedfc114affab96c3b2c7286163cc8c9
> 
> Best regards,

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Chukun Pan]

Hi,

> The pinctrl-names should be changed to "default" and not "active",
> something you can fixup or do you want a patch?

Sorry I've been a bit busy this week and forgot to send the v2 patch.
In rk3528.dtsi, the uart and upcoming i2c nodes do not have pinctrl,
so I prefer to remove them.

>> Unlike other SoCs, pinctrl-names need to be in "active" state,
>> I'm not sure about this, but otherwise the pwm-regulator will
>> not work properly.

BTW, setting the pinctrl of pwm corresponding to pwm-regulator
to "default" will cause kernel boot suspended.
Sorry but do you know why?

e.g.
```
vdd_arm: regulator-vdd-arm {
	compatible = "pwm-regulator";
	pwms = <&pwm1 0 5000 1>;
	...
};

&pwm1 {
	pinctrl-0 = <&pwm1m0_pins>;
	pinctrl-names = "default";
	status = "okay";
};
```

Thanks,
Chukun

--
2.25.1

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Jonas Karlman]

Hi Chukun,

On 2025-03-12 15:35, Chukun Pan wrote:
> Hi,
> 
>> The pinctrl-names should be changed to "default" and not "active",
>> something you can fixup or do you want a patch?
> 
> Sorry I've been a bit busy this week and forgot to send the v2 patch.
> In rk3528.dtsi, the uart and upcoming i2c nodes do not have pinctrl,
> so I prefer to remove them.
> 
>>> Unlike other SoCs, pinctrl-names need to be in "active" state,
>>> I'm not sure about this, but otherwise the pwm-regulator will
>>> not work properly.
> 
> BTW, setting the pinctrl of pwm corresponding to pwm-regulator
> to "default" will cause kernel boot suspended.
> Sorry but do you know why?

Not an issue I have seen, do you have any more logs or details? E.g.
what board you use, full regulator node, do you have operating points
defined etc.

I have runtime tested a branch at [1], that use pinctrl-names = default,
have vdd_arm and vdd_logic defined, also an opp table for cpu and gpu.

For E20C there is a commit to enable the vdd_logic, however without gpu
enabled and a mali-supply the pwm-regulator is initialized to
max-microvolt by Linux. Have instead updated U-Boot to initialize the
pwm-regulator's:

```
&vdd_arm {
	regulator-init-microvolt = <953000>;
};

&vdd_logic {
	regulator-init-microvolt = <900000>;
};
```

[1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/

Regards,
Jonas

> 
> e.g.
> ```
> vdd_arm: regulator-vdd-arm {
> 	compatible = "pwm-regulator";
> 	pwms = <&pwm1 0 5000 1>;
> 	...
> };
> 
> &pwm1 {
> 	pinctrl-0 = <&pwm1m0_pins>;
> 	pinctrl-names = "default";
> 	status = "okay";
> };
> ```
> 
> Thanks,
> Chukun
> 
> --
> 2.25.1
> 

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Heiko Stuebner]

Am Mittwoch, 12. März 2025, 16:00:00 MEZ schrieb Jonas Karlman:
> Hi Chukun,
> 
> On 2025-03-12 15:35, Chukun Pan wrote:
> > Hi,
> > 
> >> The pinctrl-names should be changed to "default" and not "active",
> >> something you can fixup or do you want a patch?

so yes of course the pinctrl needs to be default - simply because
that's the only pinctrl state mainline supports.

But judging by the fact that you're discussing working vs. non-working
below, can you please check if we should drop the patch for 6.15 till
that is solved?

Thanks a lot
Heiko

> > Sorry I've been a bit busy this week and forgot to send the v2 patch.
> > In rk3528.dtsi, the uart and upcoming i2c nodes do not have pinctrl,
> > so I prefer to remove them.
> > 
> >>> Unlike other SoCs, pinctrl-names need to be in "active" state,
> >>> I'm not sure about this, but otherwise the pwm-regulator will
> >>> not work properly.
> > 
> > BTW, setting the pinctrl of pwm corresponding to pwm-regulator
> > to "default" will cause kernel boot suspended.
> > Sorry but do you know why?
> 
> Not an issue I have seen, do you have any more logs or details? E.g.
> what board you use, full regulator node, do you have operating points
> defined etc.
> 
> I have runtime tested a branch at [1], that use pinctrl-names = default,
> have vdd_arm and vdd_logic defined, also an opp table for cpu and gpu.
> 
> For E20C there is a commit to enable the vdd_logic, however without gpu
> enabled and a mali-supply the pwm-regulator is initialized to
> max-microvolt by Linux. Have instead updated U-Boot to initialize the
> pwm-regulator's:
> 
> ```
> &vdd_arm {
> 	regulator-init-microvolt = <953000>;
> };
> 
> &vdd_logic {
> 	regulator-init-microvolt = <900000>;
> };
> ```
> 
> [1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/
> 
> Regards,
> Jonas
> 
> > 
> > e.g.
> > ```
> > vdd_arm: regulator-vdd-arm {
> > 	compatible = "pwm-regulator";
> > 	pwms = <&pwm1 0 5000 1>;
> > 	...
> > };
> > 
> > &pwm1 {
> > 	pinctrl-0 = <&pwm1m0_pins>;
> > 	pinctrl-names = "default";
> > 	status = "okay";
> > };
> > ```
> > 
> > Thanks,
> > Chukun
> > 
> > --
> > 2.25.1
> > 
> 
> 

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Chukun Pan]

Hi,

> so yes of course the pinctrl needs to be default - simply because
> that's the only pinctrl state mainline supports.

> But judging by the fact that you're discussing working vs. non-working
> below, can you please check if we should drop the patch for 6.15 till
> that is solved?

I suggest dropping this patch first, I will send v2
when this issue is solved.

Thanks,
Chukun

--
2.25.1

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Jonas Karlman]

Hi,

On 2025-03-13 08:10, Chukun Pan wrote:
> Hi,
> 
>> so yes of course the pinctrl needs to be default - simply because
>> that's the only pinctrl state mainline supports.
> 
>> But judging by the fact that you're discussing working vs. non-working
>> below, can you please check if we should drop the patch for 6.15 till
>> that is solved?
> 
> I suggest dropping this patch first, I will send v2
> when this issue is solved.

I noticed that the pwm nodes and saradc node ended up in wrong mmio
address order, so yes please drop the patch (or reorder nodes) ;-)

Chukun, please get back with more details about the issue you are having.
E.g. a stack trace, what board, DT and U-Boot you are using etc.

I have not seen any issue with PWM using the merged patch having
pinctrl-names=default.

Please see my Linux tree [1] and U-Boot tree [2], those are little ahead
of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp,
Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F
and Sige1.

Will try to post USB2.0 host and Hantro VPU patches later today.
CPU opp and GPU + opp depend on PWM and I was planning to post that
together with initial thermal support.

[1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/
[2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528

Regards,
Jonas

> 
> Thanks,
> Chukun
> 
> --
> 2.25.1
> 

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Chukun Pan]

Hi,

> I have not seen any issue with PWM using the merged patch having
> pinctrl-names=default.
>
> Please see my Linux tree [1] and U-Boot tree [2], those are little ahead
> of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp,
> Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F
> and Sige1.
>
> Please see my Linux tree [1] and U-Boot tree [2], those are little ahead
> of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp,
> Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F
> and Sige1.
> ...
> [1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/
> [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528

I tested your kernel device tree on E20C earlier today and still have
the same issue. But if I replace u-boot with the link [2] you provided,
it will work fine. For reference, I was using v2025.01 plus this series
of patches [3]. So it looks like u-boot does something and kernel doesn't?

[3] https://lore.kernel.org/u-boot/20250123224844.3104592-1-jonas@kwiboo.se/

Thanks,
Chukun

--
2.25.1

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Jonas Karlman]

Hi,

On 2025-03-13 10:01, Chukun Pan wrote:
> Hi,
> 
>> I have not seen any issue with PWM using the merged patch having
>> pinctrl-names=default.
>>
>> Please see my Linux tree [1] and U-Boot tree [2], those are little ahead
>> of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp,
>> Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F
>> and Sige1.
>>
>> Please see my Linux tree [1] and U-Boot tree [2], those are little ahead
>> of what has been posted on ML, e.g. it has working USB2.0 host, CPU opp,
>> Hantro VPU, GPU + opp, arm and logic pwm regulators for E20C, ROCK 2A/2F
>> and Sige1.
>> ...
>> [1] https://github.com/Kwiboo/linux-rockchip/commits/next-20250311-rk3528/
>> [2] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commits/rk3528
> 
> I tested your kernel device tree on E20C earlier today and still have
> the same issue. But if I replace u-boot with the link [2] you provided,
> it will work fine. For reference, I was using v2025.01 plus this series
> of patches [3]. So it looks like u-boot does something and kernel doesn't?

Interesting, good that it works with the updated U-Boot. Main change
compared to v1 is that it now use clock/reset id and DT closer to what
has been merged in mainline Linux. It also has DT params to help
initialize the two pwm regulators used by these boards.

I will try with the old v1 U-Boot series and see if I can replicated
your issue.

Regards,
Jonas

> 
> [3] https://lore.kernel.org/u-boot/20250123224844.3104592-1-jonas@kwiboo.se/
> 
> Thanks,
> Chukun
> 
> --
> 2.25.1
> 

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Chukun Pan]

Hi,

> Interesting, good that it works with the updated U-Boot. Main change
> compared to v1 is that it now use clock/reset id and DT closer to what
> has been merged in mainline Linux. It also has DT params to help
> initialize the two pwm regulators used by these boards.
>
> I will try with the old v1 U-Boot series and see if I can replicated
> your issue.

It is easy to reproduce this issue.
Make the following changes in the new series of u-boot:

```
--- a/configs/radxa-e20c-rk3528_defconfig
+++ b/configs/radxa-e20c-rk3528_defconfig
@@ -47,9 +47,7 @@ CONFIG_DM_MDIO=y
 CONFIG_DWC_ETH_QOS=y
 CONFIG_DWC_ETH_QOS_ROCKCHIP=y
 CONFIG_PHY_ROCKCHIP_INNO_USB2=y
-CONFIG_REGULATOR_PWM=y
 CONFIG_DM_REGULATOR_GPIO=y
-CONFIG_PWM_ROCKCHIP=y
 CONFIG_BAUDRATE=1500000
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550_MEM32=y
```

Or change dts:
```
--- a/arch/arm/dts/rk3528-radxa-e20c-u-boot.dtsi
+++ b/arch/arm/dts/rk3528-radxa-e20c-u-boot.dtsi
@@ -8,9 +8,9 @@
 };
 
 &vdd_arm {
-	regulator-init-microvolt = <953000>;
+	status = "disabled";
 };
 
 &vdd_logic {
-	regulator-init-microvolt = <900000>;
+	status = "disabled";
 };
```

Then the kernel will hang when loading the gpio driver:

[    0.162618] gpio gpiochip2: Static allocation of GPIO bae is deprecated, use dynamic allocation.
[    0.163558] rockchip-gpio ffb00000.gpio: probed /soc/pinctrl/gpio@ffb00000
[    0.164322] gpio gpiochip3: Static allocation of GPIO base is deprecated, use dynamic allocation.
[    0.165231] rockchip-gpio ffb10000.gpio: probed /soc/pinctrl/gpio@ffb10000
[    0.165977] gpio gpiochip4: Static allocation of GPIO base is deprecated, use dynamic allocation.
[    0.166886] rockhip-gpio ffb20000.gpio: probed /soc/pinctrl/gpio@ffb20000
[    0.170342] Internal error: Oos - Undefined instruction: 0000000002000000 [#1] SMP

Changing the debug level:
[    0.175260] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-14 to 0
[    0.175356] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-20 to 1
[    0.175968] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-14 to 0
[    0.176849] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-21 to 1
[    0.178453] rockchip-pinctrl soc:pinctrl: setting mux of GPIO4-13 to 0
(hang)

Thanks,
Chukun

--
2.25.1

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Chukun Pan]

Hi,

> Interesting, good that it works with the updated U-Boot. Main change
> compared to v1 is that it now use clock/reset id and DT closer to what
> has been merged in mainline Linux. It also has DT params to help
> initialize the two pwm regulators used by these boards.
>
> I will try with the old v1 U-Boot series and see if I can replicated
> your issue.

Hi, I found the reason. I did some experiments today:

pwm-regulator  pwm1 | pwm2 | pwm3
-----------------------------------
ArmSoM Sige1 |      | work | work |
Radxa E20C   | work | hang |      |
other 3528   | work | hang |      |

This looks weird, so I tested it further:

In the new series of u-boot:
```
--- a/arch/arm/dts/rk3528-radxa-e20c-u-boot.dtsi
+++ b/arch/arm/dts/rk3528-radxa-e20c-u-boot.dtsi
@@ -8,9 +8,9 @@
 };
 
 &vdd_arm {
-	regulator-init-microvolt = <953000>;
+	status = "disabled"; # Cancel changes, pwm works
 };
 
 &vdd_logic {
-	regulator-init-microvolt = <900000>;
+	status = "disabled"; # Cancel changes, kernel hang
 };
```

I looked at schematic and found an interesting thing:
1. ArmSoM Sige1 schematic: VDD_LOGIC Default 0.901V
2. Radxa E20C schematic: VDD_LOGIC No default voltage

Therefore, the default voltage of VDD_LOGIC (supply to
GPU and LOGIC) needs to be set to 0.9V when initializing.

This can be done by hardware or software (such as u-boot)
Should we support this in the kernel driver of regulator?

Thanks,
Chukun

--
2.25.1

Re: [PATCH 0/2] arm64: dts: rockchip: Add pwm nodes for RK3528

[Heiko Stuebner]

Hi,

Am Donnerstag, 13. März 2025, 08:10:31 MEZ schrieb Chukun Pan:
> Hi,
> 
> > so yes of course the pinctrl needs to be default - simply because
> > that's the only pinctrl state mainline supports.
> 
> > But judging by the fact that you're discussing working vs. non-working
> > below, can you please check if we should drop the patch for 6.15 till
> > that is solved?
> 
> I suggest dropping this patch first, I will send v2
> when this issue is solved.

I've dropped the patch now.


Heiko