Re: [PATCH 2/2] arm64: dts: rockchip: Add NanoPC T6

[Thomas McKahan <tmckahan@singleboardsolutions.com>]

On Mon, 7 Aug 2023 08:44:40 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 02/08/2023 07:14, Thomas McKahan wrote:
> > Add the NanoPC T6, a single board computer from FriendlyElec based on
> > the RK3588.
> > 
> > Initial device tree supports debug UART, SD, eMMC, PCIe 3, PMIC,
> > and 40 pin GPIO assignments.
> > 
> > Signed-off-by: Thomas McKahan <tmckahan@singleboardsolutions.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3588-nanopc-t6.dts    | 845 ++++++++++++++++++
> >  2 files changed, 846 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dts
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index 1ebbb3e9c2f9..e7728007fd1b 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -100,6 +100,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-rock-3a.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-io.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6b-io.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-evb1-v10.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-nanopc-t6.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-indiedroid-nova.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-khadas-edge2.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dts b/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dts
> > new file mode 100644
> > index 000000000000..2362da2c53d9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dts
> > @@ -0,0 +1,845 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
> > + * Copyright (c) 2023 Thomas McKahan
> > + *
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include <dt-bindings/usb/pd.h>
> > +#include "rk3588.dtsi"
> > +
> > +/ {
> > +	model = "FriendlyElec NanoPC-T6";
> > +	compatible = "friendlyarm,nanopc-t6", "rockchip,rk3588";
> > +

...

> > +&spi2 {
> > +	status = "okay";
> > +	assigned-clocks = <&cru CLK_SPI2>;
> > +	assigned-clock-rates = <200000000>;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&spi2m2_cs0 &spi2m2_pins>;
> > +	num-cs = <1>;
> > +
> > +	pmic@0 {
> > +		compatible = "rockchip,rk806";
> > +		spi-max-frequency = <1000000>;
> > +		reg = <0x0>;
> > +
> > +		interrupt-parent = <&gpio0>;
> > +		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pmic_pins>, <&rk806_dvs1_null>,
> > +			    <&rk806_dvs2_null>, <&rk806_dvs3_null>;
> > +
> > +		vcc1-supply = <&vcc4v0_sys>;
> > +		vcc2-supply = <&vcc4v0_sys>;
> > +		vcc3-supply = <&vcc4v0_sys>;
> > +		vcc4-supply = <&vcc4v0_sys>;
> > +		vcc5-supply = <&vcc4v0_sys>;
> > +		vcc6-supply = <&vcc4v0_sys>;
> > +		vcc7-supply = <&vcc4v0_sys>;
> > +		vcc8-supply = <&vcc4v0_sys>;
> > +		vcc9-supply = <&vcc4v0_sys>;
> > +		vcc10-supply = <&vcc4v0_sys>;
> > +		vcc11-supply = <&vcc_2v0_pldo_s3>;
> > +		vcc12-supply = <&vcc4v0_sys>;
> > +		vcc13-supply = <&vcc_1v1_nldo_s3>;
> > +		vcc14-supply = <&vcc_1v1_nldo_s3>;
> > +		vcca-supply = <&vcc4v0_sys>;
> > +
> > +		gpio-controller;
> > +		#gpio-cells = <2>;
> > +
> > +		rk806_dvs1_null: dvs1-null-pins {
> > +			pins = "gpio_pwrctrl2";
> > +			function = "pin_fun0";
> > +		};
> > +
> > +		rk806_dvs2_null: dvs2-null-pins {
> > +			pins = "gpio_pwrctrl2";
> > +			function = "pin_fun0";
> > +		};
> > +
> > +		rk806_dvs3_null: dvs3-null-pins {
> > +			pins = "gpio_pwrctrl3";
> > +			function = "pin_fun0";
> > +		};
> > +
> > +		regulators {
> > +			vdd_gpu_s0: vdd_gpu_mem_s0: dcdc-reg1 {
> > +				regulator-boot-on;  
> 
> Boolean properties are not first, but last. regulator-name is the first
> one. This odd style...
> 

I agree, however it seems the norm in Rockchip devices. This will
become an outlier in Rockchip but fall in line with the general case.
I'll put it in V2 with the other mentioned fixes unless a conflicting 
opinion is expressed. 

> > +				regulator-min-microvolt = <550000>;
> > +				regulator-max-microvolt = <950000>;
> > +				regulator-ramp-delay = <12500>;
> > +				regulator-name = "vdd_gpu_s0";
> > +				regulator-enable-ramp-delay = <400>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};  
> 
> 
> 
> Best regards,
> Krzysztof
> 

Best Regards,
Thomas

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip