All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-03-29  9:44 ` Dongjin Kim
  0 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-03-29  9:44 UTC (permalink / raw)
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Add device tree binding for Hardkernel ODROID-M1 board based on RK3568
SoC.

Signed-off-by: Dongjin Kim <tobetter@gmail.com>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index eece92f83a2d..48fd0858a795 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -664,6 +664,11 @@ properties:
           - const: rockchip,rk3568-bpi-r2pro
           - const: rockchip,rk3568
 
+      - description: Rockchip RK3568 Hardkernel ODROID-M1
+        items:
+          - const: rockchip,rk3568-odroid-m1
+          - const: rockchip,rk3568
+
 additionalProperties: true
 
 ...
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 1/2] dt-bindings: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-03-29  9:44 ` Dongjin Kim
  0 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-03-29  9:44 UTC (permalink / raw)
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Add device tree binding for Hardkernel ODROID-M1 board based on RK3568
SoC.

Signed-off-by: Dongjin Kim <tobetter@gmail.com>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index eece92f83a2d..48fd0858a795 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -664,6 +664,11 @@ properties:
           - const: rockchip,rk3568-bpi-r2pro
           - const: rockchip,rk3568
 
+      - description: Rockchip RK3568 Hardkernel ODROID-M1
+        items:
+          - const: rockchip,rk3568-odroid-m1
+          - const: rockchip,rk3568
+
 additionalProperties: true
 
 ...
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 1/2] dt-bindings: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-03-29  9:44 ` Dongjin Kim
  0 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-03-29  9:44 UTC (permalink / raw)
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

Add device tree binding for Hardkernel ODROID-M1 board based on RK3568
SoC.

Signed-off-by: Dongjin Kim <tobetter@gmail.com>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index eece92f83a2d..48fd0858a795 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -664,6 +664,11 @@ properties:
           - const: rockchip,rk3568-bpi-r2pro
           - const: rockchip,rk3568
 
+      - description: Rockchip RK3568 Hardkernel ODROID-M1
+        items:
+          - const: rockchip,rk3568-odroid-m1
+          - const: rockchip,rk3568
+
 additionalProperties: true
 
 ...
-- 
2.32.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-03-29  9:44 ` Dongjin Kim
  (?)
@ 2022-03-29  9:44   ` Dongjin Kim
  -1 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-03-29  9:44 UTC (permalink / raw)
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

This patch is to add a device tree for new board Hardkernel ODROID-M1
based on Rockchip RK3568, includes basic peripherals -
uart/eMMC/uSD/i2c and on-board ethernet.

Signed-off-by: Dongjin Kim <tobetter@gmail.com>
---
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
 2 files changed, 407 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index 4ae9f35434b8..81d160221227 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
new file mode 100644
index 000000000000..d1a5d43127e9
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2022 Hardkernel Co., Ltd.
+ *
+ */
+
+/dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+#include "rk3568.dtsi"
+
+/ {
+	model = "Hardkernel ODROID-M1";
+	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
+
+	aliases {
+		ethernet0 = &gmac0;
+		i2c0 = &i2c3;
+		i2c3 = &i2c0;
+		mmc0 = &sdhci;
+		mmc1 = &sdmmc0;
+		serial0 = &uart1;
+		serial1 = &uart0;
+	};
+
+	chosen: chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	dc_12v: dc-12v {
+		compatible = "regulator-fixed";
+		regulator-name = "dc_12v";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+	};
+
+	leds: leds {
+		compatible = "gpio-leds";
+
+		led_power: led-0 {
+			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
+			function = LED_FUNCTION_POWER;
+			color = <LED_COLOR_ID_RED>;
+			linux,default-trigger = "default-on";
+			pinctrl-names = "default";
+			pinctrl-0 = <&led_power_en>;
+		};
+		led_work: led-1 {
+			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
+			function = LED_FUNCTION_HEARTBEAT;
+			color = <LED_COLOR_ID_BLUE>;
+			linux,default-trigger = "heartbeat";
+			pinctrl-names = "default";
+			pinctrl-0 = <&led_work_en>;
+		};
+	};
+
+	vcc3v3_sys: vcc3v3-sys {
+		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>;
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu1 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu2 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu3 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&gmac0 {
+	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
+	assigned-clock-rates = <0>, <125000000>;
+	clock_in_out = "output";
+	phy-handle = <&rgmii_phy0>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac0_miim
+		     &gmac0_tx_bus2
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+	status = "okay";
+
+	tx_delay = <0x4f>;
+	rx_delay = <0x2d>;
+};
+
+&i2c0 {
+	status = "okay";
+
+	vdd_cpu: regulator@1c {
+		compatible = "tcs,tcs4525";
+		reg = <0x1c>;
+		 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>;
+		 vin-supply = <&vcc3v3_sys>;
+
+		 regulator-state-mem {
+			 regulator-off-in-suspend;
+		};
+	};
+
+	rk809: pmic@20 {
+		compatible = "rockchip,rk809";
+		reg = <0x20>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
+		#clock-cells = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_int>;
+		rockchip,system-power-controller;
+		vcc1-supply = <&vcc3v3_sys>;
+		vcc2-supply = <&vcc3v3_sys>;
+		vcc3-supply = <&vcc3v3_sys>;
+		vcc4-supply = <&vcc3v3_sys>;
+		vcc5-supply = <&vcc3v3_sys>;
+		vcc6-supply = <&vcc3v3_sys>;
+		vcc7-supply = <&vcc3v3_sys>;
+		vcc8-supply = <&vcc3v3_sys>;
+		vcc9-supply = <&vcc3v3_sys>;
+		wakeup-source;
+
+		regulators {
+			vdd_logic: DCDC_REG1 {
+				regulator-name = "vdd_logic";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_gpu: DCDC_REG2 {
+				regulator-name = "vdd_gpu";
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_ddr: DCDC_REG3 {
+				regulator-name = "vcc_ddr";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-initial-mode = <0x2>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vdd_npu: DCDC_REG4 {
+				regulator-name = "vdd_npu";
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8: DCDC_REG5 {
+				regulator-name = "vcc_1v8";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda0v9_image: LDO_REG1 {
+				regulator-name = "vdda0v9_image";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <900000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda_0v9: LDO_REG2 {
+				regulator-name = "vdda_0v9";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <900000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			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>;
+				};
+			};
+
+			vccio_acodec: LDO_REG4 {
+				regulator-name = "vccio_acodec";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vccio_sd: LDO_REG5 {
+				regulator-name = "vccio_sd";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc3v3_pmu: LDO_REG6 {
+				regulator-name = "vcc3v3_pmu";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vcca_1v8: LDO_REG7 {
+				regulator-name = "vcca_1v8";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcca1v8_pmu: LDO_REG8 {
+				regulator-name = "vcca1v8_pmu";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vcca1v8_image: LDO_REG9 {
+				regulator-name = "vcca1v8_image";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_3v3: SWITCH_REG1 {
+				regulator-name = "vcc_3v3";
+				regulator-always-on;
+				regulator-boot-on;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc3v3_sd: SWITCH_REG2 {
+				regulator-name = "vcc3v3_sd";
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+		};
+	};
+};
+
+&mdio0 {
+	rgmii_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0x0>;
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&pinctrl {
+	leds {
+		led_power_en: led_power_en {
+			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		led_work_en: led_work_en {
+			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		pmic_int: pmic_int {
+			rockchip,pins =
+				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+};
+
+&pmu_io_domains {
+	pmuio1-supply = <&vcc3v3_pmu>;
+	pmuio2-supply = <&vcc3v3_pmu>;
+	vccio1-supply = <&vccio_acodec>;
+	vccio2-supply = <&vcc_1v8>;
+	vccio3-supply = <&vccio_sd>;
+	vccio4-supply = <&vcc_1v8>;
+	vccio5-supply = <&vcc_3v3>;
+	vccio6-supply = <&vcc_3v3>;
+	vccio7-supply = <&vcc_3v3>;
+	status = "okay";
+};
+
+&saradc {
+	vref-supply = <&vcca_1v8>;
+	status = "okay";
+};
+
+&sdhci {
+	bus-width = <8>;
+	max-frequency = <200000000>;
+	non-removable;
+	pinctrl-names = "default";
+	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
+	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-sdr104;
+	vmmc-supply = <&vcc3v3_sd>;
+	vqmmc-supply = <&vccio_sd>;
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-03-29  9:44   ` Dongjin Kim
  0 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-03-29  9:44 UTC (permalink / raw)
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

This patch is to add a device tree for new board Hardkernel ODROID-M1
based on Rockchip RK3568, includes basic peripherals -
uart/eMMC/uSD/i2c and on-board ethernet.

Signed-off-by: Dongjin Kim <tobetter@gmail.com>
---
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
 2 files changed, 407 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index 4ae9f35434b8..81d160221227 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
