Re: [PATCH RESEND v2] arm64: dts: mediatek: add mmc2 support for mt8365-evk

From: Alexandre Mergnat
Date: Wed Mar 26 2025 - 06:14:49 EST


Hello Angelo,

Thanks for the review :D

I've some comment below:

On 24/03/2025 15:09, AngeloGioacchino Del Regno wrote:
Il 24/03/25 14:54, Alexandre Mergnat ha scritto:
Adds support for the MMC2 interface on the MT8365 EVK board.
It introduces a fixed regulator for the MMC2 VDD33 supply and configures
the MMC2 node with a 4-bit bus width, high-speed capabilities, UHS
modes, and appropriate power supplies. Enabled SDIO IRQ, wakeup source,
and kept power during suspend (to save firmware module) for wireless
chip functionality.

Signed-off-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
---
Changes in v2:
- Apply alphabetical order to pinctrl property items.
- Improve commit message
- Link to v1: https://lore.kernel.org/r/20250109-mmc2-support-v1-1-9b9d1b1ae35d@xxxxxxxxxxxx
---
  arch/arm64/boot/dts/mediatek/mt8365-evk.dts | 103 +++++++++++++++++++++++++---
  1 file changed, 94 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8365-evk.dts b/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
index 7d90112a7e274..a87f1b3ed6500 100644
--- a/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8365-evk.dts
@@ -53,6 +53,15 @@ memory@40000000 {
          reg = <0 0x40000000 0 0xc0000000>;
      };
+    mmc2_vdd33: mmc2_vdd33-regulator {
+        compatible = "regulator-fixed";
+        regulator-name = "mmc2_vdd33";

mmc2-vdd33 please

Ok


+        regulator-min-microvolt = <3300000>;
+        regulator-max-microvolt = <3300000>;
+        gpio = <&pio 121 0>;
+        enable-active-high;
+    };
+
      usb_otg_vbus: regulator-0 {
          compatible = "regulator-fixed";
          regulator-name = "otg_vbus";

Then, I will change this too.

@@ -197,6 +206,28 @@ &mmc1 {
      status = "okay";
  };
+&mmc2 {
+    assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL>;
+    assigned-clocks = <&topckgen CLK_TOP_MSDC50_2_SEL>;
+    bus-width = <4>;
+    cap-sd-highspeed;
+    cap-sdio-irq;
+    hs400-ds-delay = <0x12012>;
+    keep-power-in-suspend;
+    max-frequency = <200000000>;
+    non-removable;
+    pinctrl-0 = <&mmc2_default_pins>;
+    pinctrl-1 = <&mmc2_uhs_pins>;
+    pinctrl-names = "default", "state_uhs";
+    sd-uhs-sdr104;
+    sd-uhs-sdr25;
+    sd-uhs-sdr50;
+    vmmc-supply = <&mmc2_vdd33>;
+    vqmmc-supply = <&mt6357_vcn18_reg>;
+    wakeup-source;
+    status = "okay";
+};
+
  &mt6357_pmic {
      interrupts-extended = <&pio 145 IRQ_TYPE_LEVEL_HIGH>;
      interrupt-controller;
@@ -324,8 +355,8 @@ cmd-dat-pins {
                   <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;
+            input-enable;

This is a cleanup and goes to a different commit

Agree


          };
          rst-pins {
@@ -337,8 +368,8 @@ rst-pins {
      mmc0_uhs_pins: 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>;
+            drive-strength = <MTK_DRIVE_10mA>;

While at it, in a cleanup commit, if you could also remove those MTK_DRIVE_xxx and
use just the number that'd be great.

Sure


          };
          cmd-dat-pins {
@@ -351,21 +382,21 @@ cmd-dat-pins {
                   <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>;
+            drive-strength = <MTK_DRIVE_10mA>;
+            input-enable;
          };
          ds-pins {
              pinmux = <MT8365_PIN_104_MSDC0_DSL__FUNC_MSDC0_DSL>;
-            drive-strength = <MTK_DRIVE_10mA>;
              bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+            drive-strength = <MTK_DRIVE_10mA>;
          };
          rst-pins {
              pinmux = <MT8365_PIN_97_MSDC0_RSTB__FUNC_MSDC0_RSTB>;
-            drive-strength = <MTK_DRIVE_10mA>;
              bias-pull-up;
+            drive-strength = <MTK_DRIVE_10mA>;
          };
      };
@@ -386,16 +417,16 @@ cmd-dat-pins {
                   <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>;
+            input-enable;
          };
      };
      mmc1_uhs_pins: mmc1-uhs-pins {
          clk-pins {
              pinmux = <MT8365_PIN_88_MSDC1_CLK__FUNC_MSDC1_CLK>;
-            drive-strength = <8>;
              bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+            drive-strength = <8>;
          };
          cmd-dat-pins {
@@ -404,9 +435,63 @@ cmd-dat-pins {
                   <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>;
              drive-strength = <6>;
+            input-enable;
+        };
+    };
+
+    mmc2_default_pins: mmc2-default-pins {
+        clk-pins {
+            pinmux = <MT8365_PIN_81_MSDC2_CLK__FUNC_MSDC2_CLK>;
+            bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
+            drive-strength = <4>;
+        };
+
+        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>;
              bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
+            drive-strength = <4>;
+            input-enable;
+        };
+
+        sys-en-pins {
+            pinmux = <MT8365_PIN_120_DMIC1_CLK__FUNC_GPIO120>;

My schematics say that the DMIC1_CLK pin is PERST_N, DMIC_DAT0 is PWR_EN: what's
the intention here?!

In any case, this is not a mmc2 pin, but something else :-)

First of all, name in schematic are wrong, I noticed that when I've port these feature from kernel 5.15 to 6.6. After digging and talking to MTK, they confirm me that PERST_N is actually an enable pin (or power_on).

Secondly, MT7663_PERST_N (MT8365_PIN_120_DMIC1_CLK__FUNC_GPIO120) is part of mmc2 pin since is it connected to the MMC2 connector pin number 52.


Cheers,
Angelo


--
Regards,
Alexandre