2024-04-04 08:35:38

by Sascha Hauer

[permalink] [raw]
Subject: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree

From: David Jander <[email protected]>

MECSBC is a single board computer for blood analysis machines from
RR-Mechatronics, designed and manufactured by Protonic Holland, based on
the Rockchip RK3568 SoC.

Signed-off-by: David Jander <[email protected]>
Signed-off-by: Sascha Hauer <[email protected]>
---
arch/arm64/boot/dts/rockchip/Makefile | 1 +
arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts | 394 +++++++++++++++++++++++++
2 files changed, 395 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index f906a868b71ac..1152e0f6a25cb 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -104,6 +104,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-qnap-ts433.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-mecsbc.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
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts b/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts
new file mode 100644
index 0000000000000..e50d135042ec7
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts
@@ -0,0 +1,394 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+#include <dt-bindings/pwm/pwm.h>
+#include "rk3568.dtsi"
+
+/ {
+ model = "Protonic MECSBC";
+ compatible = "prt,mecsbc", "rockchip,rk3568";
+
+ aliases {
+ mmc0 = &sdhci;
+ mmc1 = &sdmmc0;
+ };
+
+ chosen: chosen {
+ stdout-path = "serial2:1500000n8";
+ };
+
+ tas2562-sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,name = "Speaker";
+ simple-audio-card,mclk-fs = <256>;
+
+ simple-audio-card,cpu {
+ sound-dai = <&i2s1_8ch>;
+ };
+
+ simple-audio-card,codec {
+ sound-dai = <&tas2562>;
+ };
+ };
+
+ vdd_gpu: regulator-vdd-gpu {
+ compatible = "pwm-regulator";
+ pwms = <&pwm1 0 5000 PWM_POLARITY_INVERTED>;
+ regulator-name = "vdd_gpu";
+ regulator-min-microvolt = <915000>;
+ regulator-max-microvolt = <1000000>;
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-settling-time-up-us = <250>;
+ pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
+ };
+
+ vdd_npu: regulator-vdd-npu {
+ compatible = "pwm-regulator";
+ pwms = <&pwm2 0 5000 PWM_POLARITY_INVERTED>;
+ regulator-name = "vdd_npu";
+ regulator-min-microvolt = <915000>;
+ regulator-max-microvolt = <1000000>;
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-settling-time-up-us = <250>;
+ pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
+ };
+
+ p3v3: p3v3-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "p3v3";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ p1v8: p1v8-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "p1v8";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+};
+
+&combphy0 {
+ status = "okay";
+};
+
+&combphy1 {
+ status = "okay";
+};
+
+&combphy2 {
+ status = "okay";
+};
+
+&cpu0 {
+ cpu-supply = <&vdd_cpu>;
+};
+
+&cpu1 {
+ cpu-supply = <&vdd_cpu>;
+};
+
+&cpu2 {
+ cpu-supply = <&vdd_cpu>;
+};
+
+&cpu3 {
+ cpu-supply = <&vdd_cpu>;
+};
+
+&gmac1 {
+ assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
+ assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
+ phy-handle = <&rgmii_phy1>;
+ phy-mode = "rgmii";
+ clock_in_out = "output";
+ pinctrl-names = "default";
+ pinctrl-0 = <&gmac1m1_miim
+ &gmac1m1_tx_bus2
+ &gmac1m1_rx_bus2
+ &gmac1m1_rgmii_clk
+ &gmac1m1_clkinout
+ &gmac1m1_rgmii_bus>;
+ status = "okay";
+ tx_delay = <0x30>;
+ rx_delay = <0x10>;
+};
+
+&gpu {
+ mali-supply = <&vdd_gpu>;
+ status = "okay";
+};
+
+&i2c0 {
+ status = "okay";
+
+ vdd_cpu: regulator@60 {
+ compatible = "fcs,fan53555";
+ reg = <0x60>;
+ fcs,suspend-voltage-selector = <1>;
+ regulator-name = "vdd_cpu";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <1150000>;
+ regulator-ramp-delay = <2300>;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+};
+
+&i2c2 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2m0_xfer>;
+};
+
+&i2c3 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c3m0_xfer>;
+ status = "okay";
+
+ tas2562: tas2562@4c {
+ compatible = "ti,tas2562";
+ reg = <0x4c>;
+ #sound-dai-cells = <0>;
+ shutdown-gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
+ interrupt-parent = <&gpio1>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tas2562>;
+ interrupts = <RK_PD1 IRQ_TYPE_LEVEL_LOW>;
+ ti,imon-slot-no = <0>;
+ };
+};
+
+&i2c5 {
+ status = "okay";
+
+ tmp1075n@48 {
+ compatible = "ti,tmp1075";
+ reg = <0x48>;
+ };
+
+ pcf8563: rtc@51 {
+ compatible = "nxp,pcf85363";
+ reg = <0x51>;
+ #clock-cells = <0>;
+ clock-output-names = "rtcic_32kout";
+ };
+};
+
+&i2s1_8ch {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2s1m0_sclktx &i2s1m0_lrcktx &i2s1m0_sdi0 &i2s1m0_sdo0>;
+ rockchip,trcm-sync-tx-only;
+ status = "okay";
+};
+
+&mdio1 {
+ rgmii_phy1: ethernet-phy@2 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <0x2>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&eth_phy1_rst>;
+ reset-assert-us = <20000>;
+ reset-deassert-us = <100000>;
+ reset-gpios = <&gpio4 RK_PB3 GPIO_ACTIVE_LOW>;
+ };
+};
+
+&pcie2x1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie20m1_pins>;
+ reset-gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
+ status = "okay";
+};
+
+&pcie30phy {
+ status = "okay";
+};
+
+&pcie3x2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie30x2m1_pins>;
+ reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
+ vpcie3v3-supply = <&p3v3>;
+ status = "okay";
+};
+
+&pinctrl {
+ ethernet {
+ eth_phy1_rst: eth_phy1_rst {
+ rockchip,pins = <4 RK_PB3 RK_FUNC_GPIO &pcfg_pull_none>;
+ };
+ };
+
+ tas2562 {
+ pinctrl_tas2562: tas2562 {
+ rockchip,pins = <1 RK_PD4 RK_FUNC_GPIO &pcfg_pull_up>;
+ };
+ };
+};
+
+&pmu_io_domains {
+ pmuio1-supply = <&p3v3>;
+ pmuio2-supply = <&p3v3>;
+ vccio1-supply = <&p1v8>;
+ vccio2-supply = <&p1v8>;
+ vccio3-supply = <&p3v3>;
+ vccio4-supply = <&p1v8>;
+ vccio5-supply = <&p3v3>;
+ vccio6-supply = <&p1v8>;
+ vccio7-supply = <&p3v3>;
+ status = "okay";
+};
+
+&saradc {
+ vref-supply = <&p1v8>;
+ status = "okay";
+};
+
+&sdhci {
+ bus-width = <8>;
+ max-frequency = <200000000>;
+ non-removable;
+ pinctrl-names = "default";
+ pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe>;
+ vmmc-supply = <&p3v3>;
+ vqmmc-supply = <&p1v8>;
+ mmc-hs200-1_8v;
+ non-removable;
+ no-sd;
+ no-sdio;
+ status = "okay";
+};
+
+&sdmmc0 {
+ bus-width = <4>;
+ cap-sd-highspeed;
+ cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>;
+ disable-wp;
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>;
+ sd-uhs-sdr50;
+ vmmc-supply = <&p3v3>;
+ vqmmc-supply = <&p3v3>;
+ status = "okay";
+};
+
+&tsadc {
+ rockchip,hw-tshut-mode = <1>;
+ rockchip,hw-tshut-polarity = <0>;
+ status = "okay";
+};
+
+&uart2 {
+ status = "okay";
+};
+
+&usb_host0_ehci {
+ status = "okay";
+};
+
+&usb_host0_ohci {
+ status = "okay";
+};
+
+&usb_host0_xhci {
+ extcon = <&usb2phy0>;
+ status = "okay";
+ dr_mode = "host";
+};
+
+&usb_host1_ehci {
+ status = "okay";
+};
+
+&usb_host1_ohci {
+ status = "okay";
+};
+
+&usb_host1_xhci {
+ status = "okay";
+};
+
+&usb2phy0 {
+ status = "okay";
+};
+
+&usb2phy0_host {
+ status = "okay";
+};
+
+&usb2phy0_otg {
+ status = "okay";
+};
+
+&usb2phy1 {
+ status = "okay";
+};
+
+&usb2phy1_host {
+ status = "okay";
+};
+
+&usb2phy1_otg {
+ status = "okay";
+};
+
+&pwm1 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm1m0_pins>;
+};
+
+&pwm2 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm2m0_pins>;
+};
+
+&gpu_opp_table {
+ compatible = "operating-points-v2";
+
+ opp-200000000 {
+ opp-hz = /bits/ 64 <200000000>;
+ opp-microvolt = <915000>;
+ };
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ opp-microvolt = <915000>;
+ };
+
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ opp-microvolt = <915000>;
+ };
+
+ opp-600000000 {
+ opp-hz = /bits/ 64 <600000000>;
+ opp-microvolt = <920000>;
+ };
+
+ opp-700000000 {
+ opp-hz = /bits/ 64 <700000000>;
+ opp-microvolt = <950000>;
+ };
+
+ opp-800000000 {
+ opp-hz = /bits/ 64 <800000000>;
+ opp-microvolt = <1000000>;
+ };
+};

