[PATCH 0/7] Add RZ/G3E SDHI support

[PATCH 0/7] Add RZ/G3E SDHI support

[Biju Das]

The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that
of the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
use SD_STATUS register to control voltage and power enable (internal
regulator).

For SD1 and SD2 channel we can either use gpio regulator or internal
regulator (using SD_STATUS register) for voltage switching.

Biju Das (7):
  dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support
  clk: renesas: r9a09g047: Add SDHI clocks/resets
  mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order
  mmc: renesas_sdhi: Add support for RZ/G3E SoC
  arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes
  arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}
  arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

 .../devicetree/bindings/mmc/renesas,sdhi.yaml |  20 +++
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |  57 ++++++++
 .../boot/dts/renesas/r9a09g047e57-smarc.dts   |  65 +++++++++
 .../boot/dts/renesas/renesas-smarc2.dtsi      |   9 ++
 .../boot/dts/renesas/rzg3e-smarc-som.dtsi     |  89 ++++++++++++
 drivers/clk/renesas/r9a09g047-cpg.c           |  31 +++++
 drivers/mmc/host/renesas_sdhi.h               |   1 +
 drivers/mmc/host/renesas_sdhi_core.c          | 131 +++++++++++++++++-
 drivers/mmc/host/tmio_mmc.h                   |   5 +
 9 files changed, 407 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Biju Das]

The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that
of the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
use SD_STATUS register to control voltage and power enable (internal
regulator).

For SD1 and SD2 channel we can either use gpio regulator or internal
regulator (using SD_STATUS register) for voltage switching.

Document RZ/G3E SDHI IP support.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../devicetree/bindings/mmc/renesas,sdhi.yaml | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index af378b9ff3f4..ef3acf0f58e0 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -68,6 +68,9 @@ properties:
               - renesas,sdhi-r9a08g045 # RZ/G3S
               - renesas,sdhi-r9a09g011 # RZ/V2M
           - const: renesas,rzg2l-sdhi
+      - items:
+          - const: renesas,sdhi-r9a09g047 # RZ/G3E
+          - const: renesas,sdhi-r9a09g057 # RZ/V2H(P)
 
   reg:
     maxItems: 1
@@ -124,6 +127,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - renesas,sdhi-r9a09g047
               - renesas,sdhi-r9a09g057
               - renesas,rzg2l-sdhi
     then:
@@ -211,6 +215,22 @@ allOf:
         sectioned off to be run by a separate second clock source to allow
         the main core clock to be turned off to save power.
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,sdhi-r9a09g047
+    then:
+      properties:
+        vqmmc-regulator:
+          type: object
+          description: VQMMC SD regulator
+          $ref: /schemas/regulator/regulator.yaml#
+          unevaluatedProperties: false
+
+      required:
+        - vqmmc-regulator
+
 required:
   - compatible
   - reg
-- 
2.43.0

[PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Biju Das]

Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
This patch depend upon [1]
[1] https://lore.kernel.org/all/20250120094715.25802-12-biju.das.jz@bp.renesas.com/
---
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 57 ++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
index 2023f70d3329..099d13b83f18 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
@@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 {
 			interrupt-controller;
 			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
 		};
+
+		sdhi0: mmc@15c00000  {
+			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
+			reg = <0x0 0x15c00000 0 0x10000>;
+			interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>,
+				 <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>;
+			clock-names = "core", "clkh", "cd", "aclk";
+			resets = <&cpg 0xa7>;
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			vqmmc_sdhi0: vqmmc-regulator {
+				regulator-name = "SDHI0-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+			};
+		};
+
+		sdhi1: mmc@15c10000 {
+			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
+			reg = <0x0 0x15c10000 0 0x10000>;
+			interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>,
+				 <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>;
+			clock-names = "core", "clkh", "cd", "aclk";
+			resets = <&cpg 0xa8>;
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			vqmmc_sdhi1: vqmmc-regulator {
+				regulator-name = "SDHI1-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+			};
+		};
+
+		sdhi2: mmc@15c20000 {
+			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
+			reg = <0x0 0x15c20000 0 0x10000>;
+			interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>,
+				 <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>;
+			clock-names = "core", "clkh", "cd", "aclk";
+			resets = <&cpg 0xa9>;
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			vqmmc_sdhi2: vqmmc-regulator {
+				regulator-name = "SDHI2-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+			};
+		};
 	};
 
 	timer {
-- 
2.43.0

[PATCH 6/7] arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}

[Biju Das]

Enable eMMC on SDHI0 and SD on SDHI2 on RZ/G3E SMARC SoM.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../boot/dts/renesas/rzg3e-smarc-som.dtsi     | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
index f4ba050beb0d..81f4f738482b 100644
--- a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
@@ -8,17 +8,79 @@
 / {
 	compatible = "renesas,rzg3e-smarcm", "renesas,r9a09g047e57", "renesas,r9a09g047";
 
+	aliases {
+		mmc0 = &sdhi0;
+		mmc2 = &sdhi2;
+	};
+
 	memory@48000000 {
 		device_type = "memory";
 		/* First 128MB is reserved for secure area. */
 		reg = <0x0 0x48000000 0x0 0xf8000000>;
 	};
+
+	reg_3p3v: regulator-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
 };
 
 &audio_extal_clk {
 	clock-frequency = <48000000>;
 };
 
+&pinctrl {
+	sdhi0_emmc_pins: sd0emmc {
+		sd0-emmc-ctrl {
+			pins = "SD0CLK", "SD0CMD";
+			renesas,output-impedance = <3>;
+		};
+
+		sd0-iovs {
+			pins = "SD0IOVS";
+			renesas,output-impedance = <3>;
+		};
+
+		sd0-emmc-data {
+			pins = "SD0DAT0", "SD0DAT1", "SD0DAT2", "SD0DAT3",
+			       "SD0DAT4", "SD0DAT5", "SD0DAT6", "SD0DAT7";
+			renesas,output-impedance = <3>;
+		};
+
+		sd0-emmc-rst {
+			pins = "SD0RSTN";
+			renesas,output-impedance = <3>;
+		};
+	};
+
+	sdhi2_pins: sd2 {
+		sd2-cd {
+			pinmux = <RZG3E_PORT_PINMUX(K, 0, 1)>; /* SD2CD */
+		};
+
+		sd2-io {
+			pinmux = <RZG3E_PORT_PINMUX(K, 1, 1)>, /* SD2IOVS */
+				 <RZG3E_PORT_PINMUX(K, 2, 1)>; /* SD2IOPWEN */
+		};
+
+		sd2-data {
+			pinmux = <RZG3E_PORT_PINMUX(H, 2, 1)>, /* SD2DAT0 */
+				 <RZG3E_PORT_PINMUX(H, 3, 1)>, /* SD2DAT1 */
+				 <RZG3E_PORT_PINMUX(H, 4, 1)>, /* SD2DAT2 */
+				 <RZG3E_PORT_PINMUX(H, 5, 1)>; /* SD2DAT3 */
+		};
+
+		sd2-ctrl {
+			pinmux = <RZG3E_PORT_PINMUX(H, 0, 1)>, /* SD2CLK */
+				 <RZG3E_PORT_PINMUX(H, 1, 1)>; /* SD2CMD */
+		};
+	};
+};
+
 &qextal_clk {
 	clock-frequency = <24000000>;
 };
@@ -27,6 +89,33 @@ &rtxin_clk {
 	clock-frequency = <32768>;
 };
 
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_emmc_pins>;
+	pinctrl-1 = <&sdhi0_emmc_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&vqmmc_sdhi0>;
+	bus-width = <8>;
+	mmc-hs200-1_8v;
+	non-removable;
+	fixed-emmc-driver-type = <1>;
+	status = "okay";
+};
+
+&sdhi2 {
+	pinctrl-0 = <&sdhi2_pins>;
+	pinctrl-1 = <&sdhi2_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&vqmmc_sdhi2>;
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+	status = "okay";
+};
+
 &wdt1 {
 	status = "okay";
 };
-- 
2.43.0

[PATCH 7/7] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

[Biju Das]

Enable SDHI1 on the RZ/G3E SMARC EVK platform using gpio regulator for
voltage switching.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../boot/dts/renesas/r9a09g047e57-smarc.dts   | 65 +++++++++++++++++++
 .../boot/dts/renesas/renesas-smarc2.dtsi      |  9 +++
 2 files changed, 74 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
index c063d47e2952..0e3d4ff31285 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
+++ b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
@@ -12,10 +12,40 @@
 #include "rzg3e-smarc-som.dtsi"
 #include "renesas-smarc2.dtsi"
 
+/*
+ * To enable uSD card on SDIO_USD:
+ *
+ * Switch bank - SW_OPT_MUX-1 (SW_SDIO_M2E):
+ *	0 - SMARC SDIO signal is connected to uSD1
+ *	1 - SMARC SDIO signal is connected to M.2 Key E connector
+ */
+
 / {
 	model = "Renesas SMARC EVK version 2 based on r9a09g047e57";
 	compatible = "renesas,smarc2-evk", "renesas,rzg3e-smarcm",
 		     "renesas,r9a09g047e57", "renesas,r9a09g047";
+
+	usd_vdd_3p3v: regulator-usd-vdd-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	vqmmc_sdhi1_ext: regulator-vqmmc-sdhi1-ext {
+		compatible = "regulator-gpio";
+
+		regulator-name = "SDHI1 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpios = <&pinctrl RZG3E_GPIO(1, 5) GPIO_ACTIVE_HIGH>;
+
+		gpios-states = <0>;
+		states = <3300000 0>, <1800000 1>;
+	};
 };
 
 &pinctrl {
@@ -23,9 +53,44 @@ scif_pins: scif {
 		pins = "SCIF_TXD", "SCIF_RXD";
 		renesas,output-impedance = <1>;
 	};
+
+	sd1-pwr-en {
+		gpio-hog;
+		gpios = <RZG3E_GPIO(1, 6) GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "sd1_pwr_en";
+	};
+
+	sdhi1_pins: sd1 {
+		sd1-cd {
+			pinmux = <RZG3E_PORT_PINMUX(1, 4, 8)>; /* SD1CD */
+		};
+
+		sd1-data {
+			pinmux = <RZG3E_PORT_PINMUX(G, 2, 1)>, /* SD1DAT0 */
+				 <RZG3E_PORT_PINMUX(G, 3, 1)>, /* SD1DAT1 */
+				 <RZG3E_PORT_PINMUX(G, 4, 1)>, /* SD1DAT2 */
+				 <RZG3E_PORT_PINMUX(G, 5, 1)>; /* SD1DAT3 */
+		};
+
+		sd1-ctrl {
+			pinmux = <RZG3E_PORT_PINMUX(G, 0, 1)>, /* SD1CLK */
+				 <RZG3E_PORT_PINMUX(G, 1, 1)>; /* SD1CMD */
+		};
+	};
 };
 
 &scif0 {
 	pinctrl-0 = <&scif_pins>;
 	pinctrl-names = "default";
 };
+
+&sdhi1 {
+	pinctrl-0 = <&sdhi1_pins>;
+	pinctrl-1 = <&sdhi1_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&usd_vdd_3p3v>;
+	vqmmc-supply = <&vqmmc_sdhi1_ext>;
+	/delete-node/ vqmmc_regulator;
+};
diff --git a/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi b/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
index e378d55e6e9b..ec79452393b0 100644
--- a/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
+++ b/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
@@ -16,9 +16,18 @@ chosen {
 
 	aliases {
 		serial3 = &scif0;
+		mmc1 = &sdhi1;
 	};
 };
 
 &scif0 {
 	status = "okay";
 };
+
+&sdhi1 {
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+
+	status = "okay";
+};
-- 
2.43.0

Re: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Tommaso Merciai]

Hi Biju,

Thanks for the patch.

On Sun, Jan 26, 2025 at 01:46:03PM +0000, Biju Das wrote:
> The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that
> of the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
> use SD_STATUS register to control voltage and power enable (internal
> regulator).
> 
> For SD1 and SD2 channel we can either use gpio regulator or internal
> regulator (using SD_STATUS register) for voltage switching.
> 
> Document RZ/G3E SDHI IP support.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> index af378b9ff3f4..ef3acf0f58e0 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -68,6 +68,9 @@ properties:
>                - renesas,sdhi-r9a08g045 # RZ/G3S
>                - renesas,sdhi-r9a09g011 # RZ/V2M
>            - const: renesas,rzg2l-sdhi
> +      - items:
> +          - const: renesas,sdhi-r9a09g047 # RZ/G3E
> +          - const: renesas,sdhi-r9a09g057 # RZ/V2H(P)
>  
>    reg:
>      maxItems: 1
> @@ -124,6 +127,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - renesas,sdhi-r9a09g047
>                - renesas,sdhi-r9a09g057
>                - renesas,rzg2l-sdhi
>      then:
> @@ -211,6 +215,22 @@ allOf:
>          sectioned off to be run by a separate second clock source to allow
>          the main core clock to be turned off to save power.
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,sdhi-r9a09g047
> +    then:
> +      properties:
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +      required:
> +        - vqmmc-regulator
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.43.0
> 

Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Tommaso Merciai]

On Sun, Jan 26, 2025 at 01:46:07PM +0000, Biju Das wrote:
> Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> This patch depend upon [1]
> [1] https://lore.kernel.org/all/20250120094715.25802-12-biju.das.jz@bp.renesas.com/
> ---
>  arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 57 ++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> index 2023f70d3329..099d13b83f18 100644
> --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 {
>  			interrupt-controller;
>  			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
>  		};
> +
> +		sdhi0: mmc@15c00000  {
> +			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> +			reg = <0x0 0x15c00000 0 0x10000>;
> +			interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>,
> +				 <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>;
> +			clock-names = "core", "clkh", "cd", "aclk";
> +			resets = <&cpg 0xa7>;
> +			power-domains = <&cpg>;
> +			status = "disabled";
> +
> +			vqmmc_sdhi0: vqmmc-regulator {
> +				regulator-name = "SDHI0-VQMMC";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +		};
> +
> +		sdhi1: mmc@15c10000 {
> +			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> +			reg = <0x0 0x15c10000 0 0x10000>;
> +			interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>,
> +				 <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>;
> +			clock-names = "core", "clkh", "cd", "aclk";
> +			resets = <&cpg 0xa8>;
> +			power-domains = <&cpg>;
> +			status = "disabled";
> +
> +			vqmmc_sdhi1: vqmmc-regulator {
> +				regulator-name = "SDHI1-VQMMC";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +		};
> +
> +		sdhi2: mmc@15c20000 {
> +			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> +			reg = <0x0 0x15c20000 0 0x10000>;
> +			interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>,
> +				 <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>;
> +			clock-names = "core", "clkh", "cd", "aclk";
> +			resets = <&cpg 0xa9>;
> +			power-domains = <&cpg>;
> +			status = "disabled";
> +
> +			vqmmc_sdhi2: vqmmc-regulator {
> +				regulator-name = "SDHI2-VQMMC";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +			};
> +		};
>  	};
>  
>  	timer {
> -- 
> 2.43.0
> 
Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Re: [PATCH 6/7] arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}

[Tommaso Merciai]

On Sun, Jan 26, 2025 at 01:46:08PM +0000, Biju Das wrote:
> Enable eMMC on SDHI0 and SD on SDHI2 on RZ/G3E SMARC SoM.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../boot/dts/renesas/rzg3e-smarc-som.dtsi     | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
> index f4ba050beb0d..81f4f738482b 100644
> --- a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
> @@ -8,17 +8,79 @@
>  / {
>  	compatible = "renesas,rzg3e-smarcm", "renesas,r9a09g047e57", "renesas,r9a09g047";
>  
> +	aliases {
> +		mmc0 = &sdhi0;
> +		mmc2 = &sdhi2;
> +	};
> +
>  	memory@48000000 {
>  		device_type = "memory";
>  		/* First 128MB is reserved for secure area. */
>  		reg = <0x0 0x48000000 0x0 0xf8000000>;
>  	};
> +
> +	reg_3p3v: regulator-3p3v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "fixed-3.3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
>  };
>  
>  &audio_extal_clk {
>  	clock-frequency = <48000000>;
>  };
>  
> +&pinctrl {
> +	sdhi0_emmc_pins: sd0emmc {
> +		sd0-emmc-ctrl {
> +			pins = "SD0CLK", "SD0CMD";
> +			renesas,output-impedance = <3>;
> +		};
> +
> +		sd0-iovs {
> +			pins = "SD0IOVS";
> +			renesas,output-impedance = <3>;
> +		};
> +
> +		sd0-emmc-data {
> +			pins = "SD0DAT0", "SD0DAT1", "SD0DAT2", "SD0DAT3",
> +			       "SD0DAT4", "SD0DAT5", "SD0DAT6", "SD0DAT7";
> +			renesas,output-impedance = <3>;
> +		};
> +
> +		sd0-emmc-rst {
> +			pins = "SD0RSTN";
> +			renesas,output-impedance = <3>;
> +		};
> +	};
> +
> +	sdhi2_pins: sd2 {
> +		sd2-cd {
> +			pinmux = <RZG3E_PORT_PINMUX(K, 0, 1)>; /* SD2CD */
> +		};
> +
> +		sd2-io {
> +			pinmux = <RZG3E_PORT_PINMUX(K, 1, 1)>, /* SD2IOVS */
> +				 <RZG3E_PORT_PINMUX(K, 2, 1)>; /* SD2IOPWEN */
> +		};
> +
> +		sd2-data {
> +			pinmux = <RZG3E_PORT_PINMUX(H, 2, 1)>, /* SD2DAT0 */
> +				 <RZG3E_PORT_PINMUX(H, 3, 1)>, /* SD2DAT1 */
> +				 <RZG3E_PORT_PINMUX(H, 4, 1)>, /* SD2DAT2 */
> +				 <RZG3E_PORT_PINMUX(H, 5, 1)>; /* SD2DAT3 */
> +		};
> +
> +		sd2-ctrl {
> +			pinmux = <RZG3E_PORT_PINMUX(H, 0, 1)>, /* SD2CLK */
> +				 <RZG3E_PORT_PINMUX(H, 1, 1)>; /* SD2CMD */
> +		};
> +	};
> +};
> +
>  &qextal_clk {
>  	clock-frequency = <24000000>;
>  };
> @@ -27,6 +89,33 @@ &rtxin_clk {
>  	clock-frequency = <32768>;
>  };
>  
> +&sdhi0 {
> +	pinctrl-0 = <&sdhi0_emmc_pins>;
> +	pinctrl-1 = <&sdhi0_emmc_pins>;
> +	pinctrl-names = "default", "state_uhs";
> +
> +	vmmc-supply = <&reg_3p3v>;
> +	vqmmc-supply = <&vqmmc_sdhi0>;
> +	bus-width = <8>;
> +	mmc-hs200-1_8v;
> +	non-removable;
> +	fixed-emmc-driver-type = <1>;
> +	status = "okay";
> +};
> +
> +&sdhi2 {
> +	pinctrl-0 = <&sdhi2_pins>;
> +	pinctrl-1 = <&sdhi2_pins>;
> +	pinctrl-names = "default", "state_uhs";
> +
> +	vmmc-supply = <&reg_3p3v>;
> +	vqmmc-supply = <&vqmmc_sdhi2>;
> +	bus-width = <4>;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
> +	status = "okay";
> +};
> +
>  &wdt1 {
>  	status = "okay";
>  };
> -- 
> 2.43.0
> 

Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Re: [PATCH 7/7] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

[Tommaso Merciai]

On Sun, Jan 26, 2025 at 01:46:09PM +0000, Biju Das wrote:
> Enable SDHI1 on the RZ/G3E SMARC EVK platform using gpio regulator for
> voltage switching.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../boot/dts/renesas/r9a09g047e57-smarc.dts   | 65 +++++++++++++++++++
>  .../boot/dts/renesas/renesas-smarc2.dtsi      |  9 +++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> index c063d47e2952..0e3d4ff31285 100644
> --- a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> +++ b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> @@ -12,10 +12,40 @@
>  #include "rzg3e-smarc-som.dtsi"
>  #include "renesas-smarc2.dtsi"
>  
> +/*
> + * To enable uSD card on SDIO_USD:
> + *
> + * Switch bank - SW_OPT_MUX-1 (SW_SDIO_M2E):
> + *	0 - SMARC SDIO signal is connected to uSD1
> + *	1 - SMARC SDIO signal is connected to M.2 Key E connector
> + */
> +
>  / {
>  	model = "Renesas SMARC EVK version 2 based on r9a09g047e57";
>  	compatible = "renesas,smarc2-evk", "renesas,rzg3e-smarcm",
>  		     "renesas,r9a09g047e57", "renesas,r9a09g047";
> +
> +	usd_vdd_3p3v: regulator-usd-vdd-3p3v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "fixed-3.3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
> +
> +	vqmmc_sdhi1_ext: regulator-vqmmc-sdhi1-ext {
> +		compatible = "regulator-gpio";
> +
> +		regulator-name = "SDHI1 VccQ";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		gpios = <&pinctrl RZG3E_GPIO(1, 5) GPIO_ACTIVE_HIGH>;
> +
> +		gpios-states = <0>;
> +		states = <3300000 0>, <1800000 1>;
> +	};
>  };
>  
>  &pinctrl {
> @@ -23,9 +53,44 @@ scif_pins: scif {
>  		pins = "SCIF_TXD", "SCIF_RXD";
>  		renesas,output-impedance = <1>;
>  	};
> +
> +	sd1-pwr-en {
> +		gpio-hog;
> +		gpios = <RZG3E_GPIO(1, 6) GPIO_ACTIVE_HIGH>;
> +		output-high;
> +		line-name = "sd1_pwr_en";
> +	};
> +
> +	sdhi1_pins: sd1 {
> +		sd1-cd {
> +			pinmux = <RZG3E_PORT_PINMUX(1, 4, 8)>; /* SD1CD */
> +		};
> +
> +		sd1-data {
> +			pinmux = <RZG3E_PORT_PINMUX(G, 2, 1)>, /* SD1DAT0 */
> +				 <RZG3E_PORT_PINMUX(G, 3, 1)>, /* SD1DAT1 */
> +				 <RZG3E_PORT_PINMUX(G, 4, 1)>, /* SD1DAT2 */
> +				 <RZG3E_PORT_PINMUX(G, 5, 1)>; /* SD1DAT3 */
> +		};
> +
> +		sd1-ctrl {
> +			pinmux = <RZG3E_PORT_PINMUX(G, 0, 1)>, /* SD1CLK */
> +				 <RZG3E_PORT_PINMUX(G, 1, 1)>; /* SD1CMD */
> +		};
> +	};
>  };
>  
>  &scif0 {
>  	pinctrl-0 = <&scif_pins>;
>  	pinctrl-names = "default";
>  };
> +
> +&sdhi1 {
> +	pinctrl-0 = <&sdhi1_pins>;
> +	pinctrl-1 = <&sdhi1_pins>;
> +	pinctrl-names = "default", "state_uhs";
> +
> +	vmmc-supply = <&usd_vdd_3p3v>;
> +	vqmmc-supply = <&vqmmc_sdhi1_ext>;
> +	/delete-node/ vqmmc_regulator;
> +};
> diff --git a/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi b/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
> index e378d55e6e9b..ec79452393b0 100644
> --- a/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
> +++ b/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
> @@ -16,9 +16,18 @@ chosen {
>  
>  	aliases {
>  		serial3 = &scif0;
> +		mmc1 = &sdhi1;
>  	};
>  };
>  
>  &scif0 {
>  	status = "okay";
>  };
> +
> +&sdhi1 {
> +	bus-width = <4>;
> +	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
> +
> +	status = "okay";
> +};
> -- 
> 2.43.0
> 
Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Re: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Rob Herring]

On Sun, Jan 26, 2025 at 01:46:03PM +0000, Biju Das wrote:
> The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that
> of the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
> use SD_STATUS register to control voltage and power enable (internal
> regulator).
> 
> For SD1 and SD2 channel we can either use gpio regulator or internal
> regulator (using SD_STATUS register) for voltage switching.
> 
> Document RZ/G3E SDHI IP support.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> index af378b9ff3f4..ef3acf0f58e0 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -68,6 +68,9 @@ properties:
>                - renesas,sdhi-r9a08g045 # RZ/G3S
>                - renesas,sdhi-r9a09g011 # RZ/V2M
>            - const: renesas,rzg2l-sdhi
> +      - items:
> +          - const: renesas,sdhi-r9a09g047 # RZ/G3E
> +          - const: renesas,sdhi-r9a09g057 # RZ/V2H(P)
>  
>    reg:
>      maxItems: 1
> @@ -124,6 +127,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - renesas,sdhi-r9a09g047
>                - renesas,sdhi-r9a09g057
>                - renesas,rzg2l-sdhi
>      then:
> @@ -211,6 +215,22 @@ allOf:
>          sectioned off to be run by a separate second clock source to allow
>          the main core clock to be turned off to save power.
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,sdhi-r9a09g047
> +    then:
> +      properties:
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +      required:
> +        - vqmmc-regulator

The driver treats this as optional. If this is required, then is 
renesas,sdhi-r9a09g047 really compatible with renesas,sdhi-r9a09g057?

Rob

RE: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Biju Das]

Hi Rob,

Thanks for the feedback.

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 27 January 2025 19:16
> Subject: Re: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support
> 
> On Sun, Jan 26, 2025 at 01:46:03PM +0000, Biju Das wrote:
> > The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that of
> > the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
> > use SD_STATUS register to control voltage and power enable (internal
> > regulator).
> >
> > For SD1 and SD2 channel we can either use gpio regulator or internal
> > regulator (using SD_STATUS register) for voltage switching.
> >
> > Document RZ/G3E SDHI IP support.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 20
> > +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > index af378b9ff3f4..ef3acf0f58e0 100644
> > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > @@ -68,6 +68,9 @@ properties:
> >                - renesas,sdhi-r9a08g045 # RZ/G3S
> >                - renesas,sdhi-r9a09g011 # RZ/V2M
> >            - const: renesas,rzg2l-sdhi
> > +      - items:
> > +          - const: renesas,sdhi-r9a09g047 # RZ/G3E
> > +          - const: renesas,sdhi-r9a09g057 # RZ/V2H(P)
> >
> >    reg:
> >      maxItems: 1
> > @@ -124,6 +127,7 @@ allOf:
> >          compatible:
> >            contains:
> >              enum:
> > +              - renesas,sdhi-r9a09g047
> >                - renesas,sdhi-r9a09g057
> >                - renesas,rzg2l-sdhi
> >      then:
> > @@ -211,6 +215,22 @@ allOf:
> >          sectioned off to be run by a separate second clock source to allow
> >          the main core clock to be turned off to save power.
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,sdhi-r9a09g047
> > +    then:
> > +      properties:
> > +        vqmmc-regulator:
> > +          type: object
> > +          description: VQMMC SD regulator
> > +          $ref: /schemas/regulator/regulator.yaml#
> > +          unevaluatedProperties: false
> > +
> > +      required:
> > +        - vqmmc-regulator
> 
> The driver treats this as optional. If this is required, then is
> renesas,sdhi-r9a09g047 really compatible with renesas,sdhi-r9a09g057?

Ok, I will make it optional for both renesas,sdhi-r9a09g057 and renesas,sdhi-r9a09g047
in the next version.

Cheers,
Biju

Re: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Geert Uytterhoeven]

Hi Biju,

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that
> of the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
> use SD_STATUS register to control voltage and power enable (internal
> regulator).
>
> For SD1 and SD2 channel we can either use gpio regulator or internal
> regulator (using SD_STATUS register) for voltage switching.
> Document RZ/G3E SDHI IP support.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -68,6 +68,9 @@ properties:
>                - renesas,sdhi-r9a08g045 # RZ/G3S
>                - renesas,sdhi-r9a09g011 # RZ/V2M
>            - const: renesas,rzg2l-sdhi
> +      - items:
> +          - const: renesas,sdhi-r9a09g047 # RZ/G3E
> +          - const: renesas,sdhi-r9a09g057 # RZ/V2H(P)

OK... but...

>
>    reg:
>      maxItems: 1
> @@ -124,6 +127,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - renesas,sdhi-r9a09g047
>                - renesas,sdhi-r9a09g057
>                - renesas,rzg2l-sdhi
>      then:
> @@ -211,6 +215,22 @@ allOf:
>          sectioned off to be run by a separate second clock source to allow
>          the main core clock to be turned off to save power.
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,sdhi-r9a09g047
> +    then:
> +      properties:
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +      required:
> +        - vqmmc-regulator
> +
>  required:
>    - compatible
>    - reg

Given RZ/V2H can use the internal regulator control, too, I think it
can be optional on both.  Then renesas,sdhi-r9a09g047 can just use
renesas,sdhi-r9a09g057 as a fallback compatible.

Note for the casual reader: as the related pins can be used as GPIOs
on all RZ/V2H SD channels, the initial idea to add support for the
internal regulator control was dropped, and replaced by the simpler
solution of using a gpio-regulator.  Unfortunately that simple option
is not available for SD0 on RZ/G3E.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Geert Uytterhoeven]

Hi Biju,

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 {
>                         interrupt-controller;
>                         interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
>                 };
> +
> +               sdhi0: mmc@15c00000  {
> +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> +                       reg = <0x0 0x15c00000 0 0x10000>;
> +                       interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>,
> +                                <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>;
> +                       clock-names = "core", "clkh", "cd", "aclk";
> +                       resets = <&cpg 0xa7>;
> +                       power-domains = <&cpg>;
> +                       status = "disabled";
> +
> +                       vqmmc_sdhi0: vqmmc-regulator {
> +                               regulator-name = "SDHI0-VQMMC";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3300000>;
> +                       };
> +               };
> +
> +               sdhi1: mmc@15c10000 {
> +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> +                       reg = <0x0 0x15c10000 0 0x10000>;
> +                       interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>,
> +                                <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>;
> +                       clock-names = "core", "clkh", "cd", "aclk";
> +                       resets = <&cpg 0xa8>;
> +                       power-domains = <&cpg>;
> +                       status = "disabled";
> +
> +                       vqmmc_sdhi1: vqmmc-regulator {
> +                               regulator-name = "SDHI1-VQMMC";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3300000>;
> +                       };
> +               };
> +
> +               sdhi2: mmc@15c20000 {
> +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> +                       reg = <0x0 0x15c20000 0 0x10000>;
> +                       interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>,
> +                                <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>;
> +                       clock-names = "core", "clkh", "cd", "aclk";
> +                       resets = <&cpg 0xa9>;
> +                       power-domains = <&cpg>;
> +                       status = "disabled";
> +
> +                       vqmmc_sdhi2: vqmmc-regulator {
> +                               regulator-name = "SDHI2-VQMMC";
> +                               regulator-min-microvolt = <1800000>;
> +                               regulator-max-microvolt = <3300000>;
> +                       };
> +               };
>         };
>
>         timer {

Shouldn't the vqmmc-regulator subnodes be added in the board DTS,
when needed (i.e. at least for SDHI[12])? Or do you expect the board DTS
to /delete-node/ them when they are not needed?

Is it possible that SDHI0 does not need the regulator control, e.g.
in case of a fixed voltage?

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 11:33
Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> biju.das.au <biju.das.au@gmail.com>
> Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes
> 
> Hi Biju,
> 
> On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 {
> >                         interrupt-controller;
> >                         interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
> >                 };
> > +
> > +               sdhi0: mmc@15c00000  {
> > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > +                       reg = <0x0 0x15c00000 0 0x10000>;
> > +                       interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>,
> > +                                <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>;
> > +                       clock-names = "core", "clkh", "cd", "aclk";
> > +                       resets = <&cpg 0xa7>;
> > +                       power-domains = <&cpg>;
> > +                       status = "disabled";
> > +
> > +                       vqmmc_sdhi0: vqmmc-regulator {
> > +                               regulator-name = "SDHI0-VQMMC";
> > +                               regulator-min-microvolt = <1800000>;
> > +                               regulator-max-microvolt = <3300000>;
> > +                       };
> > +               };
> > +
> > +               sdhi1: mmc@15c10000 {
> > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > +                       reg = <0x0 0x15c10000 0 0x10000>;
> > +                       interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>,
> > +                                <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>;
> > +                       clock-names = "core", "clkh", "cd", "aclk";
> > +                       resets = <&cpg 0xa8>;
> > +                       power-domains = <&cpg>;
> > +                       status = "disabled";
> > +
> > +                       vqmmc_sdhi1: vqmmc-regulator {
> > +                               regulator-name = "SDHI1-VQMMC";
> > +                               regulator-min-microvolt = <1800000>;
> > +                               regulator-max-microvolt = <3300000>;
> > +                       };
> > +               };
> > +
> > +               sdhi2: mmc@15c20000 {
> > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > +                       reg = <0x0 0x15c20000 0 0x10000>;
> > +                       interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>,
> > +                                    <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>;
> > +                       clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>,
> > +                                <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>;
> > +                       clock-names = "core", "clkh", "cd", "aclk";
> > +                       resets = <&cpg 0xa9>;
> > +                       power-domains = <&cpg>;
> > +                       status = "disabled";
> > +
> > +                       vqmmc_sdhi2: vqmmc-regulator {
> > +                               regulator-name = "SDHI2-VQMMC";
> > +                               regulator-min-microvolt = <1800000>;
> > +                               regulator-max-microvolt = <3300000>;
> > +                       };
> > +               };
> >         };
> >
> >         timer {
> 
> Shouldn't the vqmmc-regulator subnodes be added in the board DTS, when needed (i.e. at least for
> SDHI[12])? Or do you expect the board DTS to /delete-node/ them when they are not needed?

I agree.

I have provided an example in next patch using /delete-node/ to use gpio-regulator.
I am ok for moving it to the board DTS as well. When I sent patch, I am not sure which
is the best in terms of user point of view?

Now I got the answer to move vqmmc-regulator subnodes to add in the board DTS for
atleast SDHI[12]. I will address this in next version.

Even for SDHI0 fix type, if we plan to use fixed regulator for eMMC?

> 
> Is it possible that SDHI0 does not need the regulator control, e.g.
> in case of a fixed voltage?

Yes, for eMMC(fixed case) it is not needed. 

Without Regulator:
------------------

root@smarc-rzg3e:~# /sdhi_test.sh
Read at address  0x15C003C8 (0xffffb3f983c8): 0x00000001

[    1.884227] mmc0: new HS200 MMC card at address 0001
[    1.890480] mmcblk0: mmc0:0001 0IM20F 59.3 GiB
[    1.898777]  mmcblk0: p1

$ dd if=/dev/urandom of=/tmp/mmcblk0p1-random bs=1024 count=10240
10240+0 records in
10240+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.131063 s, 80.0 MB/s

With regulator:
----------------
root@smarc-rzg3e:~# /sdhi_test.sh
Read at address  0x15C003C8 (0xffffa1ca73c8): 0x00010001

[    1.853728] mmc0: new HS200 MMC card at address 0001
[    1.864529] mmcblk0: mmc0:0001 0IM20F 59.3 GiB
[    1.873859]  mmcblk0: p1

