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-next>] [day] [month] [year] [list]
Message-ID: <3317829.AJdgDx1Vlc@diego>
Date: Sun, 11 May 2025 15:48:35 +0200
From: Heiko StĂĽbner <heiko@...ech.de>
To: Chukun Pan <amadeus@....edu.cn>
Cc: amadeus@....edu.cn, andyshrk@....com, conor+dt@...nel.org,
 damon.ding@...k-chips.com, devicetree@...r.kernel.org, didi.debian@...ow.org,
 dsimic@...jaro.org, jbx6244@...il.com, jing@...g.rocks, krzk+dt@...nel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, sebastian.reichel@...labora.com,
 ziyao@...root.org
Subject: Re: [PATCH v7 4/5] arm64: dts: rockchip: add core dtsi for RK3562 SoC

Am Sonntag, 11. Mai 2025, 14:00:09 Mitteleuropäische Sommerzeit schrieb Chukun Pan:
> > First of all, thanks for noticing all the bits and pieces to improve.
> > I di think I have now fixed up all the "regular" pieces you mentioned
> > and amended the commit accordingly:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?id=1d2f65fa98ddcafdfd1ebcdb87105141861b584a
> 
> Thanks a lot for the quick fix! It seems there is still a little problem:
> 
> > <snip>
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3562.dtsi
> > <snip>
> +	gpu_opp_table: opp-table-gpu {
> +		compatible = "operating-points-v2";
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +			opp-microvolt = <825000 825000 1000000>;
> +		};
> +		opp-400000000 {
> +		opp-hz = /bits/ 64 <400000000>;
> 
> This line is missing a tab.

fixed :-) .

> +			opp-microvolt = <825000 825000 1000000>;
> +		};
> > <snip>
> +		spi0: spi@...20000 {
> +			compatible = "rockchip,rk3562-spi", "rockchip,rk3066-spi";
> +			reg = <0x0 0xff220000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> 
> Also needed here.

I might be blind, but I don't see a tab missing here? #adress-cells and
#size-cells are in the same level of indentation as the other properties
of spi0? I did move the -cells down though now.


> > <snip>
> > +		pwm3: pwm@...30030 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff230030 0x0 0x10>;
> > +			#pwm-cells = <3>;
> 
> Missed here.

adapted like the other pwm-nodes

> > <snip>
> > +				power-domain@12 {
> > +					reg = <12>;
> > +					#power-domain-cells = <1>;
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > ...
> > +				power-domain@13 {
> > +					reg = <13>;
> > +					#power-domain-cells = <1>;
> 
> Does #power/#address/#size need to be put under pm_qos?

moved

> > <snip>
> > +		spi1: spi@...40000 {
> > +			compatible = "rockchip,rk3066-spi";
> > +			reg = <0x0 0xff640000 0x0 0x1000>;
> > +			interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > ...
> > +		spi2: spi@...50000 {
> > +			compatible = "rockchip,rk3066-spi";
> > +			reg = <0x0 0xff650000 0x0 0x1000>;
> > +			interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>;
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> 
> Same here.

moved

> 
> > <snip>
> > +		pwm4: pwm@...00000 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff700000 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm5: pwm@...00010 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff700010 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm6: pwm@...00020 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff700020 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm7: pwm@...00030 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff700030 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm8: pwm@...10000 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff710000 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm9: pwm@...10010 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff710010 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm10: pwm@...10020 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff710020 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm11: pwm@...10030 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff710030 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm12: pwm@...20000 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff720000 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm13: pwm@...20010 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff720010 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm14: pwm@...20020 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff720020 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> > ...
> > +		pwm15: pwm@...20030 {
> > +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > +			reg = <0x0 0xff720030 0x0 0x10>;
> > +			#pwm-cells = <3>;
> > +			pinctrl-names = "active";
> 
> pinctrl and #pwm-cells forgot to change.

hopefully caught all pwms now


> > <snip>
> > +		sdmmc0: mmc@...80000 {
> > +			compatible = "rockchip,rk3562-dw-mshc",
> > +				     "rockchip,rk3288-dw-mshc";
> > +			reg = <0x0 0xff880000 0x0 0x10000>;
> > +			interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
> > +			max-frequency = <200000000>;
> > ...
> > +		sdmmc1: mmc@...90000 {
> > +			compatible = "rockchip,rk3562-dw-mshc",
> > +				     "rockchip,rk3288-dw-mshc";
> > +			reg = <0x0 0xff890000 0x0 0x10000>;
> > +			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> > +			max-frequency = <200000000>;
> 
> max-frequency should be placed below clock

moved and also moved fifo-depth upwards between clock-names
and max-frequency


> > <snip>
> > +		saradc0: adc@...30000 {
> > +			compatible = "rockchip,rk3562-saradc";
> > +			reg = <0x0 0xff730000 0x0 0x100>;
> > +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > +			#io-channel-cells = <1>;
> > ...
> > +		saradc1: adc@...a0000 {
> > +			compatible = "rockchip,rk3562-saradc";
> > +			reg = <0x0 0xffaa0000 0x0 0x100>;
> > +			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
> > +			#io-channel-cells = <1>;
> 
> `#io-channel-cells` should be put above `status = "disabled";`

moved now :-)

Hopefully this now caught all the smallish issues.


Thanks a lot
Heiko



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