--
2.39.2



2024-04-04 08:50:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree

On 04/04/2024 10:34, Sascha Hauer wrote:
> From: David Jander <[email protected]>
>
> MECSBC is a single board computer for blood analysis machines from
> RR-Mechatronics, designed and manufactured by Protonic Holland, based on
> the Rockchip RK3568 SoC.
>
> Signed-off-by: David Jander <[email protected]>
> Signed-off-by: Sascha Hauer <[email protected]>

..

> + vdd_gpu: regulator-vdd-gpu {
> + compatible = "pwm-regulator";
> + pwms = <&pwm1 0 5000 PWM_POLARITY_INVERTED>;
> + regulator-name = "vdd_gpu";
> + regulator-min-microvolt = <915000>;
> + regulator-max-microvolt = <1000000>;
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-settling-time-up-us = <250>;
> + pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
> + };
> +
> + vdd_npu: regulator-vdd-npu {
> + compatible = "pwm-regulator";
> + pwms = <&pwm2 0 5000 PWM_POLARITY_INVERTED>;
> + regulator-name = "vdd_npu";
> + regulator-min-microvolt = <915000>;
> + regulator-max-microvolt = <1000000>;
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-settling-time-up-us = <250>;
> + pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
> + };
> +
> + p3v3: p3v3-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "p3v3";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + p1v8: p1v8-regulator {

Please keep consistent naming - your other regulators are
"regulator-foo", not "foo-regulator". The "regulator-foo" is preferred
usually, because it groups devices nicely.

> + compatible = "regulator-fixed";
> + regulator-name = "p1v8";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };


..

> +&i2c3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c3m0_xfer>;
> + status = "okay";
> +
> + tas2562: tas2562@4c {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

e.g. audio-codec, speaker, amplifier

> + compatible = "ti,tas2562";
> + reg = <0x4c>;
> + #sound-dai-cells = <0>;
> + shutdown-gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
> + interrupt-parent = <&gpio1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_tas2562>;
> + interrupts = <RK_PD1 IRQ_TYPE_LEVEL_LOW>;
> + ti,imon-slot-no = <0>;
> + };
> +};
> +
> +&i2c5 {
> + status = "okay";
> +
> + tmp1075n@48 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> + compatible = "ti,tmp1075";
> + reg = <0x48>;
> + };
> +
> + pcf8563: rtc@51 {
> + compatible = "nxp,pcf85363";
> + reg = <0x51>;
> + #clock-cells = <0>;
> + clock-output-names = "rtcic_32kout";
> + };
> +};
> +

..

> +&pcie3x2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie30x2m1_pins>;
> + reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
> + vpcie3v3-supply = <&p3v3>;
> + status = "okay";
> +};
> +
> +&pinctrl {
> + ethernet {
> + eth_phy1_rst: eth_phy1_rst {

No underscores in node names.



Best regards,
Krzysztof


2024-04-04 09:12:43

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree

Hi Sascha,

Am Donnerstag, 4. April 2024, 10:34:40 CEST schrieb Sascha Hauer:
> From: David Jander <[email protected]>
>
> MECSBC is a single board computer for blood analysis machines from
> RR-Mechatronics, designed and manufactured by Protonic Holland, based on
> the Rockchip RK3568 SoC.
>
> Signed-off-by: David Jander <[email protected]>
> Signed-off-by: Sascha Hauer <[email protected]>
> ---
> arch/arm64/boot/dts/rockchip/Makefile | 1 +
> arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts | 394 +++++++++++++++++++++++++
> 2 files changed, 395 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index f906a868b71ac..1152e0f6a25cb 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -104,6 +104,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-qnap-ts433.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-mecsbc.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-radxa-e25.dtb

alphabetical sorting of entries please

> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-roc-pc.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-rock-3a.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts b/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts
> new file mode 100644
> index 0000000000000..e50d135042ec7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts
> @@ -0,0 +1,394 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/pwm/pwm.h>
> +#include "rk3568.dtsi"
> +
> +/ {
> + model = "Protonic MECSBC";
> + compatible = "prt,mecsbc", "rockchip,rk3568";
> +
> + aliases {
> + mmc0 = &sdhci;
> + mmc1 = &sdmmc0;
> + };
> +
> + chosen: chosen {
> + stdout-path = "serial2:1500000n8";
> + };
> +
> + tas2562-sound {
> + compatible = "simple-audio-card";
> + simple-audio-card,format = "i2s";
> + simple-audio-card,name = "Speaker";
> + simple-audio-card,mclk-fs = <256>;
> +
> + simple-audio-card,cpu {
> + sound-dai = <&i2s1_8ch>;
> + };
> +
> + simple-audio-card,codec {
> + sound-dai = <&tas2562>;
> + };
> + };
> +
> + vdd_gpu: regulator-vdd-gpu {
> + compatible = "pwm-regulator";
> + pwms = <&pwm1 0 5000 PWM_POLARITY_INVERTED>;
> + regulator-name = "vdd_gpu";
> + regulator-min-microvolt = <915000>;
> + regulator-max-microvolt = <1000000>;
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-settling-time-up-us = <250>;
> + pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
> + };
> +
> + vdd_npu: regulator-vdd-npu {
> + compatible = "pwm-regulator";
> + pwms = <&pwm2 0 5000 PWM_POLARITY_INVERTED>;
> + regulator-name = "vdd_npu";
> + regulator-min-microvolt = <915000>;
> + regulator-max-microvolt = <1000000>;
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-settling-time-up-us = <250>;
> + pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
> + };
> +
> + p3v3: p3v3-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "p3v3";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + p1v8: p1v8-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "p1v8";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };

please sort alphabetical by node-name

> +};
> +
> +&combphy0 {
> + status = "okay";
> +};
> +
> +&combphy1 {
> + status = "okay";
> +};
> +
> +&combphy2 {
> + status = "okay";
> +};
> +
> +&cpu0 {
> + cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu1 {
> + cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu2 {
> + cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu3 {
> + cpu-supply = <&vdd_cpu>;
> +};
> +
> +&gmac1 {
> + assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> + assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
> + phy-handle = <&rgmii_phy1>;
> + phy-mode = "rgmii";
> + clock_in_out = "output";
> + pinctrl-names = "default";
> + pinctrl-0 = <&gmac1m1_miim
> + &gmac1m1_tx_bus2
> + &gmac1m1_rx_bus2
> + &gmac1m1_rgmii_clk
> + &gmac1m1_clkinout
> + &gmac1m1_rgmii_bus>;
> + status = "okay";
> + tx_delay = <0x30>;
> + rx_delay = <0x10>;
> +};
> +
> +&gpu {
> + mali-supply = <&vdd_gpu>;
> + status = "okay";
> +};
> +
> +&i2c0 {
> + status = "okay";
> +
> + vdd_cpu: regulator@60 {
> + compatible = "fcs,fan53555";
> + reg = <0x60>;
> + fcs,suspend-voltage-selector = <1>;
> + regulator-name = "vdd_cpu";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <800000>;
> + regulator-max-microvolt = <1150000>;
> + regulator-ramp-delay = <2300>;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +};
> +
> +&i2c2 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2m0_xfer>;
> +};
> +
> +&i2c3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c3m0_xfer>;
> + status = "okay";
> +
> + tas2562: tas2562@4c {
> + compatible = "ti,tas2562";
> + reg = <0x4c>;
> + #sound-dai-cells = <0>;
> + shutdown-gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
> + interrupt-parent = <&gpio1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_tas2562>;
> + interrupts = <RK_PD1 IRQ_TYPE_LEVEL_LOW>;
> + ti,imon-slot-no = <0>;
> + };
> +};
> +
> +&i2c5 {
> + status = "okay";
> +
> + tmp1075n@48 {
> + compatible = "ti,tmp1075";
> + reg = <0x48>;
> + };
> +
> + pcf8563: rtc@51 {
> + compatible = "nxp,pcf85363";
> + reg = <0x51>;
> + #clock-cells = <0>;
> + clock-output-names = "rtcic_32kout";
> + };
> +};
> +
> +&i2s1_8ch {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2s1m0_sclktx &i2s1m0_lrcktx &i2s1m0_sdi0 &i2s1m0_sdo0>;
> + rockchip,trcm-sync-tx-only;
> + status = "okay";
> +};
> +
> +&mdio1 {
> + rgmii_phy1: ethernet-phy@2 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <0x2>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&eth_phy1_rst>;
> + reset-assert-us = <20000>;
> + reset-deassert-us = <100000>;
> + reset-gpios = <&gpio4 RK_PB3 GPIO_ACTIVE_LOW>;
> + };
> +};
> +
> +&pcie2x1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie20m1_pins>;
> + reset-gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
> + status = "okay";
> +};
> +
> +&pcie30phy {
> + status = "okay";
> +};
> +
> +&pcie3x2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie30x2m1_pins>;
> + reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
> + vpcie3v3-supply = <&p3v3>;
> + status = "okay";
> +};
> +
> +&pinctrl {
> + ethernet {
> + eth_phy1_rst: eth_phy1_rst {
> + rockchip,pins = <4 RK_PB3 RK_FUNC_GPIO &pcfg_pull_none>;
> + };
> + };
> +
> + tas2562 {
> + pinctrl_tas2562: tas2562 {
> + rockchip,pins = <1 RK_PD4 RK_FUNC_GPIO &pcfg_pull_up>;
> + };
> + };
> +};
> +
> +&pmu_io_domains {
> + pmuio1-supply = <&p3v3>;
> + pmuio2-supply = <&p3v3>;
> + vccio1-supply = <&p1v8>;
> + vccio2-supply = <&p1v8>;
> + vccio3-supply = <&p3v3>;
> + vccio4-supply = <&p1v8>;
> + vccio5-supply = <&p3v3>;
> + vccio6-supply = <&p1v8>;
> + vccio7-supply = <&p3v3>;
> + status = "okay";
> +};
> +
> +&saradc {
> + vref-supply = <&p1v8>;
> + status = "okay";
> +};
> +
> +&sdhci {
> + bus-width = <8>;
> + max-frequency = <200000000>;
> + non-removable;
> + pinctrl-names = "default";
> + pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe>;
> + vmmc-supply = <&p3v3>;
> + vqmmc-supply = <&p1v8>;
> + mmc-hs200-1_8v;
> + non-removable;
> + no-sd;
> + no-sdio;
> + status = "okay";
> +};
> +
> +&sdmmc0 {
> + bus-width = <4>;
> + cap-sd-highspeed;
> + cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>;
> + disable-wp;
> + pinctrl-names = "default";
> + pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>;
> + sd-uhs-sdr50;
> + vmmc-supply = <&p3v3>;
> + vqmmc-supply = <&p3v3>;
> + status = "okay";
> +};
> +
> +&tsadc {
> + rockchip,hw-tshut-mode = <1>;
> + rockchip,hw-tshut-polarity = <0>;
> + status = "okay";
> +};
> +
> +&uart2 {
> + status = "okay";
> +};
> +
> +&usb_host0_ehci {
> + status = "okay";
> +};
> +
> +&usb_host0_ohci {
> + status = "okay";
> +};
> +
> +&usb_host0_xhci {
> + extcon = <&usb2phy0>;
> + status = "okay";
> + dr_mode = "host";

please sort properties alphabetical, with
compatible at the top and status last.


> +};
> +
> +&usb_host1_ehci {
> + status = "okay";
> +};
> +
> +&usb_host1_ohci {
> + status = "okay";
> +};
> +
> +&usb_host1_xhci {
> + status = "okay";
> +};
> +
> +&usb2phy0 {
> + status = "okay";
> +};
> +
> +&usb2phy0_host {
> + status = "okay";
> +};
> +
> +&usb2phy0_otg {
> + status = "okay";
> +};
> +
> +&usb2phy1 {
> + status = "okay";
> +};
> +
> +&usb2phy1_host {
> + status = "okay";
> +};
> +
> +&usb2phy1_otg {
> + status = "okay";
> +};
> +
> +&pwm1 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwm1m0_pins>;
> +};
> +
> +&pwm2 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwm2m0_pins>;
> +};

