dt-bindings: Add doc for FriendlyARM NanoPi R2S

Started by David Bauer on Jul 30, 2020 · 8 messages from 4 authors · Last reply Aug 8, 2020

Subsystems (2)

Maintainers (4)

STALE2288d

open the first message on its own page

  1. [PATCH v2 2/2] rockchip: rk3328: Add support for FriendlyARM NanoPi R2S
    Related patches
Message details and related content
Add devicetree binding documentation for the FriendlyARM NanoPi R2S.

Signed-off-by: David Bauer <redacted>
---
 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 d4a4045092df..b17931ec2d86 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -104,6 +104,11 @@ properties:
               - firefly,roc-rk3399-pc-mezzanine
           - const: rockchip,rk3399
 
+      - description: FriendlyARM NanoPi R2S
+        items:
+          - const: friendlyarm,nanopi-r2s
+          - const: rockchip,rk3328
+
       - description: FriendlyElec NanoPi4 series boards
         items:
           - enum:
-- 
2.27.0


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

[PATCH v2 2/2] rockchip: rk3328: Add support for FriendlyARM NanoPi R2S

Message details and related content
This adds support for the NanoPi R2S from FriendlyARM.

Rockchip RK3328 SoC
1GB DDR4 RAM
Gigabit Ethernet (WAN)
Gigabit Ethernet (USB3) (LAN)
USB 2.0 Host Port
MicroSD slot
Reset button
WAN - LAN - SYS LED

Signed-off-by: David Bauer <redacted>
---

Changes in v2:
 - Use aligned memory for ethernet
 - Fix GPO for sdio regulator

 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3328-nanopi-r2s.dts   | 334 ++++++++++++++++++
 2 files changed, 335 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts
diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index b87b1f773083..20055c51e150 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3308-roc-cc.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3326-odroid-go2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-a1.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-roc-cc.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-evb-act8846.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts
new file mode 100644
index 000000000000..9fd148423b9d
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2020 David Bauer <mail@david-bauer.net>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/gpio/gpio.h>
+#include "rk3328.dtsi"
+
+/ {
+	model = "FriendlyARM NanoPi R2S";
+	compatible = "friendlyarm,nanopi-r2s", "rockchip,rk3328";
+
+	chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	gmac_clkin: external-gmac-clock {
+		compatible = "fixed-clock";
+		clock-frequency = <125000000>;
+		clock-output-names = "gmac_clkin";
+		#clock-cells = <0>;
+	};
+
+	vcc_sd: sdmmc-regulator {
+		compatible = "regulator-fixed";
+		gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sdmmc0m1_gpio>;
+		regulator-name = "vcc_sd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc_io>;
+	};
+
+	vcc_sdio: sdmmcio-regulator {
+		compatible = "regulator-gpio";
+		gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		states = <1800000 0x1
+			  3300000 0x0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sdio_vcc_pin>;
+		regulator-always-on;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-name = "vcc_sdio";
+		regulator-settling-time-us = <5000>;
+		regulator-type = "voltage";
+		vin-supply = <&vcc_io>;
+	};
+
+	vcc_sys: vcc-sys {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_sys";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&led_pins>;
+
+		sys {
+			gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
+			label = "nanopi-r2s:red:sys";
+		};
+
+		lan {
+			gpios = <&gpio2 RK_PB7 GPIO_ACTIVE_HIGH>;
+			label = "nanopi-r2s:green:lan";
+		};
+
+		wan {
+			gpios = <&gpio2 RK_PC2 GPIO_ACTIVE_HIGH>;
+			label = "nanopi-r2s:green:wan";
+		};
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys-polled";
+		poll-interval = <100>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&button_pins>;
+
+		reset {
+			label = "Reset Button";
+			gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_RESTART>;
+			debounce-interval = <50>;
+		};
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu1 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu2 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu3 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&gmac2io {
+	assigned-clocks = <&cru SCLK_MAC2IO>, <&cru SCLK_MAC2IO_EXT>;
+	assigned-clock-parents = <&gmac_clkin>, <&gmac_clkin>;
+	clock_in_out = "input";
+	phy-supply = <&vcc_io>;
+	phy-handle = <&rtl8211e>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmiim1_pins>;
+	snps,aal;
+	snps,reset-gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
+	snps,reset-active-low;
+	snps,reset-delays-us = <0 10000 50000>;
+	tx_delay = <0x24>;
+	rx_delay = <0x18>;
+	status = "okay";
+
+	mdio {
+		compatible = "snps,dwmac-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		rtl8211e: ethernet-phy@0 {
+			reg = <0>;
+		};
+	};
+};
+
+&i2c1 {
+	status = "okay";
+
+	rk805: rk805@18 {
+		compatible = "rockchip,rk805";
+		reg = <0x18>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
+		#clock-cells = <1>;
+		clock-output-names = "xin32k", "rk805-clkout2";
+		gpio-controller;
+		#gpio-cells = <2>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_int_l>;
+		rockchip,system-power-controller;
+		wakeup-source;
+
+		vcc1-supply = <&vcc_sys>;
+		vcc2-supply = <&vcc_sys>;
+		vcc3-supply = <&vcc_sys>;
+		vcc4-supply = <&vcc_sys>;
+		vcc5-supply = <&vcc_io>;
+		vcc6-supply = <&vcc_sys>;
+
+		regulators {
+			vdd_logic: DCDC_REG1 {
+				regulator-name = "vdd_logic";
+				regulator-min-microvolt = <712500>;
+				regulator-max-microvolt = <1450000>;
+				regulator-ramp-delay = <12500>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1000000>;
+				};
+			};
+
+			vdd_arm: DCDC_REG2 {
+				regulator-name = "vdd_arm";
+				regulator-min-microvolt = <712500>;
+				regulator-max-microvolt = <1450000>;
+				regulator-ramp-delay = <12500>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <950000>;
+				};
+			};
+
+			vcc_ddr: DCDC_REG3 {
+				regulator-name = "vcc_ddr";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vcc_io: DCDC_REG4 {
+				regulator-name = "vcc_io";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vcc_18: LDO_REG1 {
+				regulator-name = "vcc_18";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vcc18_emmc: LDO_REG2 {
+				regulator-name = "vcc18_emmc";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vdd_10: LDO_REG3 {
+				regulator-name = "vdd_10";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1000000>;
+				};
+			};
+		};
+	};
+};
+
+&io_domains {
+	status = "okay";
+
+	vccio1-supply = <&vcc_io>;
+	vccio2-supply = <&vcc18_emmc>;
+	vccio3-supply = <&vcc_sdio>;
+	vccio4-supply = <&vcc_18>;
+	vccio5-supply = <&vcc_io>;
+	vccio6-supply = <&vcc_io>;
+	pmuio-supply = <&vcc_io>;
+};
+
+&pinctrl {
+	leds {
+		led_pins: led-pins {
+			rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>,
+							<2 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>,
+							<2 RK_PC2 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	button {
+		button_pins: button-pins {
+			rockchip,pins = <0 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		pmic_int_l: pmic-int-l {
+			rockchip,pins = <2 RK_PA6 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	sd {
+		sdio_vcc_pin: sdio-vcc-pin {
+			rockchip,pins = <1 RK_PD4 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+};
+
+&sdmmc {
+	bus-width = <4>;
+	cap-mmc-highspeed;
+	cap-sd-highspeed;
+	disable-wp;
+	max-frequency = <150000000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc0_clk &sdmmc0_cmd &sdmmc0_dectn &sdmmc0_bus4>;
+	vmmc-supply = <&vcc_sd>;
+	vqmmc-supply = <&vcc_sdio>;
+	status = "okay";
+};
+
+&tsadc {
+	rockchip,hw-tshut-mode = <0>;
+	rockchip,hw-tshut-polarity = <0>;
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
+
+&u2phy {
+	status = "okay";
+
+	u2phy_host: host-port {
+		status = "okay";
+	};
+};
+
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};
-- 
2.27.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Message details and related content
On Fri, 31 Jul 2020 01:20:53 +0200, David Bauer wrote:
Add devicetree binding documentation for the FriendlyARM NanoPi R2S.

Signed-off-by: David Bauer <redacted>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)
Reviewed-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

Re: [PATCH v2 2/2] rockchip: rk3328: Add support for FriendlyARM NanoPi R2S

Message details and related content
Hi David,

Add more maintainers and mail lists for all patches in this serie.

./scripts/get_maintainer.pl --noroles --norolestats --nogit-fallback
--nogit <patch1 patch2>

git send-email --suppress-cc all --annotate --to <..> --cc <..> <patch1
patch2>

On 7/31/20 1:20 AM, David Bauer wrote:
quoted hunk
This adds support for the NanoPi R2S from FriendlyARM.

Rockchip RK3328 SoC
1GB DDR4 RAM
Gigabit Ethernet (WAN)
Gigabit Ethernet (USB3) (LAN)
USB 2.0 Host Port
MicroSD slot
Reset button
WAN - LAN - SYS LED

Signed-off-by: David Bauer <redacted>
---

Changes in v2:
 - Use aligned memory for ethernet
 - Fix GPO for sdio regulator

 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../boot/dts/rockchip/rk3328-nanopi-r2s.dts   | 334 ++++++++++++++++++
 2 files changed, 335 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts
diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index b87b1f773083..20055c51e150 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3308-roc-cc.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3326-odroid-go2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-a1.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-roc-cc.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-evb-act8846.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts
new file mode 100644
index 000000000000..9fd148423b9d
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts

27 more quoted lines

quoted hunk
+	vcc_sd: sdmmc-regulator {
+		compatible = "regulator-fixed";
+		gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sdmmc0m1_gpio>;
sort
quoted hunk
+		regulator-name = "vcc_sd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc_io>;
+	};
+
+	vcc_sdio: sdmmcio-regulator {
+		compatible = "regulator-gpio";
+		gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		states = <1800000 0x1
+			  3300000 0x0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sdio_vcc_pin>;
sort
quoted hunk
+		regulator-always-on;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-name = "vcc_sdio";
move regulator-name above regulator-always-on
quoted hunk
+		regulator-settling-time-us = <5000>;
+		regulator-type = "voltage";
+		vin-supply = <&vcc_io>;
+	};
+
+	vcc_sys: vcc-sys {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_sys";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
sort node names
move regulators below
quoted hunk
+
+	leds {
+		compatible = "gpio-leds";
+
remove empty line
+		pinctrl-names = "default";
sort pinctrl-names below pinctrl-0
+		pinctrl-0 = <&led_pins>;
use separate pinctrl nodes with consistent node names
	pinctrl-0 = <&lan_led_pin, &sys_led_pin, &wan_led_pin>;
+
+		sys {
sort node names/label

The prefered node name is '^led-[0-9a-f]$'.

sys_led: led-1 {
quoted hunk
+			gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
+			label = "nanopi-r2s:red:sys";
+		};
+
+		lan {
lan_led: led-0 {
quoted hunk
+			gpios = <&gpio2 RK_PB7 GPIO_ACTIVE_HIGH>;
+			label = "nanopi-r2s:green:lan";
+		};
+
+		wan {
wan_led: led-2 {
quoted hunk
+			gpios = <&gpio2 RK_PC2 GPIO_ACTIVE_HIGH>;
+			label = "nanopi-r2s:green:wan";
+		};
+	};
+
+	gpio_keys {
sort node name
quoted hunk
+		compatible = "gpio-keys-polled";
+		poll-interval = <100>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&button_pins>;
+
+		reset {
+			label = "Reset Button";
+			gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_RESTART>;
+			debounce-interval = <50>;
+		};
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu1 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu2 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu3 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&gmac2io {
+	assigned-clocks = <&cru SCLK_MAC2IO>, <&cru SCLK_MAC2IO_EXT>;
+	assigned-clock-parents = <&gmac_clkin>, <&gmac_clkin>;
+	clock_in_out = "input";
+	phy-supply = <&vcc_io>;
+	phy-handle = <&rtl8211e>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmiim1_pins>;
sort
quoted hunk
+	snps,aal;
+	snps,reset-gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
+	snps,reset-active-low;
+	snps,reset-delays-us = <0 10000 50000>;
+	tx_delay = <0x24>;
+	rx_delay = <0x18>;
+	status = "okay";
+
+	mdio {
+		compatible = "snps,dwmac-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		rtl8211e: ethernet-phy@0 {
+			reg = <0>;
interrupts, reset ?
quoted hunk
+		};
+	};
+};
+
+&i2c1 {
+	status = "okay";
+
+	rk805: rk805@18 {
+		compatible = "rockchip,rk805";
+		reg = <0x18>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
+		#clock-cells = <1>;
+		clock-output-names = "xin32k", "rk805-clkout2";
+		gpio-controller;
+		#gpio-cells = <2>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_int_l>;
sort
quoted hunk
+		rockchip,system-power-controller;
+		wakeup-source;
+
+		vcc1-supply = <&vcc_sys>;
+		vcc2-supply = <&vcc_sys>;
+		vcc3-supply = <&vcc_sys>;
+		vcc4-supply = <&vcc_sys>;
+		vcc5-supply = <&vcc_io>;
+		vcc6-supply = <&vcc_sys>;
+
+		regulators {
+			vdd_logic: DCDC_REG1 {
+				regulator-name = "vdd_logic";
+				regulator-min-microvolt = <712500>;
+				regulator-max-microvolt = <1450000>;
+				regulator-ramp-delay = <12500>;
+				regulator-always-on;
+				regulator-boot-on;
empty line between a node
quoted hunk
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1000000>;
+				};
+			};
+
+			vdd_arm: DCDC_REG2 {
+				regulator-name = "vdd_arm";
+				regulator-min-microvolt = <712500>;
+				regulator-max-microvolt = <1450000>;
+				regulator-ramp-delay = <12500>;
+				regulator-always-on;
+				regulator-boot-on;
same
quoted hunk
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <950000>;
+				};
+			};
+
+			vcc_ddr: DCDC_REG3 {
+				regulator-name = "vcc_ddr";
+				regulator-always-on;
+				regulator-boot-on;
same
quoted hunk
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vcc_io: DCDC_REG4 {
+				regulator-name = "vcc_io";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-always-on;
+				regulator-boot-on;
same
quoted hunk
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vcc_18: LDO_REG1 {
+				regulator-name = "vcc_18";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
same
quoted hunk
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vcc18_emmc: LDO_REG2 {
+				regulator-name = "vcc18_emmc";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
same
quoted hunk
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vdd_10: LDO_REG3 {
+				regulator-name = "vdd_10";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				regulator-boot-on;
same
quoted hunk
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1000000>;
+				};
+			};
+		};
+	};
+};
+
+&io_domains {
+	status = "okay";
status below
quoted hunk
+
+	vccio1-supply = <&vcc_io>;
+	vccio2-supply = <&vcc18_emmc>;
+	vccio3-supply = <&vcc_sdio>;
+	vccio4-supply = <&vcc_18>;
+	vccio5-supply = <&vcc_io>;
+	vccio6-supply = <&vcc_io>;
+	pmuio-supply = <&vcc_io>;
sort properties, move this to top
quoted hunk
+};
+
+&pinctrl {
+	leds {
sort node names
+		led_pins: led-pins {
use a separate sub node for each led
use a consistent node name

	lan_led_pin: lan-led-pin {}
        sys_led_pin: sys-led-pin {}
        wan_led_pin: wan-led-pin {}
quoted hunk
+			rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>,
+							<2 RK_PB7 RK_FUNC_GPIO &pcfg_pull_none>,
+							<2 RK_PC2 RK_FUNC_GPIO &pcfg_pull_none>;
(fix alignment)
quoted hunk
+		};
+	};
+
+	button {
+		button_pins: button-pins {
+			rockchip,pins = <0 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
+	pmic {
+		pmic_int_l: pmic-int-l {
+			rockchip,pins = <2 RK_PA6 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+
+	sd {
+		sdio_vcc_pin: sdio-vcc-pin {
+			rockchip,pins = <1 RK_PD4 RK_FUNC_GPIO &pcfg_pull_up>;
+		};
+	};
+};
+
+&sdmmc {
+	bus-width = <4>;
+	cap-mmc-highspeed;
remove
board has only micro SD card
+	cap-sd-highspeed;
+	disable-wp;
+	max-frequency = <150000000>;
already in dtsi
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc0_clk &sdmmc0_cmd &sdmmc0_dectn &sdmmc0_bus4>;
sort
quoted hunk
+	vmmc-supply = <&vcc_sd>;
+	vqmmc-supply = <&vcc_sdio>;
+	status = "okay";
+};
	sdmmc: mmc@ff500000 {
		compatible = "rockchip,rk3328-dw-mshc", "rockchip,rk3288-dw-mshc";
		reg = <0x0 0xff500000 0x0 0x4000>;
		interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>,
			 <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>;
		clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
		fifo-depth = <0x100>;
		max-frequency = <150000000>;
		status = "disabled";
	};
quoted hunk
+
+&tsadc {
+	rockchip,hw-tshut-mode = <0>;
+	rockchip,hw-tshut-polarity = <0>;
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
+
+&u2phy {
sort node names
quoted hunk
+	status = "okay";
+
+	u2phy_host: host-port {
+		status = "okay";
+	};
u2phy_otg ?
+};
+
&usb20_otg {               ?
	dr_mode = "host";  ?
	status = "okay";   ?
};

Do we need this as well?
quoted hunk
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Message details and related content
Hi David,

On 2020-07-31 00:20, David Bauer wrote:
quoted hunk
Add devicetree binding documentation for the FriendlyARM NanoPi R2S.

Signed-off-by: David Bauer <redacted>
---
  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 d4a4045092df..b17931ec2d86 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -104,6 +104,11 @@ properties:
                - firefly,roc-rk3399-pc-mezzanine
            - const: rockchip,rk3399
  
+      - description: FriendlyARM NanoPi R2S
Nit: as the context below hints at, the company call themselves "FriendlyElec" these days - certainly since long before this board existed - so for consistency we should probably use that name in the description (but keep the established vendor prefix).

Robin.
quoted hunk
+        items:
+          - const: friendlyarm,nanopi-r2s
+          - const: rockchip,rk3328
+
        - description: FriendlyElec NanoPi4 series boards
          items:
            - enum:
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/2] rockchip: rk3328: Add support for FriendlyARM NanoPi R2S

Message details and related content
On 2020-07-31 00:20, David Bauer wrote:
This adds support for the NanoPi R2S from FriendlyARM.

Rockchip RK3328 SoC
1GB DDR4 RAM
Gigabit Ethernet (WAN)
Gigabit Ethernet (USB3) (LAN)
USB 2.0 Host Port
MicroSD slot
Reset button
WAN - LAN - SYS LED
Neat! Mine turned up last week (far earlier than expected, yay!), and I was going to do some DT hacking, but here it is already :)
quoted hunk
Signed-off-by: David Bauer <redacted>
---

Changes in v2:
  - Use aligned memory for ethernet
  - Fix GPO for sdio regulator

  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
  .../boot/dts/rockchip/rk3328-nanopi-r2s.dts   | 334 ++++++++++++++++++
  2 files changed, 335 insertions(+)
  create mode 100644 arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts
diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index b87b1f773083..20055c51e150 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3308-roc-cc.dtb
  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3326-odroid-go2.dtb
  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-a1.dtb
  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-evb.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-nanopi-r2s.dtb
  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-rock64.dtb
  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3328-roc-cc.dtb
  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3368-evb-act8846.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts
new file mode 100644
index 000000000000..9fd148423b9d
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3328-nanopi-r2s.dts
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2020 David Bauer <mail@david-bauer.net>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/gpio/gpio.h>
+#include "rk3328.dtsi"

3 more quoted lines

Same comment as for the binding.
quoted hunk
+	compatible = "friendlyarm,nanopi-r2s", "rockchip,rk3328";
+
+	chosen {
+		stdout-path = "serial2:1500000n8";
+	};
+
+	gmac_clkin: external-gmac-clock {
+		compatible = "fixed-clock";
+		clock-frequency = <125000000>;
+		clock-output-names = "gmac_clkin";
+		#clock-cells = <0>;
+	};
+
+	vcc_sd: sdmmc-regulator {
+		compatible = "regulator-fixed";
+		gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sdmmc0m1_gpio>;
+		regulator-name = "vcc_sd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		vin-supply = <&vcc_io>;
+	};
+
+	vcc_sdio: sdmmcio-regulator {
This is "vcc_io_sd" per the schematic.
quoted hunk
+		compatible = "regulator-gpio";
+		gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		states = <1800000 0x1
+			  3300000 0x0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sdio_vcc_pin>;
+		regulator-always-on;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-name = "vcc_sdio";
+		regulator-settling-time-us = <5000>;
+		regulator-type = "voltage";
+		vin-supply = <&vcc_io>;
Strictly the supply here is vdd_5v - vcc_io is only driving the bias circuitry for the regulator adjustment.
quoted hunk
+	};
+
+	vcc_sys: vcc-sys {
This is "vdd_5v".
quoted hunk
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_sys";
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&led_pins>;
+
+		sys {
+			gpios = <&gpio0 RK_PA2 GPIO_ACTIVE_HIGH>;
+			label = "nanopi-r2s:red:sys";
+		};
+
+		lan {
+			gpios = <&gpio2 RK_PB7 GPIO_ACTIVE_HIGH>;
This seems to match the schematic, however in practice the GPIO seems to have a mind of its own and not respond to the LED settings - are we missing something with the pinctrl perhaps?
quoted hunk
+			label = "nanopi-r2s:green:lan";
+		};
+
+		wan {
+			gpios = <&gpio2 RK_PC2 GPIO_ACTIVE_HIGH>;
+			label = "nanopi-r2s:green:wan";
+		};
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys-polled";
Does this need to be polled? I would have thought regular interrupt-based gpio-keys should work, to avoid the overhead of polling for something (relatively) incredibly rare, but perhaps I'm wrong :/
quoted hunk
+		poll-interval = <100>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&button_pins>;
+
+		reset {
+			label = "Reset Button";
+			gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_RESTART>;
+			debounce-interval = <50>;
+		};
+	};
+};
+
+&cpu0 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu1 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu2 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&cpu3 {
+	cpu-supply = <&vdd_arm>;
+};
+
+&gmac2io {
+	assigned-clocks = <&cru SCLK_MAC2IO>, <&cru SCLK_MAC2IO_EXT>;
+	assigned-clock-parents = <&gmac_clkin>, <&gmac_clkin>;
+	clock_in_out = "input";
+	phy-supply = <&vcc_io>;
+	phy-handle = <&rtl8211e>;
+	phy-mode = "rgmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmiim1_pins>;
+	snps,aal;
Is this needed over and above the txpbl fix present in the main DTSI? I forget exactly where all those discussions ended up, but if it matters here then it probably matters for all RK3328 boards.
quoted hunk
+	snps,reset-gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
+	snps,reset-active-low;
+	snps,reset-delays-us = <0 10000 50000>;
Since you're describing the phy already, you can convert these to generic reset specifiers on the phy node itself (shame we don't have an interrupt wired up, oh well...)
quoted hunk
+	tx_delay = <0x24>;
+	rx_delay = <0x18>;
+	status = "okay";
+
+	mdio {
+		compatible = "snps,dwmac-mdio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		rtl8211e: ethernet-phy@0 {
+			reg = <0>;
Is that right? The schematic says the phy is strapped for address 1 (although 0 probably does manage to work since it should be interpreted as a broadcast address).
quoted hunk
+		};
+	};
+};
+
+&i2c1 {
+	status = "okay";
+
+	rk805: rk805@18 {
+		compatible = "rockchip,rk805";
+		reg = <0x18>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
The schematic says GPIO1_D0 for this.
quoted hunk
+		#clock-cells = <1>;
+		clock-output-names = "xin32k", "rk805-clkout2";
+		gpio-controller;
+		#gpio-cells = <2>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_int_l>;
+		rockchip,system-power-controller;
+		wakeup-source;
+
+		vcc1-supply = <&vcc_sys>;
+		vcc2-supply = <&vcc_sys>;
+		vcc3-supply = <&vcc_sys>;
+		vcc4-supply = <&vcc_sys>;
+		vcc5-supply = <&vcc_io>;
+		vcc6-supply = <&vcc_sys>;
+
+		regulators {
+			vdd_logic: DCDC_REG1 {
This is "vdd_log".
quoted hunk
+				regulator-name = "vdd_logic";
+				regulator-min-microvolt = <712500>;
+				regulator-max-microvolt = <1450000>;
+				regulator-ramp-delay = <12500>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1000000>;
+				};
+			};
+
+			vdd_arm: DCDC_REG2 {
+				regulator-name = "vdd_arm";
+				regulator-min-microvolt = <712500>;
+				regulator-max-microvolt = <1450000>;
+				regulator-ramp-delay = <12500>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <950000>;
+				};
+			};
+
+			vcc_ddr: DCDC_REG3 {
+				regulator-name = "vcc_ddr";
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
+			};
+
+			vcc_io: DCDC_REG4 {
This is "vcc_io_33".
quoted hunk
+				regulator-name = "vcc_io";
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <3300000>;
+				};
+			};
+
+			vcc_18: LDO_REG1 {
+				regulator-name = "vcc_18";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vcc18_emmc: LDO_REG2 {
+				regulator-name = "vcc18_emmc";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+					regulator-suspend-microvolt = <1800000>;
+				};
+			};
+
+			vdd_10: LDO_REG3 {
+				regulator-name = "vdd_10";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;

50 more quoted lines

It shouldn't hurt to enable PWM2 from the outset to make mucking about with the fan easier.
quoted hunk
+&sdmmc {
+	bus-width = <4>;
+	cap-mmc-highspeed;
+	cap-sd-highspeed;
How about UHS modes, since we have the requisite I/O voltage support?
+	disable-wp;
I believe this property is only meant for full-size MMC/SD sockets where write-protect functionality is expected but somehow broken. For micro-SD sockets the SoC pin should be hard-wired to write-enable anyway and thus it shouldn't need to be stated.
+	max-frequency = <150000000>;
This is in the DTSI already.
quoted hunk
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdmmc0_clk &sdmmc0_cmd &sdmmc0_dectn &sdmmc0_bus4>;
+	vmmc-supply = <&vcc_sd>;
+	vqmmc-supply = <&vcc_sdio>;
+	status = "okay";
+};
+
+&tsadc {
+	rockchip,hw-tshut-mode = <0>;
+	rockchip,hw-tshut-polarity = <0>;
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
+
+&u2phy {
+	status = "okay";
+
+	u2phy_host: host-port {
Just reference this by label directly, rather than as a child node here (especially since redefining the existing label is a bit confusing).
quoted hunk
+		status = "okay";
+	};
+};
No reason not to enable the OTG port either - I've hacked that in with g_serial for a cheeky console while powering the board from my laptop and it works fine.

Cheers,
Robin.
quoted hunk
+
+&usb_host0_ehci {
+	status = "okay";
+};
+
+&usb_host0_ohci {
+	status = "okay";
+};
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/2] rockchip: rk3328: Add support for FriendlyARM NanoPi R2S

Message details and related content
Hi Robin,

thanks for your review. Only adressing some non-formal comments below.

On 8/3/20 12:01 AM, Robin Murphy wrote:
This seems to match the schematic, however in practice the GPIO seems to have a mind of its own and not respond to the LED settings - are we missing something with the pinctrl perhaps?
Hmm, this is strange. It works fine on my unit (with the current OpenWrt release). Can you test this and report back?
quoted
+            label = "nanopi-r2s:green:lan";
+        };
+
+        wan {
+            gpios = <&gpio2 RK_PC2 GPIO_ACTIVE_HIGH>;
+            label = "nanopi-r2s:green:wan";
+        };
+    };
+
+    gpio_keys {
+        compatible = "gpio-keys-polled";
Does this need to be polled? I would have thought regular interrupt-based gpio-keys should work, to avoid the overhead of polling for something (relatively) incredibly rare, but perhaps I'm wrong :/
To be honest, I've missed that this is still polled. I've had this set to polled mode, as I've
had some issues (were to dumb) to get the pinmux going. I'll switch this to interrupt triggered and report back.
quoted
+        poll-interval = <100>;
+
+        pinctrl-names = "default";
+        pinctrl-0 = <&button_pins>;
+
+        reset {
+            label = "Reset Button";
+            gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_LOW>;
+            linux,code = <KEY_RESTART>;
+            debounce-interval = <50>;
+        };
+    };
+};
+
+&cpu0 {
+    cpu-supply = <&vdd_arm>;
+};
+
+&cpu1 {
+    cpu-supply = <&vdd_arm>;
+};
+
+&cpu2 {
+    cpu-supply = <&vdd_arm>;
+};
+
+&cpu3 {
+    cpu-supply = <&vdd_arm>;
+};
+
+&gmac2io {
+    assigned-clocks = <&cru SCLK_MAC2IO>, <&cru SCLK_MAC2IO_EXT>;
+    assigned-clock-parents = <&gmac_clkin>, <&gmac_clkin>;
+    clock_in_out = "input";
+    phy-supply = <&vcc_io>;
+    phy-handle = <&rtl8211e>;
+    phy-mode = "rgmii";
+    pinctrl-names = "default";
+    pinctrl-0 = <&rgmiim1_pins>;

3 more quoted lines

It seems to matter, as otherwise forwards from LAN to WAN will result in tx_fifo errors.
quoted
+    snps,reset-gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
+    snps,reset-active-low;
+    snps,reset-delays-us = <0 10000 50000>;
Since you're describing the phy already, you can convert these to generic reset specifiers on the phy node itself (shame we don't have an interrupt wired up, oh well...)
Yup, I've forgotten that there's reset-gpio bindings for the PHY itself. 
quoted
+    tx_delay = <0x24>;
+    rx_delay = <0x18>;
+    status = "okay";
+
+    mdio {
+        compatible = "snps,dwmac-mdio";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtl8211e: ethernet-phy@0 {
+            reg = <0>;
Is that right? The schematic says the phy is strapped for address 1 (although 0 probably does manage to work since it should be interpreted as a broadcast address).
I'll test this.
quoted
+        };
+    };
+};
+
+&i2c1 {
+    status = "okay";
+
+    rk805: rk805@18 {
+        compatible = "rockchip,rk805";
+        reg = <0x18>;
+        interrupt-parent = <&gpio2>;
+        interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
The schematic says GPIO1_D0 for this.
You are right, I'll fix this.
quoted
+
It shouldn't hurt to enable PWM2 from the outset to make mucking about with the fan easier.
I've seen reports of people having Fans on their NanoPi R2S, my unit does not have such 
quoted
+&sdmmc {
+    bus-width = <4>;
+    cap-mmc-highspeed;
+    cap-sd-highspeed;
How about UHS modes, since we have the requisite I/O voltage support?
I'll look into this.
quoted
+    disable-wp;
I believe this property is only meant for full-size MMC/SD sockets where write-protect functionality is expected but somehow broken. For micro-SD sockets the SoC pin should be hard-wired to write-enable anyway and thus it shouldn't need to be stated.
Thanks for this explanation, I'll remove and re-test.
quoted
+    max-frequency = <150000000>;
This is in the DTSI already.
quoted
+    pinctrl-names = "default";
+    pinctrl-0 = <&sdmmc0_clk &sdmmc0_cmd &sdmmc0_dectn &sdmmc0_bus4>;
+    vmmc-supply = <&vcc_sd>;
+    vqmmc-supply = <&vcc_sdio>;
+    status = "okay";
+};
+
+&tsadc {
+    rockchip,hw-tshut-mode = <0>;
+    rockchip,hw-tshut-polarity = <0>;
+    status = "okay";
+};
+
+&uart2 {
+    status = "okay";
+};
+
+&u2phy {
+    status = "okay";
+
+    u2phy_host: host-port {
Just reference this by label directly, rather than as a child node here (especially since redefining the existing label is a bit confusing).
quoted
+        status = "okay";
+    };
+};
No reason not to enable the OTG port either - I've hacked that in with g_serial for a cheeky console while powering the board from my laptop and it works fine.
Ouch, I've completely missed the micro-USB port is wired up to the OTG port.

I'll follow up with a v3 in the coming days. Thanks for your feedback!

Best wishes
David
Cheers,
Robin.
quoted
+
+&usb_host0_ehci {
+    status = "okay";
+};
+
+&usb_host0_ohci {
+    status = "okay";
+};


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

Re: [PATCH v2 2/2] rockchip: rk3328: Add support for FriendlyARM NanoPi R2S

Message details and related content
On 2020-08-06 07:57, David Bauer wrote:
Hi Robin,

thanks for your review. Only adressing some non-formal comments below.

On 8/3/20 12:01 AM, Robin Murphy wrote:
quoted
This seems to match the schematic, however in practice the GPIO seems to have a mind of its own and not respond to the LED settings - are we missing something with the pinctrl perhaps?
Hmm, this is strange. It works fine on my unit (with the current OpenWrt release). Can you test this and report back?
It seems like somehow that particular GPIO doesn't get configured properly and is still in its default out-of-reset state as an input. Poking the GPIO from sysfs works as expected, and in fact just unbinding and rebinding the leds-gpio driver also manages to kick things into shape. Clearly there's some subtle driver-level issue, but at least it's not a DT problem :)
quoted
quoted
+            label = "nanopi-r2s:green:lan";
+        };
+
+        wan {
+            gpios = <&gpio2 RK_PC2 GPIO_ACTIVE_HIGH>;
+            label = "nanopi-r2s:green:wan";
+        };
+    };
+
+    gpio_keys {
+        compatible = "gpio-keys-polled";
Does this need to be polled? I would have thought regular interrupt-based gpio-keys should work, to avoid the overhead of polling for something (relatively) incredibly rare, but perhaps I'm wrong :/
To be honest, I've missed that this is still polled. I've had this set to polled mode, as I've
had some issues (were to dumb) to get the pinmux going. I'll switch this to interrupt triggered and report back.
quoted
quoted
+        poll-interval = <100>;
+
+        pinctrl-names = "default";
+        pinctrl-0 = <&button_pins>;
+
+        reset {
+            label = "Reset Button";
+            gpios = <&gpio0 RK_PA0 GPIO_ACTIVE_LOW>;
+            linux,code = <KEY_RESTART>;
+            debounce-interval = <50>;
+        };
+    };
+};
+
+&cpu0 {
+    cpu-supply = <&vdd_arm>;
+};
+
+&cpu1 {
+    cpu-supply = <&vdd_arm>;
+};
+

22 more quoted lines

On second look it's already present in half the existing RK3328 DTs (including the one I wrote, ahem...), so probably does deserve promoting into the DTSI.
quoted
quoted
+    snps,reset-gpio = <&gpio1 RK_PC2 GPIO_ACTIVE_LOW>;
+    snps,reset-active-low;
+    snps,reset-delays-us = <0 10000 50000>;
Since you're describing the phy already, you can convert these to generic reset specifiers on the phy node itself (shame we don't have an interrupt wired up, oh well...)
Yup, I've forgotten that there's reset-gpio bindings for the PHY itself.
quoted
quoted
+    tx_delay = <0x24>;
+    rx_delay = <0x18>;
+    status = "okay";
+
+    mdio {
+        compatible = "snps,dwmac-mdio";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtl8211e: ethernet-phy@0 {
+            reg = <0>;
Is that right? The schematic says the phy is strapped for address 1 (although 0 probably does manage to work since it should be interpreted as a broadcast address).
I'll test this.
quoted
quoted
+        };
+    };
+};
+
+&i2c1 {
+    status = "okay";
+
+    rk805: rk805@18 {
+        compatible = "rockchip,rk805";
+        reg = <0x18>;
+        interrupt-parent = <&gpio2>;
+        interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
The schematic says GPIO1_D0 for this.

8 more quoted lines

Yeah, mine came with the version 2 heatsink/fan fitted, although upon investigation it turns out the PWM is next to useless (something's off with rise times in the external MOSFET switching setup) and the fan connector may as well just be treated as GPIO-controlled anyway.
quoted
quoted
+&sdmmc {
+    bus-width = <4>;
+    cap-mmc-highspeed;
+    cap-sd-highspeed;
How about UHS modes, since we have the requisite I/O voltage support?
I'll look into this.
FWIW SDR104 seems to be working fine for me, with the PMIC interrupt corrected so as not to break the regulator.
quoted
quoted
+    disable-wp;
I believe this property is only meant for full-size MMC/SD sockets where write-protect functionality is expected but somehow broken. For micro-SD sockets the SoC pin should be hard-wired to write-enable anyway and thus it shouldn't need to be stated.
Thanks for this explanation, I'll remove and re-test.
Actually I think I was mistaken there - having double-checked the binding definition and datasheet it looks like RK3328 doesn't expose the WP function on any pin at all, so by the letter of the binding this *should* be specified for correctness (since we don't necessarily know for sure that the controller's internal WP line isn't left floating). Sorry for the confusion.

Cheers,
Robin.
quoted
quoted
+    max-frequency = <150000000>;
This is in the DTSI already.
quoted
+    pinctrl-names = "default";
+    pinctrl-0 = <&sdmmc0_clk &sdmmc0_cmd &sdmmc0_dectn &sdmmc0_bus4>;
+    vmmc-supply = <&vcc_sd>;
+    vqmmc-supply = <&vcc_sdio>;
+    status = "okay";
+};
+
+&tsadc {
+    rockchip,hw-tshut-mode = <0>;
+    rockchip,hw-tshut-polarity = <0>;
+    status = "okay";
+};
+
+&uart2 {
+    status = "okay";
+};
+
+&u2phy {
+    status = "okay";
+
+    u2phy_host: host-port {
Just reference this by label directly, rather than as a child node here (especially since redefining the existing label is a bit confusing).
quoted
+        status = "okay";
+    };
+};
No reason not to enable the OTG port either - I've hacked that in with g_serial for a cheeky console while powering the board from my laptop and it works fine.
Ouch, I've completely missed the micro-USB port is wired up to the OTG port.

I'll follow up with a v3 in the coming days. Thanks for your feedback!

Best wishes

17 more quoted lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help