Re: [PATCH 4/7] arm64: dts: rockchip: Add pinctrl and gpio nodes for RK3528

["Heiko Stübner" <heiko@sntech.de>]

Am Sonntag, 2. März 2025, 12:14:48 MEZ schrieb Jonas Karlman:
> Hi Yao Zi,
> 
> On 2025-02-28 11:46, Yao Zi wrote:
> > On Fri, Feb 28, 2025 at 06:40:10AM +0000, Jonas Karlman wrote:
> >> Add pinctrl and gpio nodes for RK3528 and import rk3528-pinctrl.dtsi
> >> from vendor linux-6.1-stan-rkr5 kernel with the hdmi-pins-idle node
> >> removed due to missing label reference to pcfg_output_low_pull_down.
> >>
> >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> >> ---
> >> This was mostly imported from vendor kernel, however the main commit [1]
> >> list 28 signed-off-by tags, unclear who I should use as author and what
> >> signed-off-by tags to include.
> >>
> >> [1] https://github.com/rockchip-linux/kernel/commit/c17d6325959f0ec1af901e8a17919163454190a2
> >> ---
> >>  .../boot/dts/rockchip/rk3528-pinctrl.dtsi     | 1397 +++++++++++++++++
> >>  arch/arm64/boot/dts/rockchip/rk3528.dtsi      |   82 +
> >>  2 files changed, 1479 insertions(+)
> >>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3528-pinctrl.dtsi
> >>
> > 
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3528.dtsi b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> index 0fb90f5c291c..d3e2a64ff2d5 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3528.dtsi
> >> @@ -4,8 +4,10 @@
> >>   * Copyright (c) 2024 Yao Zi <ziyao@disroot.org>
> >>   */
> >>  
> >> +#include <dt-bindings/gpio/gpio.h>
> >>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>  #include <dt-bindings/interrupt-controller/irq.h>
> >> +#include <dt-bindings/pinctrl/rockchip.h>
> >>  #include <dt-bindings/clock/rockchip,rk3528-cru.h>
> >>  #include <dt-bindings/reset/rockchip,rk3528-cru.h>
> >>  
> >> @@ -17,6 +19,11 @@ / {
> >>  	#size-cells = <2>;
> >>  
> >>  	aliases {
> >> +		gpio0 = &gpio0;
> >> +		gpio1 = &gpio1;
> >> +		gpio2 = &gpio2;
> >> +		gpio3 = &gpio3;
> >> +		gpio4 = &gpio4;
> >>  		serial0 = &uart0;
> >>  		serial1 = &uart1;
> >>  		serial2 = &uart2;
> >> @@ -166,6 +173,11 @@ cru: clock-controller@ff4a0000 {
> >>  			#reset-cells = <1>;
> >>  		};
> >>  
> >> +		ioc_grf: syscon@ff540000 {
> >> +			compatible = "rockchip,rk3528-ioc-grf", "syscon";
> >> +			reg = <0x0 0xff540000 0x0 0x40000>;
> >> +		};
> >> +
> >>  		uart0: serial@ff9f0000 {
> >>  			compatible = "rockchip,rk3528-uart", "snps,dw-apb-uart";
> >>  			reg = <0x0 0xff9f0000 0x0 0x100>;
> >> @@ -264,5 +276,75 @@ saradc: adc@ffae0000 {
> >>  			#io-channel-cells = <1>;
> >>  			status = "disabled";
> >>  		};
> >> +
> >> +		pinctrl: pinctrl {
> >> +			compatible = "rockchip,rk3528-pinctrl";
> >> +			rockchip,grf = <&ioc_grf>;
> >> +			#address-cells = <2>;
> >> +			#size-cells = <2>;
> >> +			ranges;
> > 
> > I doubt whether the pincontroller should be placed under simple-bus:
> > without a reg property, it doesn't look like a MMIO device.
> > 
> > Actually it is, although all the registers stay in the ioc grf. Maybe
> > it should be considered as child of the grf.
> 
> This follows how pinctrl was added for RK3576 and what is proposed for
> RK3562 [2]. I have too little knowledge to know if this needs to change
> or if this should follow similar SoCs.
> 
> [2] https://lore.kernel.org/r/20250227111913.2344207-15-kever.yang@rock-chips.com

The reg address shouldn't matter here I think.

The "soc"-bus describes the elements contained in the soc (surrounding the
cpu cores) and the pinctrl controller definitly is part of the soc itself.

So when looking at the scope, it does belong there and also the
 gpio-controller elements do have mmio addresses :-)


Heiko