please sort phandles "&pwm2" alphabetical and status comes last


> +
> +&gpu_opp_table {
> + compatible = "operating-points-v2";
> +
> + opp-200000000 {
> + opp-hz = /bits/ 64 <200000000>;
> + opp-microvolt = <915000>;
> + };
> +
> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + opp-microvolt = <915000>;
> + };
> +
> + opp-400000000 {
> + opp-hz = /bits/ 64 <400000000>;
> + opp-microvolt = <915000>;
> + };
> +
> + opp-600000000 {
> + opp-hz = /bits/ 64 <600000000>;
> + opp-microvolt = <920000>;
> + };
> +
> + opp-700000000 {
> + opp-hz = /bits/ 64 <700000000>;
> + opp-microvolt = <950000>;
> + };
> +
> + opp-800000000 {
> + opp-hz = /bits/ 64 <800000000>;
> + opp-microvolt = <1000000>;
> + };
> +};

a comment would be nice, why the OPPs get changed


Heiko



2024-04-04 15:26:52

by Diederik de Haas

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree

On Thursday, 4 April 2024 17:10:41 CEST Andrew Lunn wrote:
> > +&gmac1 {
> > + assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> > + assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru
> > CLK_MAC1_2TOP>; + phy-handle = <&rgmii_phy1>;
> > + phy-mode = "rgmii";
> > + clock_in_out = "output";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&gmac1m1_miim
> > + &gmac1m1_tx_bus2
> > + &gmac1m1_rx_bus2
> > + &gmac1m1_rgmii_clk
> > + &gmac1m1_clkinout
> > + &gmac1m1_rgmii_bus>;
> > + status = "okay";
> > + tx_delay = <0x30>;
> > + rx_delay = <0x10>;
> > +};
>
> There was a discussion about phy-mode = "rgmii"; and these
> tx/rx_delays last month. Please could you go read that discussion and
> them make use of rgmii-id, and change the delays.

https://lore.kernel.org/linux-rockchip/[email protected]/
titled "[PATCH] arm64: dts: rockchip: qnap-ts433: Simplify network PHY connection"


Attachments:
signature.asc (235.00 B)
This is a digitally signed message part.

2024-04-04 15:45:47

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree

> +&gmac1 {
> + assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> + assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
> + phy-handle = <&rgmii_phy1>;
> + phy-mode = "rgmii";
> + clock_in_out = "output";
> + pinctrl-names = "default";
> + pinctrl-0 = <&gmac1m1_miim
> + &gmac1m1_tx_bus2
> + &gmac1m1_rx_bus2
> + &gmac1m1_rgmii_clk
> + &gmac1m1_clkinout
> + &gmac1m1_rgmii_bus>;
> + status = "okay";
> + tx_delay = <0x30>;
> + rx_delay = <0x10>;
> +};

There was a discussion about phy-mode = "rgmii"; and these
tx/rx_delays last month. Please could you go read that discussion and
them make use of rgmii-id, and change the delays.

Also, where did you copy this from? If possible, it would be good to
fix the example everybody copies into new DT blobs.

Andrew

2024-04-05 10:09:23

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree

On Thu, Apr 04, 2024 at 05:10:41PM +0200, Andrew Lunn wrote:
> > +&gmac1 {
> > + assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> > + assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
> > + phy-handle = <&rgmii_phy1>;
> > + phy-mode = "rgmii";
> > + clock_in_out = "output";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&gmac1m1_miim
> > + &gmac1m1_tx_bus2
> > + &gmac1m1_rx_bus2
> > + &gmac1m1_rgmii_clk
> > + &gmac1m1_clkinout
> > + &gmac1m1_rgmii_bus>;
> > + status = "okay";
> > + tx_delay = <0x30>;
> > + rx_delay = <0x10>;
> > +};
>
> There was a discussion about phy-mode = "rgmii"; and these
> tx/rx_delays last month. Please could you go read that discussion and
> them make use of rgmii-id, and change the delays.