$ dd if=/dev/urandom of=/tmp/mmcblk0p1-random bs=1024 count=10240
10240+0 records in
10240+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.131909 s, 79.5 MB/s


Cheers,
Biju

RE: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 11:16
> Subject: Re: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support
> 
> Hi Biju,
> 
> On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that of
> > the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
> > use SD_STATUS register to control voltage and power enable (internal
> > regulator).
> >
> > For SD1 and SD2 channel we can either use gpio regulator or internal
> > regulator (using SD_STATUS register) for voltage switching.
> > Document RZ/G3E SDHI IP support.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > @@ -68,6 +68,9 @@ properties:
> >                - renesas,sdhi-r9a08g045 # RZ/G3S
> >                - renesas,sdhi-r9a09g011 # RZ/V2M
> >            - const: renesas,rzg2l-sdhi
> > +      - items:
> > +          - const: renesas,sdhi-r9a09g047 # RZ/G3E
> > +          - const: renesas,sdhi-r9a09g057 # RZ/V2H(P)
> 
> OK... but...
> 
> >
> >    reg:
> >      maxItems: 1
> > @@ -124,6 +127,7 @@ allOf:
> >          compatible:
> >            contains:
> >              enum:
> > +              - renesas,sdhi-r9a09g047
> >                - renesas,sdhi-r9a09g057
> >                - renesas,rzg2l-sdhi
> >      then:
> > @@ -211,6 +215,22 @@ allOf:
> >          sectioned off to be run by a separate second clock source to allow
> >          the main core clock to be turned off to save power.
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,sdhi-r9a09g047
> > +    then:
> > +      properties:
> > +        vqmmc-regulator:
> > +          type: object
> > +          description: VQMMC SD regulator
> > +          $ref: /schemas/regulator/regulator.yaml#
> > +          unevaluatedProperties: false
> > +
> > +      required:
> > +        - vqmmc-regulator
> > +
> >  required:
> >    - compatible
> >    - reg
> 
> Given RZ/V2H can use the internal regulator control, too, I think it can be optional on both.  Then
> renesas,sdhi-r9a09g047 can just use
> renesas,sdhi-r9a09g057 as a fallback compatible.

Agreed, will make internal regulator control. optional for both RZ/G2H and RZ/G3E
and just use renesas,sdhi-r9a09g057 as a fallback compatible for renesas,sdhi-r9a09g047.

Cheers,
Biju

Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Geert Uytterhoeven]

Hi Biju,

On Tue, 28 Jan 2025 at 13:11, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: 28 January 2025 11:33
> Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> > biju.das.au <biju.das.au@gmail.com>
> > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes
> >
> > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> > > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 {
> > >                         interrupt-controller;
> > >                         interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
> > >                 };
> > > +
> > > +               sdhi0: mmc@15c00000  {
> > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > +                       reg = <0x0 0x15c00000 0 0x10000>;
> > > +                       interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>,
> > > +                                    <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>;
> > > +                       clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>,
> > > +                                <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>;
> > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > +                       resets = <&cpg 0xa7>;
> > > +                       power-domains = <&cpg>;
> > > +                       status = "disabled";
> > > +
> > > +                       vqmmc_sdhi0: vqmmc-regulator {
> > > +                               regulator-name = "SDHI0-VQMMC";
> > > +                               regulator-min-microvolt = <1800000>;
> > > +                               regulator-max-microvolt = <3300000>;
> > > +                       };
> > > +               };
> > > +
> > > +               sdhi1: mmc@15c10000 {
> > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > +                       reg = <0x0 0x15c10000 0 0x10000>;
> > > +                       interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>,
> > > +                                    <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>;
> > > +                       clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>,
> > > +                                <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>;
> > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > +                       resets = <&cpg 0xa8>;
> > > +                       power-domains = <&cpg>;
> > > +                       status = "disabled";
> > > +
> > > +                       vqmmc_sdhi1: vqmmc-regulator {
> > > +                               regulator-name = "SDHI1-VQMMC";
> > > +                               regulator-min-microvolt = <1800000>;
> > > +                               regulator-max-microvolt = <3300000>;
> > > +                       };
> > > +               };
> > > +
> > > +               sdhi2: mmc@15c20000 {
> > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > +                       reg = <0x0 0x15c20000 0 0x10000>;
> > > +                       interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>,
> > > +                                    <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>;
> > > +                       clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>,
> > > +                                <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>;
> > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > +                       resets = <&cpg 0xa9>;
> > > +                       power-domains = <&cpg>;
> > > +                       status = "disabled";
> > > +
> > > +                       vqmmc_sdhi2: vqmmc-regulator {
> > > +                               regulator-name = "SDHI2-VQMMC";
> > > +                               regulator-min-microvolt = <1800000>;
> > > +                               regulator-max-microvolt = <3300000>;
> > > +                       };
> > > +               };
> > >         };
> > >
> > >         timer {
> >
> > Shouldn't the vqmmc-regulator subnodes be added in the board DTS, when needed (i.e. at least for
> > SDHI[12])? Or do you expect the board DTS to /delete-node/ them when they are not needed?
>
> I agree.
>
> I have provided an example in next patch using /delete-node/ to use gpio-regulator.

Ah, my fault trying to get my reviews out sooner rather than later ;-)

> I am ok for moving it to the board DTS as well. When I sent patch, I am not sure which
> is the best in terms of user point of view?
>
> Now I got the answer to move vqmmc-regulator subnodes to add in the board DTS for
> atleast SDHI[12]. I will address this in next version.
>
> Even for SDHI0 fix type, if we plan to use fixed regulator for eMMC?
>
> >
> > Is it possible that SDHI0 does not need the regulator control, e.g.
> > in case of a fixed voltage?
>
> Yes, for eMMC(fixed case) it is not needed.

Upon second thought: as the internal regulator is always present, what
about setting its status to disabled in the SoC .dtsi, and changing
it to okay in the board DTS when needed, like done for other
components?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 6/7] arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}

[Geert Uytterhoeven]

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Enable eMMC on SDHI0 and SD on SDHI2 on RZ/G3E SMARC SoM.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 7/7] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

[Geert Uytterhoeven]

