On 2024/9/5 11:40, Chukun Pan wrote:

Hi Junhao,

quoted

...

--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3568-ariaboard-photonicat.dts

This should be 'rk3568-photonicat.dts',
e.g. "Radxa ROCK 3A" -> rk3568-rock-3a.dts

quoted

...
+	model = "Ariaboard Photonicat RK3568";
+	compatible = "ariaboard,photonicat", "rockchip,rk3568";

The official model name does not include 'RK3568'.

I will rename it.

quoted

...
+	firmware {
+		optee: optee {
+			compatible = "linaro,optee-tz";
+			method = "smc";
+		};
+	};
+
...
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		ramoops: ramoops@110000 {
+			compatible = "ramoops";
+			reg = <0 0x110000 0 0xf0000>;
+			console-size = <0x80000>;
+			ftrace-size = <0x00000>;
+			pmsg-size = <0x50000>;
+			record-size = <0x20000>;
+		};
+	};

Maybe these can be moved to rk356x.dtsi?

Yes, I will split them.

quoted

...
+	vcca1v8: regulator-1v8-vcca {

schematics: VCCA_1V8

quoted

...

+	vcc3v3_pcie: regulator-3v3-vcc-pcie {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pcie_enable_h>;

schematics: pcie_pwren_h
vcc_syson -> vcc3v3_pi6c
vcc_syson -> vcc3v3_pcie

quoted

+		regulator-always-on;
+		regulator-boot-on;

No need.

quoted

...
+	vcc5v0_sys: regulator-5v0-vcc-sys {

There is no vcc5v0_sys, but vcc_syson.

vcc_syson (5v) -> vcc3v3_sys
vcc_sysin (5v) - (mcu) -> vcc_syson
vccin_5v -> vcc_sysin

quoted

...
+	vcc5v0_usb_host: regulator-5v0-vcc-usb-host {

schematics: VCC5V0_USB30_OTG0 and usb_host_pwren_h
It's a little weird, but that's what they're calling it.
Also: VCCIN_5V -> VCC5V0_USB30_OTG0

quoted

...
+	vcc5v0_usb_modem: regulator-5v0-vcc-usb-modem {

Are you sure this regulator is 5v?

It should actually be 3.3V, I will fix it and rename to vcc3v3_usb_modem

quoted

...
+	vdda0v9: regulator-0v9-vdda {

schematics: VDDA_0V9

quoted

+	wifi_pwrseq: wifi-pwrseq {
+		compatible = "mmc-pwrseq-simple";
+		pinctrl-names = "default";
+		pinctrl-0 = <&wifi_enable_h>;

schematics: wifi_reg_on_h
Also you need to enable the clk:

		clocks = <&pmucru CLK_RTC_32K>;
		clock-names = "ext_clock";
		pinctrl-names = "default";
		pinctrl-0 = <&wifi_reg_on_h &clk32k_out1>;

quoted

+		post-power-on-delay-ms = <200>;
+		reset-gpios = <&gpio2 RK_PB1 GPIO_ACTIVE_LOW>;
+	};

quoted

...
+&pcie30phy {
+	phy-supply = <&vcc3v3_pcie>;

phy-supply = <&vcc3v3_pi6c>;

I will change it to vcc3v3_pi6c.
But there seems to be a warning here, maybe phy-supply is missing in rockchip,pcie3-phy.yaml?

/tmp/build/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dtb: phy@fe8c0000: 'phy-supply' does not match any of the regexes: 'pinctrl-[0-9]+'
        from schema $id: http://devicetree.org/schemas/phy/rockchip,pcie3-phy.yaml#

quoted

...
+&pcie3x2 {
+	max-link-speed = <1>;
+	num-lanes = <1>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie30x2m1_pins>;

These are actually pcie30x1m0_pins.

pcie30x1m0_pins seems to conflict with sdmmc0, I changed it to pcie30x1m1_pins

quoted

...
+&pmugrf {
+	reboot-mode {

Maybe these can be moved to rk356x.dtsi?

quoted

...
+&sdhci {

Missing mmc-hs200-1_8v;

[...]> 

ath10k does not need compatible.

I will remove them

quoted

...
+&uart1 {
...
+		clocks = <&pmucru CLK_RTC_32K>;
+		enable-gpios = <&gpio2 RK_PB7 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bt_enable_h>;

schematics: bt_reg_on_h
Missing clock-names = "lpo";

When I add clocks-name, check_dtbs gives me a warning

/tmp/build/arch/arm64/boot/dts/rockchip/rk3568-photonicat.dtb: bluetooth: 'clock-names' does not match any of the regexes: 'pinctrl-[0-9]+'
        from schema $id: http://devicetree.org/schemas/net/bluetooth/qualcomm-bluetooth.yaml#

Maybe clock-name is missing in qualcomm-bluetooth.yaml?

Thanks for your review, I will fix all problems in PATCH v2.

Best regards,
Junhao