[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

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

[PATCH 2/7] clk: renesas: r9a09g047: Add SDHI clocks/resets

[Biju Das]

Add SDHI{0..2} clock and reset entries.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
This patch depend upon [1]
[1] https://lore.kernel.org/all/20250120094715.25802-3-biju.das.jz@bp.renesas.com/
---
 drivers/clk/renesas/r9a09g047-cpg.c | 31 +++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/clk/renesas/r9a09g047-cpg.c b/drivers/clk/renesas/r9a09g047-cpg.c
index 1181e7b605e8..51fd24c20ed5 100644
--- a/drivers/clk/renesas/r9a09g047-cpg.c
+++ b/drivers/clk/renesas/r9a09g047-cpg.c
@@ -31,6 +31,8 @@ enum clk_ids {
 
 	/* Internal Core Clocks */
 	CLK_PLLCM33_DIV16,
+	CLK_PLLCLN_DIV2,
+	CLK_PLLCLN_DIV8,
 	CLK_PLLCLN_DIV16,
 	CLK_PLLDTY_ACPU,
 	CLK_PLLDTY_ACPU_DIV4,
@@ -71,6 +73,8 @@ static const struct cpg_core_clk r9a09g047_core_clks[] __initconst = {
 	/* Internal Core Clocks */
 	DEF_FIXED(".pllcm33_div16", CLK_PLLCM33_DIV16, CLK_PLLCM33, 1, 16),
 
+	DEF_FIXED(".pllcln_div2", CLK_PLLCLN_DIV2, CLK_PLLCLN, 1, 2),
+	DEF_FIXED(".pllcln_div8", CLK_PLLCLN_DIV8, CLK_PLLCLN, 1, 8),
 	DEF_FIXED(".pllcln_div16", CLK_PLLCLN_DIV16, CLK_PLLCLN, 1, 16),
 
 	DEF_DDIV(".plldty_acpu", CLK_PLLDTY_ACPU, CLK_PLLDTY, CDDIV0_DIVCTL2, dtable_2_64),
@@ -126,6 +130,30 @@ static const struct rzv2h_mod_clk r9a09g047_mod_clks[] __initconst = {
 						BUS_MSTOP(1, BIT(7))),
 	DEF_MOD("riic_7_ckm",			CLK_PLLCLN_DIV16, 9, 11, 4, 27,
 						BUS_MSTOP(1, BIT(8))),
+	DEF_MOD("sdhi_0_imclk",			CLK_PLLCLN_DIV8, 10, 3, 5, 3,
+						BUS_MSTOP(8, BIT(2))),
+	DEF_MOD("sdhi_0_imclk2",		CLK_PLLCLN_DIV8, 10, 4, 5, 4,
+						BUS_MSTOP(8, BIT(2))),
+	DEF_MOD("sdhi_0_clk_hs",		CLK_PLLCLN_DIV2, 10, 5, 5, 5,
+						BUS_MSTOP(8, BIT(2))),
+	DEF_MOD("sdhi_0_aclk",			CLK_PLLDTY_ACPU_DIV4, 10, 6, 5, 6,
+						BUS_MSTOP(8, BIT(2))),
+	DEF_MOD("sdhi_1_imclk",			CLK_PLLCLN_DIV8, 10, 7, 5, 7,
+						BUS_MSTOP(8, BIT(3))),
+	DEF_MOD("sdhi_1_imclk2",		CLK_PLLCLN_DIV8, 10, 8, 5, 8,
+						BUS_MSTOP(8, BIT(3))),
+	DEF_MOD("sdhi_1_clk_hs",		CLK_PLLCLN_DIV2, 10, 9, 5, 9,
+						BUS_MSTOP(8, BIT(3))),
+	DEF_MOD("sdhi_1_aclk",			CLK_PLLDTY_ACPU_DIV4, 10, 10, 5, 10,
+						BUS_MSTOP(8, BIT(3))),
+	DEF_MOD("sdhi_2_imclk",			CLK_PLLCLN_DIV8, 10, 11, 5, 11,
+						BUS_MSTOP(8, BIT(4))),
+	DEF_MOD("sdhi_2_imclk2",		CLK_PLLCLN_DIV8, 10, 12, 5, 12,
+						BUS_MSTOP(8, BIT(4))),
+	DEF_MOD("sdhi_2_clk_hs",		CLK_PLLCLN_DIV2, 10, 13, 5, 13,
+						BUS_MSTOP(8, BIT(4))),
+	DEF_MOD("sdhi_2_aclk",			CLK_PLLDTY_ACPU_DIV4, 10, 14, 5, 14,
+						BUS_MSTOP(8, BIT(4))),
 };
 
 static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
@@ -146,6 +174,9 @@ static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
 	DEF_RST(9, 14, 4, 15),		/* RIIC_6_MRST */
 	DEF_RST(9, 15, 4, 16),		/* RIIC_7_MRST */
 	DEF_RST(10, 0, 4, 17),		/* RIIC_8_MRST */
+	DEF_RST(10, 7, 4, 24),		/* SDHI_0_IXRST */
+	DEF_RST(10, 8, 4, 25),		/* SDHI_1_IXRST */
+	DEF_RST(10, 9, 4, 26),		/* SDHI_2_IXRST */
 };
 
 const struct rzv2h_cpg_info r9a09g047_cpg_info __initconst = {
-- 
2.43.0

Re: [PATCH 2/7] clk: renesas: r9a09g047: Add SDHI clocks/resets

[Tommaso Merciai]

Hi Biju,

Thanks for the patch.

On Sun, Jan 26, 2025 at 01:46:04PM +0000, Biju Das wrote:
> Add SDHI{0..2} clock and reset entries.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> This patch depend upon [1]
> [1] https://lore.kernel.org/all/20250120094715.25802-3-biju.das.jz@bp.renesas.com/
> ---
>  drivers/clk/renesas/r9a09g047-cpg.c | 31 +++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/clk/renesas/r9a09g047-cpg.c b/drivers/clk/renesas/r9a09g047-cpg.c
> index 1181e7b605e8..51fd24c20ed5 100644
> --- a/drivers/clk/renesas/r9a09g047-cpg.c
> +++ b/drivers/clk/renesas/r9a09g047-cpg.c
> @@ -31,6 +31,8 @@ enum clk_ids {
>  
>  	/* Internal Core Clocks */
>  	CLK_PLLCM33_DIV16,
> +	CLK_PLLCLN_DIV2,
> +	CLK_PLLCLN_DIV8,
>  	CLK_PLLCLN_DIV16,
>  	CLK_PLLDTY_ACPU,
>  	CLK_PLLDTY_ACPU_DIV4,
> @@ -71,6 +73,8 @@ static const struct cpg_core_clk r9a09g047_core_clks[] __initconst = {
>  	/* Internal Core Clocks */
>  	DEF_FIXED(".pllcm33_div16", CLK_PLLCM33_DIV16, CLK_PLLCM33, 1, 16),
>  
> +	DEF_FIXED(".pllcln_div2", CLK_PLLCLN_DIV2, CLK_PLLCLN, 1, 2),
> +	DEF_FIXED(".pllcln_div8", CLK_PLLCLN_DIV8, CLK_PLLCLN, 1, 8),
>  	DEF_FIXED(".pllcln_div16", CLK_PLLCLN_DIV16, CLK_PLLCLN, 1, 16),
>  
>  	DEF_DDIV(".plldty_acpu", CLK_PLLDTY_ACPU, CLK_PLLDTY, CDDIV0_DIVCTL2, dtable_2_64),
> @@ -126,6 +130,30 @@ static const struct rzv2h_mod_clk r9a09g047_mod_clks[] __initconst = {
>  						BUS_MSTOP(1, BIT(7))),
>  	DEF_MOD("riic_7_ckm",			CLK_PLLCLN_DIV16, 9, 11, 4, 27,
>  						BUS_MSTOP(1, BIT(8))),
> +	DEF_MOD("sdhi_0_imclk",			CLK_PLLCLN_DIV8, 10, 3, 5, 3,
> +						BUS_MSTOP(8, BIT(2))),
> +	DEF_MOD("sdhi_0_imclk2",		CLK_PLLCLN_DIV8, 10, 4, 5, 4,
> +						BUS_MSTOP(8, BIT(2))),
> +	DEF_MOD("sdhi_0_clk_hs",		CLK_PLLCLN_DIV2, 10, 5, 5, 5,
> +						BUS_MSTOP(8, BIT(2))),
> +	DEF_MOD("sdhi_0_aclk",			CLK_PLLDTY_ACPU_DIV4, 10, 6, 5, 6,
> +						BUS_MSTOP(8, BIT(2))),
> +	DEF_MOD("sdhi_1_imclk",			CLK_PLLCLN_DIV8, 10, 7, 5, 7,
> +						BUS_MSTOP(8, BIT(3))),
> +	DEF_MOD("sdhi_1_imclk2",		CLK_PLLCLN_DIV8, 10, 8, 5, 8,
> +						BUS_MSTOP(8, BIT(3))),
> +	DEF_MOD("sdhi_1_clk_hs",		CLK_PLLCLN_DIV2, 10, 9, 5, 9,
> +						BUS_MSTOP(8, BIT(3))),
> +	DEF_MOD("sdhi_1_aclk",			CLK_PLLDTY_ACPU_DIV4, 10, 10, 5, 10,
> +						BUS_MSTOP(8, BIT(3))),
> +	DEF_MOD("sdhi_2_imclk",			CLK_PLLCLN_DIV8, 10, 11, 5, 11,
> +						BUS_MSTOP(8, BIT(4))),
> +	DEF_MOD("sdhi_2_imclk2",		CLK_PLLCLN_DIV8, 10, 12, 5, 12,
> +						BUS_MSTOP(8, BIT(4))),
> +	DEF_MOD("sdhi_2_clk_hs",		CLK_PLLCLN_DIV2, 10, 13, 5, 13,
> +						BUS_MSTOP(8, BIT(4))),
> +	DEF_MOD("sdhi_2_aclk",			CLK_PLLDTY_ACPU_DIV4, 10, 14, 5, 14,
> +						BUS_MSTOP(8, BIT(4))),
>  };
>  
>  static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
> @@ -146,6 +174,9 @@ static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
>  	DEF_RST(9, 14, 4, 15),		/* RIIC_6_MRST */
>  	DEF_RST(9, 15, 4, 16),		/* RIIC_7_MRST */
>  	DEF_RST(10, 0, 4, 17),		/* RIIC_8_MRST */
> +	DEF_RST(10, 7, 4, 24),		/* SDHI_0_IXRST */
> +	DEF_RST(10, 8, 4, 25),		/* SDHI_1_IXRST */
> +	DEF_RST(10, 9, 4, 26),		/* SDHI_2_IXRST */
>  };
>  
>  const struct rzv2h_cpg_info r9a09g047_cpg_info __initconst = {
> -- 
> 2.43.0
>

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

Re: [PATCH 2/7] clk: renesas: r9a09g047: Add SDHI clocks/resets

[Geert Uytterhoeven]

Hi Biju,

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add SDHI{0..2} clock and reset entries.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> ---
> This patch depend upon [1]
> [1] https://lore.kernel.org/all/20250120094715.25802-3-biju.das.jz@bp.renesas.com/

Which I haven't reviewed yet, as it has been superseded, twice...
Anyway, I don't see the dependency?

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

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 3/7] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Biju Das]

Arrange local variables in reverse xmas tree for probe().

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/mmc/host/renesas_sdhi_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index f73b84bae0c4..6ea651409774 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -910,8 +910,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 		       const struct renesas_sdhi_quirks *quirks)
 {
 	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
-	struct tmio_mmc_data *mmc_data;
 	struct renesas_sdhi_dma *dma_priv;
+	struct tmio_mmc_data *mmc_data;
 	struct tmio_mmc_host *host;
 	struct renesas_sdhi *priv;
 	int num_irqs, irq, ret, i;
-- 
2.43.0

Re: [PATCH 3/7] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Tommaso Merciai]

Hi Biju,

Thanks for the patch.

On Sun, Jan 26, 2025 at 01:46:05PM +0000, Biju Das wrote:
> Arrange local variables in reverse xmas tree for probe().
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/mmc/host/renesas_sdhi_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index f73b84bae0c4..6ea651409774 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -910,8 +910,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>  		       const struct renesas_sdhi_quirks *quirks)
>  {
>  	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
> -	struct tmio_mmc_data *mmc_data;
>  	struct renesas_sdhi_dma *dma_priv;
> +	struct tmio_mmc_data *mmc_data;
>  	struct tmio_mmc_host *host;
>  	struct renesas_sdhi *priv;
>  	int num_irqs, irq, ret, i;
> -- 
> 2.43.0
> 

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

Thanks & Regards,
Tommaso

Re: [PATCH 3/7] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Geert Uytterhoeven]

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Arrange local variables in reverse xmas tree for probe().
>
> 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

[PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
However, the RZ/G3E SD0 channel has Voltage level control and PWEN pin
support via SD_STATUS register.

internal regulator support is added to control the voltage levels of
the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by populating
vqmmc-regulator child node.

SD1 and SD2 channels have gpio regulator support and internal regulator
support. Selection of the regulator is based on the regulator phandle.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/mmc/host/renesas_sdhi.h      |   1 +
 drivers/mmc/host/renesas_sdhi_core.c | 129 +++++++++++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h          |   5 ++
 3 files changed, 135 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index f12a87442338..291ddb4ad9be 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -95,6 +95,7 @@ struct renesas_sdhi {
 
 	struct reset_control *rstc;
 	struct tmio_mmc_host *host;
+	struct regulator_dev *rdev;
 };
 
 #define host_to_priv(host) \
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 6ea651409774..bbfa68ca682b 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -32,6 +32,8 @@
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/regulator/consumer.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/reset.h>
 #include <linux/sh_dma.h>
 #include <linux/slab.h>
@@ -581,12 +583,24 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
 
 	if (!preserve) {
 		if (priv->rstc) {
+			u32 sd_status;
+			/*
+			 * HW reset might have toggled the regulator state in
+			 * HW which regulator core might be unaware of so save
+			 * and restore the regulator state during HW reset.
+			 */
+			if (priv->rdev)
+				sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+
 			reset_control_reset(priv->rstc);
 			/* Unknown why but without polling reset status, it will hang */
 			read_poll_timeout(reset_control_status, ret, ret == 0, 1, 100,
 					  false, priv->rstc);
 			/* At least SDHI_VER_GEN2_SDR50 needs manual release of reset */
 			sd_ctrl_write16(host, CTL_RESET_SD, 0x0001);
+			if (priv->rdev)
+				sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
 			priv->needs_adjust_hs400 = false;
 			renesas_sdhi_set_clock(host, host->clk_cache);
 
@@ -904,15 +918,113 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
 	renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static const unsigned int renesas_sdhi_vqmmc_voltages[] = {
+	3300000, 1800000
+};
+
+static int renesas_sdhi_regulator_disable(struct regulator_dev *rdev)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+	sd_status &= ~SD_STATUS_PWEN;
+	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
+	return 0;
+}
+
+static int renesas_sdhi_regulator_enable(struct regulator_dev *rdev)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+	sd_status |= SD_STATUS_PWEN;
+	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
+	return 0;
+}
+
+static int renesas_sdhi_regulator_is_enabled(struct regulator_dev *rdev)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+
+	return (sd_status & SD_STATUS_PWEN) ? 1 : 0;
+}
+
+static int renesas_sdhi_regulator_get_voltage(struct regulator_dev *rdev)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+
+	return (sd_status & SD_STATUS_IOVS) ? 1800000 : 3300000;
+}
+
+static int renesas_sdhi_regulator_set_voltage(struct regulator_dev *rdev,
+					      int min_uV, int max_uV,
+					      unsigned int *selector)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+	if (min_uV >= 1700000 && max_uV <= 1950000) {
+		sd_status |= SD_STATUS_IOVS;
+		*selector = 1;
+	} else {
+		sd_status &= ~SD_STATUS_IOVS;
+		*selector = 0;
+	}
+	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
+	return 0;
+}
+
+static int renesas_sdhi_regulator_list_voltage(struct regulator_dev *rdev,
+					       unsigned int selector)
+{
+	if (selector >= ARRAY_SIZE(renesas_sdhi_vqmmc_voltages))
+		return -EINVAL;
+
+	return renesas_sdhi_vqmmc_voltages[selector];
+}
+
+static const struct regulator_ops renesas_sdhi_regulator_voltage_ops = {
+	.enable = renesas_sdhi_regulator_enable,
+	.disable = renesas_sdhi_regulator_disable,
+	.is_enabled = renesas_sdhi_regulator_is_enabled,
+	.list_voltage = renesas_sdhi_regulator_list_voltage,
+	.get_voltage = renesas_sdhi_regulator_get_voltage,
+	.set_voltage = renesas_sdhi_regulator_set_voltage,
+};
+
+static struct regulator_desc renesas_sdhi_vqmmc_regulator = {
+	.of_match	= of_match_ptr("vqmmc-regulator"),
+	.owner		= THIS_MODULE,
+	.type		= REGULATOR_VOLTAGE,
+	.ops		= &renesas_sdhi_regulator_voltage_ops,
+	.volt_table	= renesas_sdhi_vqmmc_voltages,
+	.n_voltages	= ARRAY_SIZE(renesas_sdhi_vqmmc_voltages),
+};
+
 int renesas_sdhi_probe(struct platform_device *pdev,
 		       const struct tmio_mmc_dma_ops *dma_ops,
 		       const struct renesas_sdhi_of_data *of_data,
 		       const struct renesas_sdhi_quirks *quirks)
 {
+	struct regulator_config rcfg = { .dev = &pdev->dev, };
 	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
 	struct renesas_sdhi_dma *dma_priv;
+	struct device *dev = &pdev->dev;
 	struct tmio_mmc_data *mmc_data;
 	struct tmio_mmc_host *host;
+	struct regulator_dev *rdev;
 	struct renesas_sdhi *priv;
 	int num_irqs, irq, ret, i;
 	struct resource *res;
@@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (ret)
 		goto efree;
 
+	rcfg.of_node = of_get_child_by_name(dev->of_node, "vqmmc-regulator");
+	if (rcfg.of_node) {
+		rcfg.driver_data = priv->host;
+
+		renesas_sdhi_vqmmc_regulator.name = "sdhi-vqmmc-regulator";
+		renesas_sdhi_vqmmc_regulator.of_match = of_match_ptr("vqmmc-regulator");
+		renesas_sdhi_vqmmc_regulator.type = REGULATOR_VOLTAGE;
+		renesas_sdhi_vqmmc_regulator.owner = THIS_MODULE;
+		rdev = devm_regulator_register(dev, &renesas_sdhi_vqmmc_regulator, &rcfg);
+		of_node_put(rcfg.of_node);
+		if (IS_ERR(rdev)) {
+			dev_err(dev, "regulator register failed err=%ld", PTR_ERR(rdev));
+			goto efree;
+		}
+		priv->rdev = rdev;
+	}
+
 	ver = sd_ctrl_read16(host, CTL_VERSION);
 	/* GEN2_SDR104 is first known SDHI to use 32bit block count */
 	if (ver < SDHI_VER_GEN2_SDR104 && mmc_data->max_blk_count > U16_MAX)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index a75755f31d31..5970ca598850 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -44,6 +44,7 @@
 #define CTL_RESET_SD 0xe0
 #define CTL_VERSION 0xe2
 #define CTL_SDIF_MODE 0xe6 /* only known on R-Car 2+ */
+#define CTL_SD_STATUS 0xf2 /* only known on RZ/{G2L,G3E,V2H} */
 
 /* Definitions for values the CTL_STOP_INTERNAL_ACTION register can take */
 #define TMIO_STOP_STP		BIT(0)
@@ -103,6 +104,10 @@
 /* Definitions for values the CTL_SDIF_MODE register can take */
 #define SDIF_MODE_HS400		BIT(0) /* only known on R-Car 2+ */
 
+/* Definitions for values the CTL_SD_STATUS register can take */
+#define SD_STATUS_PWEN		BIT(0) /* only known on RZ/{G3E,V2H} */
+#define SD_STATUS_IOVS		BIT(16) /* only known on RZ/{G3E,V2H} */
+
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
 #define TMIO_MASK_ALL           0x837f031d
-- 
2.43.0

Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Tommaso Merciai]

On Sun, Jan 26, 2025 at 01:46:06PM +0000, Biju Das wrote:
> The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
> However, the RZ/G3E SD0 channel has Voltage level control and PWEN pin
> support via SD_STATUS register.
> 
> internal regulator support is added to control the voltage levels of
> the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by populating
> vqmmc-regulator child node.
> 
> SD1 and SD2 channels have gpio regulator support and internal regulator
> support. Selection of the regulator is based on the regulator phandle.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/mmc/host/renesas_sdhi.h      |   1 +
>  drivers/mmc/host/renesas_sdhi_core.c | 129 +++++++++++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h          |   5 ++
>  3 files changed, 135 insertions(+)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
> index f12a87442338..291ddb4ad9be 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -95,6 +95,7 @@ struct renesas_sdhi {
>  
>  	struct reset_control *rstc;
>  	struct tmio_mmc_host *host;
> +	struct regulator_dev *rdev;
>  };
>  
>  #define host_to_priv(host) \
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 6ea651409774..bbfa68ca682b 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -32,6 +32,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/of_regulator.h>
>  #include <linux/reset.h>
>  #include <linux/sh_dma.h>
>  #include <linux/slab.h>
> @@ -581,12 +583,24 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
>  
>  	if (!preserve) {
>  		if (priv->rstc) {
> +			u32 sd_status;
> +			/*
> +			 * HW reset might have toggled the regulator state in
> +			 * HW which regulator core might be unaware of so save
> +			 * and restore the regulator state during HW reset.
> +			 */
> +			if (priv->rdev)
> +				sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
> +
>  			reset_control_reset(priv->rstc);
>  			/* Unknown why but without polling reset status, it will hang */
>  			read_poll_timeout(reset_control_status, ret, ret == 0, 1, 100,
>  					  false, priv->rstc);
>  			/* At least SDHI_VER_GEN2_SDR50 needs manual release of reset */
>  			sd_ctrl_write16(host, CTL_RESET_SD, 0x0001);
> +			if (priv->rdev)
> +				sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
> +
>  			priv->needs_adjust_hs400 = false;
>  			renesas_sdhi_set_clock(host, host->clk_cache);
>  
> @@ -904,15 +918,113 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
>  	renesas_sdhi_sdbuf_width(host, enable ? width : 16);
>  }
>  
> +static const unsigned int renesas_sdhi_vqmmc_voltages[] = {
> +	3300000, 1800000
> +};
> +
> +static int renesas_sdhi_regulator_disable(struct regulator_dev *rdev)
> +{
> +	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
> +	u32 sd_status;
> +
> +	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
> +	sd_status &= ~SD_STATUS_PWEN;
> +	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
> +
> +	return 0;
> +}
> +
> +static int renesas_sdhi_regulator_enable(struct regulator_dev *rdev)
> +{
> +	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
> +	u32 sd_status;
> +
> +	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
> +	sd_status |= SD_STATUS_PWEN;
> +	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
> +
> +	return 0;
> +}
> +
> +static int renesas_sdhi_regulator_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
> +	u32 sd_status;
> +
> +	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
> +
> +	return (sd_status & SD_STATUS_PWEN) ? 1 : 0;
> +}
> +
> +static int renesas_sdhi_regulator_get_voltage(struct regulator_dev *rdev)
> +{
> +	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
> +	u32 sd_status;
> +
> +	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
> +
> +	return (sd_status & SD_STATUS_IOVS) ? 1800000 : 3300000;
> +}
> +
> +static int renesas_sdhi_regulator_set_voltage(struct regulator_dev *rdev,
> +					      int min_uV, int max_uV,
> +					      unsigned int *selector)
> +{
> +	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
> +	u32 sd_status;
> +
> +	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
> +	if (min_uV >= 1700000 && max_uV <= 1950000) {
> +		sd_status |= SD_STATUS_IOVS;
> +		*selector = 1;
> +	} else {
> +		sd_status &= ~SD_STATUS_IOVS;
> +		*selector = 0;
> +	}
> +	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
> +
> +	return 0;
> +}
> +
> +static int renesas_sdhi_regulator_list_voltage(struct regulator_dev *rdev,
> +					       unsigned int selector)
> +{
> +	if (selector >= ARRAY_SIZE(renesas_sdhi_vqmmc_voltages))
> +		return -EINVAL;
> +
> +	return renesas_sdhi_vqmmc_voltages[selector];
> +}
> +
> +static const struct regulator_ops renesas_sdhi_regulator_voltage_ops = {
> +	.enable = renesas_sdhi_regulator_enable,
> +	.disable = renesas_sdhi_regulator_disable,
> +	.is_enabled = renesas_sdhi_regulator_is_enabled,
> +	.list_voltage = renesas_sdhi_regulator_list_voltage,
> +	.get_voltage = renesas_sdhi_regulator_get_voltage,
> +	.set_voltage = renesas_sdhi_regulator_set_voltage,
> +};
> +
> +static struct regulator_desc renesas_sdhi_vqmmc_regulator = {
> +	.of_match	= of_match_ptr("vqmmc-regulator"),
> +	.owner		= THIS_MODULE,
> +	.type		= REGULATOR_VOLTAGE,
> +	.ops		= &renesas_sdhi_regulator_voltage_ops,
> +	.volt_table	= renesas_sdhi_vqmmc_voltages,
> +	.n_voltages	= ARRAY_SIZE(renesas_sdhi_vqmmc_voltages),
> +};
> +
>  int renesas_sdhi_probe(struct platform_device *pdev,
>  		       const struct tmio_mmc_dma_ops *dma_ops,
>  		       const struct renesas_sdhi_of_data *of_data,
>  		       const struct renesas_sdhi_quirks *quirks)
>  {
> +	struct regulator_config rcfg = { .dev = &pdev->dev, };
>  	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
>  	struct renesas_sdhi_dma *dma_priv;
> +	struct device *dev = &pdev->dev;
>  	struct tmio_mmc_data *mmc_data;
>  	struct tmio_mmc_host *host;
> +	struct regulator_dev *rdev;
>  	struct renesas_sdhi *priv;
>  	int num_irqs, irq, ret, i;
>  	struct resource *res;
> @@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>  	if (ret)
>  		goto efree;
>  
> +	rcfg.of_node = of_get_child_by_name(dev->of_node, "vqmmc-regulator");
> +	if (rcfg.of_node) {
> +		rcfg.driver_data = priv->host;
> +
> +		renesas_sdhi_vqmmc_regulator.name = "sdhi-vqmmc-regulator";
> +		renesas_sdhi_vqmmc_regulator.of_match = of_match_ptr("vqmmc-regulator");
> +		renesas_sdhi_vqmmc_regulator.type = REGULATOR_VOLTAGE;
> +		renesas_sdhi_vqmmc_regulator.owner = THIS_MODULE;
> +		rdev = devm_regulator_register(dev, &renesas_sdhi_vqmmc_regulator, &rcfg);
> +		of_node_put(rcfg.of_node);
> +		if (IS_ERR(rdev)) {
> +			dev_err(dev, "regulator register failed err=%ld", PTR_ERR(rdev));
> +			goto efree;
> +		}
> +		priv->rdev = rdev;
> +	}
> +
>  	ver = sd_ctrl_read16(host, CTL_VERSION);
>  	/* GEN2_SDR104 is first known SDHI to use 32bit block count */
>  	if (ver < SDHI_VER_GEN2_SDR104 && mmc_data->max_blk_count > U16_MAX)
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index a75755f31d31..5970ca598850 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -44,6 +44,7 @@
>  #define CTL_RESET_SD 0xe0
>  #define CTL_VERSION 0xe2
>  #define CTL_SDIF_MODE 0xe6 /* only known on R-Car 2+ */
> +#define CTL_SD_STATUS 0xf2 /* only known on RZ/{G2L,G3E,V2H} */
>  
>  /* Definitions for values the CTL_STOP_INTERNAL_ACTION register can take */
>  #define TMIO_STOP_STP		BIT(0)
> @@ -103,6 +104,10 @@
>  /* Definitions for values the CTL_SDIF_MODE register can take */
>  #define SDIF_MODE_HS400		BIT(0) /* only known on R-Car 2+ */
>  
> +/* Definitions for values the CTL_SD_STATUS register can take */
> +#define SD_STATUS_PWEN		BIT(0) /* only known on RZ/{G3E,V2H} */
> +#define SD_STATUS_IOVS		BIT(16) /* only known on RZ/{G3E,V2H} */
> +
>  /* Define some IRQ masks */
>  /* This is the mask used at reset by the chip */
>  #define TMIO_MASK_ALL           0x837f031d
> -- 
> 2.43.0
>

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

Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Geert Uytterhoeven]

Hi Biju,

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
> However, the RZ/G3E SD0 channel has Voltage level control and PWEN pin
> support via SD_STATUS register.
>
> internal regulator support is added to control the voltage levels of
> the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by populating
> vqmmc-regulator child node.
>
> SD1 and SD2 channels have gpio regulator support and internal regulator
> support. Selection of the regulator is based on the regulator phandle.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c

> @@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>         if (ret)
>                 goto efree;
>
> +       rcfg.of_node = of_get_child_by_name(dev->of_node, "vqmmc-regulator");

If this node becomes required on RZ/V2H and RZ/G3E, and controlled
through status, you also need:

    if (!of_device_is_available(rcfg.of_node)) {
            of_node_put(rcfg.of_node);
            rcfg.of_node = NULL;
    }

Or introduce of_get_available_child_by_name()...

> +       if (rcfg.of_node) {
> +               rcfg.driver_data = priv->host;
> +
> +               renesas_sdhi_vqmmc_regulator.name = "sdhi-vqmmc-regulator";

Name conflict in case of multiple instances?

> +               renesas_sdhi_vqmmc_regulator.of_match = of_match_ptr("vqmmc-regulator");
> +               renesas_sdhi_vqmmc_regulator.type = REGULATOR_VOLTAGE;
> +               renesas_sdhi_vqmmc_regulator.owner = THIS_MODULE;
> +               rdev = devm_regulator_register(dev, &renesas_sdhi_vqmmc_regulator, &rcfg);
> +               of_node_put(rcfg.of_node);
> +               if (IS_ERR(rdev)) {
> +                       dev_err(dev, "regulator register failed err=%ld", PTR_ERR(rdev));
> +                       goto efree;
> +               }
> +               priv->rdev = rdev;
> +       }
> +
>         ver = sd_ctrl_read16(host, CTL_VERSION);
>         /* GEN2_SDR104 is first known SDHI to use 32bit block count */
>         if (ver < SDHI_VER_GEN2_SDR104 && mmc_data->max_blk_count > U16_MAX)

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 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 13:32
> Subject: Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> 
> Hi Biju,
> 
> On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
> > However, the RZ/G3E SD0 channel has Voltage level control and PWEN pin
> > support via SD_STATUS register.
> >
> > internal regulator support is added to control the voltage levels of
> > the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by
> > populating vqmmc-regulator child node.
> >
> > SD1 and SD2 channels have gpio regulator support and internal
> > regulator support. Selection of the regulator is based on the regulator phandle.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> 
> > @@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> >         if (ret)
> >                 goto efree;
> >
> > +       rcfg.of_node = of_get_child_by_name(dev->of_node,
> > + "vqmmc-regulator");
> 
> If this node becomes required on RZ/V2H and RZ/G3E, and controlled through status, you also need:
> 
>     if (!of_device_is_available(rcfg.of_node)) {
>             of_node_put(rcfg.of_node);
>             rcfg.of_node = NULL;
>     }

OK.

> 
> Or introduce of_get_available_child_by_name()...

OK, will send a separate patch for optimizing the above 2 calls.

> 
> > +       if (rcfg.of_node) {
> > +               rcfg.driver_data = priv->host;
> > +
> > +               renesas_sdhi_vqmmc_regulator.name =
> > + "sdhi-vqmmc-regulator";
> 
> Name conflict in case of multiple instances?

This regulator name is Overriden by of_regulator() and it will pick the name from there.
See below. Am I missing anything?

root@smarc-rzg3e:/cip-test-scripts# cat /sys/class/regulator/regulator.*/name
regulator-dummy
fixed-3.3V
fixed-3.3V
SDHI1 VccQ
SDHI0-VQMMC
SDHI2-VQMMC
SDHI1-VQMMC

Cheers,
Biju

RE: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Geert,

 
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: 28 January 2025 13:32
> > Subject: Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> >
> > Hi Biju,
> >
> > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
> > > However, the RZ/G3E SD0 channel has Voltage level control and PWEN
> > > pin support via SD_STATUS register.
> > >
> > > internal regulator support is added to control the voltage levels of
> > > the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by
> > > populating vqmmc-regulator child node.
> > >
> > > SD1 and SD2 channels have gpio regulator support and internal
> > > regulator support. Selection of the regulator is based on the regulator phandle.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> >
> > > @@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> > >         if (ret)
> > >                 goto efree;
> > >
> >
> > > +       if (rcfg.of_node) {
> > > +               rcfg.driver_data = priv->host;
> > > +
> > > +               renesas_sdhi_vqmmc_regulator.name =
> > > + "sdhi-vqmmc-regulator";
> >
> > Name conflict in case of multiple instances?
> 
> This regulator name is Overriden by of_regulator() and it will pick the name from there.
> See below. Am I missing anything?

Just to add, without the name it returns error see [1],

Regulator name from DT node is picked by [2] and [3]. So there won't be conflict for
Multiple instances.

[1] https://elixir.bootlin.com/linux/v6.13-rc7/source/drivers/regulator/core.c#L5597

[2] https://elixir.bootlin.com/linux/v6.13-rc7/source/drivers/regulator/core.c#L5659
[3] https://elixir.bootlin.com/linux/v6.13-rc7/source/drivers/regulator/of_regulator.c#L97

Cheers,
Biju

> 
> root@smarc-rzg3e:/cip-test-scripts# cat /sys/class/regulator/regulator.*/name
> regulator-dummy
> fixed-3.3V
> fixed-3.3V
> SDHI1 VccQ
> SDHI0-VQMMC
> SDHI2-VQMMC
> SDHI1-VQMMC
> 
> Cheers,
> Biju

Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Geert Uytterhoeven]

Hi Biju,

On Wed, 29 Jan 2025 at 15:45, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: 28 January 2025 13:32
> > > Subject: Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> > >
> > > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > > The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
> > > > However, the RZ/G3E SD0 channel has Voltage level control and PWEN
> > > > pin support via SD_STATUS register.
> > > >
> > > > internal regulator support is added to control the voltage levels of
> > > > the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by
> > > > populating vqmmc-regulator child node.
> > > >
> > > > SD1 and SD2 channels have gpio regulator support and internal
> > > > regulator support. Selection of the regulator is based on the regulator phandle.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > > > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > >
> > > > @@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> > > >         if (ret)
> > > >                 goto efree;
> > > >
> > >
> > > > +       if (rcfg.of_node) {
> > > > +               rcfg.driver_data = priv->host;
> > > > +
> > > > +               renesas_sdhi_vqmmc_regulator.name =
> > > > + "sdhi-vqmmc-regulator";
> > >
> > > Name conflict in case of multiple instances?
> >
> > This regulator name is Overriden by of_regulator() and it will pick the name from there.
> > See below. Am I missing anything?
>
> Just to add, without the name it returns error see [1],

Thanks, I had arrived at the same conclusion.

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

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

[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

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

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