[PATCH v2 0/3] ARM: dts: renesas: genmai: enable SDHI and MMCIF

[PATCH v2 0/3] ARM: dts: renesas: genmai: enable SDHI and MMCIF

[Wolfram Sang]

Here are the improved patches to enable SDHI and MMCIF for the Genmai
board.

Changes since v1 (all in patch 3):
* renamed regulator to match schematics
* added "boot-on" property to regulator node
* added vqmmc-property to mmc node

A branch with all my Genmai-related patches can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/genmai-upstreaming

Thanks and happy hacking!


Wolfram Sang (3):
  ARM: dts: renesas: genmai: enable SDHI0
  ARM: dts: renesas: r7s72100: 'bus-width' is a board property
  ARM: dts: renesas: genmai: enable MMCIF

 arch/arm/boot/dts/renesas/r7s72100-genmai.dts | 57 ++++++++++++++++++-
 arch/arm/boot/dts/renesas/r7s72100.dtsi       |  1 -
 2 files changed, 56 insertions(+), 2 deletions(-)

-- 
2.45.2

[PATCH v2 1/3] ARM: dts: renesas: genmai: enable SDHI0

[Wolfram Sang]

For this to work, User LEDs must be disabled because they share their
pins with SD data lines.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/renesas/r7s72100-genmai.dts | 23 ++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/renesas/r7s72100-genmai.dts b/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
index 2d72daa4fac2..e93f444b2442 100644
--- a/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
+++ b/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
@@ -76,7 +76,8 @@ key-1 {
 	};
 
 	leds {
-		status = "okay";
+		/* Needs SDHI0 to be disabled */
+		status = "disabled";
 		compatible = "gpio-leds";
 
 		led1 {
@@ -227,6 +228,18 @@ scif2_pins: serial2 {
 		/* P3_0 as TxD2; P3_2 as RxD2 */
 		pinmux = <RZA1_PINMUX(3, 0, 6)>, <RZA1_PINMUX(3, 2, 4)>;
 	};
+
+	sdhi0_pins: sdhi0 {
+		/* SDHI0: P4_8 up to P4_15 */
+		pinmux = <RZA1_PINMUX(4, 8, 3)>,	/* SD_CD_0 */
+			 <RZA1_PINMUX(4, 9, 3)>,	/* SD_WP_0 */
+			 <RZA1_PINMUX(4, 10, 3)>,	/* SD_D1_0 */
+			 <RZA1_PINMUX(4, 11, 3)>,	/* SD_D0_0 */
+			 <RZA1_PINMUX(4, 12, 3)>,	/* SD_CLK_0 */
+			 <RZA1_PINMUX(4, 13, 3)>,	/* SD_CMD_0 */
+			 <RZA1_PINMUX(4, 14, 3)>,	/* SD_D3_0 */
+			 <RZA1_PINMUX(4, 15, 3)>;	/* SD_D2_0 */
+	};
 };
 
 &rtc_x1_clk {
@@ -244,6 +257,14 @@ &scif2 {
 	status = "okay";
 };
 
+&sdhi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdhi0_pins>;
+
+	bus-width = <4>;
+	status = "okay";
+};
+
 &spi4 {
 	status = "okay";
 
-- 
2.45.2

Re: [PATCH v2 1/3] ARM: dts: renesas: genmai: enable SDHI0

[Geert Uytterhoeven]

Hi Wolfram,

On Sat, Sep 28, 2024 at 11:30 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> For this to work, User LEDs must be disabled because they share their
> pins with SD data lines.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> --- a/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
> +++ b/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
> @@ -244,6 +257,14 @@ &scif2 {
>         status = "okay";
>  };
>
> +&sdhi0 {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&sdhi0_pins>;
> +
> +       bus-width = <4>;
> +       status = "okay";

Any specific reason you left out the regulator for SDHI (CVCC1),
but did add the regulator for MMC (CVCC2) in PATCH 2/3?

> +};
> +
>  &spi4 {
>         status = "okay";

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 v2 1/3] ARM: dts: renesas: genmai: enable SDHI0

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]


> Any specific reason you left out the regulator for SDHI (CVCC1),
> but did add the regulator for MMC (CVCC2) in PATCH 2/3?

Yes, the MMCIF driver didn't work otherwise because it needs to
negotiate with the card. It does not seem needed for SD cards without
SDR modes. I assume that, didn't investigate further.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 1/3] ARM: dts: renesas: genmai: enable SDHI0

[Geert Uytterhoeven]

On Fri, Oct 4, 2024 at 6:21 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > Any specific reason you left out the regulator for SDHI (CVCC1),
> > but did add the regulator for MMC (CVCC2) in PATCH 2/3?
>
> Yes, the MMCIF driver didn't work otherwise because it needs to
> negotiate with the card. It does not seem needed for SD cards without
> SDR modes. I assume that, didn't investigate further.

OK.

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

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

[PATCH v2 2/3] ARM: dts: renesas: r7s72100: 'bus-width' is a board property

[Wolfram Sang]

Do not set 'bus-width' in the SoC-include DTSI. It must be set in the
board DTS file. No regressions because MMCIF was not enabled yet for
this SoC.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/renesas/r7s72100.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/renesas/r7s72100.dtsi b/arch/arm/boot/dts/renesas/r7s72100.dtsi
index fa26e4a39fd7..39999468c28b 100644
--- a/arch/arm/boot/dts/renesas/r7s72100.dtsi
+++ b/arch/arm/boot/dts/renesas/r7s72100.dtsi
@@ -333,7 +333,6 @@ mmcif: mmc@e804c800 {
 				     <GIC_SPI 267 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&mstp8_clks R7S72100_CLK_MMCIF>;
 			power-domains = <&cpg_clocks>;
-			bus-width = <8>;
 			status = "disabled";
 		};
 
-- 
2.45.2

Re: [PATCH v2 2/3] ARM: dts: renesas: r7s72100: 'bus-width' is a board property

[Geert Uytterhoeven]

On Sat, Sep 28, 2024 at 11:30 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Do not set 'bus-width' in the SoC-include DTSI. It must be set in the
> board DTS file. No regressions because MMCIF was not enabled yet for
> this SoC.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

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

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

[PATCH v2 3/3] ARM: dts: renesas: genmai: enable MMCIF

[Wolfram Sang]

Luckily, I still had an MMC card lying around. Works fine.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/renesas/r7s72100-genmai.dts | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/renesas/r7s72100-genmai.dts b/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
index e93f444b2442..2403e21f2676 100644
--- a/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
+++ b/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
@@ -88,6 +88,15 @@ led2 {
 			gpios = <&port4 11 GPIO_ACTIVE_LOW>;
 		};
 	};
+
+	cvcc2: regulator-mmc {
+		compatible = "regulator-fixed";
+		regulator-name = "CVcc2";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
 };
 
 &bsc {
@@ -179,6 +188,17 @@ eeprom@50 {
 	};
 };
 
+&mmcif {
+	pinctrl-0 = <&mmcif_pins>;
+	pinctrl-names = "default";
+	cd-gpios = <&port3 8 GPIO_ACTIVE_LOW>;
+
+	vmmc-supply = <&cvcc2>;
+	vqmmc-supply = <&cvcc2>;
+	bus-width = <8>;
+	status = "okay";
+};
+
 &mtu2 {
 	status = "okay";
 };
@@ -224,6 +244,20 @@ keyboard_pins: keyboard {
 		pinmux = <RZA1_PINMUX(3, 1, 3)>;
 	};
 
+	mmcif_pins: mmcif {
+		/* MMCIF: P3_8 is CD_GPIO, P3_10 up to P3_15, P4_0 up to P4_3 */
+		pinmux = <RZA1_PINMUX(3, 10, 8)>,	/* MMC_D1 */
+			 <RZA1_PINMUX(3, 11, 8)>,	/* MMC_D0 */
+			 <RZA1_PINMUX(3, 12, 8)>,	/* MMC_CLK */
+			 <RZA1_PINMUX(3, 13, 8)>,	/* MMC_CMD */
+			 <RZA1_PINMUX(3, 14, 8)>,	/* MMC_D3 */
+			 <RZA1_PINMUX(3, 15, 8)>,	/* MMC_D2 */
+			 <RZA1_PINMUX(4, 0, 8)>,	/* MMC_D4 */
+			 <RZA1_PINMUX(4, 1, 8)>,	/* MMC_D5 */
+			 <RZA1_PINMUX(4, 2, 8)>,	/* MMC_D6 */
+			 <RZA1_PINMUX(4, 3, 8)>;	/* MMC_D7 */
+	};
+
 	scif2_pins: serial2 {
 		/* P3_0 as TxD2; P3_2 as RxD2 */
 		pinmux = <RZA1_PINMUX(3, 0, 6)>, <RZA1_PINMUX(3, 2, 4)>;
-- 
2.45.2

Re: [PATCH v2 3/3] ARM: dts: renesas: genmai: enable MMCIF

[Geert Uytterhoeven]

Hi Wolfram,

On Sat, Sep 28, 2024 at 11:30 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Luckily, I still had an MMC card lying around. Works fine.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> --- a/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
> +++ b/arch/arm/boot/dts/renesas/r7s72100-genmai.dts
> @@ -88,6 +88,15 @@ led2 {
>                         gpios = <&port4 11 GPIO_ACTIVE_LOW>;
>                 };
>         };
> +
> +       cvcc2: regulator-mmc {
> +               compatible = "regulator-fixed";
> +               regulator-name = "CVcc2";

Odd case. "Cvcc2"?

> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +               regulator-boot-on;
> +               regulator-always-on;

The regulator is not always-on, but controlled through the MMC
CD signal.

> +       };
>  };

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 v2 3/3] ARM: dts: renesas: genmai: enable MMCIF

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]


> > +               regulator-name = "CVcc2";
> 
> Odd case. "Cvcc2"?

Fine with me.

> > +               regulator-min-microvolt = <3300000>;
> > +               regulator-max-microvolt = <3300000>;
> > +               regulator-boot-on;
> > +               regulator-always-on;
> 
> The regulator is not always-on, but controlled through the MMC
> CD signal.

Hmm, okay, but how do I model this in the DT? Can I say that a GPIO is
input-only? I can't find this in the bindings doc.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 3/3] ARM: dts: renesas: genmai: enable MMCIF

[Geert Uytterhoeven]

Hi Wolfram,

On Fri, Oct 4, 2024 at 9:53 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > > +               regulator-name = "CVcc2";
> >
> > Odd case. "Cvcc2"?
>
> Fine with me.

OK, will fix while applying.

> > > +               regulator-min-microvolt = <3300000>;
> > > +               regulator-max-microvolt = <3300000>;
> > > +               regulator-boot-on;
> > > +               regulator-always-on;
> >
> > The regulator is not always-on, but controlled through the MMC
> > CD signal.
>
> Hmm, okay, but how do I model this in the DT? Can I say that a GPIO is
> input-only? I can't find this in the bindings doc.

Right, there is no such thing as "regulator-auto-on" ;-)
From the software's point of view it is always enabled, so
"regulator-always-on" makes sense.

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

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