Hi Biju,

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Enable SDHI1 on the RZ/G3E SMARC EVK platform using gpio regulator for
> voltage switching.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> +++ b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> @@ -12,10 +12,40 @@
>  #include "rzg3e-smarc-som.dtsi"
>  #include "renesas-smarc2.dtsi"
>
> +/*
> + * To enable uSD card on SDIO_USD:
> + *
> + * Switch bank - SW_OPT_MUX-1 (SW_SDIO_M2E):
> + *     0 - SMARC SDIO signal is connected to uSD1
> + *     1 - SMARC SDIO signal is connected to M.2 Key E connector
> + */
> +
>  / {
>         model = "Renesas SMARC EVK version 2 based on r9a09g047e57";
>         compatible = "renesas,smarc2-evk", "renesas,rzg3e-smarcm",
>                      "renesas,r9a09g047e57", "renesas,r9a09g047";
> +
> +       usd_vdd_3p3v: regulator-usd-vdd-3p3v {
> +               compatible = "regulator-fixed";
> +               regulator-name = "fixed-3.3V";
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +               regulator-boot-on;
> +               regulator-always-on;
> +       };

Can't you just reuse reg_3p3v from rzg3e-smarc-som.dtsi?

> +
> +       vqmmc_sdhi1_ext: regulator-vqmmc-sdhi1-ext {
> +               compatible = "regulator-gpio";
> +
> +               regulator-name = "SDHI1 VccQ";
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <3300000>;
> +
> +               gpios = <&pinctrl RZG3E_GPIO(1, 5) GPIO_ACTIVE_HIGH>;

arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts:44.37-38 syntax error

Missing #include <dt-bindings/gpio/gpio.h>

> +
> +               gpios-states = <0>;
> +               states = <3300000 0>, <1800000 1>;
> +       };

Is there a specific reason why you are using an external regulator...

>  };
>
>  &pinctrl {
> @@ -23,9 +53,44 @@ scif_pins: scif {
>                 pins = "SCIF_TXD", "SCIF_RXD";
>                 renesas,output-impedance = <1>;
>         };
> +
> +       sd1-pwr-en {
> +               gpio-hog;
> +               gpios = <RZG3E_GPIO(1, 6) GPIO_ACTIVE_HIGH>;
> +               output-high;
> +               line-name = "sd1_pwr_en";
> +       };

... and external (fixed) power enable, instead of the internal regulator,
which uses the same two pins?

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 13:26
> Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes
> 
> Hi Biju,
> 
> On Tue, 28 Jan 2025 at 13:11, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: 28 January 2025 11:33
> > Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> > > biju.das.au <biju.das.au@gmail.com>
> > > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add
> > > SDHI0-SDHI2 nodes
> > >
> > > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> > > > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 {
> > > >                         interrupt-controller;
> > > >                         interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
> > > >                 };
> > > > +
> > > > +               sdhi0: mmc@15c00000  {
> > > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > > +                       reg = <0x0 0x15c00000 0 0x10000>;
> > > > +                       interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                                    <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                       clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>,
> > > > +                                <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>;
> > > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > > +                       resets = <&cpg 0xa7>;
> > > > +                       power-domains = <&cpg>;
> > > > +                       status = "disabled";
> > > > +
> > > > +                       vqmmc_sdhi0: vqmmc-regulator {
> > > > +                               regulator-name = "SDHI0-VQMMC";
> > > > +                               regulator-min-microvolt = <1800000>;
> > > > +                               regulator-max-microvolt = <3300000>;
> > > > +                       };
> > > > +               };
> > > > +
> > > > +               sdhi1: mmc@15c10000 {
> > > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > > +                       reg = <0x0 0x15c10000 0 0x10000>;
> > > > +                       interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                                    <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                       clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>,
> > > > +                                <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>;
> > > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > > +                       resets = <&cpg 0xa8>;
> > > > +                       power-domains = <&cpg>;
> > > > +                       status = "disabled";
> > > > +
> > > > +                       vqmmc_sdhi1: vqmmc-regulator {
> > > > +                               regulator-name = "SDHI1-VQMMC";
> > > > +                               regulator-min-microvolt = <1800000>;
> > > > +                               regulator-max-microvolt = <3300000>;
> > > > +                       };
> > > > +               };
> > > > +
> > > > +               sdhi2: mmc@15c20000 {
> > > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > > +                       reg = <0x0 0x15c20000 0 0x10000>;
> > > > +                       interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                                    <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                       clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>,
> > > > +                                <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>;
> > > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > > +                       resets = <&cpg 0xa9>;
> > > > +                       power-domains = <&cpg>;
> > > > +                       status = "disabled";
> > > > +
> > > > +                       vqmmc_sdhi2: vqmmc-regulator {
> > > > +                               regulator-name = "SDHI2-VQMMC";
> > > > +                               regulator-min-microvolt = <1800000>;
> > > > +                               regulator-max-microvolt = <3300000>;
> > > > +                       };
> > > > +               };
> > > >         };
> > > >
> > > >         timer {
> > >
> > > Shouldn't the vqmmc-regulator subnodes be added in the board DTS,
> > > when needed (i.e. at least for SDHI[12])? Or do you expect the board DTS to /delete-node/ them
> when they are not needed?
> >
> > I agree.
> >
> > I have provided an example in next patch using /delete-node/ to use gpio-regulator.
> 
> Ah, my fault trying to get my reviews out sooner rather than later ;-)
> 
> > I am ok for moving it to the board DTS as well. When I sent patch, I
> > am not sure which is the best in terms of user point of view?
> >
> > Now I got the answer to move vqmmc-regulator subnodes to add in the
> > board DTS for atleast SDHI[12]. I will address this in next version.
> >
> > Even for SDHI0 fix type, if we plan to use fixed regulator for eMMC?
> >
> > >
> > > Is it possible that SDHI0 does not need the regulator control, e.g.
> > > in case of a fixed voltage?
> >
> > Yes, for eMMC(fixed case) it is not needed.
> 
> Upon second thought: as the internal regulator is always present, what about setting its status to
> disabled in the SoC .dtsi, and changing it to okay in the board DTS when needed, like done for other
> components?

OK for me.

Cheers,
Biju

RE: [PATCH 7/7] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 14:05
> Subject: Re: [PATCH 7/7] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1
> 
> Hi Biju,
> 
> On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Enable SDHI1 on the RZ/G3E SMARC EVK platform using gpio regulator for
> > voltage switching.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> > +++ b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> > @@ -12,10 +12,40 @@
> >  #include "rzg3e-smarc-som.dtsi"
> >  #include "renesas-smarc2.dtsi"
> >
> > +/*
> > + * To enable uSD card on SDIO_USD:
> > + *
> > + * Switch bank - SW_OPT_MUX-1 (SW_SDIO_M2E):
> > + *     0 - SMARC SDIO signal is connected to uSD1
> > + *     1 - SMARC SDIO signal is connected to M.2 Key E connector
> > + */
> > +
> >  / {
> >         model = "Renesas SMARC EVK version 2 based on r9a09g047e57";
> >         compatible = "renesas,smarc2-evk", "renesas,rzg3e-smarcm",
> >                      "renesas,r9a09g047e57", "renesas,r9a09g047";
> > +
> > +       usd_vdd_3p3v: regulator-usd-vdd-3p3v {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "fixed-3.3V";
> > +               regulator-min-microvolt = <3300000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               regulator-boot-on;
> > +               regulator-always-on;
> > +       };
> 
> Can't you just reuse reg_3p3v from rzg3e-smarc-som.dtsi?

OK.

> 
> > +
> > +       vqmmc_sdhi1_ext: regulator-vqmmc-sdhi1-ext {
> > +               compatible = "regulator-gpio";
> > +
> > +               regulator-name = "SDHI1 VccQ";
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <3300000>;
> > +
> > +               gpios = <&pinctrl RZG3E_GPIO(1, 5) GPIO_ACTIVE_HIGH>;
> 
> arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts:44.37-38 syntax error
> 
> Missing #include <dt-bindings/gpio/gpio.h>

Oops missed it. It was part of out of tree ethernet patch.
I will fix it.

> 
> > +
> > +               gpios-states = <0>;
> > +               states = <3300000 0>, <1800000 1>;
> > +       };
> 
> Is there a specific reason why you are using an external regulator...

I have tested both. I just want to give an example for user,
how to choose external regulator and also, previously I tested SDIO using
M.2 Key E connector(WLAN,BT) with RZ/G3E board testing software.
So, thought of using similar changes here in mainline. 

> 
> >  };
> >
> >  &pinctrl {
> > @@ -23,9 +53,44 @@ scif_pins: scif {
> >                 pins = "SCIF_TXD", "SCIF_RXD";
> >                 renesas,output-impedance = <1>;
> >         };
> > +
> > +       sd1-pwr-en {
> > +               gpio-hog;
> > +               gpios = <RZG3E_GPIO(1, 6) GPIO_ACTIVE_HIGH>;
> > +               output-high;
> > +               line-name = "sd1_pwr_en";
> > +       };
> 
> ... and external (fixed) power enable, instead of the internal regulator, which uses the same two
> pins?

Same as above.

Cheers,
Biju

Re: [PATCH 7/7] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

[Geert Uytterhoeven]

Hi Biju,

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Enable SDHI1 on the RZ/G3E SMARC EVK platform using gpio regulator for
> voltage switching.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

> --- a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> +++ b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> @@ -23,9 +53,44 @@ scif_pins: scif {
>                 pins = "SCIF_TXD", "SCIF_RXD";
>                 renesas,output-impedance = <1>;
>         };
> +
> +       sd1-pwr-en {

arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dtb: sd1-pwr-en:
$nodename:0: 'sd1-pwr-en' does not match
'^(hog-[0-9]+|.+-hog(-[0-9]+)?)$'
        from schema $id: http://devicetree.org/schemas/gpio/gpio-hog.yaml#

> +               gpio-hog;
> +               gpios = <RZG3E_GPIO(1, 6) GPIO_ACTIVE_HIGH>;
> +               output-high;
> +               line-name = "sd1_pwr_en";
> +       };

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 13:26
> Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes
> 
> Hi Biju,
> 
> On Tue, 28 Jan 2025 at 13:11, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: 28 January 2025 11:33
> > Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> > > biju.das.au <biju.das.au@gmail.com>
> > > Subject: Re: [PATCH 5/7] arm64: dts: renesas: r9a09g047: Add
> > > SDHI0-SDHI2 nodes
> > >
> > > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
> > > > @@ -518,6 +518,63 @@ gic: interrupt-controller@14900000 {
> > > >                         interrupt-controller;
> > > >                         interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
> > > >                 };
> > > > +
> > > > +               sdhi0: mmc@15c00000  {
> > > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > > +                       reg = <0x0 0x15c00000 0 0x10000>;
> > > > +                       interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                                    <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                       clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>,
> > > > +                                <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>;
> > > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > > +                       resets = <&cpg 0xa7>;
> > > > +                       power-domains = <&cpg>;
> > > > +                       status = "disabled";
> > > > +
> > > > +                       vqmmc_sdhi0: vqmmc-regulator {
> > > > +                               regulator-name = "SDHI0-VQMMC";
> > > > +                               regulator-min-microvolt = <1800000>;
> > > > +                               regulator-max-microvolt = <3300000>;
> > > > +                       };
> > > > +               };
> > > > +
> > > > +               sdhi1: mmc@15c10000 {
> > > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > > +                       reg = <0x0 0x15c10000 0 0x10000>;
> > > > +                       interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                                    <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                       clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>,
> > > > +                                <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>;
> > > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > > +                       resets = <&cpg 0xa8>;
> > > > +                       power-domains = <&cpg>;
> > > > +                       status = "disabled";
> > > > +
> > > > +                       vqmmc_sdhi1: vqmmc-regulator {
> > > > +                               regulator-name = "SDHI1-VQMMC";
> > > > +                               regulator-min-microvolt = <1800000>;
> > > > +                               regulator-max-microvolt = <3300000>;
> > > > +                       };
> > > > +               };
> > > > +
> > > > +               sdhi2: mmc@15c20000 {
> > > > +                       compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
> > > > +                       reg = <0x0 0x15c20000 0 0x10000>;
> > > > +                       interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>,
> > > > +                                    <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                       clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>,
> > > > +                                <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>;
> > > > +                       clock-names = "core", "clkh", "cd", "aclk";
> > > > +                       resets = <&cpg 0xa9>;
> > > > +                       power-domains = <&cpg>;
> > > > +                       status = "disabled";
> > > > +
> > > > +                       vqmmc_sdhi2: vqmmc-regulator {
> > > > +                               regulator-name = "SDHI2-VQMMC";
> > > > +                               regulator-min-microvolt = <1800000>;
> > > > +                               regulator-max-microvolt = <3300000>;
> > > > +                       };
> > > > +               };
> > > >         };
> > > >
> > > >         timer {
> > >
> > > Shouldn't the vqmmc-regulator subnodes be added in the board DTS,
> > > when needed (i.e. at least for SDHI[12])? Or do you expect the board DTS to /delete-node/ them
> when they are not needed?
> >
> > I agree.
> >
> > I have provided an example in next patch using /delete-node/ to use gpio-regulator.
> 
> Ah, my fault trying to get my reviews out sooner rather than later ;-)
> 
> > I am ok for moving it to the board DTS as well. When I sent patch, I
> > am not sure which is the best in terms of user point of view?
> >
> > Now I got the answer to move vqmmc-regulator subnodes to add in the
> > board DTS for atleast SDHI[12]. I will address this in next version.
> >
> > Even for SDHI0 fix type, if we plan to use fixed regulator for eMMC?
> >
> > >
> > > Is it possible that SDHI0 does not need the regulator control, e.g.
> > > in case of a fixed voltage?
> >
> > Yes, for eMMC(fixed case) it is not needed.
> 
> Upon second thought: as the internal regulator is always present, what about setting its status to
> disabled in the SoC .dtsi, and changing it to okay in the board DTS when needed, like done for other
> components?

Agreed. Apart from that, I am planning the below changes for V2:

SD0 fixed voltage(eMMC): fixed regulator based on SoM Schematics.
SD2 : Internal regulator.

There will be a new patch for supporting SD0 non fixed voltage(SD0) using 
internal regulator.

Cheers,
Biju

RE: [PATCH 7/7] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 15:49
> Subject: Re: [PATCH 7/7] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1
> 
> Hi Biju,
> 
> On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Enable SDHI1 on the RZ/G3E SMARC EVK platform using gpio regulator for
> > voltage switching.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> > --- a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> > +++ b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
> > @@ -23,9 +53,44 @@ scif_pins: scif {
> >                 pins = "SCIF_TXD", "SCIF_RXD";
> >                 renesas,output-impedance = <1>;
> >         };
> > +
> > +       sd1-pwr-en {
> 
> arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dtb: sd1-pwr-en:
> $nodename:0: 'sd1-pwr-en' does not match '^(hog-[0-9]+|.+-hog(-[0-9]+)?)$'
>         from schema $id: http://devicetree.org/schemas/gpio/gpio-hog.yaml#

Oops. I ran a selective dtbs_check which ignored this warning. I have fixed this
Now.

Cheers,
Biju