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
