linux-kernel - Re: [PATCH v2 1/2] arm64: dts: rockchip: Add pwm nodes for RK3528

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0d638134-0c0d-4918-af47-e23d2ead3bf3@kwiboo.se>
Date: Thu, 20 Mar 2025 00:26:14 +0100
From: Jonas Karlman <jonas@...boo.se>
To: Chukun Pan <amadeus@....edu.cn>, Heiko Stuebner <heiko@...ech.de>
Cc: Yao Zi <ziyao@...root.org>, Rob Herring <robh@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org
Subject: Re: [PATCH v2 1/2] arm64: dts: rockchip: Add pwm nodes for RK3528

Hi Chukun,

On 2025-03-18 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.

The device tree should describe the hardware, not what the driver
support, so interrupts should probably be included.

However, looking closer at TRM for i.e. RK3328, RK3568 and RK3588 it
look like the following description is not a true description of the
hardware.

Each PWM controller seem to support 4 channels, here (and for older RK
SoCs) we instead describe each channel and not the controller.

Maybe something like following would better represent the hardware:

	pwm0: pwm@...90000 {
		compatible = "rockchip,rk3528-pwm";
		reg = <0x0 0xffa90000 0x0 0x10000>;
		clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
		clock-names = "pwm", "pclk";
		interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
	};

	pwm1: pwm@...98000 {
		compatible = "rockchip,rk3528-pwm";
		reg = <0x0 0xffa98000 0x0 0x10000>;
		clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
		clock-names = "pwm", "pclk";
		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
	};

Regards,
Jonas

> 
> Signed-off-by: Chukun Pan <amadeus@....edu.cn>
> ---
>  arch/arm64/boot/dts/rockchip/rk3528.dtsi | 80 ++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> index 1af0d036cf32..621fc19ac0b3 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> @@ -465,6 +465,86 @@ uart7: serial@...28000 {
>  			status = "disabled";
>  		};
>  
> +		pwm0: pwm@...90000 {
> +			compatible = "rockchip,rk3528-pwm",
> +				     "rockchip,rk3328-pwm";
> +			reg = <0x0 0xffa90000 0x0 0x10>;
> +			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
> +			clock-names = "pwm", "pclk";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
> +		pwm1: pwm@...90010 {
> +			compatible = "rockchip,rk3528-pwm",
> +				     "rockchip,rk3328-pwm";
> +			reg = <0x0 0xffa90010 0x0 0x10>;
> +			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
> +			clock-names = "pwm", "pclk";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
> +		pwm2: pwm@...90020 {
> +			compatible = "rockchip,rk3528-pwm",
> +				     "rockchip,rk3328-pwm";
> +			reg = <0x0 0xffa90020 0x0 0x10>;
> +			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
> +			clock-names = "pwm", "pclk";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
> +		pwm3: pwm@...90030 {
> +			compatible = "rockchip,rk3528-pwm",
> +				     "rockchip,rk3328-pwm";
> +			reg = <0x0 0xffa90030 0x0 0x10>;
> +			clocks = <&cru CLK_PWM0>, <&cru PCLK_PWM0>;
> +			clock-names = "pwm", "pclk";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
> +		pwm4: pwm@...98000 {
> +			compatible = "rockchip,rk3528-pwm",
> +				     "rockchip,rk3328-pwm";
> +			reg = <0x0 0xffa98000 0x0 0x10>;
> +			clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
> +			clock-names = "pwm", "pclk";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
> +		pwm5: pwm@...98010 {
> +			compatible = "rockchip,rk3528-pwm",
> +				     "rockchip,rk3328-pwm";
> +			reg = <0x0 0xffa98010 0x0 0x10>;
> +			clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
> +			clock-names = "pwm", "pclk";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
> +		pwm6: pwm@...98020 {
> +			compatible = "rockchip,rk3528-pwm",
> +				     "rockchip,rk3328-pwm";
> +			reg = <0x0 0xffa98020 0x0 0x10>;
> +			clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
> +			clock-names = "pwm", "pclk";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
> +		pwm7: pwm@...98030 {
> +			compatible = "rockchip,rk3528-pwm",
> +				     "rockchip,rk3328-pwm";
> +			reg = <0x0 0xffa98030 0x0 0x10>;
> +			clocks = <&cru CLK_PWM1>, <&cru PCLK_PWM1>;
> +			clock-names = "pwm", "pclk";
> +			#pwm-cells = <3>;
> +			status = "disabled";
> +		};
> +
>  		saradc: adc@...e0000 {
>  			compatible = "rockchip,rk3528-saradc";
>  			reg = <0x0 0xffae0000 0x0 0x10000>;