[PATCH 2/2] arm64: dts: rockchip: add dts for Firefly Station P2 aka rk3568-roc-pc
Furkan Kardame
f.kardame at manjaro.org
Sat Jun 17 04:26:19 PDT 2023
Hello Krzysztof,
Thank you for spending your precious time on reviewing this patch.
My apologies for the silly mistakes, Thank you for the detailed review.
I will fix the patch and send as version-2 to all the latest maintainer.
It have been used in manjaro kernel since long time and some of the users
are already using the device with this patch.
It will be better if it can be tested by more developers and reviewed
before it gets merged.
Recently I fixed the warning before sending the patches, Build log can be
found here:
https://gitlab.manjaro.org/manjaro-arm/packages/core/linux-rc/-/jobs/12638/raw
On Saturday 17 June 2023 10:52:59 (+03:00), Krzysztof Kozlowski wrote:
> On 16/06/2023 23:10, Furkan Kardame wrote:
> > Add dts for Firefly Station P2.
> > Working IO:
> > * eMMC
> > * HDMI
> > * LAN
> > * LED
> > * SD Card
> > * UART
> > * USB2
> > * USB3
> >
> > Signed-off-by: Furkan Kardame
> > ---
> > arch/arm64/boot/dts/rockchip/Makefile | 1 +
> > .../arm64/boot/dts/rockchip/rk3568-roc-pc.dts | 689
++++++++++++++++++
> > 2 files changed, 690 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-roc-pc.dts
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile
b/arch/arm64/boot/dts/rockchip/Makefile
> > index 2d585bbb8..2085b00f3 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -90,6 +90,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-nanopi-r5c.dtb
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-nanopi-r5s.dtb
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-radxa-e25.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-roc-pc.dtb
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-rock-3a.dtb
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-edgeble-neu6a-io.dtb
> > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-evb1-v10.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-roc-pc.dts
b/arch/arm64/boot/dts/rockchip/rk3568-roc-pc.dts
> > new file mode 100644
> > index 000000000..5946ad3ea
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-roc-pc.dts
> > @@ -0,0 +1,689 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
> > + *
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include
> > +#include
> > +#include
> > +#include "rk3568.dtsi"
> > +
> > +/ {
> > + model = "Firefly Station P2";
> > + compatible = "firefly,rk3568-roc-pc", "rockchip,rk3568";
> > +
> > + aliases {
> > + ethernet0 = &gmac0;
> > + ethernet1 = &gmac1;
> > + mmc0 = &sdmmc0;
> > + mmc1 = &sdhci;
> > + };
> > +
> > + chosen: chosen {
> > + stdout-path = "serial2:1500000n8";
> > + };
> > +> + dc_12v: dc-12v {
>
> Node names should be generic. See also explanation and list of examples
> in DT specification:
>
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> So at least regulator prefix or suffix.
>
> > + compatible = "regulator-fixed";
> > + regulator-name = "dc_12v";
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <12000000>;
> > + regulator-max-microvolt = <12000000>;
> > + };
> > +
> > + gmac0_clkin: external-gmac0-clock {
> > + compatible = "fixed-clock";
> > + clock-frequency = <125000000>;
> > + clock-output-names = "gmac0_clkin";
> > + #clock-cells = <0>;
> > + };
> > +
> > + gmac1_clkin: external-gmac1-clock {
> > + compatible = "fixed-clock";
> > + clock-frequency = <125000000>;
> > + clock-output-names = "gmac1_clkin";
> > + #clock-cells = <0>;
> > + };
> > +
> > + leds {
> > + compatible = "gpio-leds";
> > +
> > + led-user {
> > + label = "user-led";
> > + default-state = "on";
> > + gpios = <&gpio1 RK_PB2 GPIO_ACTIVE_HIGH>;
> > + linux,default-trigger = "heartbeat";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&user_led_enable_h>;
> > + retain-state-suspended;
> > + };
> > + };
> > +
> > + hdmi-con {
> > + compatible = "hdmi-connector";
> > + type = "a";
> > +
> > + port {
> > + hdmi_con_in: endpoint {
> > + remote-endpoint = <&hdmi_out_con>;
> > + };
> > + };
> > + };
> > +
> > + sdio_pwrseq: sdio-pwrseq {
> > + status = "okay";
>
> Why do you need it?
Will be removed and will add it back later with wifi patch in future.
>
> > + compatible = "mmc-pwrseq-simple";
> > + clocks = <&rk809 1>;
> > + clock-names = "ext_clock";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&wifi_enable_h>;
> > + reset-gpios = <&gpio3 RK_PD5 GPIO_ACTIVE_LOW>;
> > + };
> > +
> > + pcie30_avdd0v9: pcie30-avdd0v9 {
>
> Same naming problem.
>
> > + compatible = "regulator-fixed";
> > + regulator-name = "pcie30_avdd0v9";
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <900000>;
> > + regulator-max-microvolt = <900000>;
> > + vin-supply = <&vcc3v3_sys>;
> > + };
> > +
> > + pcie30_avdd1v8: pcie30-avdd1v8 {
>
> Ditto
>
> > + compatible = "regulator-fixed";
> > + regulator-name = "pcie30_avdd1v8";
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <1800000>;
> > + vin-supply = <&vcc3v3_sys>;
> > + };
> > +
> > + vcc3v3_sys: vcc3v3-sys {
>
> Ditto
>
> > + compatible = "regulator-fixed";
> > + regulator-name = "vcc3v3_sys";
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + vin-supply = <&dc_12v>;
> > + };
> > +
> > + vcc3v3_pcie: gpio-regulator {
>
> Oh, suffix appeared.
>
> > + compatible = "regulator-fixed";
> > + regulator-name = "vcc3v3_pcie";
> > + enable-active-high;
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&vcc3v3_pcie_en_pin>;
> > + gpio = <&gpio0 RK_PD4 GPIO_ACTIVE_HIGH>;
> > + startup-delay-us = <5000>;
> > + vin-supply = <&vcc5v0_sys>;
> > + };
> > +
> > + vcc5v0_sys: vcc5v0-sys {
>
> And gone.
>
> > + compatible = "regulator-fixed";
> > + regulator-name = "vcc5v0_sys";
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + vin-supply = <&dc_12v>;
> > + };
> > +
> > + vcc5v0_usb: vcc5v0-usb {
> > + compatible = "regulator-fixed";
> > + regulator-name = "vcc5v0_usb";
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <5000000>;
> > + regulator-max-microvolt = <5000000>;
> > + vin-supply = <&vcc5v0_sys>;
> > + };
> > +
>
> ...
>
> > +
> > + vdda0v9_pmu: LDO_REG3 {
> > + regulator-name = "vdda0v9_pmu";
> > + regulator-always-on;
> > + regulator-boot-on;
> > + regulator-min-microvolt = <900000>;
> > + regulator-max-microvolt = <900000>;
> > +
> > + regulator-state-mem {
> > + regulator-on-in-suspend;
> > + regulator-suspend-microvolt = <900000>;
> > + };
>
> Fix indentation.
>
> > + };
> > +
> > + vccio_acodec: LDO_REG4 {
> > + regulator-name = "vccio_acodec";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > +
> > + regulator-state-mem {
> > + regulator-off-in-suspend;
> > + };
> > + };
> > +
>
> > +
> > + pcie {
> > + pcie_reset_pin: pcie-reset-pin {
> > + rockchip,pins = <2 RK_PD6 RK_FUNC_GPIO &pcfg_pull_none>;
> > + };
> > + vcc3v3_pcie_en_pin: vcc3v3-pcie-en-pin {
> > + rockchip,pins = <0 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
> > + };
> > + };
> > +
> > + pmic {
> > + pmic_int: pmic_int {
>
> No underscores in node names.
>
> Are you sure this does not cause `make dtsb_check` warnings?
>
> Best regards,
> Krzysztof
>
>
Regards,
Furkan.
More information about the linux-arm-kernel
mailing list