Re: [PATCH 3/3] arm64: dts: rockchip: add dts for Ariaboard Photonicat RK3568

Chukun Pan
2024-09-05T03:57:44Z

Hi Junhao,

> ...
> --- /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

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

The official model name does not include 'RK3568'.

> ...
> + 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?

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

schematics: VCCA_1V8

> ...
+ 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

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

No need.

> ...
> + 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

> ...
> + 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

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

Are you sure this regulator is 5v?

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

schematics: VDDA_0V9

> + 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>;

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

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

phy-supply = <&vcc3v3_pi6c>;

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

These are actually pcie30x1m0_pins.

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

Maybe these can be moved to rk356x.dtsi?

> ...
> +&sdhci {

Missing mmc-hs200-1_8v;

> ...
> +&sdmmc0 {
> + bus-width = <4>;
> + cap-mmc-highspeed;

Why does sdcard need cap-mmc-highspeed?

> + max-frequency = <150000000>;
> + sd-uhs-sdr104;

The sdcard does not have 1.8v io voltage,
so this is wrong, please add no-1-8-v;

> +&sdmmc1 {
> + bus-width = <4>;
> + cap-sd-highspeed;
> + cap-sdio-irq;
> + disable-wp;

sdio wifi does not need disable-wp.

> + qca_wifi: qca-wifi@1 {
> + compatible = "qcom,ath10k";

ath10k does not need compatible.

> ...
> +&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";

--
2.25.1