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 >> + 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 >> + 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