Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 25/10/23 23:03, Laurent Pinchart ha scritto:
Add a minimal device tree for the Genio i350 Pumpkin development board,
which is based on a MediaTek MT8365.

The device tree is based on an initial version by Fabien Parent Based
written against the MediaTek BSP kernel ([1]). It has been cleaned up,
some features have been added (GPIO LEDs, ethernet), and some features
removed (audio, camera, display and dual-role USB). Those features will
be added back once the corresponding DT bindings and/or drivers become
available in the upstream kernel.

[1] https://gitlab.com/mediatek/aiot/bsp/linux/-/blob/mtk-v5.15-dev/arch/arm64/boot/dts/mediatek/mt8365-pumpkin.dts

Co-developed-by: Fabien Parent <fparent@xxxxxxxxxxxx>
Signed-off-by: Fabien Parent <fparent@xxxxxxxxxxxx>
Co-developed-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
Changes since v1:

- Drop mmc2 alias
- Reorder properties of i2c, mmc and usb nodes

Changes compared to the BSP version:

- Add ethernet controller
- Add GPIO LEDs
- Add reserved memory region for BL31
- Update board model and compatible
- Rename enable-sdio-wakeup to wakeup-source
- Drop audio support
- Drop display support
- Drop dual role USB support
- Don't use underscores in node names
- Normalize pinmux node names
- Remove unneeded labels
- Drop unneeded always-on
- Drop unused pinmux nodes
- Drop camera GPIO hog
- Update copyright
- Fix formatting
- Sort alphabetically
---
  arch/arm64/boot/dts/mediatek/Makefile         |   1 +
  .../boot/dts/mediatek/mt8365-pumpkin.dts      | 541 ++++++++++++++++++
  2 files changed, 542 insertions(+)
  create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin.dts

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index c99c3372a4b5..bbc232bdadc4 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -53,4 +53,5 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8195-cherry-tomato-r3.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt8195-demo.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt8195-evb.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-evk.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-pumpkin.dtb
  dtb-$(CONFIG_ARCH_MEDIATEK) += mt8516-pumpkin.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt8365-pumpkin.dts b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin.dts