Ok, I'll switch to rgmii-id.

>
> Also, where did you copy this from? If possible, it would be good to
> fix the example everybody copies into new DT blobs.

These are the default values used in over a dozen boards and a also
given in the example in
Documentation/devicetree/bindings/net/rockchip-dwmac.yaml.
These are also the default values the driver uses when tx_delay and
rx_delay are not given in the device tree.

I can prepare a patch to fix the example.

Do you have a pointer why setting the delays in the phy is preferred
over setting them in the network driver? In the end this requires us
to have the correct phy driver whereas setting them in the network
driver would just work for any phy driver?

Sascha


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2024-04-05 13:40:16

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree

> Do you have a pointer why setting the delays in the phy is preferred
> over setting them in the network driver? In the end this requires us
> to have the correct phy driver whereas setting them in the network
> driver would just work for any phy driver?

One reason is that nearly every other board does it in the PHY. This
is something i've been trying to standardize on for years.

Another point is that when doing it in the MAC, most MAC drivers get
it wrong. RGMII needs 2ns delays on the clock lines. That delay can be
provided by the board, making the clock lines longer. Or the MAC or
the PHY can add the delays. phy-mode in DT tells you about what the
board requires. Your board does not have extra long clock lines, so
you need rgmii-id. If the MAC decides to implement the delay, it
should modify the value passed to the PHY to be rgmii, to indicate it
has added the delays, and the PHY should not. This is what many MAC
drivers get wrong, they don't do the masking. By standardizing on the
PHY doing the delay, we avoid this, keeping the MAC driver simple, and
probably bug free in this respect.

There is admittedly some historical confusion here. The design is not
the best. If would of been much better if the design would have both
phy-mode and mac-mode.

As for using genphy, yes it might work, but there is no real
guarantee. It is always best you drive the hardware using the driver
specific to it. Consider genphy as a fallback which might be good
enough that you can ssh into the board and install the correct
module. You should not really be using it in production.

Andrew