Re: [RFC v1 5/5] arm64: dts: mediatek: Add mt7986 based Bananapi R3 Mini
From: AngeloGioacchino Del Regno
Date: Tue May 07 2024 - 09:53:34 EST
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@xxxxxxxxxxxxx>:
Il 05/05/24 18:45, Frank Wunderlich ha scritto:
From: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
Add device Tree for Bananapi R3 Mini SBC.
Co-developed-by: Eric Woudstra <ericwouds@xxxxxxxxx>
Signed-off-by: Eric Woudstra <ericwouds@xxxxxxxxx>
Co-developed-by: Tianling Shen <cnsztl@xxxxxxxxx>
Signed-off-by: Tianling Shen <cnsztl@xxxxxxxxx>
Signed-off-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxx>
+ * Eric Woudstra <ericwouds@xxxxxxxxx>
+ * Tianling Shen <cnsztl@xxxxxxxxxxxxxxx>
+ */
+
+/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";
+};
+
+ð {
+ 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 = <®_3p3v>;
+ vqmmc-supply = <®_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@xxxxxxxxx/#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