new file mode 100644
index 000000000000..9c75294c9889
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin.dts
@@ -0,0 +1,541 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021 BayLibre, SAS.
+ * Copyright (c) 2023 Ideas on Board Oy
+ *
+ * Author: Fabien Parent <fparent@xxxxxxxxxxxx>
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/pinctrl/mt8365-pinfunc.h>
+
+#include "mt8365.dtsi"
+#include "mt6357.dtsi"
+
+/ {
+	model = "OLogic Pumpkin i350 EVK";
+	compatible = "ologic,pumpkin-i350", "mediatek,mt8365";
+
+	aliases {
+		ethernet0 = &ethernet_usb;
+		mmc0 = &mmc0;
+		mmc1 = &mmc1;
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:921600n8";
+	};
+
+	firmware {
+		optee {
+			compatible = "linaro,optee-tz";
+			method = "smc";
+		};
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led-red {
+			gpios = <&gpio_exp 12 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_HEARTBEAT;
+			linux,default-trigger = "heartbeat";
+		};
+
+		led-green {
+			gpios = <&gpio_exp 13 GPIO_ACTIVE_HIGH>;
+			color = <LED_COLOR_ID_GREEN>;
+			default-state = "off";
+		};
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0 0x40000000 0 0x80000000>;
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* 192 KiB reserved for ARM Trusted Firmware (BL31) */
+		secmon@43000000 {
+			no-map;
+			reg = <0 0x43000000 0 0x30000>;


reg first, no-map last.

+		};
+
+		/*
+		 * 12 MiB reserved for OP-TEE (BL32)
+		 * +-----------------------+ 0x43e0_0000
+		 * |      SHMEM 2MiB       |
+		 * +-----------------------+ 0x43c0_0000
+		 * |        | TA_RAM  8MiB |
+		 * + TZDRAM +--------------+ 0x4340_0000
+		 * |        | TEE_RAM 2MiB |
+		 * +-----------------------+ 0x4320_0000
+		 */
+		optee@43200000 {
+			no-map;
+			reg = <0 0x43200000 0 0x00c00000>;

same here.

+		};
+	};
+
+	usb_otg_vbus: usb-otg-vbus-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "otg_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		gpio = <&pio 25 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+};
+
+&cpu0 {
+	proc-supply = <&mt6357_vproc_reg>;
+	sram-supply = <&mt6357_vsram_proc_reg>;
+};
+
+&cpu1 {
+	proc-supply = <&mt6357_vproc_reg>;
+	sram-supply = <&mt6357_vsram_proc_reg>;
+};
+
+&cpu2 {
+	proc-supply = <&mt6357_vproc_reg>;
+	sram-supply = <&mt6357_vsram_proc_reg>;
+};
+
+&cpu3 {
+	proc-supply = <&mt6357_vproc_reg>;
+	sram-supply = <&mt6357_vsram_proc_reg>;
+};
+
+&i2c0 {
+	clock-frequency = <100000>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins>;
+
+	status = "okay";
+};
+
+&i2c1 {
+	clock-frequency = <100000>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins>;
+
+	status = "okay";
+
+	gpio_exp: gpio@20 {
+		compatible = "ti,tca6416";
+		reg = <0x20>;
+		reset-gpios = <&pio 78 GPIO_ACTIVE_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&tca6416_pins>;
+
+		gpio-controller;

Please check Documentation/devicetree/bindings/dts-coding-style.rst

+		#gpio-cells = <2>;
+	};
+};
+
+&i2c2 {
+	clock-frequency = <100000>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pins>;
+
+	status = "okay";
+};
+
+&i2c3 {
+	clock-frequency = <100000>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c3_pins>;
+
+	status = "okay";
+};
+
+&keypad {
+	status = "okay";

status goes last.

Please check Documentation/devicetree/bindings/dts-coding-style.rst

+
+	pinctrl-names = "default";
+	pinctrl-0 = <&keypad_pins>;
+
+	keypad,num-rows = <2>;
+	keypad,num-columns = <1>;
+	mediatek,debounce-us = <32000>;
+	mediatek,double-keys;
+
+	linux,keymap = <MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN)
+			MATRIX_KEY(0x01, 0x00, KEY_VOLUMEUP)>;
+};
+
+/* eMMC */
+&mmc0 {
+	bus-width = <8>;
+	max-frequency = <200000000>;
+
+	cap-mmc-highspeed;
+	cap-mmc-hw-reset;
+	hs400-ds-delay = <0x12012>;
+	mmc-hs200-1_8v;
+	mmc-hs400-1_8v;
+	no-sd;
+	no-sdio;
+	non-removable;

Please order by name.

+
+	vmmc-supply = <&mt6357_vemc_reg>;
+	vqmmc-supply = <&mt6357_vio18_reg>;
+
+	pinctrl-names = "default", "state_uhs";
+	pinctrl-0 = <&mmc0_pins_default>;
+	pinctrl-1 = <&mmc0_pins_uhs>;
+
+	assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;

assigned clocks and clock parents go first.

+	assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL>;
+
+	status = "okay";
+};
+
+/* SD card connector */
+&mmc1 {
+	bus-width = <4>;
+	max-frequency = <200000000>;
+
+	cap-sd-highspeed;
+	cd-gpios = <&pio 76 GPIO_ACTIVE_LOW>;
+	sd-uhs-sdr104;
+	sd-uhs-sdr50;

Please order by name.

+
+	vmmc-supply = <&mt6357_vmch_reg>;
+	vqmmc-supply = <&mt6357_vmc_reg>;
+
+	pinctrl-names = "default", "state_uhs";
+	pinctrl-0 = <&mmc1_pins_default>;
+	pinctrl-1 = <&mmc1_pins_uhs>;
+
+	status = "okay";
+};
+
+/* WiFi module */
+&mmc2 {
+	bus-width = <4>;
+	max-frequency = <200000000>;
+
+	cap-sd-highspeed;
+	cap-sdio-irq;
+	wakeup-source;
+	hs400-ds-delay = <0x12012>;

Please order by name.

+	keep-power-in-suspend;
+	non-removable;
+	sd-uhs-sdr104;
+	sd-uhs-sdr25;
+	sd-uhs-sdr50;
+
+	vmmc-supply = <&mt6357_vemc_reg>;
+	vqmmc-supply = <&mt6357_vio18_reg>;
+
+	pinctrl-names = "default", "state_uhs";
+	pinctrl-0 = <&mmc2_pins_default>;
+	pinctrl-1 = <&mmc2_pins_uhs>;
+
+	assigned-clocks = <&topckgen CLK_TOP_MSDC50_2_SEL>;
+	assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL>;
+
+	status = "okay";
+};
+
+&mt6357_pmic {
+	interrupt-parent = <&pio>;
+	interrupts = <145 IRQ_TYPE_LEVEL_HIGH>;
+	interrupt-controller;
+	#interrupt-cells = <2>;
+};
+
+&pio {
+	i2c0_pins: i2c0-pins {
+		pins {
+			pinmux = <MT8365_PIN_57_SDA0__FUNC_SDA0_0>,
+				 <MT8365_PIN_58_SCL0__FUNC_SCL0_0>;
+			mediatek,pull-up-adv = <3>;
+			mediatek,drive-strength-adv = <00>;

I don't know what 00 is here, and it's not a valid drive strength value in uA.

Valid values are 125, 250, 500 and 1000 microamps.

P.S.: Here and everywhere else.

+			bias-pull-up;
+		};
+	};
+
+

..snip..

+	mmc0_pins_uhs: mmc0-uhs-pins{
+		clk-pins {
+			pinmux = <MT8365_PIN_99_MSDC0_CLK__FUNC_MSDC0_CLK>;
+			drive-strength = <MTK_DRIVE_10mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_103_MSDC0_DAT0__FUNC_MSDC0_DAT0>,
+				 <MT8365_PIN_102_MSDC0_DAT1__FUNC_MSDC0_DAT1>,
+				 <MT8365_PIN_101_MSDC0_DAT2__FUNC_MSDC0_DAT2>,
+				 <MT8365_PIN_100_MSDC0_DAT3__FUNC_MSDC0_DAT3>,
+				 <MT8365_PIN_96_MSDC0_DAT4__FUNC_MSDC0_DAT4>,
+				 <MT8365_PIN_95_MSDC0_DAT5__FUNC_MSDC0_DAT5>,
+				 <MT8365_PIN_94_MSDC0_DAT6__FUNC_MSDC0_DAT6>,
+				 <MT8365_PIN_93_MSDC0_DAT7__FUNC_MSDC0_DAT7>,
+				 <MT8365_PIN_98_MSDC0_CMD__FUNC_MSDC0_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_10mA>;

There's no need to use MTK_DRIVE_xxxx definitions, as they're really just defining
the same number that you can just clearly write.

In this case, it is
			drive-strength = <10>;

please change it here and everywhere else.

+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+
+		ds-pins {
+			pinmux = <MT8365_PIN_104_MSDC0_DSL__FUNC_MSDC0_DSL>;
+			drive-strength = <MTK_DRIVE_10mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		rst-pins {
+			pinmux = <MT8365_PIN_97_MSDC0_RSTB__FUNC_MSDC0_RSTB>;
+			drive-strength = <MTK_DRIVE_10mA>;
+			bias-pull-up;
+		};
+	};
+

..snip..

+
+&usb_host {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	vusb33-supply = <&mt6357_vusb33_reg>;
+
+	status = "okay";
+
+	hub@2 {
+		reg = <2>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethernet_usb: ethernet@1 {

Can you please explain why you need this node on a fully discoverable bus?
Isn't it enough to just let USB auto enumerate this device at boot?

+			compatible = "usb424,ec00";
+			reg = <1>;
+		};
+	};
+};


Cheers,
Angelo




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux