All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
@ 2023-10-25 21:03 ` Laurent Pinchart
  0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-10-25 21:03 UTC (permalink / raw)
  To: linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Hello,

This small patch series adds a device tree for the Pumpkin i350 board
made by OLogic. It starts with two small patches to add a vendor prefix
for OLogic (1/3) and a compatible string for the board (2/3), and then
adds the device tree (3/3).

The device tree is based on work from Fabien Parent, with a few features
added, and a set of devices removed:

- Audio support, as I can't easily test it at the moment due to lack of
  local access to the board.

- Display support, for the same reason.

- Dual-role USB support, due to missing bindings and drivers for the
  TUSB322 in the upstream kernel.

Support for the crypto processor (ATECC608B) and the WiFi/Bluetooth
module (MT7663S) is also missing, but this wasn't present in the BSP
either.

Once camera support for the MT8365 lands upstream, I will probably send
patches for DT integration with this board.

Laurent Pinchart (3):
  dt-bindings: vendor-prefixes: Add prefix for OLogic, Inc.
  dt-bindings: arm64: dts: mediatek: Add ologic,pumpkin-i350 board
  arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350

 .../devicetree/bindings/arm/mediatek.yaml     |   1 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 arch/arm64/boot/dts/mediatek/Makefile         |   1 +
 .../boot/dts/mediatek/mt8365-pumpkin.dts      | 541 ++++++++++++++++++
 4 files changed, 545 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin.dts


base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1
-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 0/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
@ 2023-10-25 21:03 ` Laurent Pinchart
  0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-10-25 21:03 UTC (permalink / raw)
  To: linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Hello,

This small patch series adds a device tree for the Pumpkin i350 board
made by OLogic. It starts with two small patches to add a vendor prefix
for OLogic (1/3) and a compatible string for the board (2/3), and then
adds the device tree (3/3).

The device tree is based on work from Fabien Parent, with a few features
added, and a set of devices removed:

- Audio support, as I can't easily test it at the moment due to lack of
  local access to the board.

- Display support, for the same reason.

- Dual-role USB support, due to missing bindings and drivers for the
  TUSB322 in the upstream kernel.

Support for the crypto processor (ATECC608B) and the WiFi/Bluetooth
module (MT7663S) is also missing, but this wasn't present in the BSP
either.

Once camera support for the MT8365 lands upstream, I will probably send
patches for DT integration with this board.

Laurent Pinchart (3):
  dt-bindings: vendor-prefixes: Add prefix for OLogic, Inc.
  dt-bindings: arm64: dts: mediatek: Add ologic,pumpkin-i350 board
  arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350

 .../devicetree/bindings/arm/mediatek.yaml     |   1 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 arch/arm64/boot/dts/mediatek/Makefile         |   1 +
 .../boot/dts/mediatek/mt8365-pumpkin.dts      | 541 ++++++++++++++++++
 4 files changed, 545 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin.dts


base-commit: 05d3ef8bba77c1b5f98d941d8b2d4aeab8118ef1
-- 
Regards,

Laurent Pinchart


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add prefix for OLogic, Inc.
  2023-10-25 21:03 ` Laurent Pinchart
@ 2023-10-25 21:03   ` Laurent Pinchart
  -1 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-10-25 21:03 UTC (permalink / raw)
  To: linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Add a DT vendor prefix for OLogic, Inc. OLogic is a hardware and system
design company that manufactures, among other things, development boards
with MediaTek SoCs.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 573578db9509..247b724bc01f 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -981,6 +981,8 @@ patternProperties:
     description: Oki Electric Industry Co., Ltd.
   "^olimex,.*":
     description: OLIMEX Ltd.
+  "^ologic,.*":
+    description: OLogic, Inc.
   "^olpc,.*":
     description: One Laptop Per Child
   "^oneplus,.*":
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add prefix for OLogic, Inc.
@ 2023-10-25 21:03   ` Laurent Pinchart
  0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-10-25 21:03 UTC (permalink / raw)
  To: linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Add a DT vendor prefix for OLogic, Inc. OLogic is a hardware and system
design company that manufactures, among other things, development boards
with MediaTek SoCs.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 573578db9509..247b724bc01f 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -981,6 +981,8 @@ patternProperties:
     description: Oki Electric Industry Co., Ltd.
   "^olimex,.*":
     description: OLIMEX Ltd.
+  "^ologic,.*":
+    description: OLogic, Inc.
   "^olpc,.*":
     description: One Laptop Per Child
   "^oneplus,.*":
-- 
Regards,

Laurent Pinchart


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/3] dt-bindings: arm64: dts: mediatek: Add ologic,pumpkin-i350 board
  2023-10-25 21:03 ` Laurent Pinchart
@ 2023-10-25 21:03   ` Laurent Pinchart
  -1 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-10-25 21:03 UTC (permalink / raw)
  To: linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Add a compatible string for the Pumpkin i350 board, an MT8365-based
board developed by OLogic.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 Documentation/devicetree/bindings/arm/mediatek.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index ae12b1cab9fb..dc34e62c6756 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -247,6 +247,7 @@ properties:
       - items:
           - enum:
               - mediatek,mt8365-evk
+              - ologic,pumpkin-i350
           - const: mediatek,mt8365
       - items:
           - enum:
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/3] dt-bindings: arm64: dts: mediatek: Add ologic,pumpkin-i350 board
@ 2023-10-25 21:03   ` Laurent Pinchart
  0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-10-25 21:03 UTC (permalink / raw)
  To: linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Add a compatible string for the Pumpkin i350 board, an MT8365-based
board developed by OLogic.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 Documentation/devicetree/bindings/arm/mediatek.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index ae12b1cab9fb..dc34e62c6756 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -247,6 +247,7 @@ properties:
       - items:
           - enum:
               - mediatek,mt8365-evk
+              - ologic,pumpkin-i350
           - const: mediatek,mt8365
       - items:
           - enum:
-- 
Regards,

Laurent Pinchart


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
  2023-10-25 21:03 ` Laurent Pinchart
@ 2023-10-25 21:03   ` Laurent Pinchart
  -1 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-10-25 21:03 UTC (permalink / raw)
  To: linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

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@baylibre.com>
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
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@baylibre.com>
+ */
+
+/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>;
+		};
+
+		/*
+		 * 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>;
+		};
+	};
+
+	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;
+		#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";
+
+	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;
+
+	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-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;
+
+	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>;
+	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>;
+			bias-pull-up;
+		};
+	};
+
+
+	i2c1_pins: i2c1-pins {
+		pins {
+			pinmux = <MT8365_PIN_59_SDA1__FUNC_SDA1_0>,
+				 <MT8365_PIN_60_SCL1__FUNC_SCL1_0>;
+			mediatek,pull-up-adv = <3>;
+			mediatek,drive-strength-adv = <00>;
+			bias-pull-up;
+		};
+	};
+
+	i2c2_pins: i2c2-pins {
+		pins {
+			pinmux = <MT8365_PIN_61_SDA2__FUNC_SDA2_0>,
+				 <MT8365_PIN_62_SCL2__FUNC_SCL2_0>;
+			mediatek,pull-up-adv = <3>;
+			mediatek,drive-strength-adv = <00>;
+			bias-pull-up;
+		};
+	};
+
+	i2c3_pins: i2c3-pins {
+		pins {
+			pinmux = <MT8365_PIN_63_SDA3__FUNC_SDA3_0>,
+				 <MT8365_PIN_64_SCL3__FUNC_SCL3_0>;
+			mediatek,pull-up-adv = <3>;
+			mediatek,drive-strength-adv = <00>;
+			bias-pull-up;
+		};
+	};
+
+	keypad_pins: keypad-pins {
+		pins {
+			pinmux = <MT8365_PIN_22_KPROW0__FUNC_KPROW0>,
+				 <MT8365_PIN_23_KPROW1__FUNC_KPROW1>,
+				 <MT8365_PIN_24_KPCOL0__FUNC_KPCOL0>;
+		};
+	};
+
+	mmc0_pins_default: mmc0-default-pins {
+		clk-pins {
+			pinmux = <MT8365_PIN_99_MSDC0_CLK__FUNC_MSDC0_CLK>;
+			bias-pull-down;
+		};
+
+		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;
+			bias-pull-up;
+		};
+
+		rst-pins {
+			pinmux = <MT8365_PIN_97_MSDC0_RSTB__FUNC_MSDC0_RSTB>;
+			bias-pull-up;
+		};
+	};
+
+	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>;
+			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;
+		};
+	};
+
+	mmc1_pins_default: mmc1-default-pins {
+		cd-pins {
+			pinmux = <MT8365_PIN_76_CMDAT8__FUNC_GPIO76>;
+			bias-pull-up;
+		};
+
+		clk-pins {
+			pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_89_MSDC1_DAT0__FUNC_MSDC1_DAT0>,
+				 <MT8365_PIN_90_MSDC1_DAT1__FUNC_MSDC1_DAT1>,
+				 <MT8365_PIN_91_MSDC1_DAT2__FUNC_MSDC1_DAT2>,
+				 <MT8365_PIN_92_MSDC1_DAT3__FUNC_MSDC1_DAT3>,
+				 <MT8365_PIN_87_MSDC1_CMD__FUNC_MSDC1_CMD>;
+			input-enable;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+	};
+
+	mmc1_pins_uhs: mmc1-uhs-pins {
+		cd-pins {
+			pinmux = <MT8365_PIN_76_CMDAT8__FUNC_GPIO76>;
+			bias-pull-up;
+		};
+
+		clk-pins {
+			pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_89_MSDC1_DAT0__FUNC_MSDC1_DAT0>,
+				 <MT8365_PIN_90_MSDC1_DAT1__FUNC_MSDC1_DAT1>,
+				 <MT8365_PIN_91_MSDC1_DAT2__FUNC_MSDC1_DAT2>,
+				 <MT8365_PIN_92_MSDC1_DAT3__FUNC_MSDC1_DAT3>,
+				 <MT8365_PIN_87_MSDC1_CMD__FUNC_MSDC1_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_6mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+	};
+
+	mmc2_pins_default: mmc2-default-pins {
+		clk-pins {
+			pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
+			drive-strength = <MTK_DRIVE_4mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_82_MSDC2_DAT0__FUNC_MSDC2_DAT0>,
+				 <MT8365_PIN_83_MSDC2_DAT1__FUNC_MSDC2_DAT1>,
+				 <MT8365_PIN_84_MSDC2_DAT2__FUNC_MSDC2_DAT2>,
+				 <MT8365_PIN_85_MSDC2_DAT3__FUNC_MSDC2_DAT3>,
+				 <MT8365_PIN_80_MSDC2_CMD__FUNC_MSDC2_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_4mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+
+		sysrstb-en-pins {
+			pinmux = <MT8365_PIN_74_CMDAT6__FUNC_GPIO74>;
+			output-low;
+		};
+	};
+
+	mmc2_pins_uhs: mmc2-uhs-pins {
+		clk-pins {
+			pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_82_MSDC2_DAT0__FUNC_MSDC2_DAT0>,
+				 <MT8365_PIN_83_MSDC2_DAT1__FUNC_MSDC2_DAT1>,
+				 <MT8365_PIN_84_MSDC2_DAT2__FUNC_MSDC2_DAT2>,
+				 <MT8365_PIN_85_MSDC2_DAT3__FUNC_MSDC2_DAT3>,
+				 <MT8365_PIN_80_MSDC2_CMD__FUNC_MSDC2_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+
+		ds-pins {
+			pinmux = <MT8365_PIN_86_MSDC2_DSL__FUNC_MSDC2_DSL>;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		sysrstb-en-pins {
+			pinmux = <MT8365_PIN_126_DMIC3_CLK__FUNC_GPIO126>;
+			output-high;
+		};
+	};
+
+	tca6416_pins: tca6416-pins {
+		rst-n-pins {
+			pinmux = <MT8365_PIN_78_CMHSYNC__FUNC_GPIO78>;
+			output-high;
+		};
+
+		int-n-pins {
+			pinmux = <MT8365_PIN_79_CMVSYNC__FUNC_GPIO79>;
+			input-enable;
+			bias-pull-up;
+		};
+	};
+
+	usb_pins: usb-pins {
+		hub-rst-pins {
+			pinmux = <MT8365_PIN_70_CMDAT2__FUNC_GPIO70>;
+			output-low;
+		};
+
+		id-pins {
+			pinmux = <MT8365_PIN_39_URXD2__FUNC_GPIO39>;
+			input-enable;
+			bias-pull-up;
+		};
+
+		vbus-pins {
+			pinmux = <MT8365_PIN_25_KPCOL1__FUNC_USB_DRVVBUS>;
+		};
+	};
+};
+
+&ssusb {
+	/* Host mode only for now, TUSB322 support is missing. */
+	dr_mode = "host";
+	maximum-speed = "high-speed";
+	vusb33-supply = <&mt6357_vusb33_reg>;
+
+	pinctrl-0 = <&usb_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&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 {
+			compatible = "usb424,ec00";
+			reg = <1>;
+		};
+	};
+};
-- 
Regards,

Laurent Pinchart


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
@ 2023-10-25 21:03   ` Laurent Pinchart
  0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2023-10-25 21:03 UTC (permalink / raw)
  To: linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

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@baylibre.com>
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
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@baylibre.com>
+ */
+
+/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>;
+		};
+
+		/*
+		 * 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>;
+		};
+	};
+
+	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;
+		#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";
+
+	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;
+
+	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-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;
+
+	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>;
+	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>;
+			bias-pull-up;
+		};
+	};
+
+
+	i2c1_pins: i2c1-pins {
+		pins {
+			pinmux = <MT8365_PIN_59_SDA1__FUNC_SDA1_0>,
+				 <MT8365_PIN_60_SCL1__FUNC_SCL1_0>;
+			mediatek,pull-up-adv = <3>;
+			mediatek,drive-strength-adv = <00>;
+			bias-pull-up;
+		};
+	};
+
+	i2c2_pins: i2c2-pins {
+		pins {
+			pinmux = <MT8365_PIN_61_SDA2__FUNC_SDA2_0>,
+				 <MT8365_PIN_62_SCL2__FUNC_SCL2_0>;
+			mediatek,pull-up-adv = <3>;
+			mediatek,drive-strength-adv = <00>;
+			bias-pull-up;
+		};
+	};
+
+	i2c3_pins: i2c3-pins {
+		pins {
+			pinmux = <MT8365_PIN_63_SDA3__FUNC_SDA3_0>,
+				 <MT8365_PIN_64_SCL3__FUNC_SCL3_0>;
+			mediatek,pull-up-adv = <3>;
+			mediatek,drive-strength-adv = <00>;
+			bias-pull-up;
+		};
+	};
+
+	keypad_pins: keypad-pins {
+		pins {
+			pinmux = <MT8365_PIN_22_KPROW0__FUNC_KPROW0>,
+				 <MT8365_PIN_23_KPROW1__FUNC_KPROW1>,
+				 <MT8365_PIN_24_KPCOL0__FUNC_KPCOL0>;
+		};
+	};
+
+	mmc0_pins_default: mmc0-default-pins {
+		clk-pins {
+			pinmux = <MT8365_PIN_99_MSDC0_CLK__FUNC_MSDC0_CLK>;
+			bias-pull-down;
+		};
+
+		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;
+			bias-pull-up;
+		};
+
+		rst-pins {
+			pinmux = <MT8365_PIN_97_MSDC0_RSTB__FUNC_MSDC0_RSTB>;
+			bias-pull-up;
+		};
+	};
+
+	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>;
+			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;
+		};
+	};
+
+	mmc1_pins_default: mmc1-default-pins {
+		cd-pins {
+			pinmux = <MT8365_PIN_76_CMDAT8__FUNC_GPIO76>;
+			bias-pull-up;
+		};
+
+		clk-pins {
+			pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_89_MSDC1_DAT0__FUNC_MSDC1_DAT0>,
+				 <MT8365_PIN_90_MSDC1_DAT1__FUNC_MSDC1_DAT1>,
+				 <MT8365_PIN_91_MSDC1_DAT2__FUNC_MSDC1_DAT2>,
+				 <MT8365_PIN_92_MSDC1_DAT3__FUNC_MSDC1_DAT3>,
+				 <MT8365_PIN_87_MSDC1_CMD__FUNC_MSDC1_CMD>;
+			input-enable;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+	};
+
+	mmc1_pins_uhs: mmc1-uhs-pins {
+		cd-pins {
+			pinmux = <MT8365_PIN_76_CMDAT8__FUNC_GPIO76>;
+			bias-pull-up;
+		};
+
+		clk-pins {
+			pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_89_MSDC1_DAT0__FUNC_MSDC1_DAT0>,
+				 <MT8365_PIN_90_MSDC1_DAT1__FUNC_MSDC1_DAT1>,
+				 <MT8365_PIN_91_MSDC1_DAT2__FUNC_MSDC1_DAT2>,
+				 <MT8365_PIN_92_MSDC1_DAT3__FUNC_MSDC1_DAT3>,
+				 <MT8365_PIN_87_MSDC1_CMD__FUNC_MSDC1_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_6mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+	};
+
+	mmc2_pins_default: mmc2-default-pins {
+		clk-pins {
+			pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
+			drive-strength = <MTK_DRIVE_4mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_82_MSDC2_DAT0__FUNC_MSDC2_DAT0>,
+				 <MT8365_PIN_83_MSDC2_DAT1__FUNC_MSDC2_DAT1>,
+				 <MT8365_PIN_84_MSDC2_DAT2__FUNC_MSDC2_DAT2>,
+				 <MT8365_PIN_85_MSDC2_DAT3__FUNC_MSDC2_DAT3>,
+				 <MT8365_PIN_80_MSDC2_CMD__FUNC_MSDC2_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_4mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+
+		sysrstb-en-pins {
+			pinmux = <MT8365_PIN_74_CMDAT6__FUNC_GPIO74>;
+			output-low;
+		};
+	};
+
+	mmc2_pins_uhs: mmc2-uhs-pins {
+		clk-pins {
+			pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		cmd-dat-pins {
+			pinmux = <MT8365_PIN_82_MSDC2_DAT0__FUNC_MSDC2_DAT0>,
+				 <MT8365_PIN_83_MSDC2_DAT1__FUNC_MSDC2_DAT1>,
+				 <MT8365_PIN_84_MSDC2_DAT2__FUNC_MSDC2_DAT2>,
+				 <MT8365_PIN_85_MSDC2_DAT3__FUNC_MSDC2_DAT3>,
+				 <MT8365_PIN_80_MSDC2_CMD__FUNC_MSDC2_CMD>;
+			input-enable;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+		};
+
+		ds-pins {
+			pinmux = <MT8365_PIN_86_MSDC2_DSL__FUNC_MSDC2_DSL>;
+			drive-strength = <MTK_DRIVE_8mA>;
+			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+		};
+
+		sysrstb-en-pins {
+			pinmux = <MT8365_PIN_126_DMIC3_CLK__FUNC_GPIO126>;
+			output-high;
+		};
+	};
+
+	tca6416_pins: tca6416-pins {
+		rst-n-pins {
+			pinmux = <MT8365_PIN_78_CMHSYNC__FUNC_GPIO78>;
+			output-high;
+		};
+
+		int-n-pins {
+			pinmux = <MT8365_PIN_79_CMVSYNC__FUNC_GPIO79>;
+			input-enable;
+			bias-pull-up;
+		};
+	};
+
+	usb_pins: usb-pins {
+		hub-rst-pins {
+			pinmux = <MT8365_PIN_70_CMDAT2__FUNC_GPIO70>;
+			output-low;
+		};
+
+		id-pins {
+			pinmux = <MT8365_PIN_39_URXD2__FUNC_GPIO39>;
+			input-enable;
+			bias-pull-up;
+		};
+
+		vbus-pins {
+			pinmux = <MT8365_PIN_25_KPCOL1__FUNC_USB_DRVVBUS>;
+		};
+	};
+};
+
+&ssusb {
+	/* Host mode only for now, TUSB322 support is missing. */
+	dr_mode = "host";
+	maximum-speed = "high-speed";
+	vusb33-supply = <&mt6357_vusb33_reg>;
+
+	pinctrl-0 = <&usb_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&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 {
+			compatible = "usb424,ec00";
+			reg = <1>;
+		};
+	};
+};
-- 
Regards,

Laurent Pinchart


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
  2023-10-25 21:03   ` Laurent Pinchart
@ 2023-11-27 16:10     ` Julien Masson
  -1 siblings, 0 replies; 16+ messages in thread
From: Julien Masson @ 2023-11-27 16:10 UTC (permalink / raw)
  To: Laurent Pinchart, linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson


Thanks laurent for adding the support of this board.

On Mon 27 Nov 2023 at 17:01, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> 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@baylibre.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> 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@baylibre.com>
> + */
> +
> +/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>;
> +		};
> +
> +		/*
> +		 * 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>;
> +		};
> +	};
> +
> +	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;
> +		#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";
> +
> +	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;
> +
> +	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-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;
> +
> +	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>;
> +	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>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +
> +	i2c1_pins: i2c1-pins {
> +		pins {
> +			pinmux = <MT8365_PIN_59_SDA1__FUNC_SDA1_0>,
> +				 <MT8365_PIN_60_SCL1__FUNC_SCL1_0>;
> +			mediatek,pull-up-adv = <3>;
> +			mediatek,drive-strength-adv = <00>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	i2c2_pins: i2c2-pins {
> +		pins {
> +			pinmux = <MT8365_PIN_61_SDA2__FUNC_SDA2_0>,
> +				 <MT8365_PIN_62_SCL2__FUNC_SCL2_0>;
> +			mediatek,pull-up-adv = <3>;
> +			mediatek,drive-strength-adv = <00>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	i2c3_pins: i2c3-pins {
> +		pins {
> +			pinmux = <MT8365_PIN_63_SDA3__FUNC_SDA3_0>,
> +				 <MT8365_PIN_64_SCL3__FUNC_SCL3_0>;
> +			mediatek,pull-up-adv = <3>;
> +			mediatek,drive-strength-adv = <00>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	keypad_pins: keypad-pins {
> +		pins {
> +			pinmux = <MT8365_PIN_22_KPROW0__FUNC_KPROW0>,
> +				 <MT8365_PIN_23_KPROW1__FUNC_KPROW1>,
> +				 <MT8365_PIN_24_KPCOL0__FUNC_KPCOL0>;
> +		};
> +	};
> +
> +	mmc0_pins_default: mmc0-default-pins {
> +		clk-pins {
> +			pinmux = <MT8365_PIN_99_MSDC0_CLK__FUNC_MSDC0_CLK>;
> +			bias-pull-down;
> +		};
> +
> +		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;
> +			bias-pull-up;
> +		};
> +
> +		rst-pins {
> +			pinmux = <MT8365_PIN_97_MSDC0_RSTB__FUNC_MSDC0_RSTB>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	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>;
> +			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;
> +		};
> +	};
> +
> +	mmc1_pins_default: mmc1-default-pins {
> +		cd-pins {
> +			pinmux = <MT8365_PIN_76_CMDAT8__FUNC_GPIO76>;
> +			bias-pull-up;
> +		};
> +
> +		clk-pins {
> +			pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		cmd-dat-pins {
> +			pinmux = <MT8365_PIN_89_MSDC1_DAT0__FUNC_MSDC1_DAT0>,
> +				 <MT8365_PIN_90_MSDC1_DAT1__FUNC_MSDC1_DAT1>,
> +				 <MT8365_PIN_91_MSDC1_DAT2__FUNC_MSDC1_DAT2>,
> +				 <MT8365_PIN_92_MSDC1_DAT3__FUNC_MSDC1_DAT3>,
> +				 <MT8365_PIN_87_MSDC1_CMD__FUNC_MSDC1_CMD>;
> +			input-enable;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> +		};
> +	};
> +
> +	mmc1_pins_uhs: mmc1-uhs-pins {
> +		cd-pins {
> +			pinmux = <MT8365_PIN_76_CMDAT8__FUNC_GPIO76>;
> +			bias-pull-up;
> +		};
> +
> +		clk-pins {
> +			pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		cmd-dat-pins {
> +			pinmux = <MT8365_PIN_89_MSDC1_DAT0__FUNC_MSDC1_DAT0>,
> +				 <MT8365_PIN_90_MSDC1_DAT1__FUNC_MSDC1_DAT1>,
> +				 <MT8365_PIN_91_MSDC1_DAT2__FUNC_MSDC1_DAT2>,
> +				 <MT8365_PIN_92_MSDC1_DAT3__FUNC_MSDC1_DAT3>,
> +				 <MT8365_PIN_87_MSDC1_CMD__FUNC_MSDC1_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_6mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> +		};
> +	};
> +
> +	mmc2_pins_default: mmc2-default-pins {
> +		clk-pins {
> +			pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
> +			drive-strength = <MTK_DRIVE_4mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		cmd-dat-pins {
> +			pinmux = <MT8365_PIN_82_MSDC2_DAT0__FUNC_MSDC2_DAT0>,
> +				 <MT8365_PIN_83_MSDC2_DAT1__FUNC_MSDC2_DAT1>,
> +				 <MT8365_PIN_84_MSDC2_DAT2__FUNC_MSDC2_DAT2>,
> +				 <MT8365_PIN_85_MSDC2_DAT3__FUNC_MSDC2_DAT3>,
> +				 <MT8365_PIN_80_MSDC2_CMD__FUNC_MSDC2_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_4mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> +		};
> +
> +		sysrstb-en-pins {
> +			pinmux = <MT8365_PIN_74_CMDAT6__FUNC_GPIO74>;
> +			output-low;
> +		};
> +	};
> +
> +	mmc2_pins_uhs: mmc2-uhs-pins {
> +		clk-pins {
> +			pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		cmd-dat-pins {
> +			pinmux = <MT8365_PIN_82_MSDC2_DAT0__FUNC_MSDC2_DAT0>,
> +				 <MT8365_PIN_83_MSDC2_DAT1__FUNC_MSDC2_DAT1>,
> +				 <MT8365_PIN_84_MSDC2_DAT2__FUNC_MSDC2_DAT2>,
> +				 <MT8365_PIN_85_MSDC2_DAT3__FUNC_MSDC2_DAT3>,
> +				 <MT8365_PIN_80_MSDC2_CMD__FUNC_MSDC2_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> +		};
> +
> +		ds-pins {
> +			pinmux = <MT8365_PIN_86_MSDC2_DSL__FUNC_MSDC2_DSL>;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		sysrstb-en-pins {
> +			pinmux = <MT8365_PIN_126_DMIC3_CLK__FUNC_GPIO126>;
> +			output-high;
> +		};
> +	};
> +
> +	tca6416_pins: tca6416-pins {
> +		rst-n-pins {
> +			pinmux = <MT8365_PIN_78_CMHSYNC__FUNC_GPIO78>;
> +			output-high;
> +		};
> +
> +		int-n-pins {
> +			pinmux = <MT8365_PIN_79_CMVSYNC__FUNC_GPIO79>;
> +			input-enable;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	usb_pins: usb-pins {
> +		hub-rst-pins {
> +			pinmux = <MT8365_PIN_70_CMDAT2__FUNC_GPIO70>;
> +			output-low;
> +		};
> +
> +		id-pins {
> +			pinmux = <MT8365_PIN_39_URXD2__FUNC_GPIO39>;
> +			input-enable;
> +			bias-pull-up;
> +		};
> +
> +		vbus-pins {
> +			pinmux = <MT8365_PIN_25_KPCOL1__FUNC_USB_DRVVBUS>;
> +		};
> +	};
> +};
> +
> +&ssusb {
> +	/* Host mode only for now, TUSB322 support is missing. */
> +	dr_mode = "host";
> +	maximum-speed = "high-speed";
> +	vusb33-supply = <&mt6357_vusb33_reg>;
> +
> +	pinctrl-0 = <&usb_pins>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +};
> +
> +&uart0 {
> +	status = "okay";
> +};
> +
> +&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 {
> +			compatible = "usb424,ec00";
> +			reg = <1>;
> +		};
> +	};
> +};
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 

This patch series conflict with:
f2b543a191b6 arm64: dts: mediatek: add device-tree for Genio 1200 EVK board

On i350 Pumpkin board (MT8365):
Tested-by: Julien Masson <jmasson@baylibre.com>

-- 
Julien Masson

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
@ 2023-11-27 16:10     ` Julien Masson
  0 siblings, 0 replies; 16+ messages in thread
From: Julien Masson @ 2023-11-27 16:10 UTC (permalink / raw)
  To: Laurent Pinchart, linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
	Paul Elder, Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson


Thanks laurent for adding the support of this board.

On Mon 27 Nov 2023 at 17:01, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> 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@baylibre.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> 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@baylibre.com>
> + */
> +
> +/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>;
> +		};
> +
> +		/*
> +		 * 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>;
> +		};
> +	};
> +
> +	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;
> +		#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";
> +
> +	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;
> +
> +	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-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;
> +
> +	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>;
> +	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>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +
> +	i2c1_pins: i2c1-pins {
> +		pins {
> +			pinmux = <MT8365_PIN_59_SDA1__FUNC_SDA1_0>,
> +				 <MT8365_PIN_60_SCL1__FUNC_SCL1_0>;
> +			mediatek,pull-up-adv = <3>;
> +			mediatek,drive-strength-adv = <00>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	i2c2_pins: i2c2-pins {
> +		pins {
> +			pinmux = <MT8365_PIN_61_SDA2__FUNC_SDA2_0>,
> +				 <MT8365_PIN_62_SCL2__FUNC_SCL2_0>;
> +			mediatek,pull-up-adv = <3>;
> +			mediatek,drive-strength-adv = <00>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	i2c3_pins: i2c3-pins {
> +		pins {
> +			pinmux = <MT8365_PIN_63_SDA3__FUNC_SDA3_0>,
> +				 <MT8365_PIN_64_SCL3__FUNC_SCL3_0>;
> +			mediatek,pull-up-adv = <3>;
> +			mediatek,drive-strength-adv = <00>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	keypad_pins: keypad-pins {
> +		pins {
> +			pinmux = <MT8365_PIN_22_KPROW0__FUNC_KPROW0>,
> +				 <MT8365_PIN_23_KPROW1__FUNC_KPROW1>,
> +				 <MT8365_PIN_24_KPCOL0__FUNC_KPCOL0>;
> +		};
> +	};
> +
> +	mmc0_pins_default: mmc0-default-pins {
> +		clk-pins {
> +			pinmux = <MT8365_PIN_99_MSDC0_CLK__FUNC_MSDC0_CLK>;
> +			bias-pull-down;
> +		};
> +
> +		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;
> +			bias-pull-up;
> +		};
> +
> +		rst-pins {
> +			pinmux = <MT8365_PIN_97_MSDC0_RSTB__FUNC_MSDC0_RSTB>;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	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>;
> +			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;
> +		};
> +	};
> +
> +	mmc1_pins_default: mmc1-default-pins {
> +		cd-pins {
> +			pinmux = <MT8365_PIN_76_CMDAT8__FUNC_GPIO76>;
> +			bias-pull-up;
> +		};
> +
> +		clk-pins {
> +			pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		cmd-dat-pins {
> +			pinmux = <MT8365_PIN_89_MSDC1_DAT0__FUNC_MSDC1_DAT0>,
> +				 <MT8365_PIN_90_MSDC1_DAT1__FUNC_MSDC1_DAT1>,
> +				 <MT8365_PIN_91_MSDC1_DAT2__FUNC_MSDC1_DAT2>,
> +				 <MT8365_PIN_92_MSDC1_DAT3__FUNC_MSDC1_DAT3>,
> +				 <MT8365_PIN_87_MSDC1_CMD__FUNC_MSDC1_CMD>;
> +			input-enable;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> +		};
> +	};
> +
> +	mmc1_pins_uhs: mmc1-uhs-pins {
> +		cd-pins {
> +			pinmux = <MT8365_PIN_76_CMDAT8__FUNC_GPIO76>;
> +			bias-pull-up;
> +		};
> +
> +		clk-pins {
> +			pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		cmd-dat-pins {
> +			pinmux = <MT8365_PIN_89_MSDC1_DAT0__FUNC_MSDC1_DAT0>,
> +				 <MT8365_PIN_90_MSDC1_DAT1__FUNC_MSDC1_DAT1>,
> +				 <MT8365_PIN_91_MSDC1_DAT2__FUNC_MSDC1_DAT2>,
> +				 <MT8365_PIN_92_MSDC1_DAT3__FUNC_MSDC1_DAT3>,
> +				 <MT8365_PIN_87_MSDC1_CMD__FUNC_MSDC1_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_6mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> +		};
> +	};
> +
> +	mmc2_pins_default: mmc2-default-pins {
> +		clk-pins {
> +			pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
> +			drive-strength = <MTK_DRIVE_4mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		cmd-dat-pins {
> +			pinmux = <MT8365_PIN_82_MSDC2_DAT0__FUNC_MSDC2_DAT0>,
> +				 <MT8365_PIN_83_MSDC2_DAT1__FUNC_MSDC2_DAT1>,
> +				 <MT8365_PIN_84_MSDC2_DAT2__FUNC_MSDC2_DAT2>,
> +				 <MT8365_PIN_85_MSDC2_DAT3__FUNC_MSDC2_DAT3>,
> +				 <MT8365_PIN_80_MSDC2_CMD__FUNC_MSDC2_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_4mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> +		};
> +
> +		sysrstb-en-pins {
> +			pinmux = <MT8365_PIN_74_CMDAT6__FUNC_GPIO74>;
> +			output-low;
> +		};
> +	};
> +
> +	mmc2_pins_uhs: mmc2-uhs-pins {
> +		clk-pins {
> +			pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		cmd-dat-pins {
> +			pinmux = <MT8365_PIN_82_MSDC2_DAT0__FUNC_MSDC2_DAT0>,
> +				 <MT8365_PIN_83_MSDC2_DAT1__FUNC_MSDC2_DAT1>,
> +				 <MT8365_PIN_84_MSDC2_DAT2__FUNC_MSDC2_DAT2>,
> +				 <MT8365_PIN_85_MSDC2_DAT3__FUNC_MSDC2_DAT3>,
> +				 <MT8365_PIN_80_MSDC2_CMD__FUNC_MSDC2_CMD>;
> +			input-enable;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
> +		};
> +
> +		ds-pins {
> +			pinmux = <MT8365_PIN_86_MSDC2_DSL__FUNC_MSDC2_DSL>;
> +			drive-strength = <MTK_DRIVE_8mA>;
> +			bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
> +		};
> +
> +		sysrstb-en-pins {
> +			pinmux = <MT8365_PIN_126_DMIC3_CLK__FUNC_GPIO126>;
> +			output-high;
> +		};
> +	};
> +
> +	tca6416_pins: tca6416-pins {
> +		rst-n-pins {
> +			pinmux = <MT8365_PIN_78_CMHSYNC__FUNC_GPIO78>;
> +			output-high;
> +		};
> +
> +		int-n-pins {
> +			pinmux = <MT8365_PIN_79_CMVSYNC__FUNC_GPIO79>;
> +			input-enable;
> +			bias-pull-up;
> +		};
> +	};
> +
> +	usb_pins: usb-pins {
> +		hub-rst-pins {
> +			pinmux = <MT8365_PIN_70_CMDAT2__FUNC_GPIO70>;
> +			output-low;
> +		};
> +
> +		id-pins {
> +			pinmux = <MT8365_PIN_39_URXD2__FUNC_GPIO39>;
> +			input-enable;
> +			bias-pull-up;
> +		};
> +
> +		vbus-pins {
> +			pinmux = <MT8365_PIN_25_KPCOL1__FUNC_USB_DRVVBUS>;
> +		};
> +	};
> +};
> +
> +&ssusb {
> +	/* Host mode only for now, TUSB322 support is missing. */
> +	dr_mode = "host";
> +	maximum-speed = "high-speed";
> +	vusb33-supply = <&mt6357_vusb33_reg>;
> +
> +	pinctrl-0 = <&usb_pins>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +};
> +
> +&uart0 {
> +	status = "okay";
> +};
> +
> +&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 {
> +			compatible = "usb424,ec00";
> +			reg = <1>;
> +		};
> +	};
> +};
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 

This patch series conflict with:
f2b543a191b6 arm64: dts: mediatek: add device-tree for Genio 1200 EVK board

On i350 Pumpkin board (MT8365):
Tested-by: Julien Masson <jmasson@baylibre.com>

-- 
Julien Masson

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
  2023-10-25 21:03   ` Laurent Pinchart
@ 2024-01-23 13:53     ` AngeloGioacchino Del Regno
  -1 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-23 13:53 UTC (permalink / raw)
  To: Laurent Pinchart, linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, Paul Elder, Fabien Parent,
	Alexandre Mergnat, Julien Stephan, Suhrid Subramaniam,
	Ted Larson

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@baylibre.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> 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@baylibre.com>
> + */
> +
> +/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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
@ 2024-01-23 13:53     ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-23 13:53 UTC (permalink / raw)
  To: Laurent Pinchart, linux-mediatek
  Cc: devicetree, linux-arm-kernel, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, Paul Elder, Fabien Parent,
	Alexandre Mergnat, Julien Stephan, Suhrid Subramaniam,
	Ted Larson

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@baylibre.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> 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@baylibre.com>
> + */
> +
> +/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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
  2024-01-23 13:53     ` AngeloGioacchino Del Regno
@ 2024-01-23 15:02       ` Laurent Pinchart
  -1 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2024-01-23 15:02 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: linux-mediatek, devicetree, linux-arm-kernel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, Paul Elder,
	Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Hi Angelo,

On Tue, Jan 23, 2024 at 02:53:58PM +0100, AngeloGioacchino Del Regno wrote:
> 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@baylibre.com>
> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
> > Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 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@baylibre.com>
> > + */
> > +
> > +/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

I assume you mean the "Order of Properties in Device Node" section ?
What part exactly ?

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

I don't know either, I didn't author that part. I'll look it up.

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

OK. Looks cleaner indeed.

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

To let the boot loader fill in the MAC address in DT. Would you like me
to add a comment here ?

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

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
@ 2024-01-23 15:02       ` Laurent Pinchart
  0 siblings, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2024-01-23 15:02 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: linux-mediatek, devicetree, linux-arm-kernel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, Paul Elder,
	Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Hi Angelo,

On Tue, Jan 23, 2024 at 02:53:58PM +0100, AngeloGioacchino Del Regno wrote:
> 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@baylibre.com>
> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
> > Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 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@baylibre.com>
> > + */
> > +
> > +/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

I assume you mean the "Order of Properties in Device Node" section ?
What part exactly ?

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

I don't know either, I didn't author that part. I'll look it up.

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

OK. Looks cleaner indeed.

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

To let the boot loader fill in the MAC address in DT. Would you like me
to add a comment here ?

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

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
  2024-01-23 15:02       ` Laurent Pinchart
@ 2024-01-24  8:36         ` AngeloGioacchino Del Regno
  -1 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-24  8:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-mediatek, devicetree, linux-arm-kernel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, Paul Elder,
	Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Il 23/01/24 16:02, Laurent Pinchart ha scritto:
> Hi Angelo,
> 
> On Tue, Jan 23, 2024 at 02:53:58PM +0100, AngeloGioacchino Del Regno wrote:
>> 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@baylibre.com>
>>> Signed-off-by: Fabien Parent <fparent@baylibre.com>
>>> Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
>>> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>> 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@baylibre.com>
>>> + */
>>> +
>>> +/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
> 
> I assume you mean the "Order of Properties in Device Node" section ?
> What part exactly ?
> 
>>> +		#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.
> 
> I don't know either, I didn't author that part. I'll look it up.
> 
>> 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.
> 
> OK. Looks cleaner indeed.
> 
>>> +			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?
> 
> To let the boot loader fill in the MAC address in DT. Would you like me
> to add a comment here ?
> 

This is pretty unusual... so yes please, add a comment explaining why this
node is here.
I can otherwise already see a future engineer removing this node thinking that
it's useless! :-)

Also, if this is an internal "non-removable" USB ethernet adapter, please add
that note to the comment.

Cheers,
Angelo

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350
@ 2024-01-24  8:36         ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 16+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-24  8:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-mediatek, devicetree, linux-arm-kernel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Matthias Brugger, Paul Elder,
	Fabien Parent, Alexandre Mergnat, Julien Stephan,
	Suhrid Subramaniam, Ted Larson

Il 23/01/24 16:02, Laurent Pinchart ha scritto:
> Hi Angelo,
> 
> On Tue, Jan 23, 2024 at 02:53:58PM +0100, AngeloGioacchino Del Regno wrote:
>> 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@baylibre.com>
>>> Signed-off-by: Fabien Parent <fparent@baylibre.com>
>>> Co-developed-by: Paul Elder <paul.elder@ideasonboard.com>
>>> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>> 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@baylibre.com>
>>> + */
>>> +
>>> +/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
> 
> I assume you mean the "Order of Properties in Device Node" section ?
> What part exactly ?
> 
>>> +		#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.
> 
> I don't know either, I didn't author that part. I'll look it up.
> 
>> 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.
> 
> OK. Looks cleaner indeed.
> 
>>> +			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?
> 
> To let the boot loader fill in the MAC address in DT. Would you like me
> to add a comment here ?
> 

This is pretty unusual... so yes please, add a comment explaining why this
node is here.
I can otherwise already see a future engineer removing this node thinking that
it's useless! :-)

Also, if this is an internal "non-removable" USB ethernet adapter, please add
that note to the comment.

Cheers,
Angelo

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-01-24  8:37 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-25 21:03 [PATCH v2 0/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350 Laurent Pinchart
2023-10-25 21:03 ` Laurent Pinchart
2023-10-25 21:03 ` [PATCH v2 1/3] dt-bindings: vendor-prefixes: Add prefix for OLogic, Inc Laurent Pinchart
2023-10-25 21:03   ` Laurent Pinchart
2023-10-25 21:03 ` [PATCH v2 2/3] dt-bindings: arm64: dts: mediatek: Add ologic,pumpkin-i350 board Laurent Pinchart
2023-10-25 21:03   ` Laurent Pinchart
2023-10-25 21:03 ` [PATCH v2 3/3] arm64: dts: mediatek: Add device tree for MT8365-based Pumpkin i350 Laurent Pinchart
2023-10-25 21:03   ` Laurent Pinchart
2023-11-27 16:10   ` Julien Masson
2023-11-27 16:10     ` Julien Masson
2024-01-23 13:53   ` AngeloGioacchino Del Regno
2024-01-23 13:53     ` AngeloGioacchino Del Regno
2024-01-23 15:02     ` Laurent Pinchart
2024-01-23 15:02       ` Laurent Pinchart
2024-01-24  8:36       ` AngeloGioacchino Del Regno
2024-01-24  8:36         ` AngeloGioacchino Del Regno

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.