new file mode 100644
index 000000000000..d1a5d43127e9
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2022 Hardkernel Co., Ltd.
+ *
+ */
+
+/dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+#include "rk3568.dtsi"
+
+/ {
+	model = "Hardkernel ODROID-M1";
+	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
+
+	aliases {
+		ethernet0 = &gmac0;
+		i2c0 = &i2c3;
+		i2c3 = &i2c0;
+		mmc0 = &sdhci;
+		mmc1 = &sdmmc0;
+		serial0 = &uart1;
+		serial1 = &uart0;
+	};
+
+	chosen: chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	dc_12v: dc-12v {
+		compatible = "regulator-fixed";
+		regulator-name = "dc_12v";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+	};
+
+	leds: leds {
+		compatible = "gpio-leds";
+
+		led_power: led-0 {
+			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
+			function = LED_FUNCTION_POWER;
+			color = <LED_COLOR_ID_RED>;
+			linux,default-trigger = "default-on";
+			pinctrl-names = "default";
+			pinctrl-0 = <&led_power_en>;
+		};
+		led_work: led-1 {
+			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
+			function = LED_FUNCTION_HEARTBEAT;
+			color = <LED_COLOR_ID_BLUE>;
+			linux,default-trigger = "heartbeat";
+			pinctrl-names = "default";
+			pinctrl-0 = <&led_work_en>;
+		};
+	};
+
+	vcc3v3_sys: vcc3v3-sys {
+		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>;
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu1 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu2 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu3 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&gmac0 {
+	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
+	assigned-clock-rates = <0>, <125000000>;
+	clock_in_out = "output";
+	phy-handle = <&rgmii_phy0>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac0_miim
+		     &gmac0_tx_bus2
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+	status = "okay";
+
+	tx_delay = <0x4f>;
+	rx_delay = <0x2d>;
+};
+
+&i2c0 {
+	status = "okay";
+
+	vdd_cpu: regulator@1c {
+		compatible = "tcs,tcs4525";
+		reg = <0x1c>;
+		 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>;
+		 vin-supply = <&vcc3v3_sys>;
+
+		 regulator-state-mem {
+			 regulator-off-in-suspend;
+		};
+	};
+
+	rk809: pmic@20 {
+		compatible = "rockchip,rk809";
+		reg = <0x20>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
+		#clock-cells = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_int>;
+		rockchip,system-power-controller;
+		vcc1-supply = <&vcc3v3_sys>;
+		vcc2-supply = <&vcc3v3_sys>;
+		vcc3-supply = <&vcc3v3_sys>;
+		vcc4-supply = <&vcc3v3_sys>;
+		vcc5-supply = <&vcc3v3_sys>;
+		vcc6-supply = <&vcc3v3_sys>;
+		vcc7-supply = <&vcc3v3_sys>;
+		vcc8-supply = <&vcc3v3_sys>;
+		vcc9-supply = <&vcc3v3_sys>;
+		wakeup-source;
+
+		regulators {
+			vdd_logic: DCDC_REG1 {
+				regulator-name = "vdd_logic";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_gpu: DCDC_REG2 {
+				regulator-name = "vdd_gpu";
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_ddr: DCDC_REG3 {
+				regulator-name = "vcc_ddr";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-initial-mode = <0x2>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vdd_npu: DCDC_REG4 {
+				regulator-name = "vdd_npu";
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8: DCDC_REG5 {
+				regulator-name = "vcc_1v8";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda0v9_image: LDO_REG1 {
+				regulator-name = "vdda0v9_image";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <900000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda_0v9: LDO_REG2 {
+				regulator-name = "vdda_0v9";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <900000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			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>;
+				};
+			};
+
+			vccio_acodec: LDO_REG4 {
+				regulator-name = "vccio_acodec";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vccio_sd: LDO_REG5 {
+				regulator-name = "vccio_sd";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc3v3_pmu: LDO_REG6 {
+				regulator-name = "vcc3v3_pmu";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vcca_1v8: LDO_REG7 {
+				regulator-name = "vcca_1v8";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcca1v8_pmu: LDO_REG8 {
+				regulator-name = "vcca1v8_pmu";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vcca1v8_image: LDO_REG9 {
+				regulator-name = "vcca1v8_image";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_3v3: SWITCH_REG1 {
+				regulator-name = "vcc_3v3";
+				regulator-always-on;
+				regulator-boot-on;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc3v3_sd: SWITCH_REG2 {
+				regulator-name = "vcc3v3_sd";
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+		};
+	};
+};
+
+&mdio0 {
+	rgmii_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0x0>;
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&pinctrl {
+	leds {
+		led_power_en: led_power_en {
+			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		led_work_en: led_work_en {
+			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		pmic_int: pmic_int {
+			rockchip,pins =
+				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+};
+
+&pmu_io_domains {
+	pmuio1-supply = <&vcc3v3_pmu>;
+	pmuio2-supply = <&vcc3v3_pmu>;
+	vccio1-supply = <&vccio_acodec>;
+	vccio2-supply = <&vcc_1v8>;
+	vccio3-supply = <&vccio_sd>;
+	vccio4-supply = <&vcc_1v8>;
+	vccio5-supply = <&vcc_3v3>;
+	vccio6-supply = <&vcc_3v3>;
+	vccio7-supply = <&vcc_3v3>;
+	status = "okay";
+};
+
+&saradc {
+	vref-supply = <&vcca_1v8>;
+	status = "okay";
+};
+
+&sdhci {
+	bus-width = <8>;
+	max-frequency = <200000000>;
+	non-removable;
+	pinctrl-names = "default";
+	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
+	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-sdr104;
+	vmmc-supply = <&vcc3v3_sd>;
+	vqmmc-supply = <&vccio_sd>;
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
-- 
2.32.0


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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-03-29  9:44   ` Dongjin Kim
  0 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-03-29  9:44 UTC (permalink / raw)
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

This patch is to add a device tree for new board Hardkernel ODROID-M1
based on Rockchip RK3568, includes basic peripherals -
uart/eMMC/uSD/i2c and on-board ethernet.

Signed-off-by: Dongjin Kim <tobetter@gmail.com>
---
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
 2 files changed, 407 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index 4ae9f35434b8..81d160221227 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
new file mode 100644
index 000000000000..d1a5d43127e9
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2022 Hardkernel Co., Ltd.
+ *
+ */
+
+/dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+#include "rk3568.dtsi"
+
+/ {
+	model = "Hardkernel ODROID-M1";
+	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
+
+	aliases {
+		ethernet0 = &gmac0;
+		i2c0 = &i2c3;
+		i2c3 = &i2c0;
+		mmc0 = &sdhci;
+		mmc1 = &sdmmc0;
+		serial0 = &uart1;
+		serial1 = &uart0;
+	};
+
+	chosen: chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	dc_12v: dc-12v {
+		compatible = "regulator-fixed";
+		regulator-name = "dc_12v";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+	};
+
+	leds: leds {
+		compatible = "gpio-leds";
+
+		led_power: led-0 {
+			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
+			function = LED_FUNCTION_POWER;
+			color = <LED_COLOR_ID_RED>;
+			linux,default-trigger = "default-on";
+			pinctrl-names = "default";
+			pinctrl-0 = <&led_power_en>;
+		};
+		led_work: led-1 {
+			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
+			function = LED_FUNCTION_HEARTBEAT;
+			color = <LED_COLOR_ID_BLUE>;
+			linux,default-trigger = "heartbeat";
+			pinctrl-names = "default";
+			pinctrl-0 = <&led_work_en>;
+		};
+	};
+
+	vcc3v3_sys: vcc3v3-sys {
+		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>;
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu1 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu2 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&cpu3 {
+	cpu-supply = <&vdd_cpu>;
+};
+
+&gmac0 {
+	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
+	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
+	assigned-clock-rates = <0>, <125000000>;
+	clock_in_out = "output";
+	phy-handle = <&rgmii_phy0>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac0_miim
+		     &gmac0_tx_bus2
+		     &gmac0_rx_bus2
+		     &gmac0_rgmii_clk
+		     &gmac0_rgmii_bus>;
+	status = "okay";
+
+	tx_delay = <0x4f>;
+	rx_delay = <0x2d>;
+};
+
+&i2c0 {
+	status = "okay";
+
+	vdd_cpu: regulator@1c {
+		compatible = "tcs,tcs4525";
+		reg = <0x1c>;
+		 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>;
+		 vin-supply = <&vcc3v3_sys>;
+
+		 regulator-state-mem {
+			 regulator-off-in-suspend;
+		};
+	};
+
+	rk809: pmic@20 {
+		compatible = "rockchip,rk809";
+		reg = <0x20>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
+		#clock-cells = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_int>;
+		rockchip,system-power-controller;
+		vcc1-supply = <&vcc3v3_sys>;
+		vcc2-supply = <&vcc3v3_sys>;
+		vcc3-supply = <&vcc3v3_sys>;
+		vcc4-supply = <&vcc3v3_sys>;
+		vcc5-supply = <&vcc3v3_sys>;
+		vcc6-supply = <&vcc3v3_sys>;
+		vcc7-supply = <&vcc3v3_sys>;
+		vcc8-supply = <&vcc3v3_sys>;
+		vcc9-supply = <&vcc3v3_sys>;
+		wakeup-source;
+
+		regulators {
+			vdd_logic: DCDC_REG1 {
+				regulator-name = "vdd_logic";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdd_gpu: DCDC_REG2 {
+				regulator-name = "vdd_gpu";
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_ddr: DCDC_REG3 {
+				regulator-name = "vcc_ddr";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-initial-mode = <0x2>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vdd_npu: DCDC_REG4 {
+				regulator-name = "vdd_npu";
+				regulator-init-microvolt = <900000>;
+				regulator-initial-mode = <0x2>;
+				regulator-min-microvolt = <500000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-ramp-delay = <6001>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_1v8: DCDC_REG5 {
+				regulator-name = "vcc_1v8";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda0v9_image: LDO_REG1 {
+				regulator-name = "vdda0v9_image";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <900000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vdda_0v9: LDO_REG2 {
+				regulator-name = "vdda_0v9";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <900000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			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>;
+				};
+			};
+
+			vccio_acodec: LDO_REG4 {
+				regulator-name = "vccio_acodec";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vccio_sd: LDO_REG5 {
+				regulator-name = "vccio_sd";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc3v3_pmu: LDO_REG6 {
+				regulator-name = "vcc3v3_pmu";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vcca_1v8: LDO_REG7 {
+				regulator-name = "vcca_1v8";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcca1v8_pmu: LDO_REG8 {
+				regulator-name = "vcca1v8_pmu";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vcca1v8_image: LDO_REG9 {
+				regulator-name = "vcca1v8_image";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc_3v3: SWITCH_REG1 {
+				regulator-name = "vcc_3v3";
+				regulator-always-on;
+				regulator-boot-on;
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+
+			vcc3v3_sd: SWITCH_REG2 {
+				regulator-name = "vcc3v3_sd";
+
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
+			};
+		};
+	};
+};
+
+&mdio0 {
+	rgmii_phy0: ethernet-phy@0 {
+		compatible = "ethernet-phy-ieee802.3-c22";
+		reg = <0x0>;
+		reset-assert-us = <20000>;
+		reset-deassert-us = <100000>;
+		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&pinctrl {
+	leds {
+		led_power_en: led_power_en {
+			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+		led_work_en: led_work_en {
+			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		pmic_int: pmic_int {
+			rockchip,pins =
+				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+};
+
+&pmu_io_domains {
+	pmuio1-supply = <&vcc3v3_pmu>;
+	pmuio2-supply = <&vcc3v3_pmu>;
+	vccio1-supply = <&vccio_acodec>;
+	vccio2-supply = <&vcc_1v8>;
+	vccio3-supply = <&vccio_sd>;
+	vccio4-supply = <&vcc_1v8>;
+	vccio5-supply = <&vcc_3v3>;
+	vccio6-supply = <&vcc_3v3>;
+	vccio7-supply = <&vcc_3v3>;
+	status = "okay";
+};
+
+&saradc {
+	vref-supply = <&vcca_1v8>;
+	status = "okay";
+};
+
+&sdhci {
+	bus-width = <8>;
+	max-frequency = <200000000>;
+	non-removable;
+	pinctrl-names = "default";
+	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
+	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-sdr104;
+	vmmc-supply = <&vcc3v3_sd>;
+	vqmmc-supply = <&vccio_sd>;
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
-- 
2.32.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-03-29  9:44   ` Dongjin Kim
  (?)
@ 2022-03-29 17:12     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-29 17:12 UTC (permalink / raw)
  To: Dongjin Kim, Heiko Stuebner
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

On 29/03/2022 11:44, Dongjin Kim wrote:
> This patch is to add a device tree for new board Hardkernel ODROID-M1
> based on Rockchip RK3568, includes basic peripherals -
> uart/eMMC/uSD/i2c and on-board ethernet.

I think the email got corrupted (incorrect To addresses).

> 
> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
>  2 files changed, 407 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index 4ae9f35434b8..81d160221227 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> new file mode 100644
> index 000000000000..d1a5d43127e9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> @@ -0,0 +1,406 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 Hardkernel Co., Ltd.
> + *
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include "rk3568.dtsi"
> +
> +/ {
> +	model = "Hardkernel ODROID-M1";
> +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> +
> +	aliases {
> +		ethernet0 = &gmac0;
> +		i2c0 = &i2c3;
> +		i2c3 = &i2c0;
> +		mmc0 = &sdhci;
> +		mmc1 = &sdmmc0;
> +		serial0 = &uart1;
> +		serial1 = &uart0;
> +	};
> +
> +	chosen: chosen {

No need for label.

> +		stdout-path = "serial2:1500000n8";
> +	};
> +
> +	dc_12v: dc-12v {

Generic node name, so "regulator" or "regulator-0"

> +		compatible = "regulator-fixed";
> +		regulator-name = "dc_12v";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +	};
> +
> +	leds: leds {

Label seems unused.

> +		compatible = "gpio-leds";
> +
> +		led_power: led-0 {
> +			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> +			function = LED_FUNCTION_POWER;
> +			color = <LED_COLOR_ID_RED>;
> +			linux,default-trigger = "default-on";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&led_power_en>;
> +		};
> +		led_work: led-1 {
> +			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> +			function = LED_FUNCTION_HEARTBEAT;
> +			color = <LED_COLOR_ID_BLUE>;
> +			linux,default-trigger = "heartbeat";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&led_work_en>;
> +		};
> +	};
> +
> +	vcc3v3_sys: vcc3v3-sys {

Generic node name.

> +		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>;
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu1 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu2 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu3 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&gmac0 {
> +	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> +	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> +	assigned-clock-rates = <0>, <125000000>;
> +	clock_in_out = "output";
> +	phy-handle = <&rgmii_phy0>;
> +	phy-mode = "rgmii";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac0_miim
> +		     &gmac0_tx_bus2
> +		     &gmac0_rx_bus2
> +		     &gmac0_rgmii_clk
> +		     &gmac0_rgmii_bus>;
> +	status = "okay";
> +
> +	tx_delay = <0x4f>;
> +	rx_delay = <0x2d>;
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +
> +	vdd_cpu: regulator@1c {
> +		compatible = "tcs,z`";
> +		reg = <0x1c>;
> +		 fcs,suspend-voltage-selector = <1>;

Mixed up indentation.

> +		 regulator-name = "vdd_cpu";
> +		 regulator-always-on;
> +		 regulator-boot-on;
> +		 regulator-min-microvolt = <800000>;
> +		 regulator-max-microvolt = <1150000>;
> +		 regulator-ramp-delay = <2300>;
> +		 vin-supply = <&vcc3v3_sys>;
> +
> +		 regulator-state-mem {
> +			 regulator-off-in-suspend;
> +		};
> +	};
> +
> +	rk809: pmic@20 {
> +		compatible = "rockchip,rk809";
> +		reg = <0x20>;
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> +		#clock-cells = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_int>;
> +		rockchip,system-power-controller;
> +		vcc1-supply = <&vcc3v3_sys>;
> +		vcc2-supply = <&vcc3v3_sys>;
> +		vcc3-supply = <&vcc3v3_sys>;
> +		vcc4-supply = <&vcc3v3_sys>;
> +		vcc5-supply = <&vcc3v3_sys>;
> +		vcc6-supply = <&vcc3v3_sys>;
> +		vcc7-supply = <&vcc3v3_sys>;
> +		vcc8-supply = <&vcc3v3_sys>;
> +		vcc9-supply = <&vcc3v3_sys>;
> +		wakeup-source;
> +
> +		regulators {
> +			vdd_logic: DCDC_REG1 {

No underscores in node names, unless anything requires it.

> +				regulator-name = "vdd_logic";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_gpu: DCDC_REG2 {
> +				regulator-name = "vdd_gpu";
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_ddr: DCDC_REG3 {
> +				regulator-name = "vcc_ddr";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-initial-mode = <0x2>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +				};
> +			};
> +
> +			vdd_npu: DCDC_REG4 {
> +				regulator-name = "vdd_npu";
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_1v8: DCDC_REG5 {
> +				regulator-name = "vcc_1v8";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda0v9_image: LDO_REG1 {
> +				regulator-name = "vdda0v9_image";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda_0v9: LDO_REG2 {
> +				regulator-name = "vdda_0v9";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			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>;
> +				};
> +			};
> +
> +			vccio_acodec: LDO_REG4 {
> +				regulator-name = "vccio_acodec";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vccio_sd: LDO_REG5 {
> +				regulator-name = "vccio_sd";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc3v3_pmu: LDO_REG6 {
> +				regulator-name = "vcc3v3_pmu";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <3300000>;
> +				};
> +			};
> +
> +			vcca_1v8: LDO_REG7 {
> +				regulator-name = "vcca_1v8";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcca1v8_pmu: LDO_REG8 {
> +				regulator-name = "vcca1v8_pmu";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			vcca1v8_image: LDO_REG9 {
> +				regulator-name = "vcca1v8_image";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_3v3: SWITCH_REG1 {
> +				regulator-name = "vcc_3v3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc3v3_sd: SWITCH_REG2 {
> +				regulator-name = "vcc3v3_sd";
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&mdio0 {
> +	rgmii_phy0: ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0x0>;
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> +	};
> +};
> +
> +&pinctrl {
> +	leds {
> +		led_power_en: led_power_en {

No underscores in node names. Do bindings require specific naming? I
would expect "-grp" or "-pins" suffixes, if possible.

> +			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +		led_work_en: led_work_en {
> +			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
> +	pmic {
> +		pmic_int: pmic_int {
> +			rockchip,pins =
> +				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> +		};
> +	};
> +};
> +
> +&pmu_io_domains {
> +	pmuio1-supply = <&vcc3v3_pmu>;
> +	pmuio2-supply = <&vcc3v3_pmu>;
> +	vccio1-supply = <&vccio_acodec>;
> +	vccio2-supply = <&vcc_1v8>;
> +	vccio3-supply = <&vccio_sd>;
> +	vccio4-supply = <&vcc_1v8>;
> +	vccio5-supply = <&vcc_3v3>;
> +	vccio6-supply = <&vcc_3v3>;
> +	vccio7-supply = <&vcc_3v3>;
> +	status = "okay";
> +};
> +
> +&saradc {
> +	vref-supply = <&vcca_1v8>;
> +	status = "okay";
> +};
> +
> +&sdhci {
> +	bus-width = <8>;
> +	max-frequency = <200000000>;
> +	non-removable;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> +	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-sdr104;
> +	vmmc-supply = <&vcc3v3_sd>;
> +	vqmmc-supply = <&vccio_sd>;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	status = "okay";
> +};


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-03-29 17:12     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-29 17:12 UTC (permalink / raw)
  To: Dongjin Kim, Heiko Stuebner
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

On 29/03/2022 11:44, Dongjin Kim wrote:
> This patch is to add a device tree for new board Hardkernel ODROID-M1
> based on Rockchip RK3568, includes basic peripherals -
> uart/eMMC/uSD/i2c and on-board ethernet.

I think the email got corrupted (incorrect To addresses).

> 
> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
>  2 files changed, 407 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index 4ae9f35434b8..81d160221227 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> new file mode 100644
> index 000000000000..d1a5d43127e9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> @@ -0,0 +1,406 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 Hardkernel Co., Ltd.
> + *
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include "rk3568.dtsi"
> +
> +/ {
> +	model = "Hardkernel ODROID-M1";
> +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> +
> +	aliases {
> +		ethernet0 = &gmac0;
> +		i2c0 = &i2c3;
> +		i2c3 = &i2c0;
> +		mmc0 = &sdhci;
> +		mmc1 = &sdmmc0;
> +		serial0 = &uart1;
> +		serial1 = &uart0;
> +	};
> +
> +	chosen: chosen {

No need for label.

> +		stdout-path = "serial2:1500000n8";
> +	};
> +
> +	dc_12v: dc-12v {

Generic node name, so "regulator" or "regulator-0"

> +		compatible = "regulator-fixed";
> +		regulator-name = "dc_12v";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +	};
> +
> +	leds: leds {

Label seems unused.

> +		compatible = "gpio-leds";
> +
> +		led_power: led-0 {
> +			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> +			function = LED_FUNCTION_POWER;
> +			color = <LED_COLOR_ID_RED>;
> +			linux,default-trigger = "default-on";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&led_power_en>;
> +		};
> +		led_work: led-1 {
> +			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> +			function = LED_FUNCTION_HEARTBEAT;
> +			color = <LED_COLOR_ID_BLUE>;
> +			linux,default-trigger = "heartbeat";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&led_work_en>;
> +		};
> +	};
> +
> +	vcc3v3_sys: vcc3v3-sys {

Generic node name.

> +		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>;
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu1 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu2 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu3 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&gmac0 {
> +	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> +	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> +	assigned-clock-rates = <0>, <125000000>;
> +	clock_in_out = "output";
> +	phy-handle = <&rgmii_phy0>;
> +	phy-mode = "rgmii";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac0_miim
> +		     &gmac0_tx_bus2
> +		     &gmac0_rx_bus2
> +		     &gmac0_rgmii_clk
> +		     &gmac0_rgmii_bus>;
> +	status = "okay";
> +
> +	tx_delay = <0x4f>;
> +	rx_delay = <0x2d>;
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +
> +	vdd_cpu: regulator@1c {
> +		compatible = "tcs,z`";
> +		reg = <0x1c>;
> +		 fcs,suspend-voltage-selector = <1>;

Mixed up indentation.

> +		 regulator-name = "vdd_cpu";
> +		 regulator-always-on;
> +		 regulator-boot-on;
> +		 regulator-min-microvolt = <800000>;
> +		 regulator-max-microvolt = <1150000>;
> +		 regulator-ramp-delay = <2300>;
> +		 vin-supply = <&vcc3v3_sys>;
> +
> +		 regulator-state-mem {
> +			 regulator-off-in-suspend;
> +		};
> +	};
> +
> +	rk809: pmic@20 {
> +		compatible = "rockchip,rk809";
> +		reg = <0x20>;
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> +		#clock-cells = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_int>;
> +		rockchip,system-power-controller;
> +		vcc1-supply = <&vcc3v3_sys>;
> +		vcc2-supply = <&vcc3v3_sys>;
> +		vcc3-supply = <&vcc3v3_sys>;
> +		vcc4-supply = <&vcc3v3_sys>;
> +		vcc5-supply = <&vcc3v3_sys>;
> +		vcc6-supply = <&vcc3v3_sys>;
> +		vcc7-supply = <&vcc3v3_sys>;
> +		vcc8-supply = <&vcc3v3_sys>;
> +		vcc9-supply = <&vcc3v3_sys>;
> +		wakeup-source;
> +
> +		regulators {
> +			vdd_logic: DCDC_REG1 {

No underscores in node names, unless anything requires it.

> +				regulator-name = "vdd_logic";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_gpu: DCDC_REG2 {
> +				regulator-name = "vdd_gpu";
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_ddr: DCDC_REG3 {
> +				regulator-name = "vcc_ddr";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-initial-mode = <0x2>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +				};
> +			};
> +
> +			vdd_npu: DCDC_REG4 {
> +				regulator-name = "vdd_npu";
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_1v8: DCDC_REG5 {
> +				regulator-name = "vcc_1v8";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda0v9_image: LDO_REG1 {
> +				regulator-name = "vdda0v9_image";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda_0v9: LDO_REG2 {
> +				regulator-name = "vdda_0v9";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			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>;
> +				};
> +			};
> +
> +			vccio_acodec: LDO_REG4 {
> +				regulator-name = "vccio_acodec";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vccio_sd: LDO_REG5 {
> +				regulator-name = "vccio_sd";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc3v3_pmu: LDO_REG6 {
> +				regulator-name = "vcc3v3_pmu";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <3300000>;
> +				};
> +			};
> +
> +			vcca_1v8: LDO_REG7 {
> +				regulator-name = "vcca_1v8";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcca1v8_pmu: LDO_REG8 {
> +				regulator-name = "vcca1v8_pmu";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			vcca1v8_image: LDO_REG9 {
> +				regulator-name = "vcca1v8_image";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_3v3: SWITCH_REG1 {
> +				regulator-name = "vcc_3v3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc3v3_sd: SWITCH_REG2 {
> +				regulator-name = "vcc3v3_sd";
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&mdio0 {
> +	rgmii_phy0: ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0x0>;
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> +	};
> +};
> +
> +&pinctrl {
> +	leds {
> +		led_power_en: led_power_en {

No underscores in node names. Do bindings require specific naming? I
would expect "-grp" or "-pins" suffixes, if possible.

> +			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +		led_work_en: led_work_en {
> +			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
> +	pmic {
> +		pmic_int: pmic_int {
> +			rockchip,pins =
> +				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> +		};
> +	};
> +};
> +
> +&pmu_io_domains {
> +	pmuio1-supply = <&vcc3v3_pmu>;
> +	pmuio2-supply = <&vcc3v3_pmu>;
> +	vccio1-supply = <&vccio_acodec>;
> +	vccio2-supply = <&vcc_1v8>;
> +	vccio3-supply = <&vccio_sd>;
> +	vccio4-supply = <&vcc_1v8>;
> +	vccio5-supply = <&vcc_3v3>;
> +	vccio6-supply = <&vcc_3v3>;
> +	vccio7-supply = <&vcc_3v3>;
> +	status = "okay";
> +};
> +
> +&saradc {
> +	vref-supply = <&vcca_1v8>;
> +	status = "okay";
> +};
> +
> +&sdhci {
> +	bus-width = <8>;
> +	max-frequency = <200000000>;
> +	non-removable;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> +	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-sdr104;
> +	vmmc-supply = <&vcc3v3_sd>;
> +	vqmmc-supply = <&vccio_sd>;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	status = "okay";
> +};


Best regards,
Krzysztof

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-03-29 17:12     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-29 17:12 UTC (permalink / raw)
  To: Dongjin Kim, Heiko Stuebner
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel

On 29/03/2022 11:44, Dongjin Kim wrote:
> This patch is to add a device tree for new board Hardkernel ODROID-M1
> based on Rockchip RK3568, includes basic peripherals -
> uart/eMMC/uSD/i2c and on-board ethernet.

I think the email got corrupted (incorrect To addresses).

> 
> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> ---
>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
>  2 files changed, 407 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index 4ae9f35434b8..81d160221227 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> new file mode 100644
> index 000000000000..d1a5d43127e9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> @@ -0,0 +1,406 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 Hardkernel Co., Ltd.
> + *
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include "rk3568.dtsi"
> +
> +/ {
> +	model = "Hardkernel ODROID-M1";
> +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> +
> +	aliases {
> +		ethernet0 = &gmac0;
> +		i2c0 = &i2c3;
> +		i2c3 = &i2c0;
> +		mmc0 = &sdhci;
> +		mmc1 = &sdmmc0;
> +		serial0 = &uart1;
> +		serial1 = &uart0;
> +	};
> +
> +	chosen: chosen {

No need for label.

> +		stdout-path = "serial2:1500000n8";
> +	};
> +
> +	dc_12v: dc-12v {

Generic node name, so "regulator" or "regulator-0"

> +		compatible = "regulator-fixed";
> +		regulator-name = "dc_12v";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +	};
> +
> +	leds: leds {

Label seems unused.

> +		compatible = "gpio-leds";
> +
> +		led_power: led-0 {
> +			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> +			function = LED_FUNCTION_POWER;
> +			color = <LED_COLOR_ID_RED>;
> +			linux,default-trigger = "default-on";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&led_power_en>;
> +		};
> +		led_work: led-1 {
> +			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> +			function = LED_FUNCTION_HEARTBEAT;
> +			color = <LED_COLOR_ID_BLUE>;
> +			linux,default-trigger = "heartbeat";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&led_work_en>;
> +		};
> +	};
> +
> +	vcc3v3_sys: vcc3v3-sys {

Generic node name.

> +		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>;
> +	};
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu1 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu2 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu3 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&gmac0 {
> +	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> +	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> +	assigned-clock-rates = <0>, <125000000>;
> +	clock_in_out = "output";
> +	phy-handle = <&rgmii_phy0>;
> +	phy-mode = "rgmii";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac0_miim
> +		     &gmac0_tx_bus2
> +		     &gmac0_rx_bus2
> +		     &gmac0_rgmii_clk
> +		     &gmac0_rgmii_bus>;
> +	status = "okay";
> +
> +	tx_delay = <0x4f>;
> +	rx_delay = <0x2d>;
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +
> +	vdd_cpu: regulator@1c {
> +		compatible = "tcs,z`";
> +		reg = <0x1c>;
> +		 fcs,suspend-voltage-selector = <1>;

Mixed up indentation.

> +		 regulator-name = "vdd_cpu";
> +		 regulator-always-on;
> +		 regulator-boot-on;
> +		 regulator-min-microvolt = <800000>;
> +		 regulator-max-microvolt = <1150000>;
> +		 regulator-ramp-delay = <2300>;
> +		 vin-supply = <&vcc3v3_sys>;
> +
> +		 regulator-state-mem {
> +			 regulator-off-in-suspend;
> +		};
> +	};
> +
> +	rk809: pmic@20 {
> +		compatible = "rockchip,rk809";
> +		reg = <0x20>;
> +		interrupt-parent = <&gpio0>;
> +		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> +		#clock-cells = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pmic_int>;
> +		rockchip,system-power-controller;
> +		vcc1-supply = <&vcc3v3_sys>;
> +		vcc2-supply = <&vcc3v3_sys>;
> +		vcc3-supply = <&vcc3v3_sys>;
> +		vcc4-supply = <&vcc3v3_sys>;
> +		vcc5-supply = <&vcc3v3_sys>;
> +		vcc6-supply = <&vcc3v3_sys>;
> +		vcc7-supply = <&vcc3v3_sys>;
> +		vcc8-supply = <&vcc3v3_sys>;
> +		vcc9-supply = <&vcc3v3_sys>;
> +		wakeup-source;
> +
> +		regulators {
> +			vdd_logic: DCDC_REG1 {

No underscores in node names, unless anything requires it.

> +				regulator-name = "vdd_logic";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdd_gpu: DCDC_REG2 {
> +				regulator-name = "vdd_gpu";
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_ddr: DCDC_REG3 {
> +				regulator-name = "vcc_ddr";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-initial-mode = <0x2>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +				};
> +			};
> +
> +			vdd_npu: DCDC_REG4 {
> +				regulator-name = "vdd_npu";
> +				regulator-init-microvolt = <900000>;
> +				regulator-initial-mode = <0x2>;
> +				regulator-min-microvolt = <500000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-ramp-delay = <6001>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_1v8: DCDC_REG5 {
> +				regulator-name = "vcc_1v8";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda0v9_image: LDO_REG1 {
> +				regulator-name = "vdda0v9_image";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vdda_0v9: LDO_REG2 {
> +				regulator-name = "vdda_0v9";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			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>;
> +				};
> +			};
> +
> +			vccio_acodec: LDO_REG4 {
> +				regulator-name = "vccio_acodec";
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vccio_sd: LDO_REG5 {
> +				regulator-name = "vccio_sd";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc3v3_pmu: LDO_REG6 {
> +				regulator-name = "vcc3v3_pmu";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <3300000>;
> +				regulator-max-microvolt = <3300000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <3300000>;
> +				};
> +			};
> +
> +			vcca_1v8: LDO_REG7 {
> +				regulator-name = "vcca_1v8";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcca1v8_pmu: LDO_REG8 {
> +				regulator-name = "vcca1v8_pmu";
> +				regulator-always-on;
> +				regulator-boot-on;
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-on-in-suspend;
> +					regulator-suspend-microvolt = <1800000>;
> +				};
> +			};
> +
> +			vcca1v8_image: LDO_REG9 {
> +				regulator-name = "vcca1v8_image";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc_3v3: SWITCH_REG1 {
> +				regulator-name = "vcc_3v3";
> +				regulator-always-on;
> +				regulator-boot-on;
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +
> +			vcc3v3_sd: SWITCH_REG2 {
> +				regulator-name = "vcc3v3_sd";
> +
> +				regulator-state-mem {
> +					regulator-off-in-suspend;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&mdio0 {
> +	rgmii_phy0: ethernet-phy@0 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0x0>;
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> +	};
> +};
> +
> +&pinctrl {
> +	leds {
> +		led_power_en: led_power_en {

No underscores in node names. Do bindings require specific naming? I
would expect "-grp" or "-pins" suffixes, if possible.

> +			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +		led_work_en: led_work_en {
> +			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
> +	pmic {
> +		pmic_int: pmic_int {
> +			rockchip,pins =
> +				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> +		};
> +	};
> +};
> +
> +&pmu_io_domains {
> +	pmuio1-supply = <&vcc3v3_pmu>;
> +	pmuio2-supply = <&vcc3v3_pmu>;
> +	vccio1-supply = <&vccio_acodec>;
> +	vccio2-supply = <&vcc_1v8>;
> +	vccio3-supply = <&vccio_sd>;
> +	vccio4-supply = <&vcc_1v8>;
> +	vccio5-supply = <&vcc_3v3>;
> +	vccio6-supply = <&vcc_3v3>;
> +	vccio7-supply = <&vcc_3v3>;
> +	status = "okay";
> +};
> +
> +&saradc {
> +	vref-supply = <&vcca_1v8>;
> +	status = "okay";
> +};
> +
> +&sdhci {
> +	bus-width = <8>;
> +	max-frequency = <200000000>;
> +	non-removable;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> +	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-sdr104;
> +	vmmc-supply = <&vcc3v3_sd>;
> +	vqmmc-supply = <&vccio_sd>;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	status = "okay";
> +};


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] dt-bindings: rockchip: Add Hardkernel ODROID-M1 board
  2022-03-29  9:44 ` Dongjin Kim
  (?)
@ 2022-04-04 18:10   ` Rob Herring
  -1 siblings, 0 replies; 33+ messages in thread
From: Rob Herring @ 2022-04-04 18:10 UTC (permalink / raw)
  To: Dongjin Kim
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel, Heiko Stuebner,
	devicetree

On Tue, 29 Mar 2022 18:44:45 +0900, Dongjin Kim wrote:
> Add device tree binding for Hardkernel ODROID-M1 board based on RK3568
> SoC.
> 
> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] dt-bindings: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-04 18:10   ` Rob Herring
  0 siblings, 0 replies; 33+ messages in thread
From: Rob Herring @ 2022-04-04 18:10 UTC (permalink / raw)
  To: Dongjin Kim
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel, Heiko Stuebner,
	devicetree

On Tue, 29 Mar 2022 18:44:45 +0900, Dongjin Kim wrote:
> Add device tree binding for Hardkernel ODROID-M1 board based on RK3568
> SoC.
> 
> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] dt-bindings: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-04 18:10   ` Rob Herring
  0 siblings, 0 replies; 33+ messages in thread
From: Rob Herring @ 2022-04-04 18:10 UTC (permalink / raw)
  To: Dongjin Kim
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel, Heiko Stuebner,
	devicetree

On Tue, 29 Mar 2022 18:44:45 +0900, Dongjin Kim wrote:
> Add device tree binding for Hardkernel ODROID-M1 board based on RK3568
> SoC.
> 
> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-03-29 17:12     ` Krzysztof Kozlowski
  (?)
@ 2022-04-05  2:32       ` Dongjin Kim
  -1 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-04-05  2:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On Tue, Mar 29, 2022 at 07:12:16PM +0200, Krzysztof Kozlowski wrote:
> On 29/03/2022 11:44, Dongjin Kim wrote:
> > This patch is to add a device tree for new board Hardkernel ODROID-M1
> > based on Rockchip RK3568, includes basic peripherals -
> > uart/eMMC/uSD/i2c and on-board ethernet.
> 
> I think the email got corrupted (incorrect To addresses).
> 
Thank you for reviewing and sorry for late reply.
> > 
> > Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
> >  2 files changed, 407 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index 4ae9f35434b8..81d160221227 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > new file mode 100644
> > index 000000000000..d1a5d43127e9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > @@ -0,0 +1,406 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2022 Hardkernel Co., Ltd.
> > + *
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include "rk3568.dtsi"
> > +
> > +/ {
> > +	model = "Hardkernel ODROID-M1";
> > +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> > +
> > +	aliases {
> > +		ethernet0 = &gmac0;
> > +		i2c0 = &i2c3;
> > +		i2c3 = &i2c0;
> > +		mmc0 = &sdhci;
> > +		mmc1 = &sdmmc0;
> > +		serial0 = &uart1;
> > +		serial1 = &uart0;
> > +	};
> > +
> > +	chosen: chosen {
> 
> No need for label.
> 
Ok. Will fix it.
> > +		stdout-path = "serial2:1500000n8";
> > +	};
> > +
> > +	dc_12v: dc-12v {
> 
> Generic node name, so "regulator" or "regulator-0"
> 
I've followed the node names as already merged device tree files
for other boards and thought this would be acceptable. Same for other
node names 'vcc3v3-sys' and node names with underscore below.
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "dc_12v";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +	};
> > +
> > +	leds: leds {
> 
> Label seems unused.
> 
Ok, will fix it.
> > +		compatible = "gpio-leds";
> > +
> > +		led_power: led-0 {
> > +			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> > +			function = LED_FUNCTION_POWER;
> > +			color = <LED_COLOR_ID_RED>;
> > +			linux,default-trigger = "default-on";
> > +			pinctrl-names = "default";
> > +			pinctrl-0 = <&led_power_en>;
> > +		};
> > +		led_work: led-1 {
> > +			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> > +			function = LED_FUNCTION_HEARTBEAT;
> > +			color = <LED_COLOR_ID_BLUE>;
> > +			linux,default-trigger = "heartbeat";
> > +			pinctrl-names = "default";
> > +			pinctrl-0 = <&led_work_en>;
> > +		};
> > +	};
> > +
> > +	vcc3v3_sys: vcc3v3-sys {
> 
> Generic node name.
> 
> > +		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>;
> > +	};
> > +};
> > +
> > +&cpu0 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu1 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu2 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu3 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&gmac0 {
> > +	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> > +	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> > +	assigned-clock-rates = <0>, <125000000>;
> > +	clock_in_out = "output";
> > +	phy-handle = <&rgmii_phy0>;
> > +	phy-mode = "rgmii";
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&gmac0_miim
> > +		     &gmac0_tx_bus2
> > +		     &gmac0_rx_bus2
> > +		     &gmac0_rgmii_clk
> > +		     &gmac0_rgmii_bus>;
> > +	status = "okay";
> > +
> > +	tx_delay = <0x4f>;
> > +	rx_delay = <0x2d>;
> > +};
> > +
> > +&i2c0 {
> > +	status = "okay";
> > +
> > +	vdd_cpu: regulator@1c {
> > +		compatible = "tcs,z`";
> > +		reg = <0x1c>;
> > +		 fcs,suspend-voltage-selector = <1>;
> 
> Mixed up indentation.
> 
Sorry, my bad. Will fix it.
> > +		 regulator-name = "vdd_cpu";
> > +		 regulator-always-on;
> > +		 regulator-boot-on;
> > +		 regulator-min-microvolt = <800000>;
> > +		 regulator-max-microvolt = <1150000>;
> > +		 regulator-ramp-delay = <2300>;
> > +		 vin-supply = <&vcc3v3_sys>;
> > +
> > +		 regulator-state-mem {
> > +			 regulator-off-in-suspend;
> > +		};
> > +	};
> > +
> > +	rk809: pmic@20 {
> > +		compatible = "rockchip,rk809";
> > +		reg = <0x20>;
> > +		interrupt-parent = <&gpio0>;
> > +		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> > +		#clock-cells = <1>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pmic_int>;
> > +		rockchip,system-power-controller;
> > +		vcc1-supply = <&vcc3v3_sys>;
> > +		vcc2-supply = <&vcc3v3_sys>;
> > +		vcc3-supply = <&vcc3v3_sys>;
> > +		vcc4-supply = <&vcc3v3_sys>;
> > +		vcc5-supply = <&vcc3v3_sys>;
> > +		vcc6-supply = <&vcc3v3_sys>;
> > +		vcc7-supply = <&vcc3v3_sys>;
> > +		vcc8-supply = <&vcc3v3_sys>;
> > +		vcc9-supply = <&vcc3v3_sys>;
> > +		wakeup-source;
> > +
> > +		regulators {
> > +			vdd_logic: DCDC_REG1 {
> 
> No underscores in node names, unless anything requires it.
> 
> > +				regulator-name = "vdd_logic";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_gpu: DCDC_REG2 {
> > +				regulator-name = "vdd_gpu";
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_ddr: DCDC_REG3 {
> > +				regulator-name = "vcc_ddr";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-initial-mode = <0x2>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_npu: DCDC_REG4 {
> > +				regulator-name = "vdd_npu";
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_1v8: DCDC_REG5 {
> > +				regulator-name = "vcc_1v8";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda0v9_image: LDO_REG1 {
> > +				regulator-name = "vdda0v9_image";
> > +				regulator-min-microvolt = <900000>;
> > +				regulator-max-microvolt = <900000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda_0v9: LDO_REG2 {
> > +				regulator-name = "vdda_0v9";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <900000>;
> > +				regulator-max-microvolt = <900000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			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>;
> > +				};
> > +			};
> > +
> > +			vccio_acodec: LDO_REG4 {
> > +				regulator-name = "vccio_acodec";
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vccio_sd: LDO_REG5 {
> > +				regulator-name = "vccio_sd";
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc3v3_pmu: LDO_REG6 {
> > +				regulator-name = "vcc3v3_pmu";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = <3300000>;
> > +				};
> > +			};
> > +
> > +			vcca_1v8: LDO_REG7 {
> > +				regulator-name = "vcca_1v8";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcca1v8_pmu: LDO_REG8 {
> > +				regulator-name = "vcca1v8_pmu";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = <1800000>;
> > +				};
> > +			};
> > +
> > +			vcca1v8_image: LDO_REG9 {
> > +				regulator-name = "vcca1v8_image";
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_3v3: SWITCH_REG1 {
> > +				regulator-name = "vcc_3v3";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc3v3_sd: SWITCH_REG2 {
> > +				regulator-name = "vcc3v3_sd";
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&mdio0 {
> > +	rgmii_phy0: ethernet-phy@0 {
> > +		compatible = "ethernet-phy-ieee802.3-c22";
> > +		reg = <0x0>;
> > +		reset-assert-us = <20000>;
> > +		reset-deassert-us = <100000>;
> > +		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > +	};
> > +};
> > +
> > +&pinctrl {
> > +	leds {
> > +		led_power_en: led_power_en {
> 
> No underscores in node names. Do bindings require specific naming? I
> would expect "-grp" or "-pins" suffixes, if possible.
> 
> > +			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +		led_work_en: led_work_en {
> > +			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +	};
> > +
> > +	pmic {
> > +		pmic_int: pmic_int {
> > +			rockchip,pins =
> > +				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> > +		};
> > +	};
> > +};
> > +
> > +&pmu_io_domains {
> > +	pmuio1-supply = <&vcc3v3_pmu>;
> > +	pmuio2-supply = <&vcc3v3_pmu>;
> > +	vccio1-supply = <&vccio_acodec>;
> > +	vccio2-supply = <&vcc_1v8>;
> > +	vccio3-supply = <&vccio_sd>;
> > +	vccio4-supply = <&vcc_1v8>;
> > +	vccio5-supply = <&vcc_3v3>;
> > +	vccio6-supply = <&vcc_3v3>;
> > +	vccio7-supply = <&vcc_3v3>;
> > +	status = "okay";
> > +};
> > +
> > +&saradc {
> > +	vref-supply = <&vcca_1v8>;
> > +	status = "okay";
> > +};
> > +
> > +&sdhci {
> > +	bus-width = <8>;
> > +	max-frequency = <200000000>;
> > +	non-removable;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> > +	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-sdr104;
> > +	vmmc-supply = <&vcc3v3_sd>;
> > +	vqmmc-supply = <&vccio_sd>;
> > +	status = "okay";
> > +};
> > +
> > +&uart2 {
> > +	status = "okay";
> > +};
> 
> 
> Best regards,
> Krzysztof
Thank you,
Dongjin.

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-05  2:32       ` Dongjin Kim
  0 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-04-05  2:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On Tue, Mar 29, 2022 at 07:12:16PM +0200, Krzysztof Kozlowski wrote:
> On 29/03/2022 11:44, Dongjin Kim wrote:
> > This patch is to add a device tree for new board Hardkernel ODROID-M1
> > based on Rockchip RK3568, includes basic peripherals -
> > uart/eMMC/uSD/i2c and on-board ethernet.
> 
> I think the email got corrupted (incorrect To addresses).
> 
Thank you for reviewing and sorry for late reply.
> > 
> > Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
> >  2 files changed, 407 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index 4ae9f35434b8..81d160221227 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > new file mode 100644
> > index 000000000000..d1a5d43127e9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > @@ -0,0 +1,406 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2022 Hardkernel Co., Ltd.
> > + *
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include "rk3568.dtsi"
> > +
> > +/ {
> > +	model = "Hardkernel ODROID-M1";
> > +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> > +
> > +	aliases {
> > +		ethernet0 = &gmac0;
> > +		i2c0 = &i2c3;
> > +		i2c3 = &i2c0;
> > +		mmc0 = &sdhci;
> > +		mmc1 = &sdmmc0;
> > +		serial0 = &uart1;
> > +		serial1 = &uart0;
> > +	};
> > +
> > +	chosen: chosen {
> 
> No need for label.
> 
Ok. Will fix it.
> > +		stdout-path = "serial2:1500000n8";
> > +	};
> > +
> > +	dc_12v: dc-12v {
> 
> Generic node name, so "regulator" or "regulator-0"
> 
I've followed the node names as already merged device tree files
for other boards and thought this would be acceptable. Same for other
node names 'vcc3v3-sys' and node names with underscore below.
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "dc_12v";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +	};
> > +
> > +	leds: leds {
> 
> Label seems unused.
> 
Ok, will fix it.
> > +		compatible = "gpio-leds";
> > +
> > +		led_power: led-0 {
> > +			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> > +			function = LED_FUNCTION_POWER;
> > +			color = <LED_COLOR_ID_RED>;
> > +			linux,default-trigger = "default-on";
> > +			pinctrl-names = "default";
> > +			pinctrl-0 = <&led_power_en>;
> > +		};
> > +		led_work: led-1 {
> > +			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> > +			function = LED_FUNCTION_HEARTBEAT;
> > +			color = <LED_COLOR_ID_BLUE>;
> > +			linux,default-trigger = "heartbeat";
> > +			pinctrl-names = "default";
> > +			pinctrl-0 = <&led_work_en>;
> > +		};
> > +	};
> > +
> > +	vcc3v3_sys: vcc3v3-sys {
> 
> Generic node name.
> 
> > +		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>;
> > +	};
> > +};
> > +
> > +&cpu0 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu1 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu2 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu3 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&gmac0 {
> > +	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> > +	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> > +	assigned-clock-rates = <0>, <125000000>;
> > +	clock_in_out = "output";
> > +	phy-handle = <&rgmii_phy0>;
> > +	phy-mode = "rgmii";
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&gmac0_miim
> > +		     &gmac0_tx_bus2
> > +		     &gmac0_rx_bus2
> > +		     &gmac0_rgmii_clk
> > +		     &gmac0_rgmii_bus>;
> > +	status = "okay";
> > +
> > +	tx_delay = <0x4f>;
> > +	rx_delay = <0x2d>;
> > +};
> > +
> > +&i2c0 {
> > +	status = "okay";
> > +
> > +	vdd_cpu: regulator@1c {
> > +		compatible = "tcs,z`";
> > +		reg = <0x1c>;
> > +		 fcs,suspend-voltage-selector = <1>;
> 
> Mixed up indentation.
> 
Sorry, my bad. Will fix it.
> > +		 regulator-name = "vdd_cpu";
> > +		 regulator-always-on;
> > +		 regulator-boot-on;
> > +		 regulator-min-microvolt = <800000>;
> > +		 regulator-max-microvolt = <1150000>;
> > +		 regulator-ramp-delay = <2300>;
> > +		 vin-supply = <&vcc3v3_sys>;
> > +
> > +		 regulator-state-mem {
> > +			 regulator-off-in-suspend;
> > +		};
> > +	};
> > +
> > +	rk809: pmic@20 {
> > +		compatible = "rockchip,rk809";
> > +		reg = <0x20>;
> > +		interrupt-parent = <&gpio0>;
> > +		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> > +		#clock-cells = <1>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pmic_int>;
> > +		rockchip,system-power-controller;
> > +		vcc1-supply = <&vcc3v3_sys>;
> > +		vcc2-supply = <&vcc3v3_sys>;
> > +		vcc3-supply = <&vcc3v3_sys>;
> > +		vcc4-supply = <&vcc3v3_sys>;
> > +		vcc5-supply = <&vcc3v3_sys>;
> > +		vcc6-supply = <&vcc3v3_sys>;
> > +		vcc7-supply = <&vcc3v3_sys>;
> > +		vcc8-supply = <&vcc3v3_sys>;
> > +		vcc9-supply = <&vcc3v3_sys>;
> > +		wakeup-source;
> > +
> > +		regulators {
> > +			vdd_logic: DCDC_REG1 {
> 
> No underscores in node names, unless anything requires it.
> 
> > +				regulator-name = "vdd_logic";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_gpu: DCDC_REG2 {
> > +				regulator-name = "vdd_gpu";
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_ddr: DCDC_REG3 {
> > +				regulator-name = "vcc_ddr";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-initial-mode = <0x2>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_npu: DCDC_REG4 {
> > +				regulator-name = "vdd_npu";
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_1v8: DCDC_REG5 {
> > +				regulator-name = "vcc_1v8";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda0v9_image: LDO_REG1 {
> > +				regulator-name = "vdda0v9_image";
> > +				regulator-min-microvolt = <900000>;
> > +				regulator-max-microvolt = <900000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda_0v9: LDO_REG2 {
> > +				regulator-name = "vdda_0v9";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <900000>;
> > +				regulator-max-microvolt = <900000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			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>;
> > +				};
> > +			};
> > +
> > +			vccio_acodec: LDO_REG4 {
> > +				regulator-name = "vccio_acodec";
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vccio_sd: LDO_REG5 {
> > +				regulator-name = "vccio_sd";
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc3v3_pmu: LDO_REG6 {
> > +				regulator-name = "vcc3v3_pmu";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = <3300000>;
> > +				};
> > +			};
> > +
> > +			vcca_1v8: LDO_REG7 {
> > +				regulator-name = "vcca_1v8";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcca1v8_pmu: LDO_REG8 {
> > +				regulator-name = "vcca1v8_pmu";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = <1800000>;
> > +				};
> > +			};
> > +
> > +			vcca1v8_image: LDO_REG9 {
> > +				regulator-name = "vcca1v8_image";
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_3v3: SWITCH_REG1 {
> > +				regulator-name = "vcc_3v3";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc3v3_sd: SWITCH_REG2 {
> > +				regulator-name = "vcc3v3_sd";
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&mdio0 {
> > +	rgmii_phy0: ethernet-phy@0 {
> > +		compatible = "ethernet-phy-ieee802.3-c22";
> > +		reg = <0x0>;
> > +		reset-assert-us = <20000>;
> > +		reset-deassert-us = <100000>;
> > +		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > +	};
> > +};
> > +
> > +&pinctrl {
> > +	leds {
> > +		led_power_en: led_power_en {
> 
> No underscores in node names. Do bindings require specific naming? I
> would expect "-grp" or "-pins" suffixes, if possible.
> 
> > +			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +		led_work_en: led_work_en {
> > +			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +	};
> > +
> > +	pmic {
> > +		pmic_int: pmic_int {
> > +			rockchip,pins =
> > +				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> > +		};
> > +	};
> > +};
> > +
> > +&pmu_io_domains {
> > +	pmuio1-supply = <&vcc3v3_pmu>;
> > +	pmuio2-supply = <&vcc3v3_pmu>;
> > +	vccio1-supply = <&vccio_acodec>;
> > +	vccio2-supply = <&vcc_1v8>;
> > +	vccio3-supply = <&vccio_sd>;
> > +	vccio4-supply = <&vcc_1v8>;
> > +	vccio5-supply = <&vcc_3v3>;
> > +	vccio6-supply = <&vcc_3v3>;
> > +	vccio7-supply = <&vcc_3v3>;
> > +	status = "okay";
> > +};
> > +
> > +&saradc {
> > +	vref-supply = <&vcca_1v8>;
> > +	status = "okay";
> > +};
> > +
> > +&sdhci {
> > +	bus-width = <8>;
> > +	max-frequency = <200000000>;
> > +	non-removable;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> > +	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-sdr104;
> > +	vmmc-supply = <&vcc3v3_sd>;
> > +	vqmmc-supply = <&vccio_sd>;
> > +	status = "okay";
> > +};
> > +
> > +&uart2 {
> > +	status = "okay";
> > +};
> 
> 
> Best regards,
> Krzysztof
Thank you,
Dongjin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-05  2:32       ` Dongjin Kim
  0 siblings, 0 replies; 33+ messages in thread
From: Dongjin Kim @ 2022-04-05  2:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On Tue, Mar 29, 2022 at 07:12:16PM +0200, Krzysztof Kozlowski wrote:
> On 29/03/2022 11:44, Dongjin Kim wrote:
> > This patch is to add a device tree for new board Hardkernel ODROID-M1
> > based on Rockchip RK3568, includes basic peripherals -
> > uart/eMMC/uSD/i2c and on-board ethernet.
> 
> I think the email got corrupted (incorrect To addresses).
> 
Thank you for reviewing and sorry for late reply.
> > 
> > Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
> >  2 files changed, 407 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index 4ae9f35434b8..81d160221227 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > new file mode 100644
> > index 000000000000..d1a5d43127e9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > @@ -0,0 +1,406 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2022 Hardkernel Co., Ltd.
> > + *
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include "rk3568.dtsi"
> > +
> > +/ {
> > +	model = "Hardkernel ODROID-M1";
> > +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> > +
> > +	aliases {
> > +		ethernet0 = &gmac0;
> > +		i2c0 = &i2c3;
> > +		i2c3 = &i2c0;
> > +		mmc0 = &sdhci;
> > +		mmc1 = &sdmmc0;
> > +		serial0 = &uart1;
> > +		serial1 = &uart0;
> > +	};
> > +
> > +	chosen: chosen {
> 
> No need for label.
> 
Ok. Will fix it.
> > +		stdout-path = "serial2:1500000n8";
> > +	};
> > +
> > +	dc_12v: dc-12v {
> 
> Generic node name, so "regulator" or "regulator-0"
> 
I've followed the node names as already merged device tree files
for other boards and thought this would be acceptable. Same for other
node names 'vcc3v3-sys' and node names with underscore below.
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "dc_12v";
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +	};
> > +
> > +	leds: leds {
> 
> Label seems unused.
> 
Ok, will fix it.
> > +		compatible = "gpio-leds";
> > +
> > +		led_power: led-0 {
> > +			gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> > +			function = LED_FUNCTION_POWER;
> > +			color = <LED_COLOR_ID_RED>;
> > +			linux,default-trigger = "default-on";
> > +			pinctrl-names = "default";
> > +			pinctrl-0 = <&led_power_en>;
> > +		};
> > +		led_work: led-1 {
> > +			gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> > +			function = LED_FUNCTION_HEARTBEAT;
> > +			color = <LED_COLOR_ID_BLUE>;
> > +			linux,default-trigger = "heartbeat";
> > +			pinctrl-names = "default";
> > +			pinctrl-0 = <&led_work_en>;
> > +		};
> > +	};
> > +
> > +	vcc3v3_sys: vcc3v3-sys {
> 
> Generic node name.
> 
> > +		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>;
> > +	};
> > +};
> > +
> > +&cpu0 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu1 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu2 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu3 {
> > +	cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&gmac0 {
> > +	assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> > +	assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> > +	assigned-clock-rates = <0>, <125000000>;
> > +	clock_in_out = "output";
> > +	phy-handle = <&rgmii_phy0>;
> > +	phy-mode = "rgmii";
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&gmac0_miim
> > +		     &gmac0_tx_bus2
> > +		     &gmac0_rx_bus2
> > +		     &gmac0_rgmii_clk
> > +		     &gmac0_rgmii_bus>;
> > +	status = "okay";
> > +
> > +	tx_delay = <0x4f>;
> > +	rx_delay = <0x2d>;
> > +};
> > +
> > +&i2c0 {
> > +	status = "okay";
> > +
> > +	vdd_cpu: regulator@1c {
> > +		compatible = "tcs,z`";
> > +		reg = <0x1c>;
> > +		 fcs,suspend-voltage-selector = <1>;
> 
> Mixed up indentation.
> 
Sorry, my bad. Will fix it.
> > +		 regulator-name = "vdd_cpu";
> > +		 regulator-always-on;
> > +		 regulator-boot-on;
> > +		 regulator-min-microvolt = <800000>;
> > +		 regulator-max-microvolt = <1150000>;
> > +		 regulator-ramp-delay = <2300>;
> > +		 vin-supply = <&vcc3v3_sys>;
> > +
> > +		 regulator-state-mem {
> > +			 regulator-off-in-suspend;
> > +		};
> > +	};
> > +
> > +	rk809: pmic@20 {
> > +		compatible = "rockchip,rk809";
> > +		reg = <0x20>;
> > +		interrupt-parent = <&gpio0>;
> > +		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> > +		#clock-cells = <1>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pmic_int>;
> > +		rockchip,system-power-controller;
> > +		vcc1-supply = <&vcc3v3_sys>;
> > +		vcc2-supply = <&vcc3v3_sys>;
> > +		vcc3-supply = <&vcc3v3_sys>;
> > +		vcc4-supply = <&vcc3v3_sys>;
> > +		vcc5-supply = <&vcc3v3_sys>;
> > +		vcc6-supply = <&vcc3v3_sys>;
> > +		vcc7-supply = <&vcc3v3_sys>;
> > +		vcc8-supply = <&vcc3v3_sys>;
> > +		vcc9-supply = <&vcc3v3_sys>;
> > +		wakeup-source;
> > +
> > +		regulators {
> > +			vdd_logic: DCDC_REG1 {
> 
> No underscores in node names, unless anything requires it.
> 
> > +				regulator-name = "vdd_logic";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_gpu: DCDC_REG2 {
> > +				regulator-name = "vdd_gpu";
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_ddr: DCDC_REG3 {
> > +				regulator-name = "vcc_ddr";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-initial-mode = <0x2>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdd_npu: DCDC_REG4 {
> > +				regulator-name = "vdd_npu";
> > +				regulator-init-microvolt = <900000>;
> > +				regulator-initial-mode = <0x2>;
> > +				regulator-min-microvolt = <500000>;
> > +				regulator-max-microvolt = <1350000>;
> > +				regulator-ramp-delay = <6001>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_1v8: DCDC_REG5 {
> > +				regulator-name = "vcc_1v8";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda0v9_image: LDO_REG1 {
> > +				regulator-name = "vdda0v9_image";
> > +				regulator-min-microvolt = <900000>;
> > +				regulator-max-microvolt = <900000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vdda_0v9: LDO_REG2 {
> > +				regulator-name = "vdda_0v9";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <900000>;
> > +				regulator-max-microvolt = <900000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			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>;
> > +				};
> > +			};
> > +
> > +			vccio_acodec: LDO_REG4 {
> > +				regulator-name = "vccio_acodec";
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vccio_sd: LDO_REG5 {
> > +				regulator-name = "vccio_sd";
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc3v3_pmu: LDO_REG6 {
> > +				regulator-name = "vcc3v3_pmu";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <3300000>;
> > +				regulator-max-microvolt = <3300000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = <3300000>;
> > +				};
> > +			};
> > +
> > +			vcca_1v8: LDO_REG7 {
> > +				regulator-name = "vcca_1v8";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcca1v8_pmu: LDO_REG8 {
> > +				regulator-name = "vcca1v8_pmu";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-on-in-suspend;
> > +					regulator-suspend-microvolt = <1800000>;
> > +				};
> > +			};
> > +
> > +			vcca1v8_image: LDO_REG9 {
> > +				regulator-name = "vcca1v8_image";
> > +				regulator-min-microvolt = <1800000>;
> > +				regulator-max-microvolt = <1800000>;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc_3v3: SWITCH_REG1 {
> > +				regulator-name = "vcc_3v3";
> > +				regulator-always-on;
> > +				regulator-boot-on;
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +
> > +			vcc3v3_sd: SWITCH_REG2 {
> > +				regulator-name = "vcc3v3_sd";
> > +
> > +				regulator-state-mem {
> > +					regulator-off-in-suspend;
> > +				};
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&mdio0 {
> > +	rgmii_phy0: ethernet-phy@0 {
> > +		compatible = "ethernet-phy-ieee802.3-c22";
> > +		reg = <0x0>;
> > +		reset-assert-us = <20000>;
> > +		reset-deassert-us = <100000>;
> > +		reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > +	};
> > +};
> > +
> > +&pinctrl {
> > +	leds {
> > +		led_power_en: led_power_en {
> 
> No underscores in node names. Do bindings require specific naming? I
> would expect "-grp" or "-pins" suffixes, if possible.
> 
> > +			rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +		led_work_en: led_work_en {
> > +			rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> > +		};
> > +	};
> > +
> > +	pmic {
> > +		pmic_int: pmic_int {
> > +			rockchip,pins =
> > +				<0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> > +		};
> > +	};
> > +};
> > +
> > +&pmu_io_domains {
> > +	pmuio1-supply = <&vcc3v3_pmu>;
> > +	pmuio2-supply = <&vcc3v3_pmu>;
> > +	vccio1-supply = <&vccio_acodec>;
> > +	vccio2-supply = <&vcc_1v8>;
> > +	vccio3-supply = <&vccio_sd>;
> > +	vccio4-supply = <&vcc_1v8>;
> > +	vccio5-supply = <&vcc_3v3>;
> > +	vccio6-supply = <&vcc_3v3>;
> > +	vccio7-supply = <&vcc_3v3>;
> > +	status = "okay";
> > +};
> > +
> > +&saradc {
> > +	vref-supply = <&vcca_1v8>;
> > +	status = "okay";
> > +};
> > +
> > +&sdhci {
> > +	bus-width = <8>;
> > +	max-frequency = <200000000>;
> > +	non-removable;
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> > +	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-sdr104;
> > +	vmmc-supply = <&vcc3v3_sd>;
> > +	vqmmc-supply = <&vccio_sd>;
> > +	status = "okay";
> > +};
> > +
> > +&uart2 {
> > +	status = "okay";
> > +};
> 
> 
> Best regards,
> Krzysztof
Thank you,
Dongjin.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-04-05  2:32       ` Dongjin Kim
  (?)
@ 2022-04-05  6:13         ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-05  6:13 UTC (permalink / raw)
  To: Dongjin Kim
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On 05/04/2022 04:32, Dongjin Kim wrote:
> On Tue, Mar 29, 2022 at 07:12:16PM +0200, Krzysztof Kozlowski wrote:
>> On 29/03/2022 11:44, Dongjin Kim wrote:
>>> This patch is to add a device tree for new board Hardkernel ODROID-M1
>>> based on Rockchip RK3568, includes basic peripherals -
>>> uart/eMMC/uSD/i2c and on-board ethernet.
>>
>> I think the email got corrupted (incorrect To addresses).
>>
> Thank you for reviewing and sorry for late reply.
>>>
>>> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>>>  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
>>>  2 files changed, 407 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
>>> index 4ae9f35434b8..81d160221227 100644
>>> --- a/arch/arm64/boot/dts/rockchip/Makefile
>>> +++ b/arch/arm64/boot/dts/rockchip/Makefile
>>> @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
>>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> new file mode 100644
>>> index 000000000000..d1a5d43127e9
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> @@ -0,0 +1,406 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright (c) 2022 Hardkernel Co., Ltd.
>>> + *
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +#include <dt-bindings/pinctrl/rockchip.h>
>>> +#include "rk3568.dtsi"
>>> +
>>> +/ {
>>> +	model = "Hardkernel ODROID-M1";
>>> +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
>>> +
>>> +	aliases {
>>> +		ethernet0 = &gmac0;
>>> +		i2c0 = &i2c3;
>>> +		i2c3 = &i2c0;
>>> +		mmc0 = &sdhci;
>>> +		mmc1 = &sdmmc0;
>>> +		serial0 = &uart1;
>>> +		serial1 = &uart0;
>>> +	};
>>> +
>>> +	chosen: chosen {
>>
>> No need for label.
>>
> Ok. Will fix it.
>>> +		stdout-path = "serial2:1500000n8";
>>> +	};
>>> +
>>> +	dc_12v: dc-12v {
>>
>> Generic node name, so "regulator" or "regulator-0"
>>
> I've followed the node names as already merged device tree files
> for other boards and thought this would be acceptable. Same for other
> node names 'vcc3v3-sys' and node names with underscore below.

Poor code once it gets in, it's difficult to get it out... Don't use it
as an example. :)


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-05  6:13         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-05  6:13 UTC (permalink / raw)
  To: Dongjin Kim
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On 05/04/2022 04:32, Dongjin Kim wrote:
> On Tue, Mar 29, 2022 at 07:12:16PM +0200, Krzysztof Kozlowski wrote:
>> On 29/03/2022 11:44, Dongjin Kim wrote:
>>> This patch is to add a device tree for new board Hardkernel ODROID-M1
>>> based on Rockchip RK3568, includes basic peripherals -
>>> uart/eMMC/uSD/i2c and on-board ethernet.
>>
>> I think the email got corrupted (incorrect To addresses).
>>
> Thank you for reviewing and sorry for late reply.
>>>
>>> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>>>  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
>>>  2 files changed, 407 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
>>> index 4ae9f35434b8..81d160221227 100644
>>> --- a/arch/arm64/boot/dts/rockchip/Makefile
>>> +++ b/arch/arm64/boot/dts/rockchip/Makefile
>>> @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
>>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> new file mode 100644
>>> index 000000000000..d1a5d43127e9
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> @@ -0,0 +1,406 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright (c) 2022 Hardkernel Co., Ltd.
>>> + *
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +#include <dt-bindings/pinctrl/rockchip.h>
>>> +#include "rk3568.dtsi"
>>> +
>>> +/ {
>>> +	model = "Hardkernel ODROID-M1";
>>> +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
>>> +
>>> +	aliases {
>>> +		ethernet0 = &gmac0;
>>> +		i2c0 = &i2c3;
>>> +		i2c3 = &i2c0;
>>> +		mmc0 = &sdhci;
>>> +		mmc1 = &sdmmc0;
>>> +		serial0 = &uart1;
>>> +		serial1 = &uart0;
>>> +	};
>>> +
>>> +	chosen: chosen {
>>
>> No need for label.
>>
> Ok. Will fix it.
>>> +		stdout-path = "serial2:1500000n8";
>>> +	};
>>> +
>>> +	dc_12v: dc-12v {
>>
>> Generic node name, so "regulator" or "regulator-0"
>>
> I've followed the node names as already merged device tree files
> for other boards and thought this would be acceptable. Same for other
> node names 'vcc3v3-sys' and node names with underscore below.

Poor code once it gets in, it's difficult to get it out... Don't use it
as an example. :)


Best regards,
Krzysztof

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-05  6:13         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-05  6:13 UTC (permalink / raw)
  To: Dongjin Kim
  Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
	linux-kernel

On 05/04/2022 04:32, Dongjin Kim wrote:
> On Tue, Mar 29, 2022 at 07:12:16PM +0200, Krzysztof Kozlowski wrote:
>> On 29/03/2022 11:44, Dongjin Kim wrote:
>>> This patch is to add a device tree for new board Hardkernel ODROID-M1
>>> based on Rockchip RK3568, includes basic peripherals -
>>> uart/eMMC/uSD/i2c and on-board ethernet.
>>
>> I think the email got corrupted (incorrect To addresses).
>>
> Thank you for reviewing and sorry for late reply.
>>>
>>> Signed-off-by: Dongjin Kim <tobetter@gmail.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>>>  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
>>>  2 files changed, 407 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
>>> index 4ae9f35434b8..81d160221227 100644
>>> --- a/arch/arm64/boot/dts/rockchip/Makefile
>>> +++ b/arch/arm64/boot/dts/rockchip/Makefile
>>> @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
>>>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
>>> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> new file mode 100644
>>> index 000000000000..d1a5d43127e9
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
>>> @@ -0,0 +1,406 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>> +/*
>>> + * Copyright (c) 2022 Hardkernel Co., Ltd.
>>> + *
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +#include <dt-bindings/pinctrl/rockchip.h>
>>> +#include "rk3568.dtsi"
>>> +
>>> +/ {
>>> +	model = "Hardkernel ODROID-M1";
>>> +	compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
>>> +
>>> +	aliases {
>>> +		ethernet0 = &gmac0;
>>> +		i2c0 = &i2c3;
>>> +		i2c3 = &i2c0;
>>> +		mmc0 = &sdhci;
>>> +		mmc1 = &sdmmc0;
>>> +		serial0 = &uart1;
>>> +		serial1 = &uart0;
>>> +	};
>>> +
>>> +	chosen: chosen {
>>
>> No need for label.
>>
> Ok. Will fix it.
>>> +		stdout-path = "serial2:1500000n8";
>>> +	};
>>> +
>>> +	dc_12v: dc-12v {
>>
>> Generic node name, so "regulator" or "regulator-0"
>>
> I've followed the node names as already merged device tree files
> for other boards and thought this would be acceptable. Same for other
> node names 'vcc3v3-sys' and node names with underscore below.

Poor code once it gets in, it's difficult to get it out... Don't use it
as an example. :)


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-03-29 17:12     ` Krzysztof Kozlowski
  (?)
@ 2022-04-16 12:07       ` Peter Geis
  -1 siblings, 0 replies; 33+ messages in thread
From: Peter Geis @ 2022-04-16 12:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Dongjin Kim, Heiko Stuebner, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On Tue, Mar 29, 2022 at 1:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 29/03/2022 11:44, Dongjin Kim wrote:
> > This patch is to add a device tree for new board Hardkernel ODROID-M1
> > based on Rockchip RK3568, includes basic peripherals -
> > uart/eMMC/uSD/i2c and on-board ethernet.
>
> I think the email got corrupted (incorrect To addresses).
>
> >
> > Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
> >  2 files changed, 407 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index 4ae9f35434b8..81d160221227 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > new file mode 100644
> > index 000000000000..d1a5d43127e9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > @@ -0,0 +1,406 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2022 Hardkernel Co., Ltd.
> > + *
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include "rk3568.dtsi"
> > +
> > +/ {
> > +     model = "Hardkernel ODROID-M1";
> > +     compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> > +
> > +     aliases {
> > +             ethernet0 = &gmac0;
> > +             i2c0 = &i2c3;
> > +             i2c3 = &i2c0;
> > +             mmc0 = &sdhci;
> > +             mmc1 = &sdmmc0;
> > +             serial0 = &uart1;
> > +             serial1 = &uart0;
> > +     };
> > +
> > +     chosen: chosen {
>
> No need for label.
>
> > +             stdout-path = "serial2:1500000n8";
> > +     };
> > +
> > +     dc_12v: dc-12v {
>
> Generic node name, so "regulator" or "regulator-0"

Unfortunately, this advice breaks the regulator-fixed driver, which it
seems cannot cope with a bunch of nodes all named "regulator".
Setting the regulators as regulator-0 -1 -2 leads to fun issues where
the regulator numbering in the kernel doesn't match the node numbers.
It also makes it more fun when additional regulators need to be added
and everything gets shuffled around.

If naming these uniquely to avoid confusion and collisions is such an
issue, why is it not caught by make W=1 dtbs_check?

>
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "dc_12v";
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +             regulator-min-microvolt = <12000000>;
> > +             regulator-max-microvolt = <12000000>;
> > +     };
> > +
> > +     leds: leds {
>
> Label seems unused.
>
> > +             compatible = "gpio-leds";
> > +
> > +             led_power: led-0 {
> > +                     gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> > +                     function = LED_FUNCTION_POWER;
> > +                     color = <LED_COLOR_ID_RED>;
> > +                     linux,default-trigger = "default-on";
> > +                     pinctrl-names = "default";
> > +                     pinctrl-0 = <&led_power_en>;
> > +             };
> > +             led_work: led-1 {
> > +                     gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> > +                     function = LED_FUNCTION_HEARTBEAT;
> > +                     color = <LED_COLOR_ID_BLUE>;
> > +                     linux,default-trigger = "heartbeat";
> > +                     pinctrl-names = "default";
> > +                     pinctrl-0 = <&led_work_en>;
> > +             };
> > +     };
> > +
> > +     vcc3v3_sys: vcc3v3-sys {
>
> Generic node name.
>
> > +             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>;
> > +     };
> > +};
> > +
> > +&cpu0 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu1 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu2 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu3 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&gmac0 {
> > +     assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> > +     assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> > +     assigned-clock-rates = <0>, <125000000>;
> > +     clock_in_out = "output";
> > +     phy-handle = <&rgmii_phy0>;
> > +     phy-mode = "rgmii";
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&gmac0_miim
> > +                  &gmac0_tx_bus2
> > +                  &gmac0_rx_bus2
> > +                  &gmac0_rgmii_clk
> > +                  &gmac0_rgmii_bus>;
> > +     status = "okay";
> > +
> > +     tx_delay = <0x4f>;
> > +     rx_delay = <0x2d>;
> > +};
> > +
> > +&i2c0 {
> > +     status = "okay";
> > +
> > +     vdd_cpu: regulator@1c {
> > +             compatible = "tcs,z`";
> > +             reg = <0x1c>;
> > +              fcs,suspend-voltage-selector = <1>;
>
> Mixed up indentation.
>
> > +              regulator-name = "vdd_cpu";
> > +              regulator-always-on;
> > +              regulator-boot-on;
> > +              regulator-min-microvolt = <800000>;
> > +              regulator-max-microvolt = <1150000>;
> > +              regulator-ramp-delay = <2300>;
> > +              vin-supply = <&vcc3v3_sys>;
> > +
> > +              regulator-state-mem {
> > +                      regulator-off-in-suspend;
> > +             };
> > +     };
> > +
> > +     rk809: pmic@20 {
> > +             compatible = "rockchip,rk809";
> > +             reg = <0x20>;
> > +             interrupt-parent = <&gpio0>;
> > +             interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> > +             #clock-cells = <1>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&pmic_int>;
> > +             rockchip,system-power-controller;
> > +             vcc1-supply = <&vcc3v3_sys>;
> > +             vcc2-supply = <&vcc3v3_sys>;
> > +             vcc3-supply = <&vcc3v3_sys>;
> > +             vcc4-supply = <&vcc3v3_sys>;
> > +             vcc5-supply = <&vcc3v3_sys>;
> > +             vcc6-supply = <&vcc3v3_sys>;
> > +             vcc7-supply = <&vcc3v3_sys>;
> > +             vcc8-supply = <&vcc3v3_sys>;
> > +             vcc9-supply = <&vcc3v3_sys>;
> > +             wakeup-source;
> > +
> > +             regulators {
> > +                     vdd_logic: DCDC_REG1 {
>
> No underscores in node names, unless anything requires it.
>
> > +                             regulator-name = "vdd_logic";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdd_gpu: DCDC_REG2 {
> > +                             regulator-name = "vdd_gpu";
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_ddr: DCDC_REG3 {
> > +                             regulator-name = "vcc_ddr";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-initial-mode = <0x2>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdd_npu: DCDC_REG4 {
> > +                             regulator-name = "vdd_npu";
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_1v8: DCDC_REG5 {
> > +                             regulator-name = "vcc_1v8";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdda0v9_image: LDO_REG1 {
> > +                             regulator-name = "vdda0v9_image";
> > +                             regulator-min-microvolt = <900000>;
> > +                             regulator-max-microvolt = <900000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdda_0v9: LDO_REG2 {
> > +                             regulator-name = "vdda_0v9";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <900000>;
> > +                             regulator-max-microvolt = <900000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     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>;
> > +                             };
> > +                     };
> > +
> > +                     vccio_acodec: LDO_REG4 {
> > +                             regulator-name = "vccio_acodec";
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vccio_sd: LDO_REG5 {
> > +                             regulator-name = "vccio_sd";
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc3v3_pmu: LDO_REG6 {
> > +                             regulator-name = "vcc3v3_pmu";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt = <3300000>;
> > +                             };
> > +                     };
> > +
> > +                     vcca_1v8: LDO_REG7 {
> > +                             regulator-name = "vcca_1v8";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcca1v8_pmu: LDO_REG8 {
> > +                             regulator-name = "vcca1v8_pmu";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt = <1800000>;
> > +                             };
> > +                     };
> > +
> > +                     vcca1v8_image: LDO_REG9 {
> > +                             regulator-name = "vcca1v8_image";
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_3v3: SWITCH_REG1 {
> > +                             regulator-name = "vcc_3v3";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc3v3_sd: SWITCH_REG2 {
> > +                             regulator-name = "vcc3v3_sd";
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> > +&mdio0 {
> > +     rgmii_phy0: ethernet-phy@0 {
> > +             compatible = "ethernet-phy-ieee802.3-c22";
> > +             reg = <0x0>;
> > +             reset-assert-us = <20000>;
> > +             reset-deassert-us = <100000>;
> > +             reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > +     };
> > +};
> > +
> > +&pinctrl {
> > +     leds {
> > +             led_power_en: led_power_en {
>
> No underscores in node names. Do bindings require specific naming? I
> would expect "-grp" or "-pins" suffixes, if possible.
>
> > +                     rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> > +             };
> > +             led_work_en: led_work_en {
> > +                     rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> > +             };
> > +     };
> > +
> > +     pmic {
> > +             pmic_int: pmic_int {
> > +                     rockchip,pins =
> > +                             <0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> > +             };
> > +     };
> > +};
> > +
> > +&pmu_io_domains {
> > +     pmuio1-supply = <&vcc3v3_pmu>;
> > +     pmuio2-supply = <&vcc3v3_pmu>;
> > +     vccio1-supply = <&vccio_acodec>;
> > +     vccio2-supply = <&vcc_1v8>;
> > +     vccio3-supply = <&vccio_sd>;
> > +     vccio4-supply = <&vcc_1v8>;
> > +     vccio5-supply = <&vcc_3v3>;
> > +     vccio6-supply = <&vcc_3v3>;
> > +     vccio7-supply = <&vcc_3v3>;
> > +     status = "okay";
> > +};
> > +
> > +&saradc {
> > +     vref-supply = <&vcca_1v8>;
> > +     status = "okay";
> > +};
> > +
> > +&sdhci {
> > +     bus-width = <8>;
> > +     max-frequency = <200000000>;
> > +     non-removable;
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> > +     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-sdr104;
> > +     vmmc-supply = <&vcc3v3_sd>;
> > +     vqmmc-supply = <&vccio_sd>;
> > +     status = "okay";
> > +};
> > +
> > +&uart2 {
> > +     status = "okay";
> > +};
>
>
> Best regards,
> Krzysztof
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-16 12:07       ` Peter Geis
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Geis @ 2022-04-16 12:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Dongjin Kim, Heiko Stuebner, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On Tue, Mar 29, 2022 at 1:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 29/03/2022 11:44, Dongjin Kim wrote:
> > This patch is to add a device tree for new board Hardkernel ODROID-M1
> > based on Rockchip RK3568, includes basic peripherals -
> > uart/eMMC/uSD/i2c and on-board ethernet.
>
> I think the email got corrupted (incorrect To addresses).
>
> >
> > Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
> >  2 files changed, 407 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index 4ae9f35434b8..81d160221227 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > new file mode 100644
> > index 000000000000..d1a5d43127e9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > @@ -0,0 +1,406 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2022 Hardkernel Co., Ltd.
> > + *
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include "rk3568.dtsi"
> > +
> > +/ {
> > +     model = "Hardkernel ODROID-M1";
> > +     compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> > +
> > +     aliases {
> > +             ethernet0 = &gmac0;
> > +             i2c0 = &i2c3;
> > +             i2c3 = &i2c0;
> > +             mmc0 = &sdhci;
> > +             mmc1 = &sdmmc0;
> > +             serial0 = &uart1;
> > +             serial1 = &uart0;
> > +     };
> > +
> > +     chosen: chosen {
>
> No need for label.
>
> > +             stdout-path = "serial2:1500000n8";
> > +     };
> > +
> > +     dc_12v: dc-12v {
>
> Generic node name, so "regulator" or "regulator-0"

Unfortunately, this advice breaks the regulator-fixed driver, which it
seems cannot cope with a bunch of nodes all named "regulator".
Setting the regulators as regulator-0 -1 -2 leads to fun issues where
the regulator numbering in the kernel doesn't match the node numbers.
It also makes it more fun when additional regulators need to be added
and everything gets shuffled around.

If naming these uniquely to avoid confusion and collisions is such an
issue, why is it not caught by make W=1 dtbs_check?

>
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "dc_12v";
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +             regulator-min-microvolt = <12000000>;
> > +             regulator-max-microvolt = <12000000>;
> > +     };
> > +
> > +     leds: leds {
>
> Label seems unused.
>
> > +             compatible = "gpio-leds";
> > +
> > +             led_power: led-0 {
> > +                     gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> > +                     function = LED_FUNCTION_POWER;
> > +                     color = <LED_COLOR_ID_RED>;
> > +                     linux,default-trigger = "default-on";
> > +                     pinctrl-names = "default";
> > +                     pinctrl-0 = <&led_power_en>;
> > +             };
> > +             led_work: led-1 {
> > +                     gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> > +                     function = LED_FUNCTION_HEARTBEAT;
> > +                     color = <LED_COLOR_ID_BLUE>;
> > +                     linux,default-trigger = "heartbeat";
> > +                     pinctrl-names = "default";
> > +                     pinctrl-0 = <&led_work_en>;
> > +             };
> > +     };
> > +
> > +     vcc3v3_sys: vcc3v3-sys {
>
> Generic node name.
>
> > +             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>;
> > +     };
> > +};
> > +
> > +&cpu0 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu1 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu2 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu3 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&gmac0 {
> > +     assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> > +     assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> > +     assigned-clock-rates = <0>, <125000000>;
> > +     clock_in_out = "output";
> > +     phy-handle = <&rgmii_phy0>;
> > +     phy-mode = "rgmii";
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&gmac0_miim
> > +                  &gmac0_tx_bus2
> > +                  &gmac0_rx_bus2
> > +                  &gmac0_rgmii_clk
> > +                  &gmac0_rgmii_bus>;
> > +     status = "okay";
> > +
> > +     tx_delay = <0x4f>;
> > +     rx_delay = <0x2d>;
> > +};
> > +
> > +&i2c0 {
> > +     status = "okay";
> > +
> > +     vdd_cpu: regulator@1c {
> > +             compatible = "tcs,z`";
> > +             reg = <0x1c>;
> > +              fcs,suspend-voltage-selector = <1>;
>
> Mixed up indentation.
>
> > +              regulator-name = "vdd_cpu";
> > +              regulator-always-on;
> > +              regulator-boot-on;
> > +              regulator-min-microvolt = <800000>;
> > +              regulator-max-microvolt = <1150000>;
> > +              regulator-ramp-delay = <2300>;
> > +              vin-supply = <&vcc3v3_sys>;
> > +
> > +              regulator-state-mem {
> > +                      regulator-off-in-suspend;
> > +             };
> > +     };
> > +
> > +     rk809: pmic@20 {
> > +             compatible = "rockchip,rk809";
> > +             reg = <0x20>;
> > +             interrupt-parent = <&gpio0>;
> > +             interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> > +             #clock-cells = <1>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&pmic_int>;
> > +             rockchip,system-power-controller;
> > +             vcc1-supply = <&vcc3v3_sys>;
> > +             vcc2-supply = <&vcc3v3_sys>;
> > +             vcc3-supply = <&vcc3v3_sys>;
> > +             vcc4-supply = <&vcc3v3_sys>;
> > +             vcc5-supply = <&vcc3v3_sys>;
> > +             vcc6-supply = <&vcc3v3_sys>;
> > +             vcc7-supply = <&vcc3v3_sys>;
> > +             vcc8-supply = <&vcc3v3_sys>;
> > +             vcc9-supply = <&vcc3v3_sys>;
> > +             wakeup-source;
> > +
> > +             regulators {
> > +                     vdd_logic: DCDC_REG1 {
>
> No underscores in node names, unless anything requires it.
>
> > +                             regulator-name = "vdd_logic";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdd_gpu: DCDC_REG2 {
> > +                             regulator-name = "vdd_gpu";
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_ddr: DCDC_REG3 {
> > +                             regulator-name = "vcc_ddr";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-initial-mode = <0x2>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdd_npu: DCDC_REG4 {
> > +                             regulator-name = "vdd_npu";
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_1v8: DCDC_REG5 {
> > +                             regulator-name = "vcc_1v8";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdda0v9_image: LDO_REG1 {
> > +                             regulator-name = "vdda0v9_image";
> > +                             regulator-min-microvolt = <900000>;
> > +                             regulator-max-microvolt = <900000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdda_0v9: LDO_REG2 {
> > +                             regulator-name = "vdda_0v9";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <900000>;
> > +                             regulator-max-microvolt = <900000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     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>;
> > +                             };
> > +                     };
> > +
> > +                     vccio_acodec: LDO_REG4 {
> > +                             regulator-name = "vccio_acodec";
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vccio_sd: LDO_REG5 {
> > +                             regulator-name = "vccio_sd";
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc3v3_pmu: LDO_REG6 {
> > +                             regulator-name = "vcc3v3_pmu";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt = <3300000>;
> > +                             };
> > +                     };
> > +
> > +                     vcca_1v8: LDO_REG7 {
> > +                             regulator-name = "vcca_1v8";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcca1v8_pmu: LDO_REG8 {
> > +                             regulator-name = "vcca1v8_pmu";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt = <1800000>;
> > +                             };
> > +                     };
> > +
> > +                     vcca1v8_image: LDO_REG9 {
> > +                             regulator-name = "vcca1v8_image";
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_3v3: SWITCH_REG1 {
> > +                             regulator-name = "vcc_3v3";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc3v3_sd: SWITCH_REG2 {
> > +                             regulator-name = "vcc3v3_sd";
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> > +&mdio0 {
> > +     rgmii_phy0: ethernet-phy@0 {
> > +             compatible = "ethernet-phy-ieee802.3-c22";
> > +             reg = <0x0>;
> > +             reset-assert-us = <20000>;
> > +             reset-deassert-us = <100000>;
> > +             reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > +     };
> > +};
> > +
> > +&pinctrl {
> > +     leds {
> > +             led_power_en: led_power_en {
>
> No underscores in node names. Do bindings require specific naming? I
> would expect "-grp" or "-pins" suffixes, if possible.
>
> > +                     rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> > +             };
> > +             led_work_en: led_work_en {
> > +                     rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> > +             };
> > +     };
> > +
> > +     pmic {
> > +             pmic_int: pmic_int {
> > +                     rockchip,pins =
> > +                             <0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> > +             };
> > +     };
> > +};
> > +
> > +&pmu_io_domains {
> > +     pmuio1-supply = <&vcc3v3_pmu>;
> > +     pmuio2-supply = <&vcc3v3_pmu>;
> > +     vccio1-supply = <&vccio_acodec>;
> > +     vccio2-supply = <&vcc_1v8>;
> > +     vccio3-supply = <&vccio_sd>;
> > +     vccio4-supply = <&vcc_1v8>;
> > +     vccio5-supply = <&vcc_3v3>;
> > +     vccio6-supply = <&vcc_3v3>;
> > +     vccio7-supply = <&vcc_3v3>;
> > +     status = "okay";
> > +};
> > +
> > +&saradc {
> > +     vref-supply = <&vcca_1v8>;
> > +     status = "okay";
> > +};
> > +
> > +&sdhci {
> > +     bus-width = <8>;
> > +     max-frequency = <200000000>;
> > +     non-removable;
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> > +     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-sdr104;
> > +     vmmc-supply = <&vcc3v3_sd>;
> > +     vqmmc-supply = <&vccio_sd>;
> > +     status = "okay";
> > +};
> > +
> > +&uart2 {
> > +     status = "okay";
> > +};
>
>
> Best regards,
> Krzysztof
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-16 12:07       ` Peter Geis
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Geis @ 2022-04-16 12:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Dongjin Kim, Heiko Stuebner, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On Tue, Mar 29, 2022 at 1:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 29/03/2022 11:44, Dongjin Kim wrote:
> > This patch is to add a device tree for new board Hardkernel ODROID-M1
> > based on Rockchip RK3568, includes basic peripherals -
> > uart/eMMC/uSD/i2c and on-board ethernet.
>
> I think the email got corrupted (incorrect To addresses).
>
> >
> > Signed-off-by: Dongjin Kim <tobetter@gmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >  .../boot/dts/rockchip/rk3568-odroid-m1.dts    | 406 ++++++++++++++++++
> >  2 files changed, 407 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index 4ae9f35434b8..81d160221227 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -61,3 +61,4 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-evb1-v10.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-bpi-r2-pro.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > new file mode 100644
> > index 000000000000..d1a5d43127e9
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts
> > @@ -0,0 +1,406 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2022 Hardkernel Co., Ltd.
> > + *
> > + */
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/leds/common.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include "rk3568.dtsi"
> > +
> > +/ {
> > +     model = "Hardkernel ODROID-M1";
> > +     compatible = "rockchip,rk3568-odroid-m1", "rockchip,rk3568";
> > +
> > +     aliases {
> > +             ethernet0 = &gmac0;
> > +             i2c0 = &i2c3;
> > +             i2c3 = &i2c0;
> > +             mmc0 = &sdhci;
> > +             mmc1 = &sdmmc0;
> > +             serial0 = &uart1;
> > +             serial1 = &uart0;
> > +     };
> > +
> > +     chosen: chosen {
>
> No need for label.
>
> > +             stdout-path = "serial2:1500000n8";
> > +     };
> > +
> > +     dc_12v: dc-12v {
>
> Generic node name, so "regulator" or "regulator-0"

Unfortunately, this advice breaks the regulator-fixed driver, which it
seems cannot cope with a bunch of nodes all named "regulator".
Setting the regulators as regulator-0 -1 -2 leads to fun issues where
the regulator numbering in the kernel doesn't match the node numbers.
It also makes it more fun when additional regulators need to be added
and everything gets shuffled around.

If naming these uniquely to avoid confusion and collisions is such an
issue, why is it not caught by make W=1 dtbs_check?

>
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "dc_12v";
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +             regulator-min-microvolt = <12000000>;
> > +             regulator-max-microvolt = <12000000>;
> > +     };
> > +
> > +     leds: leds {
>
> Label seems unused.
>
> > +             compatible = "gpio-leds";
> > +
> > +             led_power: led-0 {
> > +                     gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>;
> > +                     function = LED_FUNCTION_POWER;
> > +                     color = <LED_COLOR_ID_RED>;
> > +                     linux,default-trigger = "default-on";
> > +                     pinctrl-names = "default";
> > +                     pinctrl-0 = <&led_power_en>;
> > +             };
> > +             led_work: led-1 {
> > +                     gpios = <&gpio0 RK_PB7 GPIO_ACTIVE_HIGH>;
> > +                     function = LED_FUNCTION_HEARTBEAT;
> > +                     color = <LED_COLOR_ID_BLUE>;
> > +                     linux,default-trigger = "heartbeat";
> > +                     pinctrl-names = "default";
> > +                     pinctrl-0 = <&led_work_en>;
> > +             };
> > +     };
> > +
> > +     vcc3v3_sys: vcc3v3-sys {
>
> Generic node name.
>
> > +             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>;
> > +     };
> > +};
> > +
> > +&cpu0 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu1 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu2 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&cpu3 {
> > +     cpu-supply = <&vdd_cpu>;
> > +};
> > +
> > +&gmac0 {
> > +     assigned-clocks = <&cru SCLK_GMAC0_RX_TX>, <&cru SCLK_GMAC0>;
> > +     assigned-clock-parents = <&cru SCLK_GMAC0_RGMII_SPEED>;
> > +     assigned-clock-rates = <0>, <125000000>;
> > +     clock_in_out = "output";
> > +     phy-handle = <&rgmii_phy0>;
> > +     phy-mode = "rgmii";
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&gmac0_miim
> > +                  &gmac0_tx_bus2
> > +                  &gmac0_rx_bus2
> > +                  &gmac0_rgmii_clk
> > +                  &gmac0_rgmii_bus>;
> > +     status = "okay";
> > +
> > +     tx_delay = <0x4f>;
> > +     rx_delay = <0x2d>;
> > +};
> > +
> > +&i2c0 {
> > +     status = "okay";
> > +
> > +     vdd_cpu: regulator@1c {
> > +             compatible = "tcs,z`";
> > +             reg = <0x1c>;
> > +              fcs,suspend-voltage-selector = <1>;
>
> Mixed up indentation.
>
> > +              regulator-name = "vdd_cpu";
> > +              regulator-always-on;
> > +              regulator-boot-on;
> > +              regulator-min-microvolt = <800000>;
> > +              regulator-max-microvolt = <1150000>;
> > +              regulator-ramp-delay = <2300>;
> > +              vin-supply = <&vcc3v3_sys>;
> > +
> > +              regulator-state-mem {
> > +                      regulator-off-in-suspend;
> > +             };
> > +     };
> > +
> > +     rk809: pmic@20 {
> > +             compatible = "rockchip,rk809";
> > +             reg = <0x20>;
> > +             interrupt-parent = <&gpio0>;
> > +             interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> > +             #clock-cells = <1>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&pmic_int>;
> > +             rockchip,system-power-controller;
> > +             vcc1-supply = <&vcc3v3_sys>;
> > +             vcc2-supply = <&vcc3v3_sys>;
> > +             vcc3-supply = <&vcc3v3_sys>;
> > +             vcc4-supply = <&vcc3v3_sys>;
> > +             vcc5-supply = <&vcc3v3_sys>;
> > +             vcc6-supply = <&vcc3v3_sys>;
> > +             vcc7-supply = <&vcc3v3_sys>;
> > +             vcc8-supply = <&vcc3v3_sys>;
> > +             vcc9-supply = <&vcc3v3_sys>;
> > +             wakeup-source;
> > +
> > +             regulators {
> > +                     vdd_logic: DCDC_REG1 {
>
> No underscores in node names, unless anything requires it.
>
> > +                             regulator-name = "vdd_logic";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdd_gpu: DCDC_REG2 {
> > +                             regulator-name = "vdd_gpu";
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_ddr: DCDC_REG3 {
> > +                             regulator-name = "vcc_ddr";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-initial-mode = <0x2>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdd_npu: DCDC_REG4 {
> > +                             regulator-name = "vdd_npu";
> > +                             regulator-init-microvolt = <900000>;
> > +                             regulator-initial-mode = <0x2>;
> > +                             regulator-min-microvolt = <500000>;
> > +                             regulator-max-microvolt = <1350000>;
> > +                             regulator-ramp-delay = <6001>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_1v8: DCDC_REG5 {
> > +                             regulator-name = "vcc_1v8";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdda0v9_image: LDO_REG1 {
> > +                             regulator-name = "vdda0v9_image";
> > +                             regulator-min-microvolt = <900000>;
> > +                             regulator-max-microvolt = <900000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vdda_0v9: LDO_REG2 {
> > +                             regulator-name = "vdda_0v9";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <900000>;
> > +                             regulator-max-microvolt = <900000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     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>;
> > +                             };
> > +                     };
> > +
> > +                     vccio_acodec: LDO_REG4 {
> > +                             regulator-name = "vccio_acodec";
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vccio_sd: LDO_REG5 {
> > +                             regulator-name = "vccio_sd";
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc3v3_pmu: LDO_REG6 {
> > +                             regulator-name = "vcc3v3_pmu";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <3300000>;
> > +                             regulator-max-microvolt = <3300000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt = <3300000>;
> > +                             };
> > +                     };
> > +
> > +                     vcca_1v8: LDO_REG7 {
> > +                             regulator-name = "vcca_1v8";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcca1v8_pmu: LDO_REG8 {
> > +                             regulator-name = "vcca1v8_pmu";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-on-in-suspend;
> > +                                     regulator-suspend-microvolt = <1800000>;
> > +                             };
> > +                     };
> > +
> > +                     vcca1v8_image: LDO_REG9 {
> > +                             regulator-name = "vcca1v8_image";
> > +                             regulator-min-microvolt = <1800000>;
> > +                             regulator-max-microvolt = <1800000>;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc_3v3: SWITCH_REG1 {
> > +                             regulator-name = "vcc_3v3";
> > +                             regulator-always-on;
> > +                             regulator-boot-on;
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +
> > +                     vcc3v3_sd: SWITCH_REG2 {
> > +                             regulator-name = "vcc3v3_sd";
> > +
> > +                             regulator-state-mem {
> > +                                     regulator-off-in-suspend;
> > +                             };
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> > +&mdio0 {
> > +     rgmii_phy0: ethernet-phy@0 {
> > +             compatible = "ethernet-phy-ieee802.3-c22";
> > +             reg = <0x0>;
> > +             reset-assert-us = <20000>;
> > +             reset-deassert-us = <100000>;
> > +             reset-gpios = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > +     };
> > +};
> > +
> > +&pinctrl {
> > +     leds {
> > +             led_power_en: led_power_en {
>
> No underscores in node names. Do bindings require specific naming? I
> would expect "-grp" or "-pins" suffixes, if possible.
>
> > +                     rockchip,pins = <0 RK_PC6 RK_FUNC_GPIO &pcfg_pull_none>;
> > +             };
> > +             led_work_en: led_work_en {
> > +                     rockchip,pins = <0 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>;
> > +             };
> > +     };
> > +
> > +     pmic {
> > +             pmic_int: pmic_int {
> > +                     rockchip,pins =
> > +                             <0 RK_PA3 RK_FUNC_GPIO &pcfg_pull_up>;
> > +             };
> > +     };
> > +};
> > +
> > +&pmu_io_domains {
> > +     pmuio1-supply = <&vcc3v3_pmu>;
> > +     pmuio2-supply = <&vcc3v3_pmu>;
> > +     vccio1-supply = <&vccio_acodec>;
> > +     vccio2-supply = <&vcc_1v8>;
> > +     vccio3-supply = <&vccio_sd>;
> > +     vccio4-supply = <&vcc_1v8>;
> > +     vccio5-supply = <&vcc_3v3>;
> > +     vccio6-supply = <&vcc_3v3>;
> > +     vccio7-supply = <&vcc_3v3>;
> > +     status = "okay";
> > +};
> > +
> > +&saradc {
> > +     vref-supply = <&vcca_1v8>;
> > +     status = "okay";
> > +};
> > +
> > +&sdhci {
> > +     bus-width = <8>;
> > +     max-frequency = <200000000>;
> > +     non-removable;
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe &emmc_rstnout>;
> > +     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-sdr104;
> > +     vmmc-supply = <&vcc3v3_sd>;
> > +     vqmmc-supply = <&vccio_sd>;
> > +     status = "okay";
> > +};
> > +
> > +&uart2 {
> > +     status = "okay";
> > +};
>
>
> Best regards,
> Krzysztof
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-04-16 12:07       ` Peter Geis
  (?)
@ 2022-04-17 17:45         ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-17 17:45 UTC (permalink / raw)
  To: Peter Geis
  Cc: Dongjin Kim, Heiko Stuebner, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On 16/04/2022 14:07, Peter Geis wrote:

>>> +     dc_12v: dc-12v {
>>
>> Generic node name, so "regulator" or "regulator-0"
> 
> Unfortunately, this advice breaks the regulator-fixed driver, which it
> seems cannot cope with a bunch of nodes all named "regulator".

What exactly cannot cope? You cannot have different device nodes with
the same name but this is not a limitation of regulator but devicetree spec.

> Setting the regulators as regulator-0 -1 -2 leads to fun issues where
> the regulator numbering in the kernel doesn't match the node numbers.

There are no "node numbers"... maybe you mean unit addresses? But there
are none here.

> It also makes it more fun when additional regulators need to be added
> and everything gets shuffled around.

Usually adding - in subsequent DTS files - means increasing the numbers
so if you have regulator-[012] then just use regulator-[345] in other
files. I see potential mess when you combine several DTSI files, each
defining regulators, so in such case "some-name-regulator" (or reversed)
is also popular approach.

> If naming these uniquely to avoid confusion and collisions is such an
> issue, why is it not caught by make W=1 dtbs_check?

Patches are welcome. :)

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-17 17:45         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-17 17:45 UTC (permalink / raw)
  To: Peter Geis
  Cc: Dongjin Kim, Heiko Stuebner, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On 16/04/2022 14:07, Peter Geis wrote:

>>> +     dc_12v: dc-12v {
>>
>> Generic node name, so "regulator" or "regulator-0"
> 
> Unfortunately, this advice breaks the regulator-fixed driver, which it
> seems cannot cope with a bunch of nodes all named "regulator".

What exactly cannot cope? You cannot have different device nodes with
the same name but this is not a limitation of regulator but devicetree spec.

> Setting the regulators as regulator-0 -1 -2 leads to fun issues where
> the regulator numbering in the kernel doesn't match the node numbers.

There are no "node numbers"... maybe you mean unit addresses? But there
are none here.

> It also makes it more fun when additional regulators need to be added
> and everything gets shuffled around.

Usually adding - in subsequent DTS files - means increasing the numbers
so if you have regulator-[012] then just use regulator-[345] in other
files. I see potential mess when you combine several DTSI files, each
defining regulators, so in such case "some-name-regulator" (or reversed)
is also popular approach.

> If naming these uniquely to avoid confusion and collisions is such an
> issue, why is it not caught by make W=1 dtbs_check?

Patches are welcome. :)

Best regards,
Krzysztof

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-17 17:45         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-17 17:45 UTC (permalink / raw)
  To: Peter Geis
  Cc: Dongjin Kim, Heiko Stuebner, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On 16/04/2022 14:07, Peter Geis wrote:

>>> +     dc_12v: dc-12v {
>>
>> Generic node name, so "regulator" or "regulator-0"
> 
> Unfortunately, this advice breaks the regulator-fixed driver, which it
> seems cannot cope with a bunch of nodes all named "regulator".

What exactly cannot cope? You cannot have different device nodes with
the same name but this is not a limitation of regulator but devicetree spec.

> Setting the regulators as regulator-0 -1 -2 leads to fun issues where
> the regulator numbering in the kernel doesn't match the node numbers.

There are no "node numbers"... maybe you mean unit addresses? But there
are none here.

> It also makes it more fun when additional regulators need to be added
> and everything gets shuffled around.

Usually adding - in subsequent DTS files - means increasing the numbers
so if you have regulator-[012] then just use regulator-[345] in other
files. I see potential mess when you combine several DTSI files, each
defining regulators, so in such case "some-name-regulator" (or reversed)
is also popular approach.

> If naming these uniquely to avoid confusion and collisions is such an
> issue, why is it not caught by make W=1 dtbs_check?

Patches are welcome. :)

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-04-17 17:45         ` Krzysztof Kozlowski
  (?)
@ 2022-04-17 20:55           ` Heiko Stuebner
  -1 siblings, 0 replies; 33+ messages in thread
From: Heiko Stuebner @ 2022-04-17 20:55 UTC (permalink / raw)
  To: Peter Geis, Krzysztof Kozlowski
  Cc: Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

Am Sonntag, 17. April 2022, 19:45:52 CEST schrieb Krzysztof Kozlowski:
> On 16/04/2022 14:07, Peter Geis wrote:
> 
> >>> +     dc_12v: dc-12v {
> >>
> >> Generic node name, so "regulator" or "regulator-0"
> > 
> > Unfortunately, this advice breaks the regulator-fixed driver, which it
> > seems cannot cope with a bunch of nodes all named "regulator".
> 
> What exactly cannot cope? You cannot have different device nodes with
> the same name but this is not a limitation of regulator but devicetree spec.
> 
> > Setting the regulators as regulator-0 -1 -2 leads to fun issues where
> > the regulator numbering in the kernel doesn't match the node numbers.
> 
> There are no "node numbers"... maybe you mean unit addresses? But there
> are none here.
> 
> > It also makes it more fun when additional regulators need to be added
> > and everything gets shuffled around.
> 
> Usually adding - in subsequent DTS files - means increasing the numbers
> so if you have regulator-[012] then just use regulator-[345] in other
> files. I see potential mess when you combine several DTSI files, each
> defining regulators, so in such case "some-name-regulator" (or reversed)
> is also popular approach.

so going with

	dc_12v: dc-12v-regulator {
	};

i.e. doing a some-name-regulator would be an in-spec way to go?

In this case I would definitely prefer this over doing a numbered thing.

I.e. regulator-0 can create really hard to debug issues, when you have
another accidential regulator-0 for a different regulator in there, which
then would create some sort of merged node.


Heiko

> 
> > If naming these uniquely to avoid confusion and collisions is such an
> > issue, why is it not caught by make W=1 dtbs_check?
> 
> Patches are welcome. :)
> 
> Best regards,
> Krzysztof
> 





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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-17 20:55           ` Heiko Stuebner
  0 siblings, 0 replies; 33+ messages in thread
From: Heiko Stuebner @ 2022-04-17 20:55 UTC (permalink / raw)
  To: Peter Geis, Krzysztof Kozlowski
  Cc: Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

Am Sonntag, 17. April 2022, 19:45:52 CEST schrieb Krzysztof Kozlowski:
> On 16/04/2022 14:07, Peter Geis wrote:
> 
> >>> +     dc_12v: dc-12v {
> >>
> >> Generic node name, so "regulator" or "regulator-0"
> > 
> > Unfortunately, this advice breaks the regulator-fixed driver, which it
> > seems cannot cope with a bunch of nodes all named "regulator".
> 
> What exactly cannot cope? You cannot have different device nodes with
> the same name but this is not a limitation of regulator but devicetree spec.
> 
> > Setting the regulators as regulator-0 -1 -2 leads to fun issues where
> > the regulator numbering in the kernel doesn't match the node numbers.
> 
> There are no "node numbers"... maybe you mean unit addresses? But there
> are none here.
> 
> > It also makes it more fun when additional regulators need to be added
> > and everything gets shuffled around.
> 
> Usually adding - in subsequent DTS files - means increasing the numbers
> so if you have regulator-[012] then just use regulator-[345] in other
> files. I see potential mess when you combine several DTSI files, each
> defining regulators, so in such case "some-name-regulator" (or reversed)
> is also popular approach.

so going with

	dc_12v: dc-12v-regulator {
	};

i.e. doing a some-name-regulator would be an in-spec way to go?

In this case I would definitely prefer this over doing a numbered thing.

I.e. regulator-0 can create really hard to debug issues, when you have
another accidential regulator-0 for a different regulator in there, which
then would create some sort of merged node.


Heiko

> 
> > If naming these uniquely to avoid confusion and collisions is such an
> > issue, why is it not caught by make W=1 dtbs_check?
> 
> Patches are welcome. :)
> 
> Best regards,
> Krzysztof
> 





^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-17 20:55           ` Heiko Stuebner
  0 siblings, 0 replies; 33+ messages in thread
From: Heiko Stuebner @ 2022-04-17 20:55 UTC (permalink / raw)
  To: Peter Geis, Krzysztof Kozlowski
  Cc: Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

Am Sonntag, 17. April 2022, 19:45:52 CEST schrieb Krzysztof Kozlowski:
> On 16/04/2022 14:07, Peter Geis wrote:
> 
> >>> +     dc_12v: dc-12v {
> >>
> >> Generic node name, so "regulator" or "regulator-0"
> > 
> > Unfortunately, this advice breaks the regulator-fixed driver, which it
> > seems cannot cope with a bunch of nodes all named "regulator".
> 
> What exactly cannot cope? You cannot have different device nodes with
> the same name but this is not a limitation of regulator but devicetree spec.
> 
> > Setting the regulators as regulator-0 -1 -2 leads to fun issues where
> > the regulator numbering in the kernel doesn't match the node numbers.
> 
> There are no "node numbers"... maybe you mean unit addresses? But there
> are none here.
> 
> > It also makes it more fun when additional regulators need to be added
> > and everything gets shuffled around.
> 
> Usually adding - in subsequent DTS files - means increasing the numbers
> so if you have regulator-[012] then just use regulator-[345] in other
> files. I see potential mess when you combine several DTSI files, each
> defining regulators, so in such case "some-name-regulator" (or reversed)
> is also popular approach.

so going with

	dc_12v: dc-12v-regulator {
	};

i.e. doing a some-name-regulator would be an in-spec way to go?

In this case I would definitely prefer this over doing a numbered thing.

I.e. regulator-0 can create really hard to debug issues, when you have
another accidential regulator-0 for a different regulator in there, which
then would create some sort of merged node.


Heiko

> 
> > If naming these uniquely to avoid confusion and collisions is such an
> > issue, why is it not caught by make W=1 dtbs_check?
> 
> Patches are welcome. :)
> 
> Best regards,
> Krzysztof
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-04-17 20:55           ` Heiko Stuebner
  (?)
@ 2022-04-18 11:21             ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-18 11:21 UTC (permalink / raw)
  To: Heiko Stuebner, Peter Geis
  Cc: Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On 17/04/2022 22:55, Heiko Stuebner wrote:
>> Usually adding - in subsequent DTS files - means increasing the numbers
>> so if you have regulator-[012] then just use regulator-[345] in other
>> files. I see potential mess when you combine several DTSI files, each
>> defining regulators, so in such case "some-name-regulator" (or reversed)
>> is also popular approach.
> 
> so going with
> 
> 	dc_12v: dc-12v-regulator {
> 	};
> 
> i.e. doing a some-name-regulator would be an in-spec way to go?
> 
> In this case I would definitely prefer this over doing a numbered thing.
> 
> I.e. regulator-0 can create really hard to debug issues, when you have
> another accidential regulator-0 for a different regulator in there, which
> then would create some sort of merged node.

I don't think such case happens frequently, because all regulators are
usually used by something (as a phandle) thus they should have a label.
This label should be descriptive, so if one can assign same label to
entirely different regulators, then the same chances are that same
descriptive node will be used.

IOW, if you think such mistake with regulator names can happen, then the
same can happen with the label...

Anyway, answering the question - "dc-12v-regulator" is still not
matching exactly the Devicetree spec recommendation, but it's okay for
me. :)


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-18 11:21             ` Krzysztof Kozlowski
  0 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-18 11:21 UTC (permalink / raw)
  To: Heiko Stuebner, Peter Geis
  Cc: Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On 17/04/2022 22:55, Heiko Stuebner wrote:
>> Usually adding - in subsequent DTS files - means increasing the numbers
>> so if you have regulator-[012] then just use regulator-[345] in other
>> files. I see potential mess when you combine several DTSI files, each
>> defining regulators, so in such case "some-name-regulator" (or reversed)
>> is also popular approach.
> 
> so going with
> 
> 	dc_12v: dc-12v-regulator {
> 	};
> 
> i.e. doing a some-name-regulator would be an in-spec way to go?
> 
> In this case I would definitely prefer this over doing a numbered thing.
> 
> I.e. regulator-0 can create really hard to debug issues, when you have
> another accidential regulator-0 for a different regulator in there, which
> then would create some sort of merged node.

I don't think such case happens frequently, because all regulators are
usually used by something (as a phandle) thus they should have a label.
This label should be descriptive, so if one can assign same label to
entirely different regulators, then the same chances are that same
descriptive node will be used.

IOW, if you think such mistake with regulator names can happen, then the
same can happen with the label...

Anyway, answering the question - "dc-12v-regulator" is still not
matching exactly the Devicetree spec recommendation, but it's okay for
me. :)


Best regards,
Krzysztof

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-18 11:21             ` Krzysztof Kozlowski
  0 siblings, 0 replies; 33+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-18 11:21 UTC (permalink / raw)
  To: Heiko Stuebner, Peter Geis
  Cc: Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On 17/04/2022 22:55, Heiko Stuebner wrote:
>> Usually adding - in subsequent DTS files - means increasing the numbers
>> so if you have regulator-[012] then just use regulator-[345] in other
>> files. I see potential mess when you combine several DTSI files, each
>> defining regulators, so in such case "some-name-regulator" (or reversed)
>> is also popular approach.
> 
> so going with
> 
> 	dc_12v: dc-12v-regulator {
> 	};
> 
> i.e. doing a some-name-regulator would be an in-spec way to go?
> 
> In this case I would definitely prefer this over doing a numbered thing.
> 
> I.e. regulator-0 can create really hard to debug issues, when you have
> another accidential regulator-0 for a different regulator in there, which
> then would create some sort of merged node.

I don't think such case happens frequently, because all regulators are
usually used by something (as a phandle) thus they should have a label.
This label should be descriptive, so if one can assign same label to
entirely different regulators, then the same chances are that same
descriptive node will be used.

IOW, if you think such mistake with regulator names can happen, then the
same can happen with the label...

Anyway, answering the question - "dc-12v-regulator" is still not
matching exactly the Devicetree spec recommendation, but it's okay for
me. :)


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
  2022-04-18 11:21             ` Krzysztof Kozlowski
  (?)
@ 2022-04-18 11:46               ` Peter Geis
  -1 siblings, 0 replies; 33+ messages in thread
From: Peter Geis @ 2022-04-18 11:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Heiko Stuebner, Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On Mon, Apr 18, 2022 at 7:21 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 17/04/2022 22:55, Heiko Stuebner wrote:
> >> Usually adding - in subsequent DTS files - means increasing the numbers
> >> so if you have regulator-[012] then just use regulator-[345] in other
> >> files. I see potential mess when you combine several DTSI files, each
> >> defining regulators, so in such case "some-name-regulator" (or reversed)
> >> is also popular approach.
> >
> > so going with
> >
> >       dc_12v: dc-12v-regulator {
> >       };
> >
> > i.e. doing a some-name-regulator would be an in-spec way to go?
> >
> > In this case I would definitely prefer this over doing a numbered thing.
> >
> > I.e. regulator-0 can create really hard to debug issues, when you have
> > another accidential regulator-0 for a different regulator in there, which
> > then would create some sort of merged node.
>
> I don't think such case happens frequently, because all regulators are
> usually used by something (as a phandle) thus they should have a label.
> This label should be descriptive, so if one can assign same label to
> entirely different regulators, then the same chances are that same
> descriptive node will be used.
>
> IOW, if you think such mistake with regulator names can happen, then the
> same can happen with the label...
>
> Anyway, answering the question - "dc-12v-regulator" is still not
> matching exactly the Devicetree spec recommendation, but it's okay for
> me. :)

This seems like an excellent compromise, thanks!

>
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-18 11:46               ` Peter Geis
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Geis @ 2022-04-18 11:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Heiko Stuebner, Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On Mon, Apr 18, 2022 at 7:21 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 17/04/2022 22:55, Heiko Stuebner wrote:
> >> Usually adding - in subsequent DTS files - means increasing the numbers
> >> so if you have regulator-[012] then just use regulator-[345] in other
> >> files. I see potential mess when you combine several DTSI files, each
> >> defining regulators, so in such case "some-name-regulator" (or reversed)
> >> is also popular approach.
> >
> > so going with
> >
> >       dc_12v: dc-12v-regulator {
> >       };
> >
> > i.e. doing a some-name-regulator would be an in-spec way to go?
> >
> > In this case I would definitely prefer this over doing a numbered thing.
> >
> > I.e. regulator-0 can create really hard to debug issues, when you have
> > another accidential regulator-0 for a different regulator in there, which
> > then would create some sort of merged node.
>
> I don't think such case happens frequently, because all regulators are
> usually used by something (as a phandle) thus they should have a label.
> This label should be descriptive, so if one can assign same label to
> entirely different regulators, then the same chances are that same
> descriptive node will be used.
>
> IOW, if you think such mistake with regulator names can happen, then the
> same can happen with the label...
>
> Anyway, answering the question - "dc-12v-regulator" is still not
> matching exactly the Devicetree spec recommendation, but it's okay for
> me. :)

This seems like an excellent compromise, thanks!

>
>
> Best regards,
> Krzysztof

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

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] arm64: dts: rockchip: Add Hardkernel ODROID-M1 board
@ 2022-04-18 11:46               ` Peter Geis
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Geis @ 2022-04-18 11:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Heiko Stuebner, Dongjin Kim, devicetree, arm-mail-list,
	open list:ARM/Rockchip SoC...,
	Linux Kernel Mailing List

On Mon, Apr 18, 2022 at 7:21 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 17/04/2022 22:55, Heiko Stuebner wrote:
> >> Usually adding - in subsequent DTS files - means increasing the numbers
> >> so if you have regulator-[012] then just use regulator-[345] in other
> >> files. I see potential mess when you combine several DTSI files, each
> >> defining regulators, so in such case "some-name-regulator" (or reversed)
> >> is also popular approach.
> >
> > so going with
> >
> >       dc_12v: dc-12v-regulator {
> >       };
> >
> > i.e. doing a some-name-regulator would be an in-spec way to go?
> >
> > In this case I would definitely prefer this over doing a numbered thing.
> >
> > I.e. regulator-0 can create really hard to debug issues, when you have
> > another accidential regulator-0 for a different regulator in there, which
> > then would create some sort of merged node.
>
> I don't think such case happens frequently, because all regulators are
> usually used by something (as a phandle) thus they should have a label.
> This label should be descriptive, so if one can assign same label to
> entirely different regulators, then the same chances are that same
> descriptive node will be used.
>
> IOW, if you think such mistake with regulator names can happen, then the
> same can happen with the label...
>
> Anyway, answering the question - "dc-12v-regulator" is still not
> matching exactly the Devicetree spec recommendation, but it's okay for
> me. :)

This seems like an excellent compromise, thanks!

>
>
> Best regards,
> Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2022-04-18 11:47 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-29  9:44 [PATCH 1/2] dt-bindings: rockchip: Add Hardkernel ODROID-M1 board Dongjin Kim
2022-03-29  9:44 ` Dongjin Kim
2022-03-29  9:44 ` Dongjin Kim
2022-03-29  9:44 ` [PATCH 2/2] arm64: dts: " Dongjin Kim
2022-03-29  9:44   ` Dongjin Kim
2022-03-29  9:44   ` Dongjin Kim
2022-03-29 17:12   ` Krzysztof Kozlowski
2022-03-29 17:12     ` Krzysztof Kozlowski
2022-03-29 17:12     ` Krzysztof Kozlowski
2022-04-05  2:32     ` Dongjin Kim
2022-04-05  2:32       ` Dongjin Kim
2022-04-05  2:32       ` Dongjin Kim
2022-04-05  6:13       ` Krzysztof Kozlowski
2022-04-05  6:13         ` Krzysztof Kozlowski
2022-04-05  6:13         ` Krzysztof Kozlowski
2022-04-16 12:07     ` Peter Geis
2022-04-16 12:07       ` Peter Geis
2022-04-16 12:07       ` Peter Geis
2022-04-17 17:45       ` Krzysztof Kozlowski
2022-04-17 17:45         ` Krzysztof Kozlowski
2022-04-17 17:45         ` Krzysztof Kozlowski
2022-04-17 20:55         ` Heiko Stuebner
2022-04-17 20:55           ` Heiko Stuebner
2022-04-17 20:55           ` Heiko Stuebner
2022-04-18 11:21           ` Krzysztof Kozlowski
2022-04-18 11:21             ` Krzysztof Kozlowski
2022-04-18 11:21             ` Krzysztof Kozlowski
2022-04-18 11:46             ` Peter Geis
2022-04-18 11:46               ` Peter Geis
2022-04-18 11:46               ` Peter Geis
2022-04-04 18:10 ` [PATCH 1/2] dt-bindings: " Rob Herring
2022-04-04 18:10   ` Rob Herring
2022-04-04 18:10   ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.