Netdev Archive mirror
 help / color / mirror / Atom feed
* [RFC v1 0/5] Add Bananapi R3 Mini
@ 2024-05-05 16:45 Frank Wunderlich
  2024-05-05 16:45 ` [RFC v1 1/5] dt-bindings: leds: add led trigger netdev Frank Wunderlich
                   ` (5 more replies)
  0 siblings, 6 replies; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-05 16:45 UTC (permalink / raw
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

From: Frank Wunderlich <frank-w@public-files.de>

Add mt7986 based BananaPi R3 Mini SBC and fix some related binding errors.

Frank Wunderlich (5):
  dt-bindings: leds: add led trigger netdev
  dt-bindings: clock: mediatek: add address-cells and size-cells to
    ethsys
  dt-bindings: net: mediatek,net: add reset-cells
  dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini
  arm64: dts: mediatek: Add  mt7986 based Bananapi R3 Mini

 .../devicetree/bindings/arm/mediatek.yaml     |   1 +
 .../bindings/clock/mediatek,ethsys.yaml       |   6 +
 .../devicetree/bindings/leds/common.yaml      |   2 +
 .../devicetree/bindings/net/mediatek,net.yaml |   3 +
 arch/arm64/boot/dts/mediatek/Makefile         |   1 +
 .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
 6 files changed, 499 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts

-- 
2.34.1


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

* [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
  2024-05-05 16:45 [RFC v1 0/5] Add Bananapi R3 Mini Frank Wunderlich
@ 2024-05-05 16:45 ` Frank Wunderlich
  2024-05-06  8:18   ` Krzysztof Kozlowski
  2024-05-05 16:45 ` [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys Frank Wunderlich
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-05 16:45 UTC (permalink / raw
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

From: Frank Wunderlich <frank-w@public-files.de>

Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
binding.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 Documentation/devicetree/bindings/leds/common.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index 8a3c2398b10c..bf9a101e4d42 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -113,6 +113,8 @@ properties:
             # LED indicates NAND memory activity (deprecated),
             # in new implementations use "mtd"
           - nand-disk
+            # LED indicates network activity
+          - netdev
             # No trigger assigned to the LED. This is the default mode
             # if trigger is absent
           - none
-- 
2.34.1


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

* [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys
  2024-05-05 16:45 [RFC v1 0/5] Add Bananapi R3 Mini Frank Wunderlich
  2024-05-05 16:45 ` [RFC v1 1/5] dt-bindings: leds: add led trigger netdev Frank Wunderlich
@ 2024-05-05 16:45 ` Frank Wunderlich
  2024-05-06  8:18   ` Krzysztof Kozlowski
  2024-05-06 12:48   ` AngeloGioacchino Del Regno
  2024-05-05 16:45 ` [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells Frank Wunderlich
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-05 16:45 UTC (permalink / raw
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

From: Frank Wunderlich <frank-w@public-files.de>

Add missing properties already used in mt7986a.dtsi.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 .../devicetree/bindings/clock/mediatek,ethsys.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/mediatek,ethsys.yaml b/Documentation/devicetree/bindings/clock/mediatek,ethsys.yaml
index f9cddacc2eae..ce953a35f307 100644
--- a/Documentation/devicetree/bindings/clock/mediatek,ethsys.yaml
+++ b/Documentation/devicetree/bindings/clock/mediatek,ethsys.yaml
@@ -32,12 +32,18 @@ properties:
   reg:
     maxItems: 1
 
+  "#address-cells":
+    const: 1
+
   "#clock-cells":
     const: 1
 
   "#reset-cells":
     const: 1
 
+  "#size-cells":
+    const: 1
+
 required:
   - reg
   - "#clock-cells"
-- 
2.34.1


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

* [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells
  2024-05-05 16:45 [RFC v1 0/5] Add Bananapi R3 Mini Frank Wunderlich
  2024-05-05 16:45 ` [RFC v1 1/5] dt-bindings: leds: add led trigger netdev Frank Wunderlich
  2024-05-05 16:45 ` [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys Frank Wunderlich
@ 2024-05-05 16:45 ` Frank Wunderlich
  2024-05-06  8:19   ` Krzysztof Kozlowski
  2024-05-07 19:37   ` Rob Herring
  2024-05-05 16:45 ` [RFC v1 4/5] dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini Frank Wunderlich
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-05 16:45 UTC (permalink / raw
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

From: Frank Wunderlich <frank-w@public-files.de>

Add missing binding for property used in mt7986a.dtsi.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 Documentation/devicetree/bindings/net/mediatek,net.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
index e74502a0afe8..5214927c0fe8 100644
--- a/Documentation/devicetree/bindings/net/mediatek,net.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
@@ -101,6 +101,9 @@ properties:
   "#address-cells":
     const: 1
 
+  "#reset-cells":
+    const: 1
+
   "#size-cells":
     const: 0
 
-- 
2.34.1


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

* [RFC v1 4/5] dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini
  2024-05-05 16:45 [RFC v1 0/5] Add Bananapi R3 Mini Frank Wunderlich
                   ` (2 preceding siblings ...)
  2024-05-05 16:45 ` [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells Frank Wunderlich
@ 2024-05-05 16:45 ` Frank Wunderlich
  2024-05-05 16:45 ` [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi " Frank Wunderlich
  2024-05-06 20:40 ` [RFC v1 0/5] Add " Rob Herring (Arm)
  5 siblings, 0 replies; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-05 16:45 UTC (permalink / raw
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

From: Frank Wunderlich <frank-w@public-files.de>

Add MT7988A based BananaPi R3 Mini.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 Documentation/devicetree/bindings/arm/mediatek.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index 09f9ffd3ff7b..d4640e2e5fad 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -91,6 +91,7 @@ properties:
           - enum:
               - acelink,ew-7886cax
               - bananapi,bpi-r3
+              - bananapi,bpi-r3mini
               - mediatek,mt7986a-rfb
           - const: mediatek,mt7986a
       - items:
-- 
2.34.1


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

* [RFC v1 5/5] arm64: dts: mediatek: Add  mt7986 based Bananapi R3 Mini
  2024-05-05 16:45 [RFC v1 0/5] Add Bananapi R3 Mini Frank Wunderlich
                   ` (3 preceding siblings ...)
  2024-05-05 16:45 ` [RFC v1 4/5] dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini Frank Wunderlich
@ 2024-05-05 16:45 ` Frank Wunderlich
  2024-05-06  8:20   ` Krzysztof Kozlowski
  2024-05-06 12:48   ` AngeloGioacchino Del Regno
  2024-05-06 20:40 ` [RFC v1 0/5] Add " Rob Herring (Arm)
  5 siblings, 2 replies; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-05 16:45 UTC (permalink / raw
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek, Tianling Shen

From: Frank Wunderlich <frank-w@public-files.de>

Add device Tree for Bananapi R3 Mini SBC.

Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
Co-developed-by: Tianling Shen <cnsztl@gmail.com>
Signed-off-by: Tianling Shen <cnsztl@gmail.com>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 arch/arm64/boot/dts/mediatek/Makefile         |   1 +
 .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
 2 files changed, 487 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index 37b4ca3a87c9..1763b001ab06 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
new file mode 100644
index 000000000000..c764b4dc4752
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
@@ -0,0 +1,486 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (C) 2021 MediaTek Inc.
+ * Authors: Frank Wunderlich <frank-w@public-files.de>
+ *          Eric Woudstra <ericwouds@gmail.com>
+ *          Tianling Shen <cnsztl@immortalwrt.org>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/mt65xx.h>
+
+#include "mt7986a.dtsi"
+
+/ {
+	model = "Bananapi BPI-R3 Mini";
+	chassis-type = "embedded";
+	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
+
+	aliases {
+		serial0 = &uart0;
+		ethernet0 = &gmac0;
+		ethernet1 = &gmac1;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	dcin: regulator-12vd {
+		compatible = "regulator-fixed";
+		regulator-name = "12vd";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	fan: pwm-fan {
+		compatible = "pwm-fan";
+		#cooling-cells = <2>;
+		/* cooling level (0, 1, 2) - pwm inverted */
+		cooling-levels = <255 96 0>;
+		pwms = <&pwm 0 10000>;
+		status = "okay";
+	};
+
+	reg_1p8v: regulator-1p8v {
+		compatible = "regulator-fixed";
+		regulator-name = "1.8vd";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+		vin-supply = <&dcin>;
+	};
+
+	reg_3p3v: regulator-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "3.3vd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+		vin-supply = <&dcin>;
+	};
+
+	usb_vbus: regulator-usb-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
+		regulator-boot-on;
+	};
+
+	en8811_a: regulator-phy1 {
+		compatible = "regulator-fixed";
+		regulator-name = "phy1";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
+		regulator-always-on;
+	};
+
+	en8811_b: regulator-phy2 {
+		compatible = "regulator-fixed";
+		regulator-name = "phy2";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
+		regulator-always-on;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		green_led: led-0 {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_POWER;
+			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		reset-key {
+			label = "reset";
+			linux,code = <KEY_RESTART>;
+			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+};
+
+&cpu_thermal {
+	cooling-maps {
+		map0 {
+			/* active: set fan to cooling level 2 */
+			cooling-device = <&fan 2 2>;
+			trip = <&cpu_trip_active_high>;
+		};
+
+		map1 {
+			/* active: set fan to cooling level 1 */
+			cooling-device = <&fan 1 1>;
+			trip = <&cpu_trip_active_med>;
+		};
+
+		map2 {
+			/* active: set fan to cooling level 0 */
+			cooling-device = <&fan 0 0>;
+			trip = <&cpu_trip_active_low>;
+		};
+	};
+};
+
+&crypto {
+	status = "okay";
+};
+
+&eth {
+	status = "okay";
+
+	gmac0: mac@0 {
+		compatible = "mediatek,eth-mac";
+		reg = <0>;
+		phy-mode = "2500base-x";
+		phy-handle = <&phy14>;
+	};
+
+	gmac1: mac@1 {
+		compatible = "mediatek,eth-mac";
+		reg = <1>;
+		phy-mode = "2500base-x";
+		phy-handle = <&phy15>;
+	};
+
+	mdio: mdio-bus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+};
+
+&mmc0 {
+	pinctrl-names = "default", "state_uhs";
+	pinctrl-0 = <&mmc0_pins_default>;
+	pinctrl-1 = <&mmc0_pins_uhs>;
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&reg_1p8v>;
+};
+
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c_pins>;
+	status = "okay";
+
+	/* MAC Address EEPROM */
+	eeprom@50 {
+		compatible = "atmel,24c02";
+		reg = <0x50>;
+
+		address-width = <8>;
+		pagesize = <8>;
+		size = <256>;
+	};
+};
+
+&mdio {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	phy14: ethernet-phy@14 {
+		reg = <14>;
+		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
+		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
+		reset-assert-us = <10000>;
+		reset-deassert-us = <20000>;
+		phy-mode = "2500base-x";
+		full-duplex;
+		pause;
+		airoha,pnswap-rx;
+
+		leds {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			led@0 { /* en8811_a_gpio5 */
+				reg = <0>;
+				color = <LED_COLOR_ID_YELLOW>;
+				function = LED_FUNCTION_LAN;
+				function-enumerator = <1>;
+				default-state = "keep";
+				linux,default-trigger = "netdev";
+			};
+			led@1 { /* en8811_a_gpio4 */
+				reg = <1>;
+				color = <LED_COLOR_ID_GREEN>;
+				function = LED_FUNCTION_LAN;
+				function-enumerator = <2>;
+				default-state = "keep";
+				linux,default-trigger = "netdev";
+			};
+		};
+	};
+
+	phy15: ethernet-phy@15 {
+		reg = <15>;
+		interrupts-extended = <&pio 46 IRQ_TYPE_EDGE_FALLING>;
+		reset-gpios = <&pio 47 GPIO_ACTIVE_LOW>;
+		reset-assert-us = <10000>;
+		reset-deassert-us = <20000>;
+		phy-mode = "2500base-x";
+		full-duplex;
+		pause;
+		airoha,pnswap-rx;
+
+		leds {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			led@0 { /* en8811_b_gpio5 */
+				reg = <0>;
+				color = <LED_COLOR_ID_YELLOW>;
+				function = LED_FUNCTION_WAN;
+				function-enumerator = <1>;
+				default-state = "keep";
+				linux,default-trigger = "netdev";
+			};
+			led@1 { /* en8811_b_gpio4 */
+				reg = <1>;
+				color = <LED_COLOR_ID_GREEN>;
+				function = LED_FUNCTION_WAN;
+				function-enumerator = <2>;
+				default-state = "keep";
+				linux,default-trigger = "netdev";
+			};
+		};
+	};
+};
+
+&pcie {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie_pins>;
+	status = "okay";
+};
+
+&pcie_phy {
+	status = "okay";
+};
+
+&pio {
+	i2c_pins: i2c-pins {
+		mux {
+			function = "i2c";
+			groups = "i2c";
+		};
+	};
+
+	mmc0_pins_default: mmc0-pins {
+		mux {
+			function = "emmc";
+			groups = "emmc_51";
+		};
+		conf-cmd-dat {
+			pins = "EMMC_DATA_0", "EMMC_DATA_1", "EMMC_DATA_2",
+			       "EMMC_DATA_3", "EMMC_DATA_4", "EMMC_DATA_5",
+			       "EMMC_DATA_6", "EMMC_DATA_7", "EMMC_CMD";
+			input-enable;
+			drive-strength = <4>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>; /* pull-up 10K */
+		};
+		conf-clk {
+			pins = "EMMC_CK";
+			drive-strength = <6>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>; /* pull-down 50K */
+		};
+		conf-ds {
+			pins = "EMMC_DSL";
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>; /* pull-down 50K */
+		};
+		conf-rst {
+			pins = "EMMC_RSTB";
+			drive-strength = <4>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>; /* pull-up 10K */
+		};
+	};
+
+	mmc0_pins_uhs: mmc0-uhs-pins {
+		mux {
+			function = "emmc";
+			groups = "emmc_51";
+		};
+		conf-cmd-dat {
+			pins = "EMMC_DATA_0", "EMMC_DATA_1", "EMMC_DATA_2",
+			       "EMMC_DATA_3", "EMMC_DATA_4", "EMMC_DATA_5",
+			       "EMMC_DATA_6", "EMMC_DATA_7", "EMMC_CMD";
+			input-enable;
+			drive-strength = <4>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>; /* pull-up 10K */
+		};
+		conf-clk {
+			pins = "EMMC_CK";
+			drive-strength = <6>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>; /* pull-down 50K */
+		};
+		conf-ds {
+			pins = "EMMC_DSL";
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>; /* pull-down 50K */
+		};
+		conf-rst {
+			pins = "EMMC_RSTB";
+			drive-strength = <4>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>; /* pull-up 10K */
+		};
+	};
+
+	pcie_pins: pcie-pins {
+		mux {
+			function = "pcie";
+			groups = "pcie_clk", "pcie_wake", "pcie_pereset";
+		};
+	};
+
+	pwm_pins: pwm-pins {
+		mux {
+			function = "pwm";
+			groups = "pwm0";
+		};
+	};
+
+	spi_flash_pins: spi-flash-pins {
+		mux {
+			function = "spi";
+			groups = "spi0", "spi0_wp_hold";
+		};
+	};
+
+	usb_ngff_pins: usb-ngff-pins {
+		ngff-gnss-off-conf {
+			pins = "GPIO_6";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-pe-rst-conf {
+			pins = "GPIO_7";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-wwan-off-conf {
+			pins = "GPIO_8";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-pwr-off-conf {
+			pins = "GPIO_9";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-rst-conf {
+			pins = "GPIO_10";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+		ngff-coex-conf {
+			pins = "SPI1_CS";
+			drive-strength = <8>;
+			mediatek,pull-up-adv = <1>;
+		};
+	};
+
+	wf_2g_5g_pins: wf-2g-5g-pins {
+		mux {
+			function = "wifi";
+			groups = "wf_2g", "wf_5g";
+		};
+		conf {
+			pins = "WF0_HB1", "WF0_HB2", "WF0_HB3", "WF0_HB4",
+			       "WF0_HB0", "WF0_HB0_B", "WF0_HB5", "WF0_HB6",
+			       "WF0_HB7", "WF0_HB8", "WF0_HB9", "WF0_HB10",
+			       "WF0_TOP_CLK", "WF0_TOP_DATA", "WF1_HB1",
+			       "WF1_HB2", "WF1_HB3", "WF1_HB4", "WF1_HB0",
+			       "WF1_HB5", "WF1_HB6", "WF1_HB7", "WF1_HB8",
+			       "WF1_TOP_CLK", "WF1_TOP_DATA";
+			drive-strength = <4>;
+		};
+	};
+
+	wf_dbdc_pins: wf-dbdc-pins {
+		mux {
+			function = "wifi";
+			groups = "wf_dbdc";
+		};
+		conf {
+			pins = "WF0_HB1", "WF0_HB2", "WF0_HB3", "WF0_HB4",
+			       "WF0_HB0", "WF0_HB0_B", "WF0_HB5", "WF0_HB6",
+			       "WF0_HB7", "WF0_HB8", "WF0_HB9", "WF0_HB10",
+			       "WF0_TOP_CLK", "WF0_TOP_DATA", "WF1_HB1",
+			       "WF1_HB2", "WF1_HB3", "WF1_HB4", "WF1_HB0",
+			       "WF1_HB5", "WF1_HB6", "WF1_HB7", "WF1_HB8",
+			       "WF1_TOP_CLK", "WF1_TOP_DATA";
+			drive-strength = <4>;
+		};
+	};
+
+	wf_led_pins: wf-led-pins {
+		mux {
+			function = "led";
+			groups = "wifi_led";
+		};
+	};
+};
+
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm_pins>;
+	status = "okay";
+};
+
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi_flash_pins>;
+	status = "okay";
+};
+
+&ssusb {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb_ngff_pins>;
+	vusb33-supply = <&reg_3p3v>;
+	vbus-supply = <&usb_vbus>;
+	status = "okay";
+};
+
+&trng {
+	status = "okay";
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&usb_phy {
+	status = "okay";
+};
+
+&watchdog {
+	status = "okay";
+};
+
+&wifi {
+	status = "okay";
+	pinctrl-names = "default", "dbdc";
+	pinctrl-0 = <&wf_2g_5g_pins>, <&wf_led_pins>;
+	pinctrl-1 = <&wf_dbdc_pins>, <&wf_led_pins>;
+
+	led {
+		led-active-low;
+	};
+};
+
-- 
2.34.1


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

* Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
  2024-05-05 16:45 ` [RFC v1 1/5] dt-bindings: leds: add led trigger netdev Frank Wunderlich
@ 2024-05-06  8:18   ` Krzysztof Kozlowski
  2024-05-06  8:59     ` Daniel Golle
  2024-05-06 17:10     ` Aw: " Frank Wunderlich
  0 siblings, 2 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-06  8:18 UTC (permalink / raw
  To: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

On 05/05/2024 18:45, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
> binding.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  Documentation/devicetree/bindings/leds/common.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> index 8a3c2398b10c..bf9a101e4d42 100644
> --- a/Documentation/devicetree/bindings/leds/common.yaml
> +++ b/Documentation/devicetree/bindings/leds/common.yaml
> @@ -113,6 +113,8 @@ properties:
>              # LED indicates NAND memory activity (deprecated),
>              # in new implementations use "mtd"
>            - nand-disk
> +            # LED indicates network activity
> +          - netdev

"dev" is redundant (there is no flash-dev or usb-host-dev). Two network
interfaces are already provided, so your commit msg must provide
rationale why this is not enough and why this is useful/needed.

Best regards,
Krzysztof


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

* Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys
  2024-05-05 16:45 ` [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys Frank Wunderlich
@ 2024-05-06  8:18   ` Krzysztof Kozlowski
  2024-05-06 16:51     ` Aw: " Frank Wunderlich
  2024-05-06 12:48   ` AngeloGioacchino Del Regno
  1 sibling, 1 reply; 30+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-06  8:18 UTC (permalink / raw
  To: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

On 05/05/2024 18:45, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add missing properties already used in mt7986a.dtsi.

Missing for what? Or why? Provide context, IOW, explain why they are
missing.


Best regards,
Krzysztof


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

* Re: [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells
  2024-05-05 16:45 ` [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells Frank Wunderlich
@ 2024-05-06  8:19   ` Krzysztof Kozlowski
  2024-05-07 19:37   ` Rob Herring
  1 sibling, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-06  8:19 UTC (permalink / raw
  To: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

On 05/05/2024 18:45, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add missing binding for property used in mt7986a.dtsi.
> 

Again, no, it is not missing, if the device is not reset controller.

Provide explanation why this is suitable in the binding. IOW, why DTS is
right, but binding is wrong.

Best regards,
Krzysztof


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

* Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-05 16:45 ` [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi " Frank Wunderlich
@ 2024-05-06  8:20   ` Krzysztof Kozlowski
  2024-05-06 16:40     ` Aw: " Frank Wunderlich
  2024-05-06 12:48   ` AngeloGioacchino Del Regno
  1 sibling, 1 reply; 30+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-06  8:20 UTC (permalink / raw
  To: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek, Tianling Shen

On 05/05/2024 18:45, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add device Tree for Bananapi R3 Mini SBC.
> 
> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>  .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>  2 files changed, 487 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> 
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index 37b4ca3a87c9..1763b001ab06 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> new file mode 100644
> index 000000000000..c764b4dc4752
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> @@ -0,0 +1,486 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Authors: Frank Wunderlich <frank-w@public-files.de>
> + *          Eric Woudstra <ericwouds@gmail.com>
> + *          Tianling Shen <cnsztl@immortalwrt.org>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/mt65xx.h>
> +
> +#include "mt7986a.dtsi"
> +
> +/ {
> +	model = "Bananapi BPI-R3 Mini";
> +	chassis-type = "embedded";
> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
> +
> +	aliases {
> +		serial0 = &uart0;
> +		ethernet0 = &gmac0;
> +		ethernet1 = &gmac1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	dcin: regulator-12vd {

Please use name for all fixed regulators which matches current format
recommendation: 'regulator-[0-9]+v[0-9]+'

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=b6d4b3500d57370f5b3abf0701c9166b384db976

> +		compatible = "regulator-fixed";
> +		regulator-name = "12vd";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
> +	fan: pwm-fan {
> +		compatible = "pwm-fan";
> +		#cooling-cells = <2>;
> +		/* cooling level (0, 1, 2) - pwm inverted */
> +		cooling-levels = <255 96 0>;
> +		pwms = <&pwm 0 10000>;
> +		status = "okay";

Why? Where is it disabled?

> +	};
> +
> +	reg_1p8v: regulator-1p8v {

Please use name for all fixed regulators which matches current format
recommendation: 'regulator-[0-9]+v[0-9]+'

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=b6d4b3500d57370f5b3abf0701c9166b384db976

In other places as well.



Best regards,
Krzysztof


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

* Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
  2024-05-06  8:18   ` Krzysztof Kozlowski
@ 2024-05-06  8:59     ` Daniel Golle
  2024-05-06 17:10     ` Aw: " Frank Wunderlich
  1 sibling, 0 replies; 30+ messages in thread
From: Daniel Golle @ 2024-05-06  8:59 UTC (permalink / raw
  To: Krzysztof Kozlowski
  Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno, devicetree,
	Tianling Shen, netdev, linux-kernel, linux-mediatek,
	linux-arm-kernel, Eric Woudstra, linux-clk, linux-leds

On Mon, May 06, 2024 at 10:18:09AM +0200, Krzysztof Kozlowski wrote:
> On 05/05/2024 18:45, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > 
> > Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
> > binding.
> > 
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> >  Documentation/devicetree/bindings/leds/common.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> > index 8a3c2398b10c..bf9a101e4d42 100644
> > --- a/Documentation/devicetree/bindings/leds/common.yaml
> > +++ b/Documentation/devicetree/bindings/leds/common.yaml
> > @@ -113,6 +113,8 @@ properties:
> >              # LED indicates NAND memory activity (deprecated),
> >              # in new implementations use "mtd"
> >            - nand-disk
> > +            # LED indicates network activity
> > +          - netdev
> 
> "dev" is redundant (there is no flash-dev or usb-host-dev). Two network
> interfaces are already provided, so your commit msg must provide
> rationale why this is not enough and why this is useful/needed.

Also note that using 'netdev' assigned via DT via linux,default-trigger
currently doesn't work well. This is because the assignment of the trigger
from DT happens when the PHY is being attached initially, and that's
**before** the network device is registered with Linux.
As a result, LED event offloading is never used if done in this way.

I know that bindings and implementation are two different independent
things, but yet I believe that adding bindings for a feature which doesn't
really work would be misleading.

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

* Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys
  2024-05-05 16:45 ` [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys Frank Wunderlich
  2024-05-06  8:18   ` Krzysztof Kozlowski
@ 2024-05-06 12:48   ` AngeloGioacchino Del Regno
  2024-05-06 16:01     ` Frank Wunderlich
  1 sibling, 1 reply; 30+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-05-06 12:48 UTC (permalink / raw
  To: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek

Il 05/05/24 18:45, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add missing properties already used in mt7986a.dtsi.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>

Fixes tag? :-)

Cheers,
Angelo

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

* Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-05 16:45 ` [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi " Frank Wunderlich
  2024-05-06  8:20   ` Krzysztof Kozlowski
@ 2024-05-06 12:48   ` AngeloGioacchino Del Regno
  2024-05-06 16:00     ` Frank Wunderlich
  1 sibling, 1 reply; 30+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-05-06 12:48 UTC (permalink / raw
  To: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger
  Cc: Frank Wunderlich, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek, Tianling Shen

Il 05/05/24 18:45, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add device Tree for Bananapi R3 Mini SBC.
> 
> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>   arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>   .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>   2 files changed, 487 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> 
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index 37b4ca3a87c9..1763b001ab06 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> new file mode 100644
> index 000000000000..c764b4dc4752
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> @@ -0,0 +1,486 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Authors: Frank Wunderlich <frank-w@public-files.de>
> + *          Eric Woudstra <ericwouds@gmail.com>
> + *          Tianling Shen <cnsztl@immortalwrt.org>
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/mt65xx.h>
> +
> +#include "mt7986a.dtsi"
> +
> +/ {
> +	model = "Bananapi BPI-R3 Mini";
> +	chassis-type = "embedded";
> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
> +
> +	aliases {
> +		serial0 = &uart0;
> +		ethernet0 = &gmac0;
> +		ethernet1 = &gmac1;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	dcin: regulator-12vd {
> +		compatible = "regulator-fixed";
> +		regulator-name = "12vd";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
> +	fan: pwm-fan {
> +		compatible = "pwm-fan";
> +		#cooling-cells = <2>;
> +		/* cooling level (0, 1, 2) - pwm inverted */
> +		cooling-levels = <255 96 0>;

Did you try to actually invert the PWM?

Look for PWM_POLARITY_INVERTED ;-)

> +		pwms = <&pwm 0 10000>;
> +		status = "okay";
> +	};
> +
> +	reg_1p8v: regulator-1p8v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "1.8vd";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +		vin-supply = <&dcin>;
> +	};
> +
> +	reg_3p3v: regulator-3p3v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "3.3vd";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +		vin-supply = <&dcin>;
> +	};
> +
> +	usb_vbus: regulator-usb-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
> +		regulator-boot-on;
> +	};
> +
> +	en8811_a: regulator-phy1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "phy1";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
> +		regulator-always-on;
> +	};
> +
> +	en8811_b: regulator-phy2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "phy2";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
> +		regulator-always-on;
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		green_led: led-0 {
> +			color = <LED_COLOR_ID_GREEN>;
> +			function = LED_FUNCTION_POWER;
> +			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
> +			default-state = "on";
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		reset-key {
> +			label = "reset";
> +			linux,code = <KEY_RESTART>;
> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +};
> +
> +&cpu_thermal {
> +	cooling-maps {
> +		map0 {
> +			/* active: set fan to cooling level 2 */
> +			cooling-device = <&fan 2 2>;
> +			trip = <&cpu_trip_active_high>;
> +		};
> +
> +		map1 {
> +			/* active: set fan to cooling level 1 */
> +			cooling-device = <&fan 1 1>;
> +			trip = <&cpu_trip_active_med>;
> +		};
> +
> +		map2 {
> +			/* active: set fan to cooling level 0 */
> +			cooling-device = <&fan 0 0>;
> +			trip = <&cpu_trip_active_low>;
> +		};
> +	};
> +};
> +
> +&crypto {
> +	status = "okay";
> +};
> +
> +&eth {
> +	status = "okay";
> +
> +	gmac0: mac@0 {
> +		compatible = "mediatek,eth-mac";
> +		reg = <0>;
> +		phy-mode = "2500base-x";
> +		phy-handle = <&phy14>;
> +	};
> +
> +	gmac1: mac@1 {
> +		compatible = "mediatek,eth-mac";
> +		reg = <1>;
> +		phy-mode = "2500base-x";
> +		phy-handle = <&phy15>;
> +	};
> +
> +	mdio: mdio-bus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> +};
> +
> +&mmc0 {
> +	pinctrl-names = "default", "state_uhs";
> +	pinctrl-0 = <&mmc0_pins_default>;
> +	pinctrl-1 = <&mmc0_pins_uhs>;
> +	vmmc-supply = <&reg_3p3v>;
> +	vqmmc-supply = <&reg_1p8v>;
> +};
> +
> +
> +&i2c0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c_pins>;
> +	status = "okay";
> +
> +	/* MAC Address EEPROM */
> +	eeprom@50 {
> +		compatible = "atmel,24c02";
> +		reg = <0x50>;
> +
> +		address-width = <8>;
> +		pagesize = <8>;
> +		size = <256>;
> +	};
> +};
> +
> +&mdio {

You can just move all this stuff to where you declare the mdio-bus....

> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	phy14: ethernet-phy@14 {

I say that this is `phy0: ethernet-phy@14` - because this is the first PHY on this
board.

> +		reg = <14>;

Uhm.. doesn't this require the ethernet-phy-id03a2.a411 compatible?

> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
> +		reset-assert-us = <10000>;
> +		reset-deassert-us = <20000>;
> +		phy-mode = "2500base-x";
> +		full-duplex;
> +		pause;
> +		airoha,pnswap-rx;
> +
> +		leds {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			led@0 { /* en8811_a_gpio5 */
> +				reg = <0>;
> +				color = <LED_COLOR_ID_YELLOW>;
> +				function = LED_FUNCTION_LAN;
> +				function-enumerator = <1>;

Why aren't you simply using a label?

> +				default-state = "keep";
> +				linux,default-trigger = "netdev";
> +			};
> +			led@1 { /* en8811_a_gpio4 */
> +				reg = <1>;
> +				color = <LED_COLOR_ID_GREEN>;
> +				function = LED_FUNCTION_LAN;
> +				function-enumerator = <2>;
> +				default-state = "keep";
> +				linux,default-trigger = "netdev";
> +			};
> +		};
> +	};
> +
> +	phy15: ethernet-phy@15 {
> +		reg = <15>;

Same here.

Cheers,
Angelo


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

* Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-06 12:48   ` AngeloGioacchino Del Regno
@ 2024-05-06 16:00     ` Frank Wunderlich
  2024-05-06 16:12       ` Daniel Golle
  2024-05-07 13:35       ` AngeloGioacchino Del Regno
  0 siblings, 2 replies; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-06 16:00 UTC (permalink / raw
  To: AngeloGioacchino Del Regno, Frank Wunderlich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger
  Cc: Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek,
	Tianling Shen

Hi

Thanks for review.

Am 6. Mai 2024 14:48:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 05/05/24 18:45, Frank Wunderlich ha scritto:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 
>> Add device Tree for Bananapi R3 Mini SBC.
>> 
>> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
>> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>> ---
>>   arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>>   .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>>   2 files changed, 487 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>> 
>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
>> index 37b4ca3a87c9..1763b001ab06 100644
>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
>> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>> new file mode 100644
>> index 000000000000..c764b4dc4752
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>> @@ -0,0 +1,486 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>> +/*
>> + * Copyright (C) 2021 MediaTek Inc.
>> + * Authors: Frank Wunderlich <frank-w@public-files.de>
>> + *          Eric Woudstra <ericwouds@gmail.com>
>> + *          Tianling Shen <cnsztl@immortalwrt.org>
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/leds/common.h>
>> +#include <dt-bindings/pinctrl/mt65xx.h>
>> +
>> +#include "mt7986a.dtsi"
>> +
>> +/ {
>> +	model = "Bananapi BPI-R3 Mini";
>> +	chassis-type = "embedded";
>> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
>> +
>> +	aliases {
>> +		serial0 = &uart0;
>> +		ethernet0 = &gmac0;
>> +		ethernet1 = &gmac1;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +
>> +	dcin: regulator-12vd {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "12vd";
>> +		regulator-min-microvolt = <12000000>;
>> +		regulator-max-microvolt = <12000000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +	};
>> +
>> +	fan: pwm-fan {
>> +		compatible = "pwm-fan";
>> +		#cooling-cells = <2>;
>> +		/* cooling level (0, 1, 2) - pwm inverted */
>> +		cooling-levels = <255 96 0>;
>
>Did you try to actually invert the PWM?
>
>Look for PWM_POLARITY_INVERTED ;-)

Mtk pwm driver does not support it

https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211

>> +		pwms = <&pwm 0 10000>;
>> +		status = "okay";
>> +	};
>> +
>> +	reg_1p8v: regulator-1p8v {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "1.8vd";
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		vin-supply = <&dcin>;
>> +	};
>> +
>> +	reg_3p3v: regulator-3p3v {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "3.3vd";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +		vin-supply = <&dcin>;
>> +	};
>> +
>> +	usb_vbus: regulator-usb-vbus {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "usb_vbus";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
>> +		regulator-boot-on;
>> +	};
>> +
>> +	en8811_a: regulator-phy1 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "phy1";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
>> +		regulator-always-on;
>> +	};
>> +
>> +	en8811_b: regulator-phy2 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "phy2";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
>> +		regulator-always-on;
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +
>> +		green_led: led-0 {
>> +			color = <LED_COLOR_ID_GREEN>;
>> +			function = LED_FUNCTION_POWER;
>> +			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
>> +			default-state = "on";
>> +		};
>> +	};
>> +
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +
>> +		reset-key {
>> +			label = "reset";
>> +			linux,code = <KEY_RESTART>;
>> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
>> +		};
>> +	};
>> +
>> +};
>> +
>> +&cpu_thermal {
>> +	cooling-maps {
>> +		map0 {
>> +			/* active: set fan to cooling level 2 */
>> +			cooling-device = <&fan 2 2>;
>> +			trip = <&cpu_trip_active_high>;
>> +		};
>> +
>> +		map1 {
>> +			/* active: set fan to cooling level 1 */
>> +			cooling-device = <&fan 1 1>;
>> +			trip = <&cpu_trip_active_med>;
>> +		};
>> +
>> +		map2 {
>> +			/* active: set fan to cooling level 0 */
>> +			cooling-device = <&fan 0 0>;
>> +			trip = <&cpu_trip_active_low>;
>> +		};
>> +	};
>> +};
>> +
>> +&crypto {
>> +	status = "okay";
>> +};
>> +
>> +&eth {
>> +	status = "okay";
>> +
>> +	gmac0: mac@0 {
>> +		compatible = "mediatek,eth-mac";
>> +		reg = <0>;
>> +		phy-mode = "2500base-x";
>> +		phy-handle = <&phy14>;
>> +	};
>> +
>> +	gmac1: mac@1 {
>> +		compatible = "mediatek,eth-mac";
>> +		reg = <1>;
>> +		phy-mode = "2500base-x";
>> +		phy-handle = <&phy15>;
>> +	};
>> +
>> +	mdio: mdio-bus {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +	};
>> +};
>> +
>> +&mmc0 {
>> +	pinctrl-names = "default", "state_uhs";
>> +	pinctrl-0 = <&mmc0_pins_default>;
>> +	pinctrl-1 = <&mmc0_pins_uhs>;
>> +	vmmc-supply = <&reg_3p3v>;
>> +	vqmmc-supply = <&reg_1p8v>;
>> +};
>> +
>> +
>> +&i2c0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&i2c_pins>;
>> +	status = "okay";
>> +
>> +	/* MAC Address EEPROM */
>> +	eeprom@50 {
>> +		compatible = "atmel,24c02";
>> +		reg = <0x50>;
>> +
>> +		address-width = <8>;
>> +		pagesize = <8>;
>> +		size = <256>;
>> +	};
>> +};
>> +
>> +&mdio {
>
>You can just move all this stuff to where you declare the mdio-bus....

Ok,see these 2 lines are already above,so can be dropped here.

>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	phy14: ethernet-phy@14 {
>
>I say that this is `phy0: ethernet-phy@14` - because this is the first PHY on this
>board.

Ok,i change this and phy15

>> +		reg = <14>;
>
>Uhm.. doesn't this require the ethernet-phy-id03a2.a411 compatible?

I can add it,but worked without it.

There was a discussion about that and result was we don't need it in board dts,maybe add compatible in binding example.

https://patchwork.kernel.org/project/netdevbpf/patch/20240206194751.1901802-2-ericwouds@gmail.com/#25703356

>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>> +		reset-assert-us = <10000>;
>> +		reset-deassert-us = <20000>;
>> +		phy-mode = "2500base-x";
>> +		full-duplex;
>> +		pause;
>> +		airoha,pnswap-rx;
>> +
>> +		leds {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			led@0 { /* en8811_a_gpio5 */
>> +				reg = <0>;
>> +				color = <LED_COLOR_ID_YELLOW>;
>> +				function = LED_FUNCTION_LAN;
>> +				function-enumerator = <1>;
>
>Why aren't you simply using a label?

You mean the comment? I can add it of course like for regulators.

>> +				default-state = "keep";
>> +				linux,default-trigger = "netdev";
>> +			};
>> +			led@1 { /* en8811_a_gpio4 */
>> +				reg = <1>;
>> +				color = <LED_COLOR_ID_GREEN>;
>> +				function = LED_FUNCTION_LAN;
>> +				function-enumerator = <2>;
>> +				default-state = "keep";
>> +				linux,default-trigger = "netdev";
>> +			};
>> +		};
>> +	};
>> +
>> +	phy15: ethernet-phy@15 {
>> +		reg = <15>;
>
>Same here.
>
>Cheers,
>Angelo
>


regards Frank

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

* Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys
  2024-05-06 12:48   ` AngeloGioacchino Del Regno
@ 2024-05-06 16:01     ` Frank Wunderlich
  2024-05-07 13:36       ` AngeloGioacchino Del Regno
  0 siblings, 1 reply; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-06 16:01 UTC (permalink / raw
  To: AngeloGioacchino Del Regno, Frank Wunderlich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger
  Cc: Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek

Am 6. Mai 2024 14:48:58 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 05/05/24 18:45, Frank Wunderlich ha scritto:
>> From: Frank Wunderlich <frank-w@public-files.de>
>> 
>> Add missing properties already used in mt7986a.dtsi.
>> 
>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>
>Fixes tag? :-)
>
>Cheers,
>Angelo

Should i use fixes tag of binding commit or where dts (-part) was added?
regards Frank

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

* Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-06 16:00     ` Frank Wunderlich
@ 2024-05-06 16:12       ` Daniel Golle
  2024-05-07 13:35       ` AngeloGioacchino Del Regno
  1 sibling, 0 replies; 30+ messages in thread
From: Daniel Golle @ 2024-05-06 16:12 UTC (permalink / raw
  To: Frank Wunderlich
  Cc: AngeloGioacchino Del Regno, Frank Wunderlich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek,
	Tianling Shen

On Mon, May 06, 2024 at 06:00:30PM +0200, Frank Wunderlich wrote:
> Hi
> 
> Thanks for review.
> 
> Am 6. Mai 2024 14:48:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
> >Il 05/05/24 18:45, Frank Wunderlich ha scritto:
> >> From: Frank Wunderlich <frank-w@public-files.de>
> >> 
> >> Add device Tree for Bananapi R3 Mini SBC.
> >> 
> >> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
> >> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
> >> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
> >> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
> >> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> >> ---
> >>   arch/arm64/boot/dts/mediatek/Makefile         |   1 +
> >>   .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
> >>   2 files changed, 487 insertions(+)
> >>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> >> 
> >> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> >> index 37b4ca3a87c9..1763b001ab06 100644
> >> --- a/arch/arm64/boot/dts/mediatek/Makefile
> >> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> >> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
> >> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
> >>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
> >> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> >> new file mode 100644
> >> index 000000000000..c764b4dc4752
> >> --- /dev/null
> >> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> >> @@ -0,0 +1,486 @@
> >> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> >> +/*
> >> + * Copyright (C) 2021 MediaTek Inc.
> >> + * Authors: Frank Wunderlich <frank-w@public-files.de>
> >> + *          Eric Woudstra <ericwouds@gmail.com>
> >> + *          Tianling Shen <cnsztl@immortalwrt.org>
> >> + */
> >> +
> >> +/dts-v1/;
> >> +
> >> +#include <dt-bindings/gpio/gpio.h>
> >> +#include <dt-bindings/input/input.h>
> >> +#include <dt-bindings/leds/common.h>
> >> +#include <dt-bindings/pinctrl/mt65xx.h>
> >> +
> >> +#include "mt7986a.dtsi"
> >> +
> >> +/ {
> >> +	model = "Bananapi BPI-R3 Mini";
> >> +	chassis-type = "embedded";
> >> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
> >> +
> >> +	aliases {
> >> +		serial0 = &uart0;
> >> +		ethernet0 = &gmac0;
> >> +		ethernet1 = &gmac1;
> >> +	};
> >> +
> >> +	chosen {
> >> +		stdout-path = "serial0:115200n8";
> >> +	};
> >> +
> >> +	dcin: regulator-12vd {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "12vd";
> >> +		regulator-min-microvolt = <12000000>;
> >> +		regulator-max-microvolt = <12000000>;
> >> +		regulator-boot-on;
> >> +		regulator-always-on;
> >> +	};
> >> +
> >> +	fan: pwm-fan {
> >> +		compatible = "pwm-fan";
> >> +		#cooling-cells = <2>;
> >> +		/* cooling level (0, 1, 2) - pwm inverted */
> >> +		cooling-levels = <255 96 0>;
> >
> >Did you try to actually invert the PWM?
> >
> >Look for PWM_POLARITY_INVERTED ;-)
> 
> Mtk pwm driver does not support it
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
> 
> >> +		pwms = <&pwm 0 10000>;
> >> +		status = "okay";
> >> +	};
> >> +
> >> +	reg_1p8v: regulator-1p8v {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "1.8vd";
> >> +		regulator-min-microvolt = <1800000>;
> >> +		regulator-max-microvolt = <1800000>;
> >> +		regulator-boot-on;
> >> +		regulator-always-on;
> >> +		vin-supply = <&dcin>;
> >> +	};
> >> +
> >> +	reg_3p3v: regulator-3p3v {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "3.3vd";
> >> +		regulator-min-microvolt = <3300000>;
> >> +		regulator-max-microvolt = <3300000>;
> >> +		regulator-boot-on;
> >> +		regulator-always-on;
> >> +		vin-supply = <&dcin>;
> >> +	};
> >> +
> >> +	usb_vbus: regulator-usb-vbus {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "usb_vbus";
> >> +		regulator-min-microvolt = <5000000>;
> >> +		regulator-max-microvolt = <5000000>;
> >> +		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
> >> +		regulator-boot-on;
> >> +	};
> >> +
> >> +	en8811_a: regulator-phy1 {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "phy1";
> >> +		regulator-min-microvolt = <3300000>;
> >> +		regulator-max-microvolt = <3300000>;
> >> +		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
> >> +		regulator-always-on;
> >> +	};
> >> +
> >> +	en8811_b: regulator-phy2 {
> >> +		compatible = "regulator-fixed";
> >> +		regulator-name = "phy2";
> >> +		regulator-min-microvolt = <3300000>;
> >> +		regulator-max-microvolt = <3300000>;
> >> +		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
> >> +		regulator-always-on;
> >> +	};
> >> +
> >> +	leds {
> >> +		compatible = "gpio-leds";
> >> +
> >> +		green_led: led-0 {
> >> +			color = <LED_COLOR_ID_GREEN>;
> >> +			function = LED_FUNCTION_POWER;
> >> +			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
> >> +			default-state = "on";
> >> +		};
> >> +	};
> >> +
> >> +	gpio-keys {
> >> +		compatible = "gpio-keys";
> >> +
> >> +		reset-key {
> >> +			label = "reset";
> >> +			linux,code = <KEY_RESTART>;
> >> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
> >> +		};
> >> +	};
> >> +
> >> +};
> >> +
> >> +&cpu_thermal {
> >> +	cooling-maps {
> >> +		map0 {
> >> +			/* active: set fan to cooling level 2 */
> >> +			cooling-device = <&fan 2 2>;
> >> +			trip = <&cpu_trip_active_high>;
> >> +		};
> >> +
> >> +		map1 {
> >> +			/* active: set fan to cooling level 1 */
> >> +			cooling-device = <&fan 1 1>;
> >> +			trip = <&cpu_trip_active_med>;
> >> +		};
> >> +
> >> +		map2 {
> >> +			/* active: set fan to cooling level 0 */
> >> +			cooling-device = <&fan 0 0>;
> >> +			trip = <&cpu_trip_active_low>;
> >> +		};
> >> +	};
> >> +};
> >> +
> >> +&crypto {
> >> +	status = "okay";
> >> +};
> >> +
> >> +&eth {
> >> +	status = "okay";
> >> +
> >> +	gmac0: mac@0 {
> >> +		compatible = "mediatek,eth-mac";
> >> +		reg = <0>;
> >> +		phy-mode = "2500base-x";
> >> +		phy-handle = <&phy14>;
> >> +	};
> >> +
> >> +	gmac1: mac@1 {
> >> +		compatible = "mediatek,eth-mac";
> >> +		reg = <1>;
> >> +		phy-mode = "2500base-x";
> >> +		phy-handle = <&phy15>;
> >> +	};
> >> +
> >> +	mdio: mdio-bus {
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> >> +	};
> >> +};
> >> +
> >> +&mmc0 {
> >> +	pinctrl-names = "default", "state_uhs";
> >> +	pinctrl-0 = <&mmc0_pins_default>;
> >> +	pinctrl-1 = <&mmc0_pins_uhs>;
> >> +	vmmc-supply = <&reg_3p3v>;
> >> +	vqmmc-supply = <&reg_1p8v>;
> >> +};
> >> +
> >> +
> >> +&i2c0 {
> >> +	pinctrl-names = "default";
> >> +	pinctrl-0 = <&i2c_pins>;
> >> +	status = "okay";
> >> +
> >> +	/* MAC Address EEPROM */
> >> +	eeprom@50 {
> >> +		compatible = "atmel,24c02";
> >> +		reg = <0x50>;
> >> +
> >> +		address-width = <8>;
> >> +		pagesize = <8>;
> >> +		size = <256>;
> >> +	};
> >> +};
> >> +
> >> +&mdio {
> >
> >You can just move all this stuff to where you declare the mdio-bus....
> 
> Ok,see these 2 lines are already above,so can be dropped here.
> 
> >> +	#address-cells = <1>;
> >> +	#size-cells = <0>;
> >> +
> >> +	phy14: ethernet-phy@14 {
> >
> >I say that this is `phy0: ethernet-phy@14` - because this is the first PHY on this
> >board.
> 
> Ok,i change this and phy15
> 
> >> +		reg = <14>;
> >
> >Uhm.. doesn't this require the ethernet-phy-id03a2.a411 compatible?
> 
> I can add it,but worked without it.
> 
> There was a discussion about that and result was we don't need it in board dts,maybe add compatible in binding example.
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20240206194751.1901802-2-ericwouds@gmail.com/#25703356

I confirm that setting the PHY ID in DT is **not** needed.

The PHY probes fine and it's possible to read the PHY ID even before
firmware has been loaded.

> 
> >> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
> >> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
> >> +		reset-assert-us = <10000>;
> >> +		reset-deassert-us = <20000>;
> >> +		phy-mode = "2500base-x";
> >> +		full-duplex;
> >> +		pause;
> >> +		airoha,pnswap-rx;
> >> +
> >> +		leds {
> >> +			#address-cells = <1>;
> >> +			#size-cells = <0>;
> >> +
> >> +			led@0 { /* en8811_a_gpio5 */
> >> +				reg = <0>;
> >> +				color = <LED_COLOR_ID_YELLOW>;
> >> +				function = LED_FUNCTION_LAN;
> >> +				function-enumerator = <1>;
> >
> >Why aren't you simply using a label?
> 
> You mean the comment? I can add it of course like for regulators.
> 
> >> +				default-state = "keep";
> >> +				linux,default-trigger = "netdev";

Using linux,default-trigger = "netdev" will NOT work as intended,
see also the reply to your other patch where you are adding netdev
trigger to dt-bindings.
If you do this, it will seemingly work, but if you check
/sys/class/leds/foo/hw_control
it will always be 0, and hardware offloading will hence never be used.
Please just set the LED trigger in userspace for now.

> >> +			};
> >> +			led@1 { /* en8811_a_gpio4 */
> >> +				reg = <1>;
> >> +				color = <LED_COLOR_ID_GREEN>;
> >> +				function = LED_FUNCTION_LAN;
> >> +				function-enumerator = <2>;
> >> +				default-state = "keep";
> >> +				linux,default-trigger = "netdev";
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	phy15: ethernet-phy@15 {
> >> +		reg = <15>;
> >
> >Same here.
> >
> >Cheers,
> >Angelo
> >
> 
> 
> regards Frank
> 

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

* Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-06  8:20   ` Krzysztof Kozlowski
@ 2024-05-06 16:40     ` Frank Wunderlich
  0 siblings, 0 replies; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-06 16:40 UTC (permalink / raw
  To: Krzysztof Kozlowski
  Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno, Eric Woudstra,
	Tianling Shen, devicetree, linux-kernel, linux-clk, linux-leds,
	netdev, linux-arm-kernel, linux-mediatek, Tianling Shen

Hi

Thanks for review



> Gesendet: Montag, 06. Mai 2024 um 10:20 Uhr
> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> An: "Frank Wunderlich" <linux@fw-web.de>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski"
> > +	dcin: regulator-12vd {
>
> Please use name for all fixed regulators which matches current format
> recommendation: 'regulator-[0-9]+v[0-9]+'
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=b6d4b3500d57370f5b3abf0701c9166b384db976
>
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "12vd";
> > +		regulator-min-microvolt = <12000000>;
> > +		regulator-max-microvolt = <12000000>;
> > +		regulator-boot-on;
> > +		regulator-always-on;
> > +	};
> > +
> > +	fan: pwm-fan {
> > +		compatible = "pwm-fan";
> > +		#cooling-cells = <2>;
> > +		/* cooling level (0, 1, 2) - pwm inverted */
> > +		cooling-levels = <255 96 0>;
> > +		pwms = <&pwm 0 10000>;
> > +		status = "okay";
>
> Why? Where is it disabled?

you're right, i'll drop it

> > +	};
> > +
> > +	reg_1p8v: regulator-1p8v {
>
> Please use name for all fixed regulators which matches current format
> recommendation: 'regulator-[0-9]+v[0-9]+'
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git/commit/?id=b6d4b3500d57370f5b3abf0701c9166b384db976
>
> In other places as well.

ok i change it like this (label doesn't matter, right?):

dcin: regulator-12v {
reg_1p8v: regulator-1v8 {
reg_3p3v: regulator-3v3 {
usb_vbus: regulator-5v {


> Best regards,
> Krzysztof

regards Frank

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

* Aw: Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys
  2024-05-06  8:18   ` Krzysztof Kozlowski
@ 2024-05-06 16:51     ` Frank Wunderlich
  2024-05-07 19:35       ` Rob Herring
  0 siblings, 1 reply; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-06 16:51 UTC (permalink / raw
  To: Krzysztof Kozlowski
  Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno, Eric Woudstra,
	Tianling Shen, devicetree, linux-kernel, linux-clk, linux-leds,
	netdev, linux-arm-kernel, linux-mediatek, Frank Wunderlich

> Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr
> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> On 05/05/2024 18:45, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> >
> > Add missing properties already used in mt7986a.dtsi.
>
> Missing for what? Or why? Provide context, IOW, explain why they are
> missing.

ethernet-node in mt7986a.dtsi hast reset-cells-property

https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm64/boot/dts/mediatek/mt7986a.dtsi#L559

and

address-cells and size-cells are used here:

https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm64/boot/dts/mediatek/mt7986a.dtsi#L495

i saw the warnings while checking my r3mini dts...

arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: syscon@15000000: '#address-cells', '#size-cells' do not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/clock/mediatek,ethsys.yaml#
arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: ethernet@15100000: Unevaluated properties are not allowed ('#reset-cells' was unexpected)
	from schema $id: http://devicetree.org/schemas/net/mediatek,net.yaml#

so i thought it is a good idea to fix this now ;)

but basicly not related to my dts as these are inherited, so i can drop binding-changes...

regards Frank

> Best regards,
> Krzysztof


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

* Aw: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
  2024-05-06  8:18   ` Krzysztof Kozlowski
  2024-05-06  8:59     ` Daniel Golle
@ 2024-05-06 17:10     ` Frank Wunderlich
  2024-05-07  6:10       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-06 17:10 UTC (permalink / raw
  To: Krzysztof Kozlowski
  Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno, Eric Woudstra,
	Tianling Shen, devicetree, linux-kernel, linux-clk, linux-leds,
	netdev, linux-arm-kernel, linux-mediatek

Hi

> Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr
> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> An: "Frank Wunderlich" <linux@fw-web.de>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>, "Michael Turquette" <mturquette@baylibre.com>, "Stephen Boyd" <sboyd@kernel.org>, "Pavel Machek" <pavel@ucw.cz>, "Lee Jones" <lee@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
> Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Eric Woudstra" <ericwouds@gmail.com>, "Tianling Shen" <cnsztl@immortalwrt.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-leds@vger.kernel.org, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org
> Betreff: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
>
> On 05/05/2024 18:45, Frank Wunderlich wrote:
> > From: Frank Wunderlich <frank-w@public-files.de>
> >
> > Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
> > binding.
> >
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > ---
> >  Documentation/devicetree/bindings/leds/common.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
> > index 8a3c2398b10c..bf9a101e4d42 100644
> > --- a/Documentation/devicetree/bindings/leds/common.yaml
> > +++ b/Documentation/devicetree/bindings/leds/common.yaml
> > @@ -113,6 +113,8 @@ properties:
> >              # LED indicates NAND memory activity (deprecated),
> >              # in new implementations use "mtd"
> >            - nand-disk
> > +            # LED indicates network activity
> > +          - netdev
>
> "dev" is redundant (there is no flash-dev or usb-host-dev). Two network
> interfaces are already provided, so your commit msg must provide
> rationale why this is not enough and why this is useful/needed.

i only see 1 network binding...and this is labeled/described with wlan and phy

        # LED is triggered by WLAN activity
      - pattern: "^phy[0-9]+tx$"

which second do you mean?

btw. usb + disk has 3 trigger and "netdev" is already used in some dts, so i thought adding the binding is a good idea

arch/arm/boot/dts/rockchip/rk3128-xpi-3128.dts:107:			 * linux,default-trigger = "netdev";
arch/arm/boot/dts/nxp/imx/imx53-m53menlo.dts:52:			linux,default-trigger = "netdev";
arch/arm/boot/dts/intel/ixp/intel-ixp42x-dlink-dsm-g600.dts:51:			linux,default-trigger = "netdev";
arch/arm/boot/dts/intel/ixp/intel-ixp42x-iomega-nas100d.dts:39:			linux,default-trigger = "netdev";
arch/arm/boot/dts/ti/omap/am5729-beagleboneai.dts:138:			linux,default-trigger = "netdev";
arch/arm/boot/dts/ti/omap/am335x-netcan-plus-1xx.dts:27:			linux,default-trigger = "netdev";
arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:107:			linux,default-trigger = "netdev";
arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:113:			linux,default-trigger = "netdev";

first one has it as comment that not yet in binding and with not that it needs to be set in userspace
like Daniel stated in the dts patch (does not work for phys)...so i drop this patch and the property
in the dts.

> Best regards,
> Krzysztof

regards Frank

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

* Re: [RFC v1 0/5] Add Bananapi R3 Mini
  2024-05-05 16:45 [RFC v1 0/5] Add Bananapi R3 Mini Frank Wunderlich
                   ` (4 preceding siblings ...)
  2024-05-05 16:45 ` [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi " Frank Wunderlich
@ 2024-05-06 20:40 ` Rob Herring (Arm)
  5 siblings, 0 replies; 30+ messages in thread
From: Rob Herring (Arm) @ 2024-05-06 20:40 UTC (permalink / raw
  To: Frank Wunderlich
  Cc: Michael Turquette, Tianling Shen, linux-arm-kernel,
	Matthias Brugger, Pavel Machek, linux-clk, Paolo Abeni,
	AngeloGioacchino Del Regno, devicetree, Stephen Boyd,
	linux-kernel, Eric Dumazet, linux-mediatek, linux-leds,
	David S. Miller, Lee Jones, Conor Dooley, Frank Wunderlich,
	Krzysztof Kozlowski, netdev, Jakub Kicinski, Eric Woudstra


On Sun, 05 May 2024 18:45:44 +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add mt7986 based BananaPi R3 Mini SBC and fix some related binding errors.
> 
> Frank Wunderlich (5):
>   dt-bindings: leds: add led trigger netdev
>   dt-bindings: clock: mediatek: add address-cells and size-cells to
>     ethsys
>   dt-bindings: net: mediatek,net: add reset-cells
>   dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini
>   arm64: dts: mediatek: Add  mt7986 based Bananapi R3 Mini
> 
>  .../devicetree/bindings/arm/mediatek.yaml     |   1 +
>  .../bindings/clock/mediatek,ethsys.yaml       |   6 +
>  .../devicetree/bindings/leds/common.yaml      |   2 +
>  .../devicetree/bindings/net/mediatek,net.yaml |   3 +
>  arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>  .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>  6 files changed, 499 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
> 
> --
> 2.34.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y mediatek/mt7986a-bananapi-bpi-r3-mini.dtb' for 20240505164549.65644-1-linux@fw-web.de:

arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: crypto@10320000: interrupts: [[0, 116, 4], [0, 117, 4], [0, 118, 4], [0, 119, 4]] is too short
	from schema $id: http://devicetree.org/schemas/crypto/inside-secure,safexcel.yaml#
arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: crypto@10320000: interrupt-names: ['ring0', 'ring1', 'ring2', 'ring3'] is too short
	from schema $id: http://devicetree.org/schemas/crypto/inside-secure,safexcel.yaml#






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

* Re: Aw: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
  2024-05-06 17:10     ` Aw: " Frank Wunderlich
@ 2024-05-07  6:10       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-07  6:10 UTC (permalink / raw
  To: Frank Wunderlich
  Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, AngeloGioacchino Del Regno, Eric Woudstra,
	Tianling Shen, devicetree, linux-kernel, linux-clk, linux-leds,
	netdev, linux-arm-kernel, linux-mediatek

On 06/05/2024 19:10, Frank Wunderlich wrote:
> Hi
> 
>> Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr
>> Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
>> An: "Frank Wunderlich" <linux@fw-web.de>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Conor Dooley" <conor+dt@kernel.org>, "Michael Turquette" <mturquette@baylibre.com>, "Stephen Boyd" <sboyd@kernel.org>, "Pavel Machek" <pavel@ucw.cz>, "Lee Jones" <lee@kernel.org>, "David S. Miller" <davem@davemloft.net>, "Eric Dumazet" <edumazet@google.com>, "Jakub Kicinski" <kuba@kernel.org>, "Paolo Abeni" <pabeni@redhat.com>, "Matthias Brugger" <matthias.bgg@gmail.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>> Cc: "Frank Wunderlich" <frank-w@public-files.de>, "Eric Woudstra" <ericwouds@gmail.com>, "Tianling Shen" <cnsztl@immortalwrt.org>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-leds@vger.kernel.org, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org
>> Betreff: Re: [RFC v1 1/5] dt-bindings: leds: add led trigger netdev
>>
>> On 05/05/2024 18:45, Frank Wunderlich wrote:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Add led trigger implemented with config-symbol LEDS_TRIGGER_NETDEV to
>>> binding.
>>>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>> ---
>>>  Documentation/devicetree/bindings/leds/common.yaml | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
>>> index 8a3c2398b10c..bf9a101e4d42 100644
>>> --- a/Documentation/devicetree/bindings/leds/common.yaml
>>> +++ b/Documentation/devicetree/bindings/leds/common.yaml
>>> @@ -113,6 +113,8 @@ properties:
>>>              # LED indicates NAND memory activity (deprecated),
>>>              # in new implementations use "mtd"
>>>            - nand-disk
>>> +            # LED indicates network activity
>>> +          - netdev
>>
>> "dev" is redundant (there is no flash-dev or usb-host-dev). Two network
>> interfaces are already provided, so your commit msg must provide
>> rationale why this is not enough and why this is useful/needed.
> 
> i only see 1 network binding...and this is labeled/described with wlan and phy
> 
>         # LED is triggered by WLAN activity
>       - pattern: "^phy[0-9]+tx$"
> 
> which second do you mean?
> 
> btw. usb + disk has 3 trigger and "netdev" is already used in some dts, so i thought adding the binding is a good idea
> 
> arch/arm/boot/dts/rockchip/rk3128-xpi-3128.dts:107:			 * linux,default-trigger = "netdev";
> arch/arm/boot/dts/nxp/imx/imx53-m53menlo.dts:52:			linux,default-trigger = "netdev";
> arch/arm/boot/dts/intel/ixp/intel-ixp42x-dlink-dsm-g600.dts:51:			linux,default-trigger = "netdev";
> arch/arm/boot/dts/intel/ixp/intel-ixp42x-iomega-nas100d.dts:39:			linux,default-trigger = "netdev";
> arch/arm/boot/dts/ti/omap/am5729-beagleboneai.dts:138:			linux,default-trigger = "netdev";
> arch/arm/boot/dts/ti/omap/am335x-netcan-plus-1xx.dts:27:			linux,default-trigger = "netdev";
> arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:107:			linux,default-trigger = "netdev";
> arch/mips/boot/dts/ralink/gardena_smart_gateway_mt7688.dts:113:			linux,default-trigger = "netdev";
> 

Then please check previous discussions:
https://lore.kernel.org/all/20230217230346.GA2217008-robh@kernel.org/

Best regards,
Krzysztof


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

* Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-06 16:00     ` Frank Wunderlich
  2024-05-06 16:12       ` Daniel Golle
@ 2024-05-07 13:35       ` AngeloGioacchino Del Regno
  2024-05-08 18:25         ` Aw: " Frank Wunderlich
  1 sibling, 1 reply; 30+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-05-07 13:35 UTC (permalink / raw
  To: frank-w, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Michael Turquette, Stephen Boyd, Pavel Machek,
	Lee Jones, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Matthias Brugger
  Cc: Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek,
	Tianling Shen

Il 06/05/24 18:00, Frank Wunderlich ha scritto:
> Hi
> 
> Thanks for review.
> 
> Am 6. Mai 2024 14:48:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 05/05/24 18:45, Frank Wunderlich ha scritto:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Add device Tree for Bananapi R3 Mini SBC.
>>>
>>> Co-developed-by: Eric Woudstra <ericwouds@gmail.com>
>>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>>> Co-developed-by: Tianling Shen <cnsztl@gmail.com>
>>> Signed-off-by: Tianling Shen <cnsztl@gmail.com>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>> ---
>>>    arch/arm64/boot/dts/mediatek/Makefile         |   1 +
>>>    .../mediatek/mt7986a-bananapi-bpi-r3-mini.dts | 486 ++++++++++++++++++
>>>    2 files changed, 487 insertions(+)
>>>    create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
>>> index 37b4ca3a87c9..1763b001ab06 100644
>>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>>> @@ -11,6 +11,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
>>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-mini.dtb
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
>>>    dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>>> new file mode 100644
>>> index 000000000000..c764b4dc4752
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dts
>>> @@ -0,0 +1,486 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Copyright (C) 2021 MediaTek Inc.
>>> + * Authors: Frank Wunderlich <frank-w@public-files.de>
>>> + *          Eric Woudstra <ericwouds@gmail.com>
>>> + *          Tianling Shen <cnsztl@immortalwrt.org>
>>> + */
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/input/input.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +#include <dt-bindings/pinctrl/mt65xx.h>
>>> +
>>> +#include "mt7986a.dtsi"
>>> +
>>> +/ {
>>> +	model = "Bananapi BPI-R3 Mini";
>>> +	chassis-type = "embedded";
>>> +	compatible = "bananapi,bpi-r3mini", "mediatek,mt7986a";
>>> +
>>> +	aliases {
>>> +		serial0 = &uart0;
>>> +		ethernet0 = &gmac0;
>>> +		ethernet1 = &gmac1;
>>> +	};
>>> +
>>> +	chosen {
>>> +		stdout-path = "serial0:115200n8";
>>> +	};
>>> +
>>> +	dcin: regulator-12vd {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "12vd";
>>> +		regulator-min-microvolt = <12000000>;
>>> +		regulator-max-microvolt = <12000000>;
>>> +		regulator-boot-on;
>>> +		regulator-always-on;
>>> +	};
>>> +
>>> +	fan: pwm-fan {
>>> +		compatible = "pwm-fan";
>>> +		#cooling-cells = <2>;
>>> +		/* cooling level (0, 1, 2) - pwm inverted */
>>> +		cooling-levels = <255 96 0>;
>>
>> Did you try to actually invert the PWM?
>>
>> Look for PWM_POLARITY_INVERTED ;-)
> 
> Mtk pwm driver does not support it
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
> 

You're right, sorry - I confused the general purpose PWM controller with the
rather specific DISP_PWM controller (which does support polarity inversion).

It's good - but I'd appreciate if you can please add a comment stating that
the PWM values are inverted in SW because the controller does *not* support
polarity inversion... so that next time someone looks at this will immediately
understand what's going on and why :-)

>>> +		pwms = <&pwm 0 10000>;
>>> +		status = "okay";
>>> +	};
>>> +
>>> +	reg_1p8v: regulator-1p8v {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "1.8vd";
>>> +		regulator-min-microvolt = <1800000>;
>>> +		regulator-max-microvolt = <1800000>;
>>> +		regulator-boot-on;
>>> +		regulator-always-on;
>>> +		vin-supply = <&dcin>;
>>> +	};
>>> +
>>> +	reg_3p3v: regulator-3p3v {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "3.3vd";
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +		regulator-boot-on;
>>> +		regulator-always-on;
>>> +		vin-supply = <&dcin>;
>>> +	};
>>> +
>>> +	usb_vbus: regulator-usb-vbus {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "usb_vbus";
>>> +		regulator-min-microvolt = <5000000>;
>>> +		regulator-max-microvolt = <5000000>;
>>> +		gpios = <&pio 20 GPIO_ACTIVE_LOW>;
>>> +		regulator-boot-on;
>>> +	};
>>> +
>>> +	en8811_a: regulator-phy1 {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "phy1";
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +		gpio = <&pio 16 GPIO_ACTIVE_LOW>;
>>> +		regulator-always-on;
>>> +	};
>>> +
>>> +	en8811_b: regulator-phy2 {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "phy2";
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +		gpio = <&pio 17 GPIO_ACTIVE_LOW>;
>>> +		regulator-always-on;
>>> +	};
>>> +
>>> +	leds {
>>> +		compatible = "gpio-leds";
>>> +
>>> +		green_led: led-0 {
>>> +			color = <LED_COLOR_ID_GREEN>;
>>> +			function = LED_FUNCTION_POWER;
>>> +			gpios = <&pio 19 GPIO_ACTIVE_HIGH>;
>>> +			default-state = "on";
>>> +		};
>>> +	};
>>> +
>>> +	gpio-keys {
>>> +		compatible = "gpio-keys";
>>> +
>>> +		reset-key {
>>> +			label = "reset";
>>> +			linux,code = <KEY_RESTART>;
>>> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
>>> +		};
>>> +	};
>>> +
>>> +};
>>> +
>>> +&cpu_thermal {
>>> +	cooling-maps {
>>> +		map0 {
>>> +			/* active: set fan to cooling level 2 */
>>> +			cooling-device = <&fan 2 2>;
>>> +			trip = <&cpu_trip_active_high>;
>>> +		};
>>> +
>>> +		map1 {
>>> +			/* active: set fan to cooling level 1 */
>>> +			cooling-device = <&fan 1 1>;
>>> +			trip = <&cpu_trip_active_med>;
>>> +		};
>>> +
>>> +		map2 {
>>> +			/* active: set fan to cooling level 0 */
>>> +			cooling-device = <&fan 0 0>;
>>> +			trip = <&cpu_trip_active_low>;
>>> +		};
>>> +	};
>>> +};
>>> +
>>> +&crypto {
>>> +	status = "okay";
>>> +};
>>> +
>>> +&eth {
>>> +	status = "okay";
>>> +
>>> +	gmac0: mac@0 {
>>> +		compatible = "mediatek,eth-mac";
>>> +		reg = <0>;
>>> +		phy-mode = "2500base-x";
>>> +		phy-handle = <&phy14>;
>>> +	};
>>> +
>>> +	gmac1: mac@1 {
>>> +		compatible = "mediatek,eth-mac";
>>> +		reg = <1>;
>>> +		phy-mode = "2500base-x";
>>> +		phy-handle = <&phy15>;
>>> +	};
>>> +
>>> +	mdio: mdio-bus {
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +	};
>>> +};
>>> +
>>> +&mmc0 {
>>> +	pinctrl-names = "default", "state_uhs";
>>> +	pinctrl-0 = <&mmc0_pins_default>;
>>> +	pinctrl-1 = <&mmc0_pins_uhs>;
>>> +	vmmc-supply = <&reg_3p3v>;
>>> +	vqmmc-supply = <&reg_1p8v>;
>>> +};
>>> +
>>> +
>>> +&i2c0 {
>>> +	pinctrl-names = "default";
>>> +	pinctrl-0 = <&i2c_pins>;
>>> +	status = "okay";
>>> +
>>> +	/* MAC Address EEPROM */
>>> +	eeprom@50 {
>>> +		compatible = "atmel,24c02";
>>> +		reg = <0x50>;
>>> +
>>> +		address-width = <8>;
>>> +		pagesize = <8>;
>>> +		size = <256>;
>>> +	};
>>> +};
>>> +
>>> +&mdio {
>>
>> You can just move all this stuff to where you declare the mdio-bus....
> 
> Ok,see these 2 lines are already above,so can be dropped here.
> 
>>> +	#address-cells = <1>;
>>> +	#size-cells = <0>;
>>> +
>>> +	phy14: ethernet-phy@14 {
>>
>> I say that this is `phy0: ethernet-phy@14` - because this is the first PHY on this
>> board.
> 
> Ok,i change this and phy15
> 
>>> +		reg = <14>;
>>
>> Uhm.. doesn't this require the ethernet-phy-id03a2.a411 compatible?
> 
> I can add it,but worked without it.
> 
> There was a discussion about that and result was we don't need it in board dts,maybe add compatible in binding example.
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20240206194751.1901802-2-ericwouds@gmail.com/#25703356
> 

Ah, okay. Leave it then.

>>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>>> +		reset-assert-us = <10000>;
>>> +		reset-deassert-us = <20000>;
>>> +		phy-mode = "2500base-x";
>>> +		full-duplex;
>>> +		pause;
>>> +		airoha,pnswap-rx;
>>> +
>>> +		leds {
>>> +			#address-cells = <1>;
>>> +			#size-cells = <0>;
>>> +
>>> +			led@0 { /* en8811_a_gpio5 */
>>> +				reg = <0>;
>>> +				color = <LED_COLOR_ID_YELLOW>;
>>> +				function = LED_FUNCTION_LAN;
>>> +				function-enumerator = <1>;
>>
>> Why aren't you simply using a label?
> 
> You mean the comment? I can add it of course like for regulators.
> 

I mean in place of the function-enumerator... that's practically used to
distinguish between instances, it's not too common to see it, and usually
"label" replaces exactly that - just that, instead of a different number,
it gets a different name with no (usually) meaningless numbers :-)

Cheers!

>>> +				default-state = "keep";
>>> +				linux,default-trigger = "netdev";
>>> +			};
>>> +			led@1 { /* en8811_a_gpio4 */
>>> +				reg = <1>;
>>> +				color = <LED_COLOR_ID_GREEN>;
>>> +				function = LED_FUNCTION_LAN;
>>> +				function-enumerator = <2>;
>>> +				default-state = "keep";
>>> +				linux,default-trigger = "netdev";
>>> +			};
>>> +		};
>>> +	};
>>> +
>>> +	phy15: ethernet-phy@15 {
>>> +		reg = <15>;
>>
>> Same here.
>>
>> Cheers,
>> Angelo
>>
> 
> 
> regards Frank


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

* Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys
  2024-05-06 16:01     ` Frank Wunderlich
@ 2024-05-07 13:36       ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 30+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-05-07 13:36 UTC (permalink / raw
  To: frank-w, Frank Wunderlich, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Michael Turquette, Stephen Boyd, Pavel Machek,
	Lee Jones, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Matthias Brugger
  Cc: Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek

Il 06/05/24 18:01, Frank Wunderlich ha scritto:
> Am 6. Mai 2024 14:48:58 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 05/05/24 18:45, Frank Wunderlich ha scritto:
>>> From: Frank Wunderlich <frank-w@public-files.de>
>>>
>>> Add missing properties already used in mt7986a.dtsi.
>>>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>
>> Fixes tag? :-)
>>
>> Cheers,
>> Angelo
> 
> Should i use fixes tag of binding commit or where dts (-part) was added?
> regards Frank

You're fixing the binding, so, the binding one :-)

Cheers,
Angelo

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

* Re: Aw: Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys
  2024-05-06 16:51     ` Aw: " Frank Wunderlich
@ 2024-05-07 19:35       ` Rob Herring
  2024-05-07 21:22         ` Aw: " Frank Wunderlich
  0 siblings, 1 reply; 30+ messages in thread
From: Rob Herring @ 2024-05-07 19:35 UTC (permalink / raw
  To: Frank Wunderlich
  Cc: Krzysztof Kozlowski, Frank Wunderlich, Krzysztof Kozlowski,
	Conor Dooley, Michael Turquette, Stephen Boyd, Pavel Machek,
	Lee Jones, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
	Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek

On Mon, May 06, 2024 at 06:51:43PM +0200, Frank Wunderlich wrote:
> > Gesendet: Montag, 06. Mai 2024 um 10:18 Uhr
> > Von: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> > On 05/05/2024 18:45, Frank Wunderlich wrote:
> > > From: Frank Wunderlich <frank-w@public-files.de>
> > >
> > > Add missing properties already used in mt7986a.dtsi.
> >
> > Missing for what? Or why? Provide context, IOW, explain why they are
> > missing.
> 
> ethernet-node in mt7986a.dtsi hast reset-cells-property
> 
> https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm64/boot/dts/mediatek/mt7986a.dtsi#L559
> 
> and
> 
> address-cells and size-cells are used here:
> 
> https://elixir.bootlin.com/linux/v6.9-rc1/source/arch/arm64/boot/dts/mediatek/mt7986a.dtsi#L495
> 
> i saw the warnings while checking my r3mini dts...
> 
> arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: syscon@15000000: '#address-cells', '#size-cells' do not match any of the regexes: 'pinctrl-[0-9]+'
> 	from schema $id: http://devicetree.org/schemas/clock/mediatek,ethsys.yaml#
> arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-mini.dtb: ethernet@15100000: Unevaluated properties are not allowed ('#reset-cells' was unexpected)
> 	from schema $id: http://devicetree.org/schemas/net/mediatek,net.yaml#
> 
> so i thought it is a good idea to fix this now ;)

The dts is already fixed dropping these properties in linux-next.

If you don't have child nodes with reg/ranges, then you never need 
#address-cells or #size-cells.

Rob

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

* Re: [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells
  2024-05-05 16:45 ` [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells Frank Wunderlich
  2024-05-06  8:19   ` Krzysztof Kozlowski
@ 2024-05-07 19:37   ` Rob Herring
  1 sibling, 0 replies; 30+ messages in thread
From: Rob Herring @ 2024-05-07 19:37 UTC (permalink / raw
  To: Frank Wunderlich
  Cc: Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno, Frank Wunderlich, Eric Woudstra,
	Tianling Shen, devicetree, linux-kernel, linux-clk, linux-leds,
	netdev, linux-arm-kernel, linux-mediatek

On Sun, May 05, 2024 at 06:45:47PM +0200, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Add missing binding for property used in mt7986a.dtsi.
> 
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> ---
>  Documentation/devicetree/bindings/net/mediatek,net.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
> index e74502a0afe8..5214927c0fe8 100644
> --- a/Documentation/devicetree/bindings/net/mediatek,net.yaml
> +++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
> @@ -101,6 +101,9 @@ properties:
>    "#address-cells":
>      const: 1
>  
> +  "#reset-cells":
> +    const: 1

Also fixed already.

Rob

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

* Aw: Re:  Re: [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys
  2024-05-07 19:35       ` Rob Herring
@ 2024-05-07 21:22         ` Frank Wunderlich
  0 siblings, 0 replies; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-07 21:22 UTC (permalink / raw
  To: Rob Herring
  Cc: Krzysztof Kozlowski, Frank Wunderlich, Krzysztof Kozlowski,
	Conor Dooley, Michael Turquette, Stephen Boyd, Pavel Machek,
	Lee Jones, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno,
	Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek

Hi

> Gesendet: Dienstag, 07. Mai 2024 um 21:35 Uhr
> Von: "Rob Herring" <robh@kernel.org>

> The dts is already fixed dropping these properties in linux-next.
>
> If you don't have child nodes with reg/ranges, then you never need
> #address-cells or #size-cells.

thx for pointing to this,

so i only need part 4+5 when 6.10-rc1 is out (as i drop netdev trigger from dts in v2).

any comments for these?

regards Frank

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

* Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-07 13:35       ` AngeloGioacchino Del Regno
@ 2024-05-08 18:25         ` Frank Wunderlich
  2024-05-09 10:10           ` AngeloGioacchino Del Regno
  0 siblings, 1 reply; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-08 18:25 UTC (permalink / raw
  To: AngeloGioacchino Del Regno
  Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek, Tianling Shen

Hi

> Gesendet: Dienstag, 07. Mai 2024 um 15:35 Uhr
> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>
> Il 06/05/24 18:00, Frank Wunderlich ha scritto:

> >>> +	fan: pwm-fan {
> >>> +		compatible = "pwm-fan";
> >>> +		#cooling-cells = <2>;
> >>> +		/* cooling level (0, 1, 2) - pwm inverted */
> >>> +		cooling-levels = <255 96 0>;
> >>
> >> Did you try to actually invert the PWM?
> >>
> >> Look for PWM_POLARITY_INVERTED ;-)
> >
> > Mtk pwm driver does not support it
> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
> >
>
> You're right, sorry - I confused the general purpose PWM controller with the
> rather specific DISP_PWM controller (which does support polarity inversion).
>
> It's good - but I'd appreciate if you can please add a comment stating that
> the PWM values are inverted in SW because the controller does *not* support
> polarity inversion... so that next time someone looks at this will immediately
> understand what's going on and why :-)

so i would change comment like this:

		/* cooling level (0, 1, 2)
		 * signal is inverted on board
		 * mtk pwm driver does not support
		 * PWM_POLARITY_INVERTED */

> >>> +		pwms = <&pwm 0 10000>;
> >>> +		status = "okay";
> >>> +	};
> >>> +
> >>> +	phy14: ethernet-phy@14 {
...
> >>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
> >>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
> >>> +		reset-assert-us = <10000>;
> >>> +		reset-deassert-us = <20000>;
> >>> +		phy-mode = "2500base-x";
> >>> +		full-duplex;
> >>> +		pause;
> >>> +		airoha,pnswap-rx;
> >>> +
> >>> +		leds {
> >>> +			#address-cells = <1>;
> >>> +			#size-cells = <0>;
> >>> +
> >>> +			led@0 { /* en8811_a_gpio5 */
> >>> +				reg = <0>;
> >>> +				color = <LED_COLOR_ID_YELLOW>;
> >>> +				function = LED_FUNCTION_LAN;
> >>> +				function-enumerator = <1>;
> >>
> >> Why aren't you simply using a label?
> >
> > You mean the comment? I can add it of course like for regulators.
> >
>
> I mean in place of the function-enumerator... that's practically used to
> distinguish between instances, it's not too common to see it, and usually
> "label" replaces exactly that - just that, instead of a different number,
> it gets a different name with no (usually) meaningless numbers :-)

as far as i understand using label also makes "function" property useless, after discussing
this with eric i would drop both on all 4 places by labels like these:

label = "yellow-lan";
label = "green-lan";
...

not sure if we should drop color property too...

> >>> +				default-state = "keep";
> >>> +				linux,default-trigger = "netdev";
> >>> +			};
> >>> +			led@1 { /* en8811_a_gpio4 */
> >>> +				reg = <1>;
> >>> +				color = <LED_COLOR_ID_GREEN>;
> >>> +				function = LED_FUNCTION_LAN;
> >>> +				function-enumerator = <2>;
> >>> +				default-state = "keep";
> >>> +				linux,default-trigger = "netdev";
> >>> +			};
> >>> +		};
> >>> +	};
> >>> +
> >>> +	phy15: ethernet-phy@15 {
> >>> +		reg = <15>;
> >>
> >> Same here.
> >>
> >> Cheers,
> >> Angelo

regards Frank

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

* Re: Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-08 18:25         ` Aw: " Frank Wunderlich
@ 2024-05-09 10:10           ` AngeloGioacchino Del Regno
  2024-05-09 10:30             ` Frank Wunderlich
  0 siblings, 1 reply; 30+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-05-09 10:10 UTC (permalink / raw
  To: Frank Wunderlich
  Cc: Frank Wunderlich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Michael Turquette, Stephen Boyd, Pavel Machek, Lee Jones,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Matthias Brugger, Eric Woudstra, Tianling Shen, devicetree,
	linux-kernel, linux-clk, linux-leds, netdev, linux-arm-kernel,
	linux-mediatek, Tianling Shen

Il 08/05/24 20:25, Frank Wunderlich ha scritto:
> Hi
> 
>> Gesendet: Dienstag, 07. Mai 2024 um 15:35 Uhr
>> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>>
>> Il 06/05/24 18:00, Frank Wunderlich ha scritto:
> 
>>>>> +	fan: pwm-fan {
>>>>> +		compatible = "pwm-fan";
>>>>> +		#cooling-cells = <2>;
>>>>> +		/* cooling level (0, 1, 2) - pwm inverted */
>>>>> +		cooling-levels = <255 96 0>;
>>>>
>>>> Did you try to actually invert the PWM?
>>>>
>>>> Look for PWM_POLARITY_INVERTED ;-)
>>>
>>> Mtk pwm driver does not support it
>>>
>>> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
>>>
>>
>> You're right, sorry - I confused the general purpose PWM controller with the
>> rather specific DISP_PWM controller (which does support polarity inversion).
>>
>> It's good - but I'd appreciate if you can please add a comment stating that
>> the PWM values are inverted in SW because the controller does *not* support
>> polarity inversion... so that next time someone looks at this will immediately
>> understand what's going on and why :-)
> 
> so i would change comment like this:
> 
> 		/* cooling level (0, 1, 2)
> 		 * signal is inverted on board
> 		 * mtk pwm driver does not support
> 		 * PWM_POLARITY_INVERTED */
> 

There you go:

/*
  * The signal is inverted on this board and the general purpose
  * PWM HW IP in this SoC does not support polarity inversion.
  */
/* Cooling level < 0  1  2> */
cooling-levels = <255 96 0>;



>>>>> +		pwms = <&pwm 0 10000>;
>>>>> +		status = "okay";
>>>>> +	};
>>>>> +
>>>>> +	phy14: ethernet-phy@14 {
> ...
>>>>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>>>>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>>>>> +		reset-assert-us = <10000>;
>>>>> +		reset-deassert-us = <20000>;
>>>>> +		phy-mode = "2500base-x";
>>>>> +		full-duplex;
>>>>> +		pause;
>>>>> +		airoha,pnswap-rx;
>>>>> +
>>>>> +		leds {
>>>>> +			#address-cells = <1>;
>>>>> +			#size-cells = <0>;
>>>>> +
>>>>> +			led@0 { /* en8811_a_gpio5 */
>>>>> +				reg = <0>;
>>>>> +				color = <LED_COLOR_ID_YELLOW>;
>>>>> +				function = LED_FUNCTION_LAN;
>>>>> +				function-enumerator = <1>;
>>>>
>>>> Why aren't you simply using a label?
>>>
>>> You mean the comment? I can add it of course like for regulators.
>>>
>>
>> I mean in place of the function-enumerator... that's practically used to
>> distinguish between instances, it's not too common to see it, and usually
>> "label" replaces exactly that - just that, instead of a different number,
>> it gets a different name with no (usually) meaningless numbers :-)
> 
> as far as i understand using label also makes "function" property useless, after discussing
> this with eric i would drop both on all 4 places by labels like these:
> 
> label = "yellow-lan";
> label = "green-lan";
> ...
> 
> not sure if we should drop color property too...
> 

I'm looking at the leds binding (leds/common.yaml) right now.

My suggestion of using 'label' was actually wrong - and your devicetree was
actually right!!! (apart from the default-trigger that may not work)

Infact, the documentation says, in brief:

- function-enumerator is ignored if label is present
- function doesn't say that gets ignored
- color doesn't say that gets ignored
- label says:
   - If not present -> get string from node name
   - function-enumerator ignored
   - This property is deprecated

...but the 'label' binding does not say 'deprecated: true', which is something
that must be fixed!


So, I'm sorry for the confusion, the noise and the useless loss of time around
this - you can keep the LED nodes as they are, and that's a lesson for the future
me reviewing another node like this one.

P.S.: This shouldn't have been a RFC, as the patches are more than RFC quality!!!

Cheers,
Angelo

>>>>> +				default-state = "keep";
>>>>> +				linux,default-trigger = "netdev";
>>>>> +			};
>>>>> +			led@1 { /* en8811_a_gpio4 */
>>>>> +				reg = <1>;
>>>>> +				color = <LED_COLOR_ID_GREEN>;
>>>>> +				function = LED_FUNCTION_LAN;
>>>>> +				function-enumerator = <2>;
>>>>> +				default-state = "keep";
>>>>> +				linux,default-trigger = "netdev";
>>>>> +			};
>>>>> +		};
>>>>> +	};
>>>>> +
>>>>> +	phy15: ethernet-phy@15 {
>>>>> +		reg = <15>;
>>>>
>>>> Same here.
>>>>
>>>> Cheers,
>>>> Angelo
> 
> regards Frank


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

* Re: Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-09 10:10           ` AngeloGioacchino Del Regno
@ 2024-05-09 10:30             ` Frank Wunderlich
  2024-05-09 10:35               ` AngeloGioacchino Del Regno
  0 siblings, 1 reply; 30+ messages in thread
From: Frank Wunderlich @ 2024-05-09 10:30 UTC (permalink / raw
  To: AngeloGioacchino Del Regno, Frank Wunderlich
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek,
	Tianling Shen

Am 9. Mai 2024 12:10:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>Il 08/05/24 20:25, Frank Wunderlich ha scritto:
>> Hi
>> 
>>> Gesendet: Dienstag, 07. Mai 2024 um 15:35 Uhr
>>> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>>> 
>>> Il 06/05/24 18:00, Frank Wunderlich ha scritto:
>> 
>>>>>> +	fan: pwm-fan {
>>>>>> +		compatible = "pwm-fan";
>>>>>> +		#cooling-cells = <2>;
>>>>>> +		/* cooling level (0, 1, 2) - pwm inverted */
>>>>>> +		cooling-levels = <255 96 0>;
>>>>> 
>>>>> Did you try to actually invert the PWM?
>>>>> 
>>>>> Look for PWM_POLARITY_INVERTED ;-)
>>>> 
>>>> Mtk pwm driver does not support it
>>>> 
>>>> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
>>>> 
>>> 
>>> You're right, sorry - I confused the general purpose PWM controller with the
>>> rather specific DISP_PWM controller (which does support polarity inversion).
>>> 
>>> It's good - but I'd appreciate if you can please add a comment stating that
>>> the PWM values are inverted in SW because the controller does *not* support
>>> polarity inversion... so that next time someone looks at this will immediately
>>> understand what's going on and why :-)
>> 
>> so i would change comment like this:
>> 
>> 		/* cooling level (0, 1, 2)
>> 		 * signal is inverted on board
>> 		 * mtk pwm driver does not support
>> 		 * PWM_POLARITY_INVERTED */
>> 
>
>There you go:
>
>/*
> * The signal is inverted on this board and the general purpose
> * PWM HW IP in this SoC does not support polarity inversion.
> */
>/* Cooling level < 0  1  2> */
>cooling-levels = <255 96 0>;

Thanks for clearing structure of the comment,but imho actually it is a driver issue (for all mtk SoC). Not sure it is really a hardware limitation. So i would change this to "... and the PWM driver does not support polarity inversion."

>>>>>> +		pwms = <&pwm 0 10000>;
>>>>>> +		status = "okay";
>>>>>> +	};
>>>>>> +
>>>>>> +	phy14: ethernet-phy@14 {
>> ...
>>>>>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>>>>>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>>>>>> +		reset-assert-us = <10000>;
>>>>>> +		reset-deassert-us = <20000>;
>>>>>> +		phy-mode = "2500base-x";
>>>>>> +		full-duplex;
>>>>>> +		pause;
>>>>>> +		airoha,pnswap-rx;
>>>>>> +
>>>>>> +		leds {
>>>>>> +			#address-cells = <1>;
>>>>>> +			#size-cells = <0>;
>>>>>> +
>>>>>> +			led@0 { /* en8811_a_gpio5 */
>>>>>> +				reg = <0>;
>>>>>> +				color = <LED_COLOR_ID_YELLOW>;
>>>>>> +				function = LED_FUNCTION_LAN;
>>>>>> +				function-enumerator = <1>;
>>>>> 
>>>>> Why aren't you simply using a label?
>>>> 
>>>> You mean the comment? I can add it of course like for regulators.
>>>> 
>>> 
>>> I mean in place of the function-enumerator... that's practically used to
>>> distinguish between instances, it's not too common to see it, and usually
>>> "label" replaces exactly that - just that, instead of a different number,
>>> it gets a different name with no (usually) meaningless numbers :-)
>> 
>> as far as i understand using label also makes "function" property useless, after discussing
>> this with eric i would drop both on all 4 places by labels like these:
>> 
>> label = "yellow-lan";
>> label = "green-lan";
>> ...
>> 
>> not sure if we should drop color property too...
>> 
>
>I'm looking at the leds binding (leds/common.yaml) right now.
>
>My suggestion of using 'label' was actually wrong - and your devicetree was
>actually right!!! (apart from the default-trigger that may not work)
>
>Infact, the documentation says, in brief:
>
>- function-enumerator is ignored if label is present
>- function doesn't say that gets ignored
>- color doesn't say that gets ignored
>- label says:
>  - If not present -> get string from node name
>  - function-enumerator ignored
>  - This property is deprecated
>
>...but the 'label' binding does not say 'deprecated: true', which is something
>that must be fixed!

Ok,i can try to add the property to binding (independ of this series). Imho label was cleaner than function and function-enumerator...

>So, I'm sorry for the confusion, the noise and the useless loss of time around
>this - you can keep the LED nodes as they are, and that's a lesson for the future
>me reviewing another node like this one.

Don't worry, we are all humas...i missed looking in linux-next for the other binding-patches.

>P.S.: This shouldn't have been a RFC, as the patches are more than RFC quality!!!

I sent it as RFC because i had not expected to be merged before next is closed.

>Cheers,
>Angelo
>
>>>>>> +				default-state = "keep";
>>>>>> +				linux,default-trigger = "netdev";
>>>>>> +			};
>>>>>> +			led@1 { /* en8811_a_gpio4 */
>>>>>> +				reg = <1>;
>>>>>> +				color = <LED_COLOR_ID_GREEN>;
>>>>>> +				function = LED_FUNCTION_LAN;
>>>>>> +				function-enumerator = <2>;
>>>>>> +				default-state = "keep";
>>>>>> +				linux,default-trigger = "netdev";
>>>>>> +			};
>>>>>> +		};
>>>>>> +	};
>>>>>> +
>>>>>> +	phy15: ethernet-phy@15 {
>>>>>> +		reg = <15>;


regards Frank

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

* Re: Aw: Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
  2024-05-09 10:30             ` Frank Wunderlich
@ 2024-05-09 10:35               ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 30+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-05-09 10:35 UTC (permalink / raw
  To: Frank Wunderlich, Frank Wunderlich
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Turquette,
	Stephen Boyd, Pavel Machek, Lee Jones, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	Eric Woudstra, Tianling Shen, devicetree, linux-kernel, linux-clk,
	linux-leds, netdev, linux-arm-kernel, linux-mediatek,
	Tianling Shen

Il 09/05/24 12:30, Frank Wunderlich ha scritto:
> Am 9. Mai 2024 12:10:59 MESZ schrieb AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>:
>> Il 08/05/24 20:25, Frank Wunderlich ha scritto:
>>> Hi
>>>
>>>> Gesendet: Dienstag, 07. Mai 2024 um 15:35 Uhr
>>>> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
>>>>
>>>> Il 06/05/24 18:00, Frank Wunderlich ha scritto:
>>>
>>>>>>> +	fan: pwm-fan {
>>>>>>> +		compatible = "pwm-fan";
>>>>>>> +		#cooling-cells = <2>;
>>>>>>> +		/* cooling level (0, 1, 2) - pwm inverted */
>>>>>>> +		cooling-levels = <255 96 0>;
>>>>>>
>>>>>> Did you try to actually invert the PWM?
>>>>>>
>>>>>> Look for PWM_POLARITY_INVERTED ;-)
>>>>>
>>>>> Mtk pwm driver does not support it
>>>>>
>>>>> https://elixir.bootlin.com/linux/latest/source/drivers/pwm/pwm-mediatek.c#L211
>>>>>
>>>>
>>>> You're right, sorry - I confused the general purpose PWM controller with the
>>>> rather specific DISP_PWM controller (which does support polarity inversion).
>>>>
>>>> It's good - but I'd appreciate if you can please add a comment stating that
>>>> the PWM values are inverted in SW because the controller does *not* support
>>>> polarity inversion... so that next time someone looks at this will immediately
>>>> understand what's going on and why :-)
>>>
>>> so i would change comment like this:
>>>
>>> 		/* cooling level (0, 1, 2)
>>> 		 * signal is inverted on board
>>> 		 * mtk pwm driver does not support
>>> 		 * PWM_POLARITY_INVERTED */
>>>
>>
>> There you go:
>>
>> /*
>> * The signal is inverted on this board and the general purpose
>> * PWM HW IP in this SoC does not support polarity inversion.
>> */
>> /* Cooling level < 0  1  2> */
>> cooling-levels = <255 96 0>;
> 
> Thanks for clearing structure of the comment,but imho actually it is a driver issue (for all mtk SoC). Not sure it is really a hardware limitation. So i would change this to "... and the PWM driver does not support polarity inversion."
> 
>>>>>>> +		pwms = <&pwm 0 10000>;
>>>>>>> +		status = "okay";
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	phy14: ethernet-phy@14 {
>>> ...
>>>>>>> +		interrupts-extended = <&pio 48 IRQ_TYPE_EDGE_FALLING>;
>>>>>>> +		reset-gpios = <&pio 49 GPIO_ACTIVE_LOW>;
>>>>>>> +		reset-assert-us = <10000>;
>>>>>>> +		reset-deassert-us = <20000>;
>>>>>>> +		phy-mode = "2500base-x";
>>>>>>> +		full-duplex;
>>>>>>> +		pause;
>>>>>>> +		airoha,pnswap-rx;
>>>>>>> +
>>>>>>> +		leds {
>>>>>>> +			#address-cells = <1>;
>>>>>>> +			#size-cells = <0>;
>>>>>>> +
>>>>>>> +			led@0 { /* en8811_a_gpio5 */
>>>>>>> +				reg = <0>;
>>>>>>> +				color = <LED_COLOR_ID_YELLOW>;
>>>>>>> +				function = LED_FUNCTION_LAN;
>>>>>>> +				function-enumerator = <1>;
>>>>>>
>>>>>> Why aren't you simply using a label?
>>>>>
>>>>> You mean the comment? I can add it of course like for regulators.
>>>>>
>>>>
>>>> I mean in place of the function-enumerator... that's practically used to
>>>> distinguish between instances, it's not too common to see it, and usually
>>>> "label" replaces exactly that - just that, instead of a different number,
>>>> it gets a different name with no (usually) meaningless numbers :-)
>>>
>>> as far as i understand using label also makes "function" property useless, after discussing
>>> this with eric i would drop both on all 4 places by labels like these:
>>>
>>> label = "yellow-lan";
>>> label = "green-lan";
>>> ...
>>>
>>> not sure if we should drop color property too...
>>>
>>
>> I'm looking at the leds binding (leds/common.yaml) right now.
>>
>> My suggestion of using 'label' was actually wrong - and your devicetree was
>> actually right!!! (apart from the default-trigger that may not work)
>>
>> Infact, the documentation says, in brief:
>>
>> - function-enumerator is ignored if label is present
>> - function doesn't say that gets ignored
>> - color doesn't say that gets ignored
>> - label says:
>>   - If not present -> get string from node name
>>   - function-enumerator ignored
>>   - This property is deprecated
>>
>> ...but the 'label' binding does not say 'deprecated: true', which is something
>> that must be fixed!
> 
> Ok,i can try to add the property to binding (independ of this series). Imho label was cleaner than function and function-enumerator...
> 

Oh I sort of agree with you, I liked the label more, as it's more consistent with
everything else... but oh well. :-)

>> So, I'm sorry for the confusion, the noise and the useless loss of time around
>> this - you can keep the LED nodes as they are, and that's a lesson for the future
>> me reviewing another node like this one.
> 
> Don't worry, we are all humas...i missed looking in linux-next for the other binding-patches.
> 
>> P.S.: This shouldn't have been a RFC, as the patches are more than RFC quality!!!
> 
> I sent it as RFC because i had not expected to be merged before next is closed.
> 

Ah at least from my side, no worries... when I see RFC I generally expect to see
"dubious/head-scratching stuff", not "sub-optimal timing to send a patch" :-P

Cheers!
Angelo

>>
>>>>>>> +				default-state = "keep";
>>>>>>> +				linux,default-trigger = "netdev";
>>>>>>> +			};
>>>>>>> +			led@1 { /* en8811_a_gpio4 */
>>>>>>> +				reg = <1>;
>>>>>>> +				color = <LED_COLOR_ID_GREEN>;
>>>>>>> +				function = LED_FUNCTION_LAN;
>>>>>>> +				function-enumerator = <2>;
>>>>>>> +				default-state = "keep";
>>>>>>> +				linux,default-trigger = "netdev";
>>>>>>> +			};
>>>>>>> +		};
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	phy15: ethernet-phy@15 {
>>>>>>> +		reg = <15>;
> 
> 
> regards Frank


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

end of thread, other threads:[~2024-05-09 10:35 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-05 16:45 [RFC v1 0/5] Add Bananapi R3 Mini Frank Wunderlich
2024-05-05 16:45 ` [RFC v1 1/5] dt-bindings: leds: add led trigger netdev Frank Wunderlich
2024-05-06  8:18   ` Krzysztof Kozlowski
2024-05-06  8:59     ` Daniel Golle
2024-05-06 17:10     ` Aw: " Frank Wunderlich
2024-05-07  6:10       ` Krzysztof Kozlowski
2024-05-05 16:45 ` [RFC v1 2/5] dt-bindings: clock: mediatek: add address-cells and size-cells to ethsys Frank Wunderlich
2024-05-06  8:18   ` Krzysztof Kozlowski
2024-05-06 16:51     ` Aw: " Frank Wunderlich
2024-05-07 19:35       ` Rob Herring
2024-05-07 21:22         ` Aw: " Frank Wunderlich
2024-05-06 12:48   ` AngeloGioacchino Del Regno
2024-05-06 16:01     ` Frank Wunderlich
2024-05-07 13:36       ` AngeloGioacchino Del Regno
2024-05-05 16:45 ` [RFC v1 3/5] dt-bindings: net: mediatek,net: add reset-cells Frank Wunderlich
2024-05-06  8:19   ` Krzysztof Kozlowski
2024-05-07 19:37   ` Rob Herring
2024-05-05 16:45 ` [RFC v1 4/5] dt-bindings: arm64: dts: mediatek: add BananaPi R3 Mini Frank Wunderlich
2024-05-05 16:45 ` [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi " Frank Wunderlich
2024-05-06  8:20   ` Krzysztof Kozlowski
2024-05-06 16:40     ` Aw: " Frank Wunderlich
2024-05-06 12:48   ` AngeloGioacchino Del Regno
2024-05-06 16:00     ` Frank Wunderlich
2024-05-06 16:12       ` Daniel Golle
2024-05-07 13:35       ` AngeloGioacchino Del Regno
2024-05-08 18:25         ` Aw: " Frank Wunderlich
2024-05-09 10:10           ` AngeloGioacchino Del Regno
2024-05-09 10:30             ` Frank Wunderlich
2024-05-09 10:35               ` AngeloGioacchino Del Regno
2024-05-06 20:40 ` [RFC v1 0/5] Add " Rob Herring (Arm)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).