Re: [RFC v1 12/12] arm64: dts: mt7986: add Bananapi R3

From: Krzysztof Kozlowski
Date: Tue Oct 18 2022 - 11:38:58 EST

Next message: Roberto Sassu: "Re: [PATCH 0/9] integrity: Move hooks into LSM"
Previous message: Yazen Ghannam: "[PATCH] EDAC/mc_sysfs: Increase legacy channel support to 12"
In reply to: Frank Wunderlich: "[RFC v1 12/12] arm64: dts: mt7986: add Bananapi R3"
Next in thread: Frank Wunderlich: "[RFC v1 08/12] arm64: dts: mt7986: add usb related device nodes"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 17/10/2022 06:41, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
>
> Add support for Bananapi R3 SBC.
>
> - SD/eMMC support (switching first 4 bits of data-bus with sw6/D)
> - all rj45 ports and both SFP working (eth1/lan4)
> - PCIe and all USB-Ports + SIM-Slot tested
> - i2c and all uarts tested
> - wifi tested

Thank you for your patch. There is something to discuss/improve.

>
> Signed-off-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
> ---
> SPI-NAND/NOR switched (CS by sw5/C) not yet included
> this is done with DT-Overlays in my tree, but i have no idea yet,
> how to upstream
>
> break some lines in wifi-eeprom-data because of checkpatch warnings.
> originally there were 8 x int32 per line
> ---
> arch/arm64/boot/dts/mediatek/Makefile | 2 +
> .../mediatek/mt7986a-bananapi-bpi-r3-emmc.dts | 34 +
> .../mediatek/mt7986a-bananapi-bpi-r3-sd.dts | 29 +
> .../dts/mediatek/mt7986a-bananapi-bpi-r3.dtsi | 609 ++++++++++++++++++
> 4 files changed, 674 insertions(+)
> create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-emmc.dts
> create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-sd.dts
> create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dtsi
>
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index 0ec90cb3ef28..c22cd3e6b98f 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -8,6 +8,8 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd.dtb> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtb

Alphabetical order

> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986b-rfb.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8167-pumpkin.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm.dtb
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-emmc.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-emmc.dts
> new file mode 100644
> index 000000000000..859b4180ca11
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-emmc.dts
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Author: Sam.Shih <sam.shih@xxxxxxxxxxxx>
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +#include "mt7986a-bananapi-bpi-r3.dtsi"
> +
> +/ {
> + model = "Bananapi BPI-R3 (emmc)";
> +};
> +
> +&mmc0 {
> + pinctrl-names = "default", "state_uhs";
> + pinctrl-0 = <&mmc0_pins_default>;
> + pinctrl-1 = <&mmc0_pins_uhs>;
> + bus-width = <8>;
> + max-frequency = <200000000>;
> + cap-mmc-highspeed;
> + mmc-hs200-1_8v;
> + mmc-hs400-1_8v;
> + hs400-ds-delay = <0x14014>;
> + vmmc-supply = <&reg_3p3v>;
> + vqmmc-supply = <&reg_1p8v>;
> + non-removable;
> + no-sd;
> + no-sdio;
> + status = "okay";
> +};
> +
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-sd.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-sd.dts
> new file mode 100644
> index 000000000000..57200407ab86
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-sd.dts
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Author: Sam.Shih <sam.shih@xxxxxxxxxxxx>
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +#include "mt7986a-bananapi-bpi-r3.dtsi"
> +
> +/ {
> + model = "Bananapi BPI-R3 (sdmmc)";
> +};
> +
> +&mmc0 {
> + //sdcard
> + pinctrl-names = "default", "state_uhs";
> + pinctrl-0 = <&mmc0_pins_default>;
> + pinctrl-1 = <&mmc0_pins_uhs>;
> + bus-width = <4>;
> + max-frequency = <52000000>;
> + cap-sd-highspeed;
> + vmmc-supply = <&reg_3p3v>;
> + vqmmc-supply = <&reg_1p8v>;
> + status = "okay";
> +};
> +
> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dtsi b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dtsi
> new file mode 100644
> index 000000000000..b2317e894855
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3.dtsi
> @@ -0,0 +1,609 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2021 MediaTek Inc.
> + * Authors: Sam.Shih <sam.shih@xxxxxxxxxxxx>
> + * Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
> + * Daniel Golle <daniel@xxxxxxxxxxxxxx>
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +
> +#include "mt7986a.dtsi"
> +
> +/ {
> + model = "Bananapi BPI-R3";
> + compatible = "bananapi,bpi-r3", "mediatek,mt7986a";
> +
> + aliases {
> + serial0 = &uart0;
> + ethernet0 = &gmac0;
> + ethernet1 = &gmac1;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + factory {

key/button prefix

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).

> + label = "reset";
> + linux,code = <KEY_RESTART>;
> + gpios = <&pio 9 GPIO_ACTIVE_LOW>;
> + };
> +
> + wps {
> + label = "wps";
> + linux,code = <KEY_WPS_BUTTON>;
> + gpios = <&pio 10 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> + /* i2c of the left SFP cage (wan) */
> + i2c_sfp1: i2c-gpio-0 {
> + compatible = "i2c-gpio";
> + sda-gpios = <&pio 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> + scl-gpios = <&pio 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> + i2c-gpio,delay-us = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + /* i2c of the right SFP cage (lan) */
> + i2c_sfp2: i2c-gpio-1 {
> + compatible = "i2c-gpio";
> + sda-gpios = <&pio 18 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> + scl-gpios = <&pio 19 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> + i2c-gpio,delay-us = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + green_led: led-0 {
> + color = <LED_COLOR_ID_GREEN>;
> + function = LED_FUNCTION_POWER;
> + gpios = <&pio 69 GPIO_ACTIVE_HIGH>;
> + default-state = "on";
> + };
> +
> + blue_led: led-1 {
> + color = <LED_COLOR_ID_BLUE>;
> + function = LED_FUNCTION_STATUS;
> + gpios = <&pio 86 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + };
> + };

Blank line
> + memory@40000000 {
> + device_type = "memory";
> + reg = <0 0x40000000 0 0x40000000>;
> + };
> +
> + reg_1p8v: regulator-1p8v {
> + compatible = "regulator-fixed";
> + regulator-name = "fixed-1.8V";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + reg_3p3v: regulator-3p3v {
> + compatible = "regulator-fixed";
> + regulator-name = "fixed-3.3V";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + reg_5v: regulator-5v {
> + compatible = "regulator-fixed";
> + regulator-name = "fixed-5V";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + /* left SFP cage (wan) */
> + sfp1: sfp1 {

sfp-1

> + compatible = "sff,sfp";
> + i2c-bus = <&i2c_sfp1>;
> + los-gpio = <&pio 46 GPIO_ACTIVE_HIGH>;
> + moddef0-gpio = <&pio 49 GPIO_ACTIVE_LOW>;
> + tx-disable-gpio = <&pio 20 GPIO_ACTIVE_HIGH>;
> + tx-fault-gpio = <&pio 7 GPIO_ACTIVE_HIGH>;
> + };
> +
> + /* right SFP cage (lan) */
> + sfp2: sfp2 {

sfp-2

> + compatible = "sff,sfp";
> + i2c-bus = <&i2c_sfp2>;
> + los-gpios = <&pio 31 GPIO_ACTIVE_HIGH>;
> + mod-def0-gpios = <&pio 47 GPIO_ACTIVE_LOW>;
> + tx-disable-gpios = <&pio 15 GPIO_ACTIVE_HIGH>;
> + tx-fault-gpios = <&pio 48 GPIO_ACTIVE_HIGH>;
> + };
> +};
> +
> +&crypto {
> + status = "okay";
> +};
> +
> +&eth {
> + status = "okay";
> +
> + gmac0: mac@0 {
> + compatible = "mediatek,eth-mac";
> + reg = <0>;
> + phy-mode = "2500base-x";
> +
> + fixed-link {
> + speed = <2500>;
> + full-duplex;
> + pause;
> + };
> + };
> +
> + gmac1: mac@1 {
> + compatible = "mediatek,eth-mac";
> + reg = <1>;
> + phy-mode = "2500base-x";
> + sfp = <&sfp1>;
> + managed = "in-band-status";
> + };
> +
> + mdio: mdio-bus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +};
> +
> +&mdio {
> + switch: switch@0 {
> + compatible = "mediatek,mt7531";
> + reg = <31>;

This does not match your unit address.

> + reset-gpios = <&pio 5 GPIO_ACTIVE_HIGH>;
> + };
> +};
> +
> +&i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c_pins>;
> + status = "okay";
> +};
> +
> +&pcie {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie_pins>;
> + status = "okay";
> +};
> +
> +&pcie_phy {
> + status = "okay";
> +};
> +
> +&pio {
> + i2c_pins: i2c-pins-3-4 {

Usually bindings expect certain suffix or prefix. Are you sure this
passes dtbs_check?

Best regards,
Krzysztof

Next message: Roberto Sassu: "Re: [PATCH 0/9] integrity: Move hooks into LSM"
Previous message: Yazen Ghannam: "[PATCH] EDAC/mc_sysfs: Increase legacy channel support to 12"
In reply to: Frank Wunderlich: "[RFC v1 12/12] arm64: dts: mt7986: add Bananapi R3"
Next in thread: Frank Wunderlich: "[RFC v1 08/12] arm64: dts: mt7986: add usb related device nodes"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]