From: "Heiko Stübner" <heiko@sntech.de>
To: Chukun Pan <amadeus@jmu.edu.cn>
Cc: amadeus@jmu.edu.cn, andyshrk@163.com, conor+dt@kernel.org,
damon.ding@rock-chips.com, devicetree@vger.kernel.org,
didi.debian@cknow.org, dsimic@manjaro.org, jbx6244@gmail.com,
jing@jing.rocks, krzk+dt@kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
sebastian.reichel@collabora.com, ziyao@disroot.org
Subject: Re: [PATCH v7 4/5] arm64: dts: rockchip: add core dtsi for RK3562 SoC
Date: Sun, 11 May 2025 15:48:35 +0200 [thread overview]
Message-ID: <3317829.AJdgDx1Vlc@diego> (raw)
In-Reply-To: <20250511120009.37031-1-amadeus@jmu.edu.cn>
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@ff220000 {
> + 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@ff230030 {
> > + 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@ff640000 {
> > + 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@ff650000 {
> > + 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@ff700000 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff700000 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm5: pwm@ff700010 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff700010 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm6: pwm@ff700020 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff700020 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm7: pwm@ff700030 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff700030 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm8: pwm@ff710000 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff710000 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm9: pwm@ff710010 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff710010 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm10: pwm@ff710020 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff710020 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm11: pwm@ff710030 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff710030 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm12: pwm@ff720000 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff720000 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm13: pwm@ff720010 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff720010 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm14: pwm@ff720020 {
> > + compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> > + reg = <0x0 0xff720020 0x0 0x10>;
> > + #pwm-cells = <3>;
> > + pinctrl-names = "active";
> > ...
> > + pwm15: pwm@ff720030 {
> > + 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@ff880000 {
> > + 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@ff890000 {
> > + 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@ff730000 {
> > + compatible = "rockchip,rk3562-saradc";
> > + reg = <0x0 0xff730000 0x0 0x100>;
> > + interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > + #io-channel-cells = <1>;
> > ...
> > + saradc1: adc@ffaa0000 {
> > + 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
next prev parent reply other threads:[~2025-05-11 13:51 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-10 14:09 [PATCH] arm64: dts: rockchip: Fix PWM pinctrl names Yao Zi
2025-03-12 7:46 ` Heiko Stuebner
2025-05-11 10:00 ` [PATCH v7 4/5] arm64: dts: rockchip: add core dtsi for RK3562 SoC Chukun Pan
2025-05-11 10:14 ` Heiko Stübner
2025-05-11 12:00 ` Chukun Pan
2025-05-11 13:48 ` Heiko Stübner [this message]
2025-05-11 15:01 ` Chukun Pan
2025-05-11 15:32 ` Heiko Stübner
-- strict thread matches above, loose matches on Subject: below --
2025-05-09 10:23 [PATCH v7 0/5] rockchip: Add rk3562 SoC and evb support Kever Yang
2025-05-09 10:23 ` [PATCH v7 4/5] arm64: dts: rockchip: add core dtsi for RK3562 SoC Kever Yang
2025-05-16 7:01 ` Chukun Pan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3317829.AJdgDx1Vlc@diego \
--to=heiko@sntech.de \
--cc=amadeus@jmu.edu.cn \
--cc=andyshrk@163.com \
--cc=conor+dt@kernel.org \
--cc=damon.ding@rock-chips.com \
--cc=devicetree@vger.kernel.org \
--cc=didi.debian@cknow.org \
--cc=dsimic@manjaro.org \
--cc=jbx6244@gmail.com \
--cc=jing@jing.rocks \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=sebastian.reichel@collabora.com \
--cc=ziyao@disroot.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).