[PATCH 0/3] Add eMMC/Micro SD support

[PATCH 0/3] Add eMMC/Micro SD support

[Biju Das]

This patch series aims to add eMMC/Micro SD support on RZ/G2L SMARC EVK
platform.

This patch series has dependency upon [1]
[1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=559045

Biju Das (3):
  arm64: dts: renesas: r9a07g044: Add SDHI nodes
  arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC on SMARC platform
  arm64: dts: renesas: rzg2l-smarc: Enable microSD on SMARC platform

 arch/arm64/boot/dts/renesas/r9a07g044.dtsi    |  32 ++++
 .../boot/dts/renesas/rzg2l-smarc-som.dtsi     | 145 ++++++++++++++++++
 arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi  |  62 ++++++++
 3 files changed, 239 insertions(+)

-- 
2.17.1

[PATCH 1/3] arm64: dts: renesas: r9a07g044: Add SDHI nodes

[Biju Das]

Add SDHI{0, 1} nodes to RZ/G2L SoC DTSI.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
This patch has dependency upon [1]
[1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=559045
---
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 1f01737d2def..0b0372a02515 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -456,6 +456,38 @@
 			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
 		};
 
+		sdhi0: mmc@11c00000  {
+			compatible = "renesas,sdhi-r9a07g044",
+				     "renesas,rcar-gen3-sdhi";
+			reg = <0x0 0x11c00000 0 0x10000>;
+			interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A07G044_SDHI0_IMCLK>,
+				 <&cpg CPG_MOD R9A07G044_SDHI0_IMCLK2>,
+				 <&cpg CPG_MOD R9A07G044_SDHI0_CLK_HS>,
+				 <&cpg CPG_MOD R9A07G044_SDHI0_ACLK>;
+			clock-names = "imclk", "imclk2", "clk_hs", "aclk";
+			resets = <&cpg R9A07G044_SDHI0_IXRST>;
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
+
+		sdhi1: mmc@11c10000 {
+			compatible = "renesas,sdhi-r9a07g044",
+				     "renesas,rcar-gen3-sdhi";
+			reg = <0x0 0x11c10000 0 0x10000>;
+			interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A07G044_SDHI1_IMCLK>,
+				 <&cpg CPG_MOD R9A07G044_SDHI1_IMCLK2>,
+				 <&cpg CPG_MOD R9A07G044_SDHI1_CLK_HS>,
+				 <&cpg CPG_MOD R9A07G044_SDHI1_ACLK>;
+			clock-names = "imclk", "imclk2", "clk_hs", "aclk";
+			resets = <&cpg R9A07G044_SDHI1_IXRST>;
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
+
 		phyrst: usbphy-ctrl@11c40000 {
 			compatible = "renesas,r9a07g044-usbphy-ctrl",
 				     "renesas,rzg2l-usbphy-ctrl";
-- 
2.17.1

[PATCH 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC on SMARC platform

[Biju Das]

RZ/G2L SoM has both 64Gb eMMC and micro SD connected to SDHI0.

Both these interfaces are mutually exclusive and the SD0 device
selection is based on the XOR between GPIO_SD0_DEV_SEL and SW1[2]
switch position.

This patch sets GPIO_SD0_DEV_SEL to high in DT. Use the below switch
setting logic for device selection between eMMC and microSD slot
connected to SDHI0.

Set SW1[2] to position 2/OFF for selecting eMMC
Set SW1[2] to position 3/ON for selecting micro SD

This patch enables eMMC on RZ/G2L SMARC platform by default.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
This patch has dependency upon [1]
[1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=559045
---
 .../boot/dts/renesas/rzg2l-smarc-som.dtsi     | 145 ++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
index da1ee2206e1a..d83072009225 100644
--- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
@@ -5,14 +5,55 @@
  * Copyright (C) 2021 Renesas Electronics Corp.
  */
 
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
 
+/* SW1[2] should be at position 2/OFF to enable 64Gb eMMC */
+#define EMMC	1
+
+/*
+ * To enable uSD card on CN3,
+ * SW1[2] should be at position 3/ON.
+ * Disable eMMC by setting "#define EMMC	0" above.
+ */
+#define SDHI	(!EMMC)
+
 / {
 	memory@48000000 {
 		device_type = "memory";
 		/* first 128MB is reserved for secure area. */
 		reg = <0x0 0x48000000 0x0 0x78000000>;
 	};
+
+	reg_1p8v: regulator0 {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-1.8V";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	reg_3p3v: regulator1 {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	vccq_sdhi0: regulator-vccq-sdhi0 {
+		compatible = "regulator-gpio";
+
+		regulator-name = "SDHI0 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		states = <3300000 1 1800000 0>;
+		regulator-boot-on;
+		gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>;
+		regulator-always-on;
+	};
 };
 
 &adc {
@@ -32,4 +73,108 @@
 	adc_pins: adc {
 		pinmux = <RZG2L_PORT_PINMUX(9, 0, 2)>; /* ADC_TRG */
 	};
+
+	/*
+	 * SDO device selection is XOR between GPIO_SD0_DEV_SEL and SW1[2]
+	 * The below switch logic can be used to select the device between
+	 * eMMC and microSD, after setting GPIO_SD0_DEV_SEL to high in DT.
+	 * SW1[2] should be at position 2/OFF to enable 64Gb eMMC
+	 * SW1[2] should be at position 3/ON to enable uSD card CN3
+	 */
+	gpio_sd0_dev_sel {
+		gpio-hog;
+		gpios = <RZG2L_GPIO(41, 1) GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "gpio_sd0_dev_sel";
+	};
+
+	sd0_pwr_en {
+		gpio-hog;
+		gpios = <RZG2L_GPIO(4, 1) GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "sd0_pwr_en";
+	};
+
+	sdhi0_emmc_pins: sd0emmc {
+		sd0_emmc_data {
+			pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3",
+			       "SD0_DATA4", "SD0_DATA5", "SD0_DATA6", "SD0_DATA7";
+			power-source  = <1800>;
+		};
+
+		sd0_emmc_ctrl {
+			pins = "SD0_CLK", "SD0_CMD";
+			power-source  = <1800>;
+		};
+
+		sd0_emmc_rst {
+			pins = "SD0_RST#";
+			power-source  = <1800>;
+		};
+	};
+
+	sdhi0_pins: sd0 {
+		sd0_data {
+			pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3";
+			power-source  = <3300>;
+		};
+
+		sd0_ctrl {
+			pins = "SD0_CLK", "SD0_CMD";
+			power-source  = <3300>;
+		};
+
+		sd0_mux {
+			pinmux = <RZG2L_PORT_PINMUX(47, 0, 2)>; /* SD0_CD */
+		};
+	};
+
+	sdhi0_pins_uhs: sd0_uhs {
+		sd1_data_uhs {
+			pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3";
+			power-source  = <1800>;
+		};
+
+		sd0_ctrl_uhs {
+			pins = "SD0_CLK", "SD0_CMD";
+			power-source  = <1800>;
+		};
+
+		sd0_mux_uhs {
+			pinmux = <RZG2L_PORT_PINMUX(47, 0, 2)>; /* SD0_CD */
+		};
+	};
+
 };
+
+#if SDHI
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_pins>;
+	pinctrl-1 = <&sdhi0_pins_uhs>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&vccq_sdhi0>;
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+	status = "okay";
+};
+#endif
+
+#if EMMC
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_emmc_pins>;
+	pinctrl-1 = <&sdhi0_emmc_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&reg_1p8v>;
+	bus-width = <8>;
+	mmc-hs200-1_8v;
+	non-removable;
+	fixed-emmc-driver-type = <1>;
+	status = "okay";
+};
+#endif
+
-- 
2.17.1

[PATCH 3/3] arm64: dts: renesas: rzg2l-smarc: Enable microSD on SMARC platform

[Biju Das]

This patch enables microSD card slot connected to SDHI1 on RZ/G2L SMARC
platform.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
This patch has dependency upon [1] and [2]
[1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=559045
[2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20211006171605.6861-1-biju.das.jz@bp.renesas.com/
---
 arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi | 62 ++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
index a02784fab46a..a7d01a10d286 100644
--- a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
@@ -70,6 +70,16 @@
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
 	};
+
+	vccq_sdhi1: regulator-vccq-sdhi1 {
+		compatible = "regulator-gpio";
+		regulator-name = "SDHI1 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&pinctrl RZG2L_GPIO(39, 1) GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1 1800000 0>;
+	};
 };
 
 &audio_clk1{
@@ -199,6 +209,45 @@
 			 <RZG2L_PORT_PINMUX(38, 1, 1)>;	/* RxD */
 	};
 
+	sd1_pwr_en {
+		gpio-hog;
+		gpios = <RZG2L_GPIO(39, 2) GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "sd1_pwr_en";
+	};
+
+	sdhi1_pins: sd1 {
+		sd1_data {
+			pins = "SD1_DATA0", "SD1_DATA1", "SD1_DATA2", "SD1_DATA3";
+			power-source  = <3300>;
+		};
+
+		sd1_ctrl {
+			pins = "SD1_CLK", "SD1_CMD";
+			power-source  = <3300>;
+		};
+
+		sd1_mux {
+			pinmux = <RZG2L_PORT_PINMUX(19, 0, 1)>; /* SD1_CD */
+		};
+	};
+
+	sdhi1_pins_uhs: sd1_uhs {
+		sd1_data_uhs {
+			pins = "SD1_DATA0", "SD1_DATA1", "SD1_DATA2", "SD1_DATA3";
+			power-source  = <1800>;
+		};
+
+		sd1_ctrl_uhs {
+			pins = "SD1_CLK", "SD1_CMD";
+			power-source  = <1800>;
+		};
+
+		sd1_mux_uhs {
+			pinmux = <RZG2L_PORT_PINMUX(19, 0, 1)>; /* SD1_CD */
+		};
+	};
+
 	sound_clk_pins: sound_clk {
 		pins = "AUDIO_CLK1", "AUDIO_CLK2";
 		input-enable;
@@ -229,6 +278,19 @@
 	status = "okay";
 };
 
+&sdhi1 {
+	pinctrl-0 = <&sdhi1_pins>;
+	pinctrl-1 = <&sdhi1_pins_uhs>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&vccq_sdhi1>;
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+	status = "okay";
+};
+
 &ssi0 {
 	pinctrl-0 = <&ssi0_pins>;
 	pinctrl-names = "default";
-- 
2.17.1

Re: [PATCH 1/3] arm64: dts: renesas: r9a07g044: Add SDHI nodes

[Geert Uytterhoeven]

On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add SDHI{0, 1} nodes to RZ/G2L SoC DTSI.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.16.

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 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC on SMARC platform

[Geert Uytterhoeven]

Hi Biju,

On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> RZ/G2L SoM has both 64Gb eMMC and micro SD connected to SDHI0.
>
> Both these interfaces are mutually exclusive and the SD0 device
> selection is based on the XOR between GPIO_SD0_DEV_SEL and SW1[2]
> switch position.
>
> This patch sets GPIO_SD0_DEV_SEL to high in DT. Use the below switch
> setting logic for device selection between eMMC and microSD slot
> connected to SDHI0.
>
> Set SW1[2] to position 2/OFF for selecting eMMC
> Set SW1[2] to position 3/ON for selecting micro SD
>
> This patch enables eMMC on RZ/G2L SMARC platform by default.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!


> --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> @@ -5,14 +5,55 @@
>   * Copyright (C) 2021 Renesas Electronics Corp.
>   */
>
> +#include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
>
> +/* SW1[2] should be at position 2/OFF to enable 64Gb eMMC */

64 GB (the MTFC64GASAQHD datasheet isn't clear about the exact size
and the meaning of GB)

> +#define EMMC   1
> +
> +/*
> + * To enable uSD card on CN3,
> + * SW1[2] should be at position 3/ON.
> + * Disable eMMC by setting "#define EMMC       0" above.
> + */
> +#define SDHI   (!EMMC)
> +
>  / {
>         memory@48000000 {
>                 device_type = "memory";
>                 /* first 128MB is reserved for secure area. */
>                 reg = <0x0 0x48000000 0x0 0x78000000>;
>         };
> +
> +       reg_1p8v: regulator0 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "fixed-1.8V";
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <1800000>;
> +               regulator-boot-on;
> +               regulator-always-on;
> +       };
> +
> +       reg_3p3v: regulator1 {
> +               compatible = "regulator-fixed";
> +               regulator-name = "fixed-3.3V";
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +               regulator-boot-on;
> +               regulator-always-on;
> +       };
> +
> +       vccq_sdhi0: regulator-vccq-sdhi0 {
> +               compatible = "regulator-gpio";
> +
> +               regulator-name = "SDHI0 VccQ";
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <3300000>;
> +               states = <3300000 1 1800000 0>;

"make dtbs_check" says:

    regulator-vccq-sdhi0: states:0: [3300000, 1, 1800000, 0] is too long

Please group the two states using angular brackets.

> +               regulator-boot-on;
> +               gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>;
> +               regulator-always-on;
> +       };
>  };
>
>  &adc {
> @@ -32,4 +73,108 @@
>         adc_pins: adc {
>                 pinmux = <RZG2L_PORT_PINMUX(9, 0, 2)>; /* ADC_TRG */
>         };
> +
> +       /*
> +        * SDO device selection is XOR between GPIO_SD0_DEV_SEL and SW1[2]
> +        * The below switch logic can be used to select the device between
> +        * eMMC and microSD, after setting GPIO_SD0_DEV_SEL to high in DT.
> +        * SW1[2] should be at position 2/OFF to enable 64Gb eMMC
> +        * SW1[2] should be at position 3/ON to enable uSD card CN3
> +        */
> +       gpio_sd0_dev_sel {

    gpio_sd0_dev_sel: $nodename:0: 'gpio_sd0_dev_sel' does not match
'^(hog-[0-9]+|.+-hog(-[0-9]+)?)$'
    (for all hogs)

Please use e.g. "sd0-dev-sel-hog".

> +               gpio-hog;
> +               gpios = <RZG2L_GPIO(41, 1) GPIO_ACTIVE_HIGH>;
> +               output-high;
> +               line-name = "gpio_sd0_dev_sel";
> +       };
> +
> +       sd0_pwr_en {

gpio-sd0-pwr-en-hog (with "gpio-" prefix, to match schematics)

> +               gpio-hog;
> +               gpios = <RZG2L_GPIO(4, 1) GPIO_ACTIVE_HIGH>;
> +               output-high;
> +               line-name = "sd0_pwr_en";

gpio_sd0_pwr_en

> +       };

The rest looks good to me.

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 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC on SMARC platform

[Geert Uytterhoeven]

Hi Biju,

On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> RZ/G2L SoM has both 64Gb eMMC and micro SD connected to SDHI0.
>
> Both these interfaces are mutually exclusive and the SD0 device
> selection is based on the XOR between GPIO_SD0_DEV_SEL and SW1[2]
> switch position.
>
> This patch sets GPIO_SD0_DEV_SEL to high in DT. Use the below switch
> setting logic for device selection between eMMC and microSD slot
> connected to SDHI0.
>
> Set SW1[2] to position 2/OFF for selecting eMMC
> Set SW1[2] to position 3/ON for selecting micro SD
>
> This patch enables eMMC on RZ/G2L SMARC platform by default.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

> --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi

> +       vccq_sdhi0: regulator-vccq-sdhi0 {
> +               compatible = "regulator-gpio";
> +
> +               regulator-name = "SDHI0 VccQ";
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <3300000>;
> +               states = <3300000 1 1800000 0>;
> +               regulator-boot-on;
> +               gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>;

Is this correct?
According to the schematics, the GPIO should be high to select 3.3V.

> +               regulator-always-on;
> +       };
>  };

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 3/3] arm64: dts: renesas: rzg2l-smarc: Enable microSD on SMARC platform

[Geert Uytterhoeven]

Hi Biju,

On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> This patch enables microSD card slot connected to SDHI1 on RZ/G2L SMARC
> platform.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
> @@ -70,6 +70,16 @@
>                 regulator-min-microvolt = <5000000>;
>                 regulator-max-microvolt = <5000000>;
>         };
> +
> +       vccq_sdhi1: regulator-vccq-sdhi1 {
> +               compatible = "regulator-gpio";
> +               regulator-name = "SDHI1 VccQ";
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <3300000>;
> +               gpios = <&pinctrl RZG2L_GPIO(39, 1) GPIO_ACTIVE_HIGH>;

Is this correct? Schematics say GPIO high is 3.3V.

> +               gpios-states = <1>;
> +               states = <3300000 1 1800000 0>;
> +       };
>  };
>
>  &audio_clk1{
> @@ -199,6 +209,45 @@
>                          <RZG2L_PORT_PINMUX(38, 1, 1)>; /* RxD */
>         };
>
> +       sd1_pwr_en {

sd1-pwr-en-hog (like in the example in
Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml ;-)


> +               gpio-hog;
> +               gpios = <RZG2L_GPIO(39, 2) GPIO_ACTIVE_HIGH>;
> +               output-high;
> +               line-name = "sd1_pwr_en";
> +       };

The rest looks good to me.

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 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC on SMARC platform

[Biju Das]

Hi Geert,

Thanks for the review.

> Subject: Re: [PATCH 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC
> on SMARC platform
> 
> Hi Biju,
> 
> On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > RZ/G2L SoM has both 64Gb eMMC and micro SD connected to SDHI0.
> >
> > Both these interfaces are mutually exclusive and the SD0 device
> > selection is based on the XOR between GPIO_SD0_DEV_SEL and SW1[2]
> > switch position.
> >
> > This patch sets GPIO_SD0_DEV_SEL to high in DT. Use the below switch
> > setting logic for device selection between eMMC and microSD slot
> > connected to SDHI0.
> >
> > Set SW1[2] to position 2/OFF for selecting eMMC Set SW1[2] to position
> > 3/ON for selecting micro SD
> >
> > This patch enables eMMC on RZ/G2L SMARC platform by default.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> > --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> 
> > +       vccq_sdhi0: regulator-vccq-sdhi0 {
> > +               compatible = "regulator-gpio";
> > +
> > +               regulator-name = "SDHI0 VccQ";
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               states = <3300000 1 1800000 0>;
> > +               regulator-boot-on;
> > +               gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>;
> 
> Is this correct?
> According to the schematics, the GPIO should be high to select 3.3V.

Yes, But it is "AND" Operation between SD0_DEV_SEL and GPIO_SD0_PWR_SEL.

For eMMC, SD0_PWR_SEL will be always 1.8V

For Micro SD, SD0_PWR_SEL will be 3.3V when GPIO_SD0_PWR_SEL is high
          SD0_PWR_SEL will be 1.8V when GPIO_SD0_PWR_SEL is low.

The table next to it has the details.

Regards,
Biju

> 
> > +               regulator-always-on;
> > +       };
> >  };
> 
> 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 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC on SMARC platform

[Geert Uytterhoeven]

Hi Biju,

On Fri, Oct 8, 2021 at 2:43 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC
> > on SMARC platform
> > On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > RZ/G2L SoM has both 64Gb eMMC and micro SD connected to SDHI0.
> > >
> > > Both these interfaces are mutually exclusive and the SD0 device
> > > selection is based on the XOR between GPIO_SD0_DEV_SEL and SW1[2]
> > > switch position.
> > >
> > > This patch sets GPIO_SD0_DEV_SEL to high in DT. Use the below switch
> > > setting logic for device selection between eMMC and microSD slot
> > > connected to SDHI0.
> > >
> > > Set SW1[2] to position 2/OFF for selecting eMMC Set SW1[2] to position
> > > 3/ON for selecting micro SD
> > >
> > > This patch enables eMMC on RZ/G2L SMARC platform by default.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > > --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> >
> > > +       vccq_sdhi0: regulator-vccq-sdhi0 {
> > > +               compatible = "regulator-gpio";
> > > +
> > > +               regulator-name = "SDHI0 VccQ";
> > > +               regulator-min-microvolt = <1800000>;
> > > +               regulator-max-microvolt = <3300000>;
> > > +               states = <3300000 1 1800000 0>;
> > > +               regulator-boot-on;
> > > +               gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>;
> >
> > Is this correct?
> > According to the schematics, the GPIO should be high to select 3.3V.
>
> Yes, But it is "AND" Operation between SD0_DEV_SEL and GPIO_SD0_PWR_SEL.
>
> For eMMC, SD0_PWR_SEL will be always 1.8V
>
> For Micro SD, SD0_PWR_SEL will be 3.3V when GPIO_SD0_PWR_SEL is high
>           SD0_PWR_SEL will be 1.8V when GPIO_SD0_PWR_SEL is low.

Doesn't the first state in states = <3300000 1 1800000 0> correspond
to GPIO_SD0_PWR_SEL being low?

Oh no, the second cell is the GPIO state...
Why is it common to order these in reverse order? :-(

Sorry, you're right. Please ignore my comment, also for the next patch.

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 3/3] arm64: dts: renesas: rzg2l-smarc: Enable microSD on SMARC platform

[Geert Uytterhoeven]

On Fri, Oct 8, 2021 at 2:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > This patch enables microSD card slot connected to SDHI1 on RZ/G2L SMARC
> > platform.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
> > @@ -70,6 +70,16 @@
> >                 regulator-min-microvolt = <5000000>;
> >                 regulator-max-microvolt = <5000000>;
> >         };
> > +
> > +       vccq_sdhi1: regulator-vccq-sdhi1 {
> > +               compatible = "regulator-gpio";
> > +               regulator-name = "SDHI1 VccQ";
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               gpios = <&pinctrl RZG2L_GPIO(39, 1) GPIO_ACTIVE_HIGH>;
>
> Is this correct? Schematics say GPIO high is 3.3V.

Yes it is, as per the value "1" in states below.

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

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 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC on SMARC platform

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 2/3] arm64: dts: renesas: rzg2l-smarc-som: Enable eMMC
> on SMARC platform
> 
> Hi Biju,
> 
> On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > RZ/G2L SoM has both 64Gb eMMC and micro SD connected to SDHI0.
> >
> > Both these interfaces are mutually exclusive and the SD0 device
> > selection is based on the XOR between GPIO_SD0_DEV_SEL and SW1[2]
> > switch position.
> >
> > This patch sets GPIO_SD0_DEV_SEL to high in DT. Use the below switch
> > setting logic for device selection between eMMC and microSD slot
> > connected to SDHI0.
> >
> > Set SW1[2] to position 2/OFF for selecting eMMC Set SW1[2] to position
> > 3/ON for selecting micro SD
> >
> > This patch enables eMMC on RZ/G2L SMARC platform by default.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> 
> > --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-som.dtsi
> > @@ -5,14 +5,55 @@
> >   * Copyright (C) 2021 Renesas Electronics Corp.
> >   */
> >
> > +#include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
> >
> > +/* SW1[2] should be at position 2/OFF to enable 64Gb eMMC */
> 

> 64 GB (the MTFC64GASAQHD datasheet isn't clear about the exact size and
> the meaning of GB)

It is 64 GB.

> 
> > +#define EMMC   1
> > +
> > +/*
> > + * To enable uSD card on CN3,
> > + * SW1[2] should be at position 3/ON.
> > + * Disable eMMC by setting "#define EMMC       0" above.
> > + */
> > +#define SDHI   (!EMMC)
> > +
> >  / {
> >         memory@48000000 {
> >                 device_type = "memory";
> >                 /* first 128MB is reserved for secure area. */
> >                 reg = <0x0 0x48000000 0x0 0x78000000>;
> >         };
> > +
> > +       reg_1p8v: regulator0 {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "fixed-1.8V";
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <1800000>;
> > +               regulator-boot-on;
> > +               regulator-always-on;
> > +       };
> > +
> > +       reg_3p3v: regulator1 {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "fixed-3.3V";
> > +               regulator-min-microvolt = <3300000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               regulator-boot-on;
> > +               regulator-always-on;
> > +       };
> > +
> > +       vccq_sdhi0: regulator-vccq-sdhi0 {
> > +               compatible = "regulator-gpio";
> > +
> > +               regulator-name = "SDHI0 VccQ";
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               states = <3300000 1 1800000 0>;
> 
> "make dtbs_check" says:
> 
>     regulator-vccq-sdhi0: states:0: [3300000, 1, 1800000, 0] is too long
> 
> Please group the two states using angular brackets.

OK.

> 
> > +               regulator-boot-on;
> > +               gpios = <&pinctrl RZG2L_GPIO(39, 0) GPIO_ACTIVE_HIGH>;
> > +               regulator-always-on;
> > +       };
> >  };
> >
> >  &adc {
> > @@ -32,4 +73,108 @@
> >         adc_pins: adc {
> >                 pinmux = <RZG2L_PORT_PINMUX(9, 0, 2)>; /* ADC_TRG */
> >         };
> > +
> > +       /*
> > +        * SDO device selection is XOR between GPIO_SD0_DEV_SEL and
> SW1[2]
> > +        * The below switch logic can be used to select the device
> between
> > +        * eMMC and microSD, after setting GPIO_SD0_DEV_SEL to high in
> DT.
> > +        * SW1[2] should be at position 2/OFF to enable 64Gb eMMC
> > +        * SW1[2] should be at position 3/ON to enable uSD card CN3
> > +        */
> > +       gpio_sd0_dev_sel {
> 
>     gpio_sd0_dev_sel: $nodename:0: 'gpio_sd0_dev_sel' does not match
> '^(hog-[0-9]+|.+-hog(-[0-9]+)?)$'
>     (for all hogs)
> 
> Please use e.g. "sd0-dev-sel-hog".

OK.

> 
> > +               gpio-hog;
> > +               gpios = <RZG2L_GPIO(41, 1) GPIO_ACTIVE_HIGH>;
> > +               output-high;
> > +               line-name = "gpio_sd0_dev_sel";

OK. Here as well.

> > +       };
> > +
> > +       sd0_pwr_en {
> 
> gpio-sd0-pwr-en-hog (with "gpio-" prefix, to match schematics)
OK.

> 
> > +               gpio-hog;
> > +               gpios = <RZG2L_GPIO(4, 1) GPIO_ACTIVE_HIGH>;
> > +               output-high;
> > +               line-name = "sd0_pwr_en";
> 
> gpio_sd0_pwr_en
OK.

Will add this changes to next version.

Regards,
Biju

> 
> > +       };
> 
> The rest looks good to me.
> 
> 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 3/3] arm64: dts: renesas: rzg2l-smarc: Enable microSD on SMARC platform

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 3/3] arm64: dts: renesas: rzg2l-smarc: Enable microSD
> on SMARC platform
> 
> Hi Biju,
> 
> On Thu, Oct 7, 2021 at 5:55 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > This patch enables microSD card slot connected to SDHI1 on RZ/G2L
> > SMARC platform.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
> > @@ -70,6 +70,16 @@
> >                 regulator-min-microvolt = <5000000>;
> >                 regulator-max-microvolt = <5000000>;
> >         };
> > +
> > +       vccq_sdhi1: regulator-vccq-sdhi1 {
> > +               compatible = "regulator-gpio";
> > +               regulator-name = "SDHI1 VccQ";
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               gpios = <&pinctrl RZG2L_GPIO(39, 1) GPIO_ACTIVE_HIGH>;
> 
> Is this correct? Schematics say GPIO high is 3.3V.
> 
> > +               gpios-states = <1>;
> > +               states = <3300000 1 1800000 0>;

OK. Will fix this like previous patch.
> > +       };
> >  };
> >
> >  &audio_clk1{
> > @@ -199,6 +209,45 @@
> >                          <RZG2L_PORT_PINMUX(38, 1, 1)>; /* RxD */
> >         };
> >
> > +       sd1_pwr_en {
> 
> sd1-pwr-en-hog (like in the example in
> Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml ;-)
> 

OK.

Will fix this in next version.

Regards,
Biju

> 
> > +               gpio-hog;
> > +               gpios = <RZG2L_GPIO(39, 2) GPIO_ACTIVE_HIGH>;
> > +               output-high;
> > +               line-name = "sd1_pwr_en";
> > +       };
> 
> The rest looks good to me.
> 